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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

query re: IssueData & plugins in 5.x #8411

Open
SignpostMarv opened this issue Aug 15, 2022 · 4 comments
Open

query re: IssueData & plugins in 5.x #8411

SignpostMarv opened this issue Aug 15, 2022 · 4 comments

Comments

@SignpostMarv
Copy link
Contributor

With IssueData being flagged as internal in the 5.x beta, is the interface for getting a hold of issues in the AfterAnalysisEvent going to change? I'm currently getting a bunch of InternalProperty issues being flagged since pulling in 5.x 馃

@psalm-github-bot
Copy link

Hey @SignpostMarv, can you reproduce the issue on https://psalm.dev ?

@orklah
Copy link
Collaborator

orklah commented Aug 15, 2022

PR for reference: #7268

I think the @internal was added in bulk based on the directory of the class. It doesn't seem to make sense to have an internal class sent to plugins.

So I'd personally vote to move the IssueData out of the Internal namespace and remove the @internal

@SignpostMarv
Copy link
Contributor Author

@orklah I'm seeing that in psalm 5.7, some classes are flagged as non-internal, but the constructors are flagged as internal- this seems to be a good way to allow types to be consumed etc.

With IssueData for example, because the entire class is flagged as internal then any use of the properties is also flagged with InternalProperty instances- it seems that taking @internal off the class and shifting it to the constructor only would allow the properties to be consumed without issue and leave only the constructor to be flagged in unit tests (which is where #9424 comes in).

@weirdan
Copy link
Collaborator

weirdan commented Feb 28, 2023

So I'd personally vote to move the IssueData out of the Internal namespace and remove the @internal

I'd also make it final if we do that move.

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

3 participants