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

Detect unused properties that are written to inside arrays #7129

Merged
merged 2 commits into from Dec 11, 2021

Conversation

muglug
Copy link
Collaborator

@muglug muglug commented Dec 11, 2021

In my ongoing deep audit of the Psalm codebase I found a property that wasn't being read, and tracked it down to an issue detecting unused properties with array types.

This detects those, and removes similar properties from Psalm's codebase.

@muglug muglug added the release:fix The PR will be included in 'Fixes' section of the release notes label Dec 11, 2021
@orklah
Copy link
Collaborator

orklah commented Dec 11, 2021

That's cool! Thanks!

@orklah orklah merged commit 858be40 into master Dec 11, 2021
@weirdan weirdan added the release:removed The PR will be included in 'Removed' section of the release notes label Dec 11, 2021
@weirdan
Copy link
Collaborator

weirdan commented Dec 11, 2021

Those public properties are part of the public API, so they should be deprecated first and then dropped in a major release.

@orklah @muglug I don't think it should be merged just yet.

@muglug
Copy link
Collaborator Author

muglug commented Dec 11, 2021

True, but I think the only property that has any chance of being used used is the constant attributes one. It’s your call to revert the second commit, but I’m happy to PR the return of the constant attributes property for a smaller diff.

weirdan added a commit to weirdan/psalm that referenced this pull request Dec 26, 2021
This partially fixes BC break introduced in vimeo#7129
@weirdan weirdan added release:feature The PR will be included in 'Features' section of the release notes and removed release:fix The PR will be included in 'Fixes' section of the release notes labels Dec 26, 2021
@muglug muglug deleted the muglug-detect-unused-properties-with-arrays branch January 15, 2022 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature The PR will be included in 'Features' section of the release notes release:removed The PR will be included in 'Removed' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants