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

SubNodeNames are known at static analysis time #1035

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Oct 7, 2024

this PR turns the SubNodeNames into constant arrays and corresponding phpdocs, which makes them available to PHPStan at static analysis time.

this information is useful to better cover codebases which utilize PHP-Parser and in need of dynamic property lookups like

			foreach ($node->getSubNodeNames() as $subNodeName) {
				$subNode = $node->{$subNodeName};
                // ...
			}

when the type of $node is known.

closes #1033

@staabm staabm marked this pull request as ready for review October 7, 2024 08:01
@nikic
Copy link
Owner

nikic commented Oct 9, 2024

I can kind of see how this could be useful in theory, but it's not really clear to me where we'd see a benefit in practice. Can you share an example where e.g. this gets rid of a baseline suppression in phpstan-src or similar?

@staabm
Copy link
Contributor Author

staabm commented Oct 10, 2024

see e.g. https://phpstan.org/r/9041358f-043d-4171-9780-f9d9b4b3c534

before this PR here, the unused property cannot be detected.

after this PR we get

  • as initially suggested, the unused property now gets detected
  • the error on line 26 is a unrelated PHPStan bug
➜  phpstan-src git:(2.0.x) ✗ php bin/phpstan analyze --debug test.php
Note: Using configuration file /Users/staabm/workspace/phpstan-src/phpstan.neon.dist.
/Users/staabm/workspace/phpstan-src/test.php
 ------ ---------------------------------------------------------------------------------------------------- 
  Line   test.php                                                                                            
 ------ ---------------------------------------------------------------------------------------------------- 
  11     Property Processor::$unusedProperty is never read, only written.                                    
         🪪  property.onlyWritten                                                                            
         💡 See: https://phpstan.org/developing-extensions/always-read-written-properties                    
  26     Access to an undefined property PhpParser\Node\Expr\FuncCall|PhpParser\Node\Expr\MethodCall::$var.  
         🪪  property.notFound                                                                               
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property                
 ------ ---------------------------------------------------------------------------------------------------- 

this gets rid of a baseline suppression in phpstan-src or similar?

I can't give you such a example, as because of the dynamic use errors are not detected which would be otherwise.
(it does not fix a false-positive, but fixes a false-negative)

@staabm staabm marked this pull request as draft October 10, 2024 11:19
@staabm
Copy link
Contributor Author

staabm commented Oct 10, 2024

hmm.. I tried this PR on the whole phpstan-src codebase and it somehow has strange side-effects. I have another look

staabm added a commit to staabm/phpstan-src that referenced this pull request Oct 10, 2024
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.

Add Node->getSubNodes()
2 participants