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

Scan preloaded stubs earlier #6213

Merged
merged 1 commit into from
Jul 31, 2021

Conversation

weirdan
Copy link
Collaborator

@weirdan weirdan commented Jul 31, 2021

This should prevent Psalm from sometimes marking user-defined classes as
built-in.

Fixes #5126
Fixes #5626

@weirdan weirdan requested a review from orklah July 31, 2021 20:07
@orklah
Copy link
Collaborator

orklah commented Jul 31, 2021

Would it also solve #5626 and #6203 ?

There's a couple repo that uses preloading:
https://github.com/lperamo/otra
https://github.com/nextcloud/notify_push

maybe we could try to see nothing breaks

Unfortunately I don't know those parts of the code so I'm not gonna be a big help here

@VincentLanglet
Copy link
Contributor

Tried on my work project and my personal project, it seems to fixed both.
Big thanks.

Just, you sometimes run

$this->config->visitPreloadedStubFiles($this->codebase, $this->progress);
$this->visitAutoloadFiles();

and sometimes

$this->visitAutoloadFiles();
$this->config->visitPreloadedStubFiles($this->codebase, $this->progress);

don't know if it change something.

@weirdan
Copy link
Collaborator Author

weirdan commented Jul 31, 2021

don't know if it change something.

It shouldn't, but I'd better get them consistent, lest anyone get confused by this later (myself included).

@weirdan
Copy link
Collaborator Author

weirdan commented Jul 31, 2021

notify_push seems to be fine, otra looks broken (missing directories and files), but it's not related to the PR.

I don't think it'll solve #6203, but I'm pretty sure it'll fix #5626 (unfortunately it misses reproducer).

This should prevent Psalm from sometimes marking user-defined classes as
built-in.

Fixes vimeo#5126
Fixes vimeo#5626
@weirdan weirdan force-pushed the scan-preloaded-stubs-earlier branch from c3f55dd to 2562e37 Compare July 31, 2021 22:37
@weirdan weirdan added this to the 4.9.2 milestone Jul 31, 2021
@weirdan weirdan merged commit edffb1a into vimeo:master Jul 31, 2021
@weirdan weirdan deleted the scan-preloaded-stubs-earlier branch July 31, 2021 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants