Skip to content

Conversation

@kelvinou01
Copy link
Contributor

@kelvinou01 kelvinou01 commented Nov 29, 2025

Before this PR

  1. StrictUnusedVariable opts out of SuppressibleTreePathScanner's suppression mechanism, and checks for suppressions manually. This interferes with suppressible-error-prone's removeUnused mode, which relies on modifying SuppressibleTreePathScanner.
  2. StrictUnusedVariable has a quirky setup to deal with alt-names, so that@SuppressWarnings("UnusedVariable") suppresses it, but errorProneOpotions.disable("UnusedVariable") does not disable it. This also interferes with RemoveUnused mode, as it doesn't know @SuppressWarnings("UnusedVariable") is a suppression for StrictUnusedVariable

After this PR

  1. I think UnusedVariable (from which StrictUnusedVariable is mostly copied from) didn't use SuppressibleTreePathScanner because the class was new at the time (2022), but now we can switch to using SuppressibleTreePathScanner to get our modifications
  2. It is reasonable that @SuppressWarnings("UnusedVariable") should only suppress UnusedVariable, @SuppressWarnings("StrictUnusedVariable") should only suppress StrictUnusedVariable, and SuppressWarnings("unused") is the catch-all suppression for all unused checks (including UnusedMethod). Let's implement this behavior so removeUnused mode can work with StrictUnusedVariable.

==COMMIT_MSG==
StrictUnusedVariable should respect -XoptIgnoreSuppressionAnnotations; @SuppressWarnings("UnusedVariable") no longer suppresses StrictUnusedVariable
==COMMIT_MSG==

Possible downsides?

@changelog-app
Copy link

changelog-app bot commented Nov 29, 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

StrictUnusedVariable should respect -XoptIgnoreSuppressionAnnotations

Check the box to generate changelog(s)

  • Generate changelog entry

@kelvinou01 kelvinou01 changed the title StrictUnusedVariable should respect -XoptIgnoreSuppressionAnnotations Make StrictUnusedVariable use SuppressibleTreePathScanner, and it shouldn't be suppressed by @SuppressWarnings("UnusedVariable") Nov 30, 2025
@changelog-app
Copy link

changelog-app bot commented Nov 30, 2025

Successfully generated changelog entry!

Need to regenerate?

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


📋Changelog Preview

🐛 Fixes

  • StrictUnusedVariable should respect -XoptIgnoreSuppressionAnnotations (#3384)

@kelvinou01 kelvinou01 changed the title Make StrictUnusedVariable use SuppressibleTreePathScanner, and it shouldn't be suppressed by @SuppressWarnings("UnusedVariable") Make StrictUnusedVariable use SuppressibleTreePathScanner, @SuppressWarnings("UnusedVariable") no longer suppresses StrictUnusedVariable Nov 30, 2025
@kelvinou01 kelvinou01 changed the title Make StrictUnusedVariable use SuppressibleTreePathScanner, @SuppressWarnings("UnusedVariable") no longer suppresses StrictUnusedVariable Make StrictUnusedVariable use SuppressibleTreePathScanner; @SuppressWarnings("UnusedVariable") no longer suppresses StrictUnusedVariable Nov 30, 2025
@kelvinou01 kelvinou01 changed the title Make StrictUnusedVariable use SuppressibleTreePathScanner; @SuppressWarnings("UnusedVariable") no longer suppresses StrictUnusedVariable StrictUnusedVariable should respect -XoptIgnoreSuppressionAnnotations; @SuppressWarnings("UnusedVariable") no longer suppresses StrictUnusedVariable Dec 1, 2025
@kelvinou01
Copy link
Contributor Author

kelvinou01 commented Dec 1, 2025

When this rolls out, places with @SuppressWarnings("UnusedVariable") will additionally suppress for-rollout:StrictUnusedVariable. Then, when removeUnused mode is fixed, we can remove the unused suppressions on UnusedVariable.

@kelvinou01
Copy link
Contributor Author

kelvinou01 commented Dec 1, 2025

DO NOT MERGE (YET)

We're currently moving all the error-prone infrastructure in gradle-baseline (including baseline-error-prone, baseline-nullaway) to https://github.com/palantir/baseline-error-prone. This PR should be opened in the new repository once setup is completed.

@kelvinou01
Copy link
Contributor Author

Hey there, our error-prone infrastructure has been moved to https://github.com/palantir/baseline-error-prone.

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.

1 participant