Conversation
| [ | ||
| 'Impure method MethodSignaturePureUnlessCallable\ImpureChildOfAllMethodsPureParent::run() overrides method MethodSignaturePureUnlessCallable\AllMethodsPureParent::run() marked @pure-unless-callable-is-impure.', | ||
| 62, | ||
| ], |
There was a problem hiding this comment.
I am not sure about this one.
@pure-unless-callable-is-impure means its maybe pure (and maybe impure).
should it be allowed to override a "maybe pure", "maybe impure" method with "always pure" or "always impure"?
|
For line 62, this PR changes the message, not whether PHPStan reports the override. Before, the class-level The tag states a conditional promise: the call is pure whenever the callback is pure. PHPStan keeps that apart from "maybe". A parent without any purity tag has While checking this I found a hole in the always-pure case. A |
Might be worth to start with an issue @staabm @zonuexe about what we're planning to allow/forbid about phpdoc override ; wdyt ? Before this and #6669 PR I feel like we should allow to override the phpdoc with a bigger-purity but never with a bigger impurity. Maybe first we should report this, WDYT ? |
|
I agree with that rule: an override may promise more purity, never less. For the allow/forbid list, I ran your example on
#6667 and #6669 don't change which overrides |
86eceaf to
7361cfb
Compare
Follow-up to #6666 (comment). In a class marked
@phpstan-all-methods-pure, the class-level tag overrode a method's own@pure-unless-parameter-passedor@pure-unless-callable-is-impureand made the method unconditionally pure. PHPStan then missed a caller that passes$countor an impure callback:It also reported
Method Replacer::replace() is marked as pure but parameter $count is passed by reference.on the method itself.Changes
PhpDocsResolverandPhpClassReflectionExtension::createUserlandMethodReflection()apply@phpstan-all-methods-pure/@phpstan-all-methods-impureonly to a method without a@pure-unless-*tag, written or inherited.@phpstan-purealready works this way: an inherited@phpstan-purewins over the class-level tags.PhpDocsResolver.cppandPhpClassReflectionExtension.cppget the same condition, followed by aBump expected turbo versioncommit. Thephp-class-reflection-familydifferential and a newwalk-tracefixture cover the new cases.Change to the #6666 test
AllMethodsImpureChildinmethod-signature-pure-unless-parameter-passed.phpis a@phpstan-all-methods-impureclass implementing an interface method marked@pure-unless-parameter-passed.MethodSignatureRuleno longer reports it: the inherited tag now wins over the class-level one, the same as an inherited@phpstan-purewould. PHPStan checks its body against the inherited conditional purity instead and reports theechothere. The fixture now covers the other direction too, an impure override of a method declared in a@phpstan-all-methods-pureclass, for both tags.Verified locally
make testswith and without the extension,make phpstan,make cs, the strict build,smoke.php,signature-parity.php,side-by-side.php,walk-trace.phpover the default corpus, and byte-identical--error-format=rawoutput with the extension loaded and not loaded over the Pure fixtures and the reproducers.