Skip to content

Inconsisten analysis results #12972

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

Closed
staabm opened this issue May 5, 2025 · 8 comments · May be fixed by phpstan/phpstan-src#3988
Closed

Inconsisten analysis results #12972

staabm opened this issue May 5, 2025 · 8 comments · May be fixed by phpstan/phpstan-src#3988

Comments

@staabm
Copy link
Contributor

staabm commented May 5, 2025

Bug report

I am currently debugging a strange problem.

I have a file, when analyzed with a path command like
vendor/bin/phpstan analyze lib/cms/ -c app/portal/phpstan.neon.dist --debug reports error, like

Method CMS\service\CmsImageService::handleImageModuleSave() should return bool but return statement is missing.

running analysis over the very same file with a different command, e.g.
vendor/bin/phpstan analyze lib/cms/service/ -c app/portal/phpstan.neon.dist --debug
or
vendor/bin/phpstan analyze lib/cms/service/CmsImageService.php -c app/portal/phpstan.neon.dist --debug

yields 0 errors.


I have debugged the situation to a point, in which I can see that the different error results occur, because the Analyzer gets a different AST node-tree.

in the error case, the Analyzer gets a AST in which the ClassMethod nodes contain a empty array of statements (even though the file beeing analyzed does have code in its methods)

Image

when the very same file is analyzed with one of the working commands the AST looks like this:

Image

I am not yet able to create a small reproducer and atm I only see this problem in a single project

Code snippet that reproduces the problem

n/a

Expected output

consistent behaviour

Did PHPStan help you today? Did it make you happy in any way?

No response

@staabm
Copy link
Contributor Author

staabm commented May 5, 2025

I can see the root cause of the different AST now.

in the error case, the file beeing analyzed is not recognized as beeing one of the analyzedFIles in the PathRoutingParser, and therefore will only be parsed with a simple-parser instead of a rich-parser:

Image

@ondrejmirtes
Copy link
Member

Yeah, this usually happens when symlinks are involved, or when a file with exactly the same contents is discovered by PHPStan outside of analysed paths (although I fixed some bugs related to that).

@staabm
Copy link
Contributor Author

staabm commented May 5, 2025

In my case the root problem is, that a custom autoloader builds a "nearly correct" path to the file (which has wrong casing), but the macos filesystem still says
is_file('/Users/m.staab/Documents/GitHub/philipp/lib/CMS/service/CmsImageService.php') -> true

even though in the filesystem the path actually is
/Users/m.staab/Documents/GitHub/philipp/lib/cms/service/CmsImageService.php,

therefore the "wrong" path is required() and php-src remembers this wrong path, and returns it later on in ReflectionClass->getFileName() 💥

@ondrejmirtes
Copy link
Member

The joys of case-insensitive filesystems 😊 Probably not much we can do about that.

@staabm
Copy link
Contributor Author

staabm commented May 5, 2025

I hate our home grown custom autoloader - especially in hybrid with a composer autoloader, which would do a better job when beeing alone.

thanks for the emotional support while crying thru all this shit ;)

@staabm staabm closed this as completed May 5, 2025
@staabm
Copy link
Contributor Author

staabm commented May 5, 2025

Would it be worth to detect a case-insensitive filesystem and compare paths in a insensitive way?

@ondrejmirtes
Copy link
Member

I'm not even sure why an autoloader and a reflection filename have say in this. Would be interesting to see this reproduced here in E2E suite running on windows-latest.

@staabm
Copy link
Contributor Author

staabm commented May 12, 2025

I'm not even sure why an autoloader and a reflection filename have say in this.

➜  repro git:(bug12972) ✗ ls -la   
total 16
drwxr-xr-x@  4 m.staab  staff   128 May 12 16:01 .
drwxr-xr-x@ 42 m.staab  staff  1344 May 12 16:01 ..
-rw-r--r--@  1 m.staab  staff    20 May 12 15:58 OTHER.php
-rw-r--r--@  1 m.staab  staff   123 May 12 15:59 repro.php

➜  phpstan-src git:(bug12972) ✗ cat OTHER.php   
<?php

class Foo {}
➜  phpstan-src git:(bug12972) ✗ cat repro.php
<?php

require __DIR__.'/other.php'; // include file with different case, as a autoloader could do it

$reflection = new ReflectionClass(Foo::class);
var_dump($reflection->getFileName());
➜  phpstan-src git:(bug12972) ✗ php repro.php 
string(53) "/Users/m.staab/Documents/GitHub/phpstan-src/other.php"

see in the last line of the output, that $reflection->getFileName() returns the path, equal to the path used for require, even though in the filesystem the path has a different casing.

Would be interesting to see this reproduced here in E2E suite running on windows-latest.

regression test and fix in phpstan/phpstan-src#3988

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 a pull request may close this issue.

2 participants