Skip to content

Reprocess missing class-like storage events#10724

Closed
ohader wants to merge 4 commits intovimeo:masterfrom
ohader:10350-intersections
Closed

Reprocess missing class-like storage events#10724
ohader wants to merge 4 commits intovimeo:masterfrom
ohader:10350-intersections

Conversation

@ohader
Copy link
Copy Markdown
Contributor

@ohader ohader commented Feb 19, 2024 •
edited
Loading

The scenario described in #10350 fails, since not all other references used in intersection types were complete resolved when scanning a particular file. Instead of catching exceptions during the type resolution process - which would just "mute" the problem and drop important information - those failures in resolving class-like storages are tracked for a potential rescanning.

This PR does the following:

  • introduces a new ClassStorageNotFoundException (instead of global InvalidArgumentException) to handle missing class-like storage references specifically
  • tracks those storage failures during the scanning process in $files_to_rescan for later processing
  • adds bool return type to \Psalm\Internal\Codebase\Scanner::scanAPath() - whether scan was successful
  • modifies Codebase::scanFiles() to actually reprocess those files to be rescanned - after changes of the previous run were populated to the codebase
  • adds test cases for the known scenario of issue Internal Psalm Error: Could not get class storage for <interface> #10350

Fixes: #7520
Fixes: #10350
Fixes: #10152

TODO

  • tackle \Psalm\Internal\Codebase\Scanner::scanAPath() return type in forked processes - currently only tested with $pool_size = 1 in test cases

@ohader
Add more tests for native intersection types in class properties
c022deb
@ohader
Introduce new ClassStorageNotFoundException for missing class storages
3bd76e3
@ohader
Rescan files on missing class storage items
29a4b00 
In case not all class storage items were available during
a given file scan, those failed files will be rescanned
later with more populated details in the codebase.

There's a maximum of 10 rescans to avoid endless recursions.
@ohader ohader marked this pull request as draft February 19, 2024 14:20
@ohader ohader mentioned this pull request Feb 19, 2024
Merged
@ohader

This comment was marked as off-topic.

Sign in to view
@weirdan
Copy link
Copy Markdown
Collaborator

weirdan commented Feb 19, 2024

introduces a new ClassStorageNotFoundException (instead of global InvalidArgumentException) to handle missing class-like storage references specifically

It was literally my next step 😁 :

image

@weirdan
Copy link
Copy Markdown
Collaborator

weirdan commented Feb 19, 2024

tracks those storage failures during the scanning process in $files_to_rescan for later processing

I wonder if rescanning whole files is really necessary. My idea was to walk classlikes / functionlikes after the codebase population and tighten the intersections. E.g. if the return method was left as MySessionHandler&SessionHandlerInterface where MySessionHandler implements SessionHandlerInterface (because either MySessionHandler or SessionHandlerInterface were unavailable during the initial scan) it would be tightened to MySessionHandler.

@ohader
Copy link
Copy Markdown
Contributor Author

ohader commented Feb 19, 2024 •
edited
Loading

I wonder if rescanning whole files is really necessary. My idea was to walk classlikes / functionlikes after the codebase population and tighten the intersections. E.g. if the return method was left as MySessionHandler&SessionHandlerInterface where MySessionHandler implements SessionHandlerInterface (because either MySessionHandler or SessionHandlerInterface were unavailable during the initial scan) it would be tightened to MySessionHandler.

I had something similar in my mind as well, but did not find a good way to "communicate" from the specific inner parts to the outer parts that would trigger those optimizations after codebase population. Basically like:

  • hold unresolved/failed statements/names in memory (probably required for anonymous classes)
  • process different strategies to resolve the types (e.g. simplifying the types like ArrayObject&Countable → ArrayObject; if that fails, reprocess the parser statements; if that fails reprocess the whole file) - this might be an additional setting in psalm.xml config file
  • in case not all previously unresolved statements/names could be resolved, the process could either fail with an exception or continue with a generic type, e.g. TMixed - this might be another config option in psalm.xml

@simonberger
Copy link
Copy Markdown

simonberger commented Feb 20, 2024

I would have tested if this attempt makes a difference for the crash I still have, but unfortunately you branched it from psalm 6 instead of 5 which I not yet are able to install.

@ohader
Copy link
Copy Markdown
Contributor Author

ohader commented Feb 20, 2024

I would have tested if this attempt makes a difference for the crash I still have, but unfortunately you branched it from psalm 6 instead of 5 which I not yet are able to install.

Uh, sorry... I usually start working in main (master) and the back-port to stable branches. I forgot it's different here...
Let's close this PR (I can take care of the forward port later, once we found a good foundation code-wise).

→ 5.x PR at #10730

@ohader ohader closed this Feb 20, 2024
@ohader ohader mentioned this pull request Feb 20, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ohader @weirdan @simonberger