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

psalm.phar fails to check the source it was built from #618

Closed
weirdan opened this issue Mar 25, 2018 · 4 comments
Closed

psalm.phar fails to check the source it was built from #618

weirdan opened this issue Mar 25, 2018 · 4 comments

Comments

@weirdan
Copy link
Collaborator

weirdan commented Mar 25, 2018

Starting from 83811e6 psalm.phar reports errors when run on the psalm source it was built from. I was able to track it down to differences in bundled src/Psalm/PropertyMap.php, where failing version had PhpParser\* replaced with PsalmPhar\\PhpParser, and it seems it was done deliberately (see patcher changes in that commit).

I was under impression that file was meant to serve as a kind data asset, so it it shouldn't be scoped (well, maybe namespace part, but nothing else).

These patcher changes did not produce any differences in any of the other files that patcher is meant for (stubs and call map).

@muglug
Copy link
Collaborator

muglug commented Mar 25, 2018

Yup, this is by design. The code in src/Psalm/PropertyMap.php that is not in Phan's equivalent is only there to help Psalm analyse itself.

I added an additional step to ensure that the scoped source passes its own Psalm checks, more or less (i.e. that the scoping doesn't break any behaviour).

The linked commit includes that self-check on the scoped files when running bin/build-phar.sh

@weirdan
Copy link
Collaborator Author

weirdan commented Mar 25, 2018

Well, I see some generally useful definitions in that file, as well as those Psalm-specific changes (PhpParser\*). To me it seems that psalm needs a way to configure those per-project. If this was the case, phar could bundle generic property map, and psalm.xml would add anything needed to check the project at hand (psalm itself in this case).

@muglug
Copy link
Collaborator

muglug commented Mar 25, 2018

To me it seems that psalm needs a way to configure those per-project

Agreed!

@muglug muglug closed this as completed Mar 26, 2018
@muglug
Copy link
Collaborator

muglug commented Mar 26, 2018

Related to #482

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

No branches or pull requests

2 participants