Skip to content

ArrayFilterRule should care bout treatPhpDocTypesAsCertain #2065

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

Merged
merged 2 commits into from
Dec 8, 2022

Conversation

VincentLanglet
Copy link
Contributor

Hi @ondrejmirtes,

When using treatPhpDocTypesAsCertain: false, a call array_filter shouldn't be reported as useless if the type is coming from phpdoc.

So

if ($this->treatPhpDocTypesAsCertain) {
     $arrayType = $scope->getType($args[0]->value);
} else {
     $arrayType = $scope->getNativeType($args[0]->value);
}

should be used instead of $arrayType = $scope->getType($args[0]->value);

But I don't know if there is something to do in order to not forgot this for others rules,
like changing getType to return the nativeType if treatPhpDocTypesAsCertain is false or exposing another method.
Maybe you would have an idea @staabm, @herndlm or @rajyan ?

@staabm
Copy link
Contributor

staabm commented Dec 8, 2022

as I understood it, treatPhpDocTypesAsCertain is missing in a lot of places and the current in-progress refactoring will help us to concentrate the if (treatPhpDocTypesAsCertain)-logic into the Scope class, so we don't need to sprinkle it all over the rules.

@ondrejmirtes
Copy link
Member

@staabm That's a wrong understanding of the current work in progress. Rules will specifically need to ask about getNativeType as opposed to getType with the treat parameter set to false.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff doesn't make it obvious which errors are no longer reported. Please create a new file specific to your issue and test it with both true and false. Thanks. Otherwise 👍

@ondrejmirtes ondrejmirtes merged commit 1a59fe1 into phpstan:1.9.x Dec 8, 2022
@ondrejmirtes
Copy link
Member

Thank you!

@VincentLanglet VincentLanglet deleted the arrayFilter branch December 8, 2022 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants