Skip to content

Avoid warning about non-nullable class-property in traits.#5479

Open
peter17 wants to merge 2 commits intophpstan:2.2.xfrom
peter17:patch1
Open

Avoid warning about non-nullable class-property in traits.#5479
peter17 wants to merge 2 commits intophpstan:2.2.xfrom
peter17:patch1

Conversation

@peter17
Copy link
Copy Markdown

@peter17 peter17 commented Apr 15, 2026

Fixes #14416

@phpstan-bot
Copy link
Copy Markdown
Collaborator

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.

Copy link
Copy Markdown
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.

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.

Comment thread src/Rules/IssetCheck.php Outdated
* @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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are you adding a new method if you can just change the original one?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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...

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.

Warns too much about non-nullable class-property

3 participants