Request for BugChecker.checkSuppression to be initialized _after_ construction
- Dominant language
- Java
- Stars
- 7.2k
- Forks
- 820
- Avg merge
- 5h 9m
- Merged PRs (30d)
- 50
Description
### Description of the problem / feature request:
We'd like for `BugChecker.checkSuppression` to be initialized after full checker construction (e.g. lazily, on first use), rather than as part of the `BugChecker()` constructor.
In particularly, we'd like for `BugChecker` subclasses to be fully instantiated before `this.customSuppressionAnnotations()` is called to populate this predicate.
### Feature requests: what underlying problem are you trying to solve with this feature?
We are building an Error Prone checker ([NullAway](https://github.com/uber/NullAway)), which defines customs exclusion/suppression annotations as part of its configuration (i.e. `ErrorProneFlags`). This means that, as part of its constructor, it will set up a `this.customSuppressionAnnotations` field, which will be then returned by the `this.customSuppressionAnnotations()` method.
In earlier Error Prone versions, this was enough to add flag-passed annotations to the list of custom suppressions.
However, for EP 2.4.x, after [this optimization](https://github.com/google/error-prone/commit/cf075b47d7747614cce2d20ebca98be6cc8a360b#diff-76beb6c76fa9727e670f08daaf248845R114) (which otherwise sounds great, btw! 1.1% javac time reduction is a big win!), this approach is no longer viable for us. Because `BugChecker()` (the superclass constructor) executes before `NullAway(ErrorProneFlags flags)`, we are no longer able to change the result of `this.customSuppressionAnnotations()` before it gets queried and frozen into the `BugChecker.checkSuppression` predicate.
We can't use a static initializer or a field initializer to get ahead of the `BugChecker()` constructor, as we need access to the `ErrorProneFlags` object.
### What version of Error Prone are you using?
Upgrading 2.3.2 to 2.4.0 uncovered this issue for us.
### Have you found anything relevant by searching the web?
See the [analysis](https://github.com/uber/NullAway/pull/412#issuecomment-663190522) on NullAway's GitHub. We have a PR to mitigate this, but it involves adding overhead on our end.
An ideal solution, which would not require compromising the performance optimization that introduced this limitation, is to still freeze all custom suppressions into the immutable `BugChecker.checkSuppression` predicate, but to do so after full object initialization. This could be either a method called after the constructor when retrieving a new `BugChecker` instance, or else just lazy-initialization whenever `BugChecker.suppressedByAnyOf(...)` is called.
I can submit a PR for the later case, but I first want to confirm this is seen as valid issue / feature request by EP.
Another option is to provide a setter for `BugChecker.checkSuppression`, but this involves increasing the API surface of EP. `BugChecker.customSuppressionAnnotations()` is already protected, and I'd argue developers would expect it to be called after object construction rather than only during super-class initialization.
Contributor guide
Assessment
This issue has not been assessed yet.