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

refactor(2095): Refactor UnclosedResourcesProcessor #557

Merged
merged 7 commits into from
Jun 7, 2021

Conversation

slarse
Copy link
Collaborator

@slarse slarse commented Jun 3, 2021

I got stuck trying to fix #540 because this processor is a bit complex. This PR completely refactors the UnclosedResourcesProcessor to make it easier to reason about.

@slarse
Copy link
Collaborator Author

slarse commented Jun 3, 2021

Coverage on diff less than 100% as some conditions never evaluate to false (for example, we don't have any case where isAssignmentWithWriteToResolvedLocalVariable(parent) evaluates to false). But this preserves the semantics from before the refactoring: the coverage wasn't 100% before the refactoring either.

@slarse slarse requested a review from fermadeiral June 3, 2021 16:27
@slarse slarse closed this Jun 7, 2021
@slarse slarse reopened this Jun 7, 2021
@slarse
Copy link
Collaborator Author

slarse commented Jun 7, 2021

Just closing/opening to try to run CI again, Codecov is failing across all projects...

@slarse
Copy link
Collaborator Author

slarse commented Jun 7, 2021

Codecov uploads are just failing across every single project... I'm sick of Codecov failing now, time to look for an alternative.

EDIT: I'm now ignoring Codecov upload failures in Sorald. I don't have time to replace it right now, we'll just have to do code coverage manually for the time being.

@fermadeiral
Copy link
Collaborator

@slarse, it looks good to me.

Should I merge?

@slarse
Copy link
Collaborator Author

slarse commented Jun 7, 2021

@fermadeiral Yes, I've verified locally that coverage is acceptable, you can take my word for it :)

@fermadeiral fermadeiral merged commit 8ac1873 into master Jun 7, 2021
@fermadeiral fermadeiral deleted the issue/540-refactor-UnclosedResourcesProcessor branch June 7, 2021 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Processor for S2095 (UnclosedResources) introduces new resource leak
2 participants