Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

preg_split sometimes returns array of arrays #324

Merged
merged 1 commit into from Sep 22, 2020
Merged

preg_split sometimes returns array of arrays #324

merged 1 commit into from Sep 22, 2020

Conversation

spaze
Copy link
Contributor

@spaze spaze commented Sep 20, 2020

When PREG_SPLIT_OFFSET_CAPTURE flag is given

preg_split('/-/', 'a-b-c', -1, PREG_SPLIT_OFFSET_CAPTURE);

See https://3v4l.org/B4Qf5

Then "this changes the return value in an array where every element is an array consisting of the matched string at offset 0 and its string offset into subject at offset 1."

@dktapps
Copy link
Contributor

dktapps commented Sep 20, 2020

Wouldn't a dynamic return type extension be better for this?

@ondrejmirtes
Copy link
Member

Thank you, but yes, dynamic return type extension would be better so that the user doesn't have to check against values that cannot occur in their situation :)

@spaze I'm sure you won't have any problems writing one as you've already written some custom PHPStan rules :) Here are some pointers:

* Basic info: https://phpstan.org/developing-extensions/dynamic-return-type-extensions

  • This is a similar extension that also reads various constants (although it's probably much more complicated than one needed for preg_split: https://github.com/phpstan/phpstan-src/blob/master/src/Type/Php/FilterVarDynamicReturnTypeExtension.php
  • This is a useful method:
    private function getConstant(string $constantName): int
    {
    $constant = $this->reflectionProvider->getConstant(new Node\Name($constantName), null);
    $valueType = $constant->getValueType();
    if (!$valueType instanceof ConstantIntegerType) {
    throw new \PHPStan\ShouldNotHappenException(sprintf('Constant %s does not have integer type.', $constantName));
    }
    return $valueType->getValue();
    }
    (feel free to copy it to your extension as well)
  • I'm not sure if preg_split flags can be combined using bitwise operators, if yes then the extension has to account for it.
  • Test the implementation by adding a new data provider to NodeScopeResolverTest::testFileAsserts, call assertType in the test file and test various inputs :)

When PREG_SPLIT_OFFSET_CAPTURE is given, "this changes the return value in an array where every element is an array consisting of the matched string at offset 0 and its string offset into subject at offset 1."
@spaze
Copy link
Contributor Author

spaze commented Sep 20, 2020

Thanks, I've vaguely remembered there was a better way of doing things like this but...

I've now changed this PR to use a dynamic type extension for that.

And thanks @ondrejmirtes for those hints, helped a lot!

@spaze spaze changed the title [Signatures] preg_split sometimes returns array of arrays preg_split sometimes returns array of arrays Sep 20, 2020
@ondrejmirtes ondrejmirtes merged commit 1be28f3 into phpstan:master Sep 22, 2020
@ondrejmirtes
Copy link
Member

Perfect, thank you :)

@spaze spaze deleted the patch-2 branch September 22, 2020 11:38
@spaze
Copy link
Contributor Author

spaze commented Sep 24, 2020

Thanks for the release @ondrejmirtes! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants