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

isset and empty constructs appears as PropertyWrites in propertyUsages of ClassPropertiesNode #5337

Closed
janedbal opened this issue Jul 20, 2021 · 8 comments · Fixed by phpstan/phpstan-src#1174
Labels

Comments

@janedbal
Copy link
Contributor

Bug report

Assuming following code, there will be two PropertyWrite elements in any custom rule hooked to ClassPropertiesNode (one for assign, one for empty). Detected by rule like this.

Code snippet that reproduces the problem

https://phpstan.org/r/f9dee572-568e-462f-a4c1-24f56594f804

Expected output

Only one property write to appear.

@ondrejmirtes
Copy link
Member

I'd like an example where this shows a false-negative/false-positive in a PHPStan core rule. It probably marks a property as written even if it's not. Based on the example I'll decide how sever this bug is.

You can always filter it on your side.

@janedbal
Copy link
Contributor Author

janedbal commented Jul 20, 2021

Biggest issue from core I see is that it affects UninitializedPropertyRule so this code is not reported to have uninitialized field $field. Smaller problem is the error you see in example, it says, it is only written, which is not true.

@ondrejmirtes
Copy link
Member

This is coming from the Scope::isInExpressionAssign() method which is used to detect whether it's about a write or a read. This method is also used to detect whether accessing non-existent things should be reported or not. I can't think of a clean solution for this.

@phpstan-bot
Copy link
Contributor

@janedbal After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-No errors
+5: Property Clazz::$prefix is never read, only written.
Full report
Line Error
5 Property Clazz::$prefix is never read, only written.

@phpstan-bot
Copy link
Contributor

@janedbal After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-No errors
+5: Property Clazz::$prefix is never read, only written.
Full report
Line Error
5 Property Clazz::$prefix is never read, only written.

@phpstan-bot
Copy link
Contributor

@janedbal After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
- 6: Property Foo::$field is never read, only written.
+-1: Internal error: PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule::findMethod(): Argument #2 ($classReflection) must be of type PHPStan\Reflection\ClassReflection, null given, called in /var/task/vendor/phpstan/phpstan-strict-rules/src/Rules/Methods/WrongCaseOfInheritedMethodRule.php on line 40
+Run PHPStan with --debug option and post the stack trace to:
+https://github.com/phpstan/phpstan/issues/new?template=Bug_report.md
Full report
Line Error
-1 Internal error: PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule::findMethod(): Argument #2 ($classReflection) must be of type PHPStan\Reflection\ClassReflection, null given, called in /var/task/vendor/phpstan/phpstan-strict-rules/src/Rules/Methods/WrongCaseOfInheritedMethodRule.php on line 40Run PHPStan with --debug option and post the stack trace to:https://github.com/phpstan/phpstan/issues/new?template=Bug_report.md

@phpstan-bot
Copy link
Contributor

@janedbal After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-6: Property Foo::$field is never read, only written.
+6: Property Foo::$field is unused.
Full report
Line Error
6 Property Foo::$field is unused.

@github-actions
Copy link

github-actions bot commented Feb 9, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants