Repository navigation
Any simple way to completely disable messages from plugin checks at default SUGGESTION level? #5387
Description
See uber/NullAway#1363. We included a new RequireExplicitNullMarking check in NullAway 0.12.13 at SUGGESTION level. It's matches get reported as NOTEs since it's at SUGGESTION, so they still show up in the console for a Gradle build by default (I imagine their printing is build-system dependent). Is there any easy way to provide this check but have it as completely off by default, with no printed messages? I think I can hack something using VisitorState.severityMap() to just return Description.NO_MATCH when the check is at SUGGESTION but it'd be nice to have a way to completely disable the check. Thanks!
Activity
msridhar commentedon Dec 3, 2025
@rickie I think a bunch of the error-prone-support checks are at
SUGGESTIONlevel by default; have you all run into this issue?msridhar commentedon Dec 5, 2025
@cushon assuming this is not possible now except via hacks, would you be open to some kind of PR to enable this? Adding some kind of
OFFseverity seems like an obvious way to accomplish it, but not sure what else that might impact. I modified our code to work around this issue for now, but the change seems fragile.cushon commentedon Dec 5, 2025
I think that having a
-XepDisableAllSuggestionsflag would be consistent with other configuration we already have. Would that meet the requirements here?I think that internally checks with 'suggestion' severity are mostly things that get run as refactorings, not surfaced in build logs or code review. With the external Error Prone integrations, build diagnostics are sort of the only hammer we have, so suggestions get surfaced there. Similar to #5367, I tend to think that only putting errors in build logs is often a good choice, and finding other ways to surface non-error level checks.
Maybe we should have a more generic mechanism to configure all checks of a particular default severity as a different severity, or just go ahead and add all combinations of
-XepDisableAll{Suggestions,Warnings,Errors}and-XepAll{Suggestions,Warnings,Errors}As{Suggestions,Warnings,Errors}. I'm not requesting a PR for that, maybe let's just add-XepDisableAllSuggestionsfor now :)rickie commentedon Dec 5, 2025
@msridhar We don't run into this issue, as we always resolve all cases when introducing a new check.
In our build, we use:
combined with the failOnWarning. Because of that, we don't have many violations. It will only give some messages when someone is working on code before it's merged.
msridhar commentedon Dec 5, 2025
My goal is to be able to ship a plugin check that is disabled by default (like the "Experimental" checks shipped with Error Prone). So I don't think
-XepDisableAllSuggestionsdoes quite what I want, since by default our check would still be enabled. IfSUGGESTIONlevel checks were disabled by default and needed to be explicitly enabled (so, something like-XepEnableAllSuggestions), that would do the trick. So I think I'm proposing something similar to #5367 but for suggestions instead.tbroyer commentedon Dec 5, 2025
There should probably be a new annotation, or a new attribute in
@BugPattern, to signify that the check should be disabled by default, while still providing a default severity for when one wants to enable it without specific level, like for disabled-by-default built-in checks.cushon commentedon Dec 6, 2025
That SGTM
I am also curious what you all think of disabling suggestions and warnings by default, and only emitting errors by default, and allowing suggestions and warnings to be enabled with
-XepEnableAllSuggestions/-XepEnableAllWarnings? if that would be disruptive I don't think there's a rush to do it, but in hindsight it seems like a better experience.tbroyer commentedon Dec 6, 2025
I wouldn't mind suggestions being disabled by default, partly because there's none enabled by default, and I don't think I use any check that defaults to a suggestion level.
I use
-Werrorthough, so that would have to be replaced with-XepAllWarningsAsErrorsor complemented with-XepEnableAllWarnings. Such a change would be disruptive though, as all of a sudden you update Error Prone and many checks aren't applied anymore without notice (other than a few words in the release notes).Would it be possible to have (discovered / extension) suggestion-level checks disabled by default but without entirely turning off the suggestion level? I.e. the NullAway check (from 0.12.13) being disabled by default, but
-Xep:RequireExplicitNullMarking(without level) enabling it at suggestion level? I personally wouldn't use it, but it might be a good compromise (assuming people don't use the suggestion level much in their custom checks)Personally, I'd say the NullAway check should "default to error (or warning) level, disabled by default", so enabling it with its default level would be "meaningful" (like other disabled-by-default built-in checks, e.g. SystemExitOutsideMain or DefaultLocale)
msridhar commentedon Dec 6, 2025
I agree that updating Error Prone to disable warnings by default could be disruptive. If we add the feature to the
@BugPatternannotation to disable a check unless explicitly enabled, I would use it for the extra NullAway check and also probably set the default level to WARN. For suggestion-level checks it seems they don't come up that often in practice. I only used SUGGESTION because I thought it was equivalent to disabled-by-default for some reason.cushon commentedon Dec 8, 2025
Having a mechanism for indicate that plugin checks should be disabled by default, separate from their default severity, SGTM.
(I see that we once had a
SeverityLevel.NOT_A_PROBLEMto indiciate that a check should be disabled, but that conflates whether it's enabled with what it's severity is: ff0c9b2.)msridhar commentedon Dec 8, 2025
I can try to put up a PR for this but it will probably take me some time...in case someone else wants to take it on 🙂
feat(nullaway): add a checker for unrecognized JSpecify annotation lo…
feat(nullaway): add a checker for unrecognized JSpecify annotation lo…
feat(nullaway): add a checker for unrecognized JSpecify annotation lo…
feat(nullaway): add a checker for unrecognized JSpecify annotation lo…
feat(nullaway): add a checker for unrecognized JSpecify annotation lo…
feat(nullaway): add a checker for unrecognized JSpecify annotation lo…
Add a check for nullness annotations in unrecognized locations
Add a check for nullness annotations in unrecognized locations
Add a check for nullness annotations in locations JSpecify does not r…
Add a check for nullness annotations in locations JSpecify does not r…
Add a check for nullness annotations in locations JSpecify does not r…
Add a check for nullness annotations in locations JSpecify does not r…