You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/11/30 15:37:05 UTC
[GitHub] [solr] rmuir commented on a diff in pull request #1200: Invert error-prone configuration to be allow-list vs deny-list
rmuir commented on code in PR #1200:
URL: https://github.com/apache/solr/pull/1200#discussion_r1036127682
##########
gradle/validation/error-prone.gradle:
##########
@@ -62,53 +62,421 @@ allprojects { prj ->
options.errorprone.disableWarningsInGeneratedCode = true
options.errorprone.errorproneArgs = [
- // bug patterns related to our tests
- '-Xep:ExtendingJUnitAssert:OFF', // we inherit from LuceneTestCase which extends Assert
- '-Xep:UseCorrectAssertInTests:OFF', // and this is a consequence of the above
- '-Xep:CatchFail:OFF', // our code is generally descriptive enough, fix case by case if tests fail
- '-Xep:JUnit4TestNotRun:OFF', // RandomizedRunner finds unannotated test methods no problem
- '-Xep:StaticAssignmentInConstructor:OFF', // we assign SolrTestCaseJ4.configString in many tests, difficult to untangle
+ '-XepDisableAllChecks', // only enable specific checks
+ '-XepAllErrorsAsWarnings', // warnings still fail build by default, but allows usage of -Pjavac.failOnWarnings=false
+
+ // List of enabled/disabled checks
+ // Please keep this synced with https://errorprone.info/bugpatterns when upgrading!
+
+ // On by Default : ERROR
+
+ '-Xep:AlwaysThrows:ERROR',
+ '-Xep:AndroidInjectionBeforeSuper:ERROR',
+ '-Xep:ArrayEquals:ERROR',
+ '-Xep:ArrayFillIncompatibleType:ERROR',
+ '-Xep:ArrayHashCode:ERROR',
+ '-Xep:ArrayToString:ERROR',
+ '-Xep:ArraysAsListPrimitiveArray:ERROR',
+ '-Xep:AsyncCallableReturnsNull:ERROR',
+ '-Xep:AsyncFunctionReturnsNull:ERROR',
+ '-Xep:AutoValueBuilderDefaultsInConstructor:ERROR',
+ '-Xep:AutoValueConstructorOrderChecker:ERROR',
+ '-Xep:BadAnnotationImplementation:ERROR',
+ '-Xep:BadShiftAmount:ERROR',
+ '-Xep:BanJNDI:ERROR',
+ '-Xep:BoxedPrimitiveEquality:ERROR',
+ '-Xep:BundleDeserializationCast:ERROR',
+ '-Xep:ChainingConstructorIgnoresParameter:ERROR',
+ '-Xep:CheckNotNullMultipleTimes:ERROR',
+ '-Xep:CheckReturnValue:ERROR',
+ '-Xep:CollectionToArraySafeParameter:ERROR',
'-Xep:ComparableType:OFF', // SolrTestCaseJ4.Doc and Fld are messy
Review Comment:
i think i ended up commenting them out in the followup PR. It does make it easier for my editor to see the disabled ones.
I'm not concerned about disabling-all-checks not actually disabling all checks. I think it only enables a few critical ones, such as unicode directional checker. These checks override `disableable = false` in their annotation in the source code, e.g. https://github.com/google/error-prone/blob/4c7dff57339bba6b4cf41fc776a51de6c6bd5e00/core/src/main/java/com/google/errorprone/bugpatterns/UnicodeDirectionalityCharacters.java#L34
And if you explicitly disable them, error-prone fails:
```
Execution failed for task ':lucene:core:compileJava'.
> UnicodeDirectionalityCharacters may not be disabled
```
So I think it is best to make life easier on ourselves by using the comments? In this example, yeah we check for these unicode characters elsewhere in our build, so it is redundant, but it is not possible to disable with error-prone so best to just have it configured as `ERROR` and move on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org