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

False positives related to UndefinedClass #6517

Open
ndench opened this issue Sep 22, 2021 · 17 comments
Open

False positives related to UndefinedClass #6517

ndench opened this issue Sep 22, 2021 · 17 comments
Labels

Comments

@ndench
Copy link
Contributor

ndench commented Sep 22, 2021

Over the past week, Psalm in our CI build has been returning a lot of errors related to UndefinedClass.
It's always related to the same one or two classed that it thinks are undefined, however they are not.
I've fully checked the path to the classes multiple times and they definitely exist, in the right place, with the right character cases in their FQCN and path. Additionally, the app successfully loads the classes all the time and I've written a short script that requires the autoloader and instantiates the classes that psalm can't find which I run right before running psalm.

I'm unable to reproduce this in my local dev environment, but it's very consistent in the CI environment. I also can't seem to make any sort of reproducer.

I've tried:

  • Both manually specifying the autoloader and not specifying it at all in psalm.xml
  • Clearing the psalm cache
  • Running with --no-cache
  • Running psalm multiple times in CI
  • Running composer dump-autoload and composer dump-autoload -o before running psalm

From the couple of classes it cannot find, it generates a bunch of other errors such as MoreSpecificReturnType, LessSpecificReturnType, MixedInferredReturnType, UndefinedDocBlockClass, MixedMethodCall, MixedInferredReturnType, etc. I believe these all stem from not being able to determine the type of the objects because their class is "undefined".

Our codebase is very modern and we've been using psalm from day 1, so it's not like we have a custom autoloader or anything weird. I know that it will be very hard for anyone to track this down and I'm certainly very happy to do the work. But I have no idea where to go now in order to debug further.

@orklah
Copy link
Collaborator

orklah commented Sep 22, 2021

When you say you tried with --no-cache, you mean that the issue is reproduced in your CI event when not using cache?

Can you try with --threads=1 ?

Do you see what those classes may have in common?

@ndench
Copy link
Contributor Author

ndench commented Sep 22, 2021

Yes, everything I listed in the issue description I tried and the issue is still reproduced in the CI environment but not the local dev environment.

I can't see anything that the classes have that is not in plenty of other classes throughout the codebase as well (and even in the same namespace). FYI they're Doctrine entities, so they have a bunch of properties with annotations, a constructor, a bunch of getters/setters and some other domain functions but they're pretty light. All the other entities have all this as well.

I've tried with --threads=1 and that seems to solve the issue. I also just now tried with --debug as well, and that also solves the issue. I'm not really sure why specifying these options allows psalm to find the classes though?

@orklah
Copy link
Collaborator

orklah commented Sep 22, 2021

IIRC, debug also puts the number of threads to 1.

This means it's a concurrency issue. It may also be the reason why your local environment doesn't reproduce (either because you're on windows or in an environment that doesn't allows threads or a different number)

It may be because of a cyclic dependancy but I have no idea how Psalm handle that kind of things...

@ndench
Copy link
Contributor Author

ndench commented Sep 22, 2021

Oh right, that makes sense. Do you have any idea how I could debug this further?

FWIW my dev environment is ubuntu 18.04 and CI is running on a docker container based off node:12-stretch.

@ndench
Copy link
Contributor Author

ndench commented Sep 22, 2021

I've also tried deleting everything out of the offending classes so they're just an empty class, but this still causes the error.
So I don't think it can be a cyclic dependency because those classes are no longer depending on any other classes?

@orklah
Copy link
Collaborator

orklah commented Sep 22, 2021

I'm not sure, I'd try experimenting with the number of threads to see if there's a sweet spot with as many threads as possible without crashing. I'm not sure if --debug-by-line works alongside with threads but if it does, maybe check the order of classes analysis when it crash opposed to when it doesn't crash. I'm not sure what you could do even if you get this information though

@ndench
Copy link
Contributor Author

ndench commented Sep 23, 2021

Thanks for the help. It seems that threads=2 work consistently for now (ie. finds the classes properly). However I have no idea why. --debug-by-line generates a lot of output so it's quite hard to sift through. I've tried running psalm over smaller portions of the codebase to make it easier to digest the debug output. However it doesn't seem to reproduce the error unless it's run across the entire codebase.

I'll keep digging into the debug output to try identify the root cause, but might take a while.

Update:
threads= only intermittently works. The issue is still reproduced 50% of the time.

@ndench
Copy link
Contributor Author

ndench commented Sep 23, 2021

I've run with --debug-by-line with each of the following:

  • --threads=1 -> does not reproduce the issue
  • --threads=2 -> intermittently reproduce the issue
  • --threads=3 -> does reproduce the issue
  • --threads=4 -> does reproduce the issue

When the issue is reproduced, there are 2 classes that result in UndefinedClass and it's always the same 2 classes. Occasionally only 1 of them will give UndefinedClass and the other does not. But there doesn't seem to be any other classes this happens on. Both the classes are fairly simple Doctrine entities, there are plenty of other entities (both more simple and more complex) in the same namespace and other namespaces. There must be something unique about these classes for them to fail so consistently, but I can't seem to identify it.

With --debug-by-line psalm prints the following logs for both the classes (regardless of whether the issue is reproduced or not):

  1. Parsing /src/Path/To/Entity.php
  2. Deep scanning /src/Path/To/Entity.php
  3. Have populated App\Path\To\Entity
  4. Getting /src/Path/To/Entity.php
  5. Analyzing /src/Path/To/Entity.php

1 and 2 always happen immediately after each other, as do 4 and 5. Although the order in which entity is processed can change.
eg. in one build the order might happen like:

  • Parsing EntityA
  • Deep scanning EntityA
  • Parsing EntityB
  • Deep scanning EntityB
  • Have populated EntityA
  • Have populated EntityB
  • Getting EntityA
  • Analyzing EntityA
  • Getting EntityB
  • Analyzing EntityB

But in the next build, EntityB might come first:

  • Parsing EntityB
  • Deep scanning EntityB
  • Parsing EntityA
  • Deep scanning EntityA
  • Have populated EntityB
  • Have populated EntityA
  • Getting EntityB
  • Analyzing EntityB
  • Getting EntityA
  • Analyzing EntityA

The order doesn't really seem to matter IMHO, since the issue is very consistent but the order is not. Even their order compared to other classes is different across all builds. Although I haven't broken down the order of every class because there's way to many to make much sense of that.

Additionally, for each of the classes the following errors are identified by psalm when the issue is reproduced:

- UnusedProperty - cannot find any references to private property` for each property in the class
- UndefinedClass - cannot set properties of undefined class` for each place the property is written in the class
- UndefinedClass - Class, interface or enum named App\Path\To\Entity does not exist` every time the entity is the type of a property or parameter
- UndefinedDocblockClass - Docblock-defined class, interface or enum App\Path\To\Entity does not exist` every time the entity
- MoreSpecificReturnType - The declared return type  'App\Path\To\Entity&App\Path\To\Entity' is more specific than the inferred return type 'object'
- LessSpecificReturnType - The type 'object' is more general than the declared return type 'App\Path\To\Entity&App\Path\To\Entity

After digging through this for a while, I cannot identify anything that seems to be the root cause. Not sure if any of the above give you any ideas as to where to dig further?

Happy to share the logs privately if you think that will help? Just flick me your email address on LinkedIn or something.

@weirdan
Copy link
Collaborator

weirdan commented Oct 5, 2021

Does adding or removing an entity change anything?

@ndench
Copy link
Contributor Author

ndench commented Oct 6, 2021

Adding an entity doesn't change anything. Removing the entities that cause the problem result in even more of the same errors (ie. UndefinedClass, etc). Which seems to mean that psalm is able to find the class sometimes and not others.

@tdgroot
Copy link

tdgroot commented Oct 15, 2021

I am also experiencing the same issues where specific classes are found 'undefined' in CI (specifically Bitbucket Pipelines), while tests pass perfectly fine on local environment. I have tried increasing memory, adding --no-cache and --no-diff, but to no avail.

I'll try adding the --threads=1 flag and report if that fixes the issue.

The issue i's also not specific to Doctrine, because I'm experiencing this on a Magento 2 project.

@orklah
Copy link
Collaborator

orklah commented Oct 15, 2021

Do you use anything out of ordinary? Plugins, Stubs, autoloader...?

@ndench
Copy link
Contributor Author

ndench commented Oct 17, 2021

I'm using the doctrine plugin and just the ordinary composer autoloader. No custom stubs (besides the doctrine plugin).

@orklah orklah added the bug label Oct 20, 2021
@kieljohn
Copy link

I believe we are also affected by this, and the --threads=1 appears to reduce the impact.

./vendor/bin/psalm -v
Psalm 4.30.0@d0bc6e25d89f649e4f36a534f330f8bb4643dd69

We are using no psalm plug-ins on this product, (Symfony 3.x codebase)

Macbook Pro M1 running in PHP 8.1, although also happening within docker images built FROM php:7.4-fpm in CI.

php -v
PHP 8.1.12 (cli) (built: Nov  2 2022 15:48:23) (NTS)
<?xml version="1.0"?>
<psalm
    errorLevel="2"
    reportMixedIssues="false"
    resolveFromConfigFile="true"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns="https://getpsalm.org/schema/config"
    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
    errorBaseline="./psalm-baseline.xml"
    autoloader="./vendor/autoload.php"
>

Re-running psalm successively in a script 10x times will lead to different number of errors outputted each time.

./vendor/bin/psalm --no-file-cache --no-reflection-cache --no-cache --long-progress --no-suggestions --config=psalm.xml

x10 will result in

31 errors found
31 errors found
100 errors found
31 errors found
31 errors found
31 errors found
100 errors found
31 errors found
31 errors found
31 errors found

The extra errors reported are typically:

  • Undefined

--threads=1 will consistently report 31 errors found over 10 iterations. We will be updating the CI to run in a single thread to keep things moving for our development team.

Posting as it might help others and as a 'me too', but I appreciate it might not be enough to debug

@orklah
Copy link
Collaborator

orklah commented Nov 22, 2022

@kieljohn I'd be interested to know if you can reproduce on our master branch (which will soon be released as Psalm 5). There has been some internal changes that can prevent some multithreading issues, it may resolve this hopefully

@gsteel
Copy link
Contributor

gsteel commented Jan 11, 2023

Experiencing the same issue on psalm 5.4 but in reverse where CI is fine, but locally the same class (every time) is undefined. Having read the comments here, running --threads=1 --no-cache solved the issue. I tried a mixture of options including --no-reflection-cache, individually and combined until I found the winning combination.

Disabling plugins had no effect. Issues reported are consistent every time. My baseline is empty, but removing it from config also had no effect.

@ArtemGoutsoul
Copy link
Contributor

Experiencing a similar issue in 5.17. Very hard to reproduce.
In CI (dh actions -> docker -> psalm) I fixed it by disabling cache, but running with multiple threads.
Locally I run in docker, with persistent cache. My solution was to disable cache, still running with multiple threads though.

Seems like psalm fails to load some classes when caching is enabled and there's cached data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants