Sync ErrorProne configuration with OpenTelemetry Java instrumentation

buildSrc/src/main/kotlin/ai.errorprone-conventions.gradle.kts

@@ -22,57 +22,118 @@ tasks {
         disableWarningsInGeneratedCode.set(true)
         allDisabledChecksAsWarnings.set(true)
 
+        // Ignore warnings for generated classes
         excludedPaths.set(".*/build/generated/.*")
 
-        if (System.getenv("CI") == null) {
-          disable("SystemOut")
-        }
-
-        // Still Java 8
-        disable("Varifier")
+        // it's very convenient to debug stuff in the javaagent using System.out.println
+        // and we don't want to conditionally only check this in CI
+        // because then the remote gradle cache won't work for local builds
+        // so we check this via checkstyle instead
+        disable("SystemOut")
 
-        // Intellij does a nice job of displaying parameter names
         disable("BooleanParameter")
 
-        // Needed for legacy 2.x bridge
-        disable("JavaUtilDate")
+        // We often override a method returning Iterable which this makes tedious
+        // for questionable value.
+        disable("PreferredInterfaceType")
 
         // Doesn't work well with Java 8
         disable("FutureReturnValueIgnored")
 
-        // Needs Java 9+
-        disable("JavaDurationGetSecondsToToSeconds")
+        // Still Java 8
+        disable("Varifier")
+
+        // Doesn't currently use Var annotations.
+        disable("Var") // "-Xep:Var:OFF"
+
+        // ImmutableRefactoring suggests using com.google.errorprone.annotations.Immutable,
+        // but currently uses javax.annotation.concurrent.Immutable
+        disable("ImmutableRefactoring")
 
-        // Require Guava
+        // AutoValueImmutableFields suggests returning Guava types from API methods
         disable("AutoValueImmutableFields")
-        disable("StringSplitter")
+        // Suggests using Guava types for fields but we don't use Guava
         disable("ImmutableMemberCollection")
 
-        // Don't currently use this (to indicate a local variable that's mutated) but could
-        // consider for future.
-        disable("Var")
+        // Fully qualified names may be necessary when deprecating a class to avoid
+        // deprecation warning.
+        disable("UnnecessarilyFullyQualified")
 
-        // Don't support Android without desugar
+        // TODO (trask) use animal sniffer
+        disable("Java8ApiChecker")
         disable("AndroidJdkLibsChecker")
+
+        // apparently disabling android doesn't disable this
         disable("StaticOrDefaultInterfaceMethod")
 
-        // needed temporarily while hosting azure-monitor-opentelemetry-exporter in this repo
-        disable("MissingSummary")
-        disable("UnnecessaryDefaultInEnumSwitch")
-        disable("InconsistentOverloads")
+        // We don't depend on Guava so use normal splitting
+        disable("StringSplitter")
+
+        // Prevents lazy initialization
+        disable("InitializeInline")
+
+        // Seems to trigger even when a deprecated method isn't called anywhere.
+        // We don't get much benefit from it anyways.
+        disable("InlineMeSuggester")
+
+        disable("DoNotCallSuggester")
+
+        // We have nullaway so don't need errorprone nullable checks which have more false positives.
+        disable("FieldMissingNullable")
+        disable("ParameterMissingNullable")
+        disable("ReturnMissingNullable")
+        disable("VoidMissingNullable")
+
+        // allow UPPERCASE type parameter names
+        disable("TypeParameterNaming")
+
+        // In bytecode instrumentation it's very common to separate across onEnter / onExit
+        // TODO: Only disable for javaagent instrumentation modules.
+        disable("MustBeClosedChecker")
+
+        // Common to avoid an allocation. Revisit if it's worth opt-in suppressing instead of
+        // disabling entirely.
+        disable("MixedMutabilityReturnType")
+
+        // We end up using obsolete types if a library we're instrumenting uses them.
+        disable("JdkObsolete")
+        disable("JavaUtilDate")
+
+        // TODO: Remove this, we use this pattern in several tests and it will mean some moving.
+        disable("DefaultPackage")
+
+        // We don't have custom checks like OpenTelemetry, so disable the standard utility class check
+        disable("PrivateConstructorForUtilityClass")
 
         disable("CanIgnoreReturnValueSuggester")
 
-        disable("NonFinalStaticField")
+        // TODO: Remove this, probably after instrumenter API migration instead of dealing with
+        // older APIs.
+        disable("InconsistentOverloads")
+
+        // lots of low level APIs use arrays
+        disable("AvoidObjectArrays")
+
+        disable("BanClassLoader")
 
         // YodaConditions may improve safety in some cases. The argument of increased
         // cognitive load is dubious.
         disable("YodaCondition")
 
+        disable("NonFinalStaticField")
+
         // Requires adding compile dependency to JSpecify
         disable("AddNullMarkedToPackageInfo")
 
-        if (name.contains("Jmh")) {
+        // needed temporarily while hosting azure-monitor-opentelemetry-exporter in this repo
+        disable("MissingSummary")
+        disable("UnnecessaryDefaultInEnumSwitch")
+
+        // Needs Java 9+
+        disable("JavaDurationGetSecondsToToSeconds")
+
+        if (name.contains("Jmh") || name.contains("Test")) {
+          // Allow underscore in test-type method names
           disable("MemberName")
         }
       }