From 9e6d69c84a407a61facd2ba4052db0b2c099597e Mon Sep 17 00:00:00 2001 From: Vincent Potucek Date: Fri, 10 Oct 2025 16:44:45 +0200 Subject: [PATCH 1/2] Add `error-prone.picnic.tech` featuring `RedundantStringConversion` --- gradle/libs.versions.toml | 3 +- ...ld.java-nullability-conventions.gradle.kts | 53 ++++++++++--------- .../PreInterruptThreadDumpPrinter.java | 2 +- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 4a4c74a6aeff..d79eb3cd23fc 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -32,7 +32,8 @@ bndlib = { module = "biz.aQute.bnd:biz.aQute.bndlib", version.ref = "bnd" } checkstyle = { module = "com.puppycrawl.tools:checkstyle", version.ref = "checkstyle" } classgraph = { module = "io.github.classgraph:classgraph", version = "4.8.184" } commons-io = { module = "commons-io:commons-io", version = "2.20.0" } -errorProne-core = { module = "com.google.errorprone:error_prone_core", version = "2.42.0" } +error-prone-contrib = { module = "tech.picnic.error-prone-support:error-prone-contrib", version = "0.25.0" } +error-prone-core = { module = "com.google.errorprone:error_prone_core", version = "2.42.0" } fastcsv = { module = "de.siegmar:fastcsv", version = "4.1.0" } groovy = { module = "org.apache.groovy:groovy", version = "5.0.1" } groovy2-bom = { module = "org.codehaus.groovy:groovy-bom", version = "2.5.23" } diff --git a/gradle/plugins/common/src/main/kotlin/junitbuild.java-nullability-conventions.gradle.kts b/gradle/plugins/common/src/main/kotlin/junitbuild.java-nullability-conventions.gradle.kts index 2bef782b14ed..060ea5edb90a 100644 --- a/gradle/plugins/common/src/main/kotlin/junitbuild.java-nullability-conventions.gradle.kts +++ b/gradle/plugins/common/src/main/kotlin/junitbuild.java-nullability-conventions.gradle.kts @@ -9,7 +9,8 @@ plugins { } dependencies { - errorprone(dependencyFromLibs("errorProne-core")) + errorprone(dependencyFromLibs("error-prone-contrib")) + errorprone(dependencyFromLibs("error-prone-core")) errorprone(dependencyFromLibs("nullaway")) constraints { errorprone("com.google.guava:guava") { @@ -17,41 +18,44 @@ dependencies { require("33.4.8-jre") } because("Older versions use deprecated methods in sun.misc.Unsafe") + // https://github.com/junit-team/junit-framework/pull/5039#discussion_r2414490581 } } } -nullaway { - onlyNullMarked = true -} - tasks.withType().configureEach { options.errorprone { val shouldDisableErrorProne = java.toolchain.implementation.orNull == JvmImplementation.J9 if (name == "compileJava" && !shouldDisableErrorProne) { disable( - - // This check is opinionated wrt. which method names it considers unsuitable for import which includes - // a few of our own methods in `ReflectionUtils` etc. - "BadImport", - - // The findings of this check are subjective because a named constant can be more readable in many cases - "UnnecessaryLambda", - - // Resolving findings for these checks requires ErrorProne's annotations which we don't want to use - "AnnotateFormatMethod", + "AnnotateFormatMethod", // We don`t want to use ErrorProne`s annotations. + "BadImport", // This check is opinionated wrt. which method names it considers unsuitable for import which includes a few of our own methods in `ReflectionUtils` etc. "DoNotCallSuggester", - "InlineMeSuggester", "ImmutableEnumChecker", - - // Resolving findings for this check requires using Guava which we don't want to use - "StringSplitter", - - // Produces a lot of findings that we consider to be false positives, for example for package-private - // classes and methods - "MissingSummary", + "InlineMeSuggester", + "MissingSummary", // Produces a lot of findings that we consider to be false positives, for example for package-private classes and methods. + "StringSplitter", // We don`t want to use Guava. + "UnnecessaryLambda", // The findings of this check are subjective because a named constant can be more readable in many cases. + // picnic (https://error-prone.picnic.tech) + "ConstantNaming", + "DirectReturn", // https://github.com/junit-team/junit-framework/pull/5006#discussion_r2403984446 + "FormatStringConcatenation", + "IdentityConversion", + "LexicographicalAnnotationAttributeListing", + "LexicographicalAnnotationListing", + "MissingTestCall", + "NestedOptionals", + "NonStaticImport", + "OptionalOrElseGet", + "PrimitiveComparison", + "StaticImport", + "TimeZoneUsage", + ) + error( + "PackageLocation", + "RedundantStringConversion", + "RedundantStringEscape", ) - error("PackageLocation") } else { disableAllChecks = true } @@ -61,6 +65,7 @@ tasks.withType().configureEach { } else { enable() } + onlyNullMarked = true isJSpecifyMode = true customContractAnnotations.add("org.junit.platform.commons.annotation.Contract") checkContracts = true diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/PreInterruptThreadDumpPrinter.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/PreInterruptThreadDumpPrinter.java index 283710eb1f1e..94147129d87b 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/PreInterruptThreadDumpPrinter.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/PreInterruptThreadDumpPrinter.java @@ -49,7 +49,7 @@ public void beforeThreadInterrupt(PreInterruptContext preInterruptContext, Exten sb.append(NL); // Use the same prefix as java.lang.Throwable.printStackTrace(PrintStreamOrWriter) sb.append("\tat "); - sb.append(stackTraceElement.toString()); + sb.append(stackTraceElement); } sb.append(NL); } From f0bd798670e89289a1d7b7357ada8805003e4a4c Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sun, 12 Oct 2025 12:49:09 +0000 Subject: [PATCH 2/2] Apply suggestions from code review Add comments to each disabled bug pattern --- .../junitbuild.java-nullability-conventions.gradle.kts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gradle/plugins/common/src/main/kotlin/junitbuild.java-nullability-conventions.gradle.kts b/gradle/plugins/common/src/main/kotlin/junitbuild.java-nullability-conventions.gradle.kts index 060ea5edb90a..8d1979601c64 100644 --- a/gradle/plugins/common/src/main/kotlin/junitbuild.java-nullability-conventions.gradle.kts +++ b/gradle/plugins/common/src/main/kotlin/junitbuild.java-nullability-conventions.gradle.kts @@ -28,11 +28,11 @@ tasks.withType().configureEach { val shouldDisableErrorProne = java.toolchain.implementation.orNull == JvmImplementation.J9 if (name == "compileJava" && !shouldDisableErrorProne) { disable( - "AnnotateFormatMethod", // We don`t want to use ErrorProne`s annotations. + "AnnotateFormatMethod", // We don`t want to use ErrorProne's annotations. "BadImport", // This check is opinionated wrt. which method names it considers unsuitable for import which includes a few of our own methods in `ReflectionUtils` etc. - "DoNotCallSuggester", - "ImmutableEnumChecker", - "InlineMeSuggester", + "DoNotCallSuggester", // We don`t want to use ErrorProne's annotations. + "ImmutableEnumChecker", // We don`t want to use ErrorProne's annotations. + "InlineMeSuggester", // We don`t want to use ErrorProne's annotations. "MissingSummary", // Produces a lot of findings that we consider to be false positives, for example for package-private classes and methods. "StringSplitter", // We don`t want to use Guava. "UnnecessaryLambda", // The findings of this check are subjective because a named constant can be more readable in many cases.