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

Fix #3461 by removing file from issues after removing last issue #3462

Merged
merged 1 commit into from
May 27, 2020
Merged

Fix #3461 by removing file from issues after removing last issue #3462

merged 1 commit into from
May 27, 2020

Conversation

still-dreaming-1
Copy link
Contributor

No description provided.

@still-dreaming-1
Copy link
Contributor Author

still-dreaming-1 commented May 26, 2020

You can ignore the branch name being a little strange. This ended up going in a different direction than I imagined. I was originally thinking I would just change the $issues type as used in AfterAnalysisInterface. I tried changing it to @param array<string, non-empty-list<IssueData>> $issues, hoping that would cause Psalm to reveal where that is not honored in the code. But there is a Psalm suppression preventing it from surfacing, and it says it is because of a bug. I tried deleting that comment, but then there was other technical debt that got revealed that I didn't know how to deal with. So I abandoned that approach and just fixed the bug directly in IssueBuffer::remove() as suggested by @muglug.

I tested this in my plugin, and it does fix #3461.

@muglug muglug merged commit 700b5dd into vimeo:master May 27, 2020
@muglug
Copy link
Collaborator

muglug commented May 27, 2020

Awesome, thanks!

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

Successfully merging this pull request may close these issues.

2 participants