Skip to content

Conversation

asukaminato0721
Copy link
Contributor

fix #1309

@meta-cla meta-cla bot added the cla signed label Oct 16, 2025
@asukaminato0721 asukaminato0721 marked this pull request as ready for review October 16, 2025 18:49
@asukaminato0721 asukaminato0721 changed the title fix 1309 fix reports the wrong number of removed ignores Oct 16, 2025
@yangdanny97
Copy link
Contributor

Hi, do you mind writing up a quick summary of the nature of the bug and how you fixed it? It would make the logic a bit easier to follow. I can see that it fixes the issue, but it would be useful to know why certain things happen.

@asukaminato0721
Copy link
Contributor Author

old: incremented the counter for every candidate line flagged by find_unused_ignores, even if the line no longer contained a suppression marker.

new: split the work into' remove_unused_ignores_internal', returning a SmallMap<PathBuf, usize> of actual removals. This allows the public to log the correct totals and provides tests with direct access to the counts.

In the inner loop, now check the regex.is_match(line) before counting and mutating the line; untouched lines at the target positions are copied through without bumping the tally, so empty or whitespace-only rows don’t inflate the numbers.

Copy link

meta-codesync bot commented Oct 17, 2025

@rchen152 has imported this pull request. If you are a Meta employee, you can view this in D84954481.

Copy link
Contributor

@yangdanny97 yangdanny97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review automatically exported from Phabricator review in Meta.

Copy link

meta-codesync bot commented Oct 20, 2025

@rchen152 merged this pull request in 1676458.

@asukaminato0721 asukaminato0721 deleted the 1309 branch October 20, 2025 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--remove-unused-ignores reports the wrong number of removed ignores

3 participants