Skip to content

Conversation

@kelvinou01
Copy link
Contributor

Before this PR

We are working on -PerrorProneRemoveUnused, a mode which removes all unused suppressions. This mode requires us to turn on the ignore suppressions option in error-prone, or else we can't descend into suppressed trees to chec.

Error-prone's SuppressibleTreePathScanner doesn't respect this flag. This is not expected behavior, as ErrorProneScanner respects the flag. We've opened google/error-prone#5255, but I think it will take some time to be merged. In the mean time, let's add the option to insert some bytecode so the SuppressibleTreePathScanner respects this flag when we need it to.

After this PR

==COMMIT_MSG==
Add the ability to ignore suppressions in SuppressibleTreePathScanner
==COMMIT_MSG==

Possible downsides?

@changelog-app
Copy link

changelog-app bot commented Oct 16, 2025

Generate changelog in changelog/@unreleased

Type (Select exactly one)

  • Feature (Adding new functionality)
  • Improvement (Improving existing functionality)
  • Fix (Fixing an issue with existing functionality)
  • Break (Creating a new major version by breaking public APIs)
  • Deprecation (Removing functionality in a non-breaking way)
  • Migration (Automatically moving data/functionality to a new system)

Description

Add the ability to ignore suppressions in SuppressibleTreePathScanner

Check the box to generate changelog(s)

  • Generate changelog entry

@changelog-app
Copy link

changelog-app bot commented Oct 16, 2025

Successfully generated changelog entry!

What happened?

Your changelog entries have been stored in the database as part of our migration to ChangelogV3.

Need to regenerate?

Simply interact with the changelog bot comment again to regenerate these entries.


📋Changelog Preview

✨ Features

  • Add the ability to ignore suppressions in SuppressibleTreePathScanner (#200)

Comment on lines 60 to 61
.collect(Collectors.toMap(
Map.Entry::getKey, Map.Entry::getValue, (val1, val2) -> val1 + "," + val2));
Copy link
Contributor

Choose a reason for hiding this comment

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

EntryStream has a slightly shorter .toMap(mergeFunction) method.

Copy link
Contributor

Choose a reason for hiding this comment

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

So this is letting us natively has the same option multiple times, which then builds them up as a list (aka comma separated string argument)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we actually need this change?

Copy link
Contributor Author

@kelvinou01 kelvinou01 Oct 16, 2025

Choose a reason for hiding this comment

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

Oops I think this was required to handle some extra options in the next PR. Can't remember why exactly it was needed, but let's remove it from this PR.

@bulldozer-bot bulldozer-bot bot merged commit b997df0 into develop Oct 16, 2025
5 checks passed
@bulldozer-bot bulldozer-bot bot deleted the okelvin/ignore-suppressions branch October 16, 2025 16:40
@autorelease3
Copy link

autorelease3 bot commented Oct 16, 2025

Released 2.21.0

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.

3 participants