Avoid warning about non-nullable class-property in traits.#5479
Avoid warning about non-nullable class-property in traits.#5479peter17 wants to merge 2 commits intophpstan:2.2.xfrom
Conversation
|
You've opened the pull request against the latest branch 2.2.x. PHPStan 2.2 is not going to be released for months. If your code is relevant on 2.1.x and you want it to be released sooner, please rebase your pull request and change its target to 2.1.x. |
ondrejmirtes
left a comment
There was a problem hiding this comment.
The tests are insufficient. You need to use CompositeRule and add ConstantConditionInTraitRule.
The PR that introduced ConstantConditionInTraitHelper does that in all tests for rules it changed.
Also add cases where the rule should and should not report errors. In all rule tests you touched.
| * @param ErrorIdentifier $identifier | ||
| * @param callable(Type): ?string $typeMessageCallback | ||
| */ | ||
| public function checkWithTraitHandling(Expr $expr, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope, string $operatorDescription, string $identifier, callable $typeMessageCallback, string $ruleName): ?IdentifierRuleError |
There was a problem hiding this comment.
Why are you adding a new method if you can just change the original one?
There was a problem hiding this comment.
Hi @ondrejmirtes thanks for your review. The original check method is very complex, with several return cases and also calls itself. I'm not sure how to manage changing it without breaking it... I proposed a different approach in my new commit so the exposed name does not change and move the logic to a checkInternal, maybe it is more understandable like this...
Fixes #14416