Pull #18820: Consolidate CI to fix error-prone rewrite and spotbugs execution #2825

.github/workflows/ci.yml

@@ -7,13 +7,12 @@ on:
   push:
     branches: [main, release]
   workflow_dispatch:
-
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
 jobs:
-  sanity-check:
-    name: Sanity Check 🦢
+  sanity:
+    name: Sanity 🦢
     runs-on: ubuntu-latest
     env:
       buildcacheuser: ${{ secrets.BUILDCACHE_USER }}
@@ -25,16 +24,26 @@ jobs:
           fetch-depth: 0
       - uses: actions/setup-java@v5
         with:
-          distribution: "temurin"
+          distribution: temurin
           java-version: 21
-      - name: gradle caching
+      - name: Gradle Setup 🐘
         uses: gradle/actions/setup-gradle@v5
+      - name: Error Prone 🚧️
+        run: ./gradlew assemble -Derror-prone=true
+      # run: ./gradlew assemble -Derror-prone=false
+      - name: OpenRewrite ☑️ # 4m 55s (2 min compile, 3 min rewrite)
+        run: ./gradlew rewriteDryRun -DfailOnDryRunResults=true
+      # run: ./gradlew rewriteDryRun -DfailOnDryRunResults=false
       - name: Spotless ✨
         run: ./gradlew spotlessCheck
-      - name: assemble testClasses
-        run: ./gradlew assemble testClasses
+      - name: Spotbugs 🐞
+        run: ./gradlew spotbugsMain
+      - name: Assemble 🏗️
+        run: ./gradlew assemble
+      - name: TestClasses 📋
+        run: ./gradlew testClasses
   build:
-    needs: sanity-check
+    needs: sanity
     strategy:
       fail-fast: false
       matrix:
@@ -60,7 +69,7 @@ jobs:
         uses: actions/checkout@v6
       - uses: actions/setup-java@v5
         with:
-          distribution: "temurin"
+          distribution: temurin
           java-version: ${{ matrix.jre }}
       - name: gradle caching
         uses: gradle/actions/setup-gradle@v5
@@ -88,11 +97,11 @@ jobs:
       - name: Test idea
         if: matrix.kind == 'idea'
         run: |
-          download_link=$(curl https://data.services.jetbrains.com/products/releases\?code\=IIC\&latest\=true\&type\=release | jq -r '.IIC[0].downloads.linux.link')
+          download_link=$(curl -s "https://data.services.jetbrains.com/products/releases?code=IIC&latest=true&type=release" | jq -r '.IIC[0].downloads.linux.link')
           curl --location "$download_link" -o idea.tar.gz
           tar -xf idea.tar.gz
           cd idea-IC*
-          export PATH=${PATH}:$(pwd)/bin
+          export PATH="${PATH}:$(pwd)/bin"
           cd ..
           ./gradlew testIdea
       - name: junit result
@@ -102,20 +111,19 @@ jobs:
           check_name: JUnit ${{ matrix.kind }} ${{ matrix.jre }} ${{ matrix.os }}
           report_paths: '*/build/test-results/*/TEST-*.xml'
           check_retries: true
-
   # Status check that is required in branch protection rules.
   final-status:
     needs:
-      - sanity-check
+      - sanity
       - build
     runs-on: ubuntu-latest
     if: always()
     steps:
       - name: Check
         run: |
-          results=$(tr -d '\n' <<< '${{ toJSON(needs.*.result) }}')
+          results=$(tr -d '\n' <<< "${{ toJSON(needs.*.result) }}")
           if ! grep -q -v -E '(failure|cancelled)' <<< "$results"; then
-            echo "One or more required jobs failed"
+            echo "One or more required jobs failed. ❌"
             exit 1
           fi
-          echo "All required jobs completed successfully."
+          echo "All required jobs completed successfully. ✅"

CONTRIBUTING.md

@@ -184,8 +184,14 @@ concerning what you are working on:
 Before creating a pull request, you might want to format (yes, spotless is  formatted by spotless)
 the code and check for possible bugs
 
-* `./gradlew spotlessApply`
-* `./gradlew spotbugsMain`
+* Error Prone 🚧
+  * `./gradlew assemble -Derror-prone=true`
+* OpenRewrite ☑️
+  * `./gradlew rewriteRun`
+* Spotless ✨
+  * `./gradlew spotlessApply`
+* Spotbugs 🐞
+  * `./gradlew spotbugsMain`
 
 These checks are also run by the automated pipeline when you submit a pull request, if
 the pipeline fails, first check if the code is formatted and no bugs were found.

PULL_REQUEST_TEMPLATE.md

@@ -1,3 +1,5 @@
+## DO NOT FORCE PUSH
+
 Please **DO NOT FORCE PUSH**. Don't worry about messy history, it's easier to do code review if we can tell what happened after the review, and force pushing breaks that.
 
 Please make sure that your [PR allows edits from maintainers](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork).  Sometimes it's faster for us to just fix something than it is to describe how to fix it.
@@ -16,3 +18,17 @@ If your change only affects a build plugin, and not the lib, then you only need
 If your change affects lib in an end-user-visible way (fixing a bug, updating a version) then you need to update `CHANGES.md` for both the lib and all build plugins.  Users of a build plugin shouldn't have to refer to lib to see changes that affect them.
 
 This makes it easier for the maintainers to quickly release your changes :)
+
+## Check and format code
+
+Before creating a pull request, you might want to format (yes, spotless is  formatted by spotless)
+the code and check for possible bugs
+
+* Error Prone 🚧
+  * `./gradlew assemble -Derror-prone=true`
+* OpenRewrite ☑️
+  * `./gradlew rewriteRun`
+* Spotless ✨
+  * `./gradlew spotlessApply`
+* Spotbugs 🐞
+  * `./gradlew spotbugsMain`

README.md

@@ -8,7 +8,7 @@ Spotless can format <antlr | c | c# | c++ | css | flow | graphql | groovy | h
 
 You probably want one of the links below:
 
-## [❇️ Spotless for Gradle](plugin-gradle) (with integrations for [VS Code](https://marketplace.visualstudio.com/items?itemName=richardwillis.vscode-spotless-gradle) and [IntelliJ](https://plugins.jetbrains.com/plugin/18321-spotless-gradle))
+## [✨️ Spotless for Gradle](plugin-gradle) (with integrations for [VS Code](https://marketplace.visualstudio.com/items?itemName=richardwillis.vscode-spotless-gradle) and [IntelliJ](https://plugins.jetbrains.com/plugin/18321-spotless-gradle))
 
 ```console
 user@machine repo % ./gradlew build
@@ -25,7 +25,7 @@ user@machine repo % ./gradlew build
 BUILD SUCCESSFUL
 ```
 
-## [❇️ Spotless for Maven](plugin-maven)
+## [✨️ Spotless for Maven](plugin-maven)
 
 ```console
 user@machine repo % mvn spotless:check
@@ -40,7 +40,7 @@ user@machine repo % mvn spotless:check
 [INFO] BUILD SUCCESS
 ```
 
-## [❇️ Spotless for SBT (external for now)](https://github.com/moznion/sbt-spotless)
+## [✨️ Spotless for SBT (external for now)](https://github.com/moznion/sbt-spotless)
 ## [Other build systems](CONTRIBUTING.md#how-to-add-a-new-plugin-for-a-build-system)
 
 ## How it works (for potential contributors)

build.gradle

@@ -10,6 +10,7 @@ apply from: rootProject.file('gradle/java-publish.gradle')
 apply from: rootProject.file('gradle/changelog.gradle')
 apply from: rootProject.file('gradle/rewrite.gradle')
 allprojects {
+	apply from: rootProject.file('gradle/error-prone.gradle')
 	apply from: rootProject.file('gradle/spotless.gradle')
 }
 apply from: rootProject.file('gradle/spotless-freshmark.gradle')

gradle/error-prone.gradle

@@ -4,11 +4,10 @@ apply plugin: 'net.ltgt.errorprone'
 
 tasks.withType(JavaCompile).configureEach {
 	options.errorprone {
+		getenv('error-prone')?.toBoolean() ? enable() : disable()
 		disableAllWarnings = true // https://github.com/diffplug/spotless/issues/2745 https://github.com/google/error-prone/issues/5365
 		disable(
-				// consider fix, or reasoning.
 				'AnnotateFormatMethod', // We don`t want to use ErrorProne's annotations.
-				'DoNotCallSuggester', // We don`t want to use ErrorProne's annotations.
 				'FunctionalInterfaceMethodChanged',
 				'ImmutableEnumChecker', // We don`t want to use ErrorProne's annotations.
 				'InlineMeSuggester', // We don`t want to use ErrorProne's annotations.
@@ -26,17 +25,7 @@ tasks.withType(JavaCompile).configureEach {
 		// bug: this only happens when the file is dirty.
 		// might be an up2date (caching) issue, as file is currently in corrupt state.
 		// ForbidGradleInternal(import
-		excludedPaths.set(
-				'.*/GradleIntegrationHarness.java|'+
-				'.*/SelfTest.java'
-				)
-		if (!getenv().containsKey('CI') && getenv('IN_PLACE')?.toBoolean()) {
-			errorproneArgs.addAll(
-					'-XepPatchLocation:IN_PLACE',
-					'-XepPatchChecks:' +
-					'MissingOverride'
-					)
-		}
+		excludedPaths.set('.*/GradleIntegrationHarness.java')
 	}
 }
 

gradle/rewrite.gradle

@@ -1,6 +1,5 @@
-if (!project.hasProperty('rewrite')) {
-	return
-}
+import static java.lang.System.getenv
+
 apply plugin: 'org.openrewrite.rewrite'
 rewrite {
 	activeRecipe('com.diffplug.spotless.openrewrite.SanityCheck')
@@ -21,11 +20,10 @@ rewrite {
 			'**testlib/src/main/resources**'
 			)
 	exportDatatables = true
-	failOnDryRunResults = true
+	failOnDryRunResults = getenv('failOnDryRunResults')?.toBoolean()
 }
 dependencies {
-	rewrite('org.openrewrite.recipe:rewrite-migrate-java:3.22.0')
-	rewrite('org.openrewrite.recipe:rewrite-rewrite:0.16.0')
-	rewrite('org.openrewrite.recipe:rewrite-static-analysis:2.22.0')
-	rewrite('org.openrewrite.recipe:rewrite-third-party:0.31.2')
+	rewrite('org.openrewrite.recipe:rewrite-migrate-java:3.26.0')
+	rewrite('org.openrewrite.recipe:rewrite-static-analysis:2.26.0')
+	rewrite('org.openrewrite.recipe:rewrite-third-party:0.34.0')
 }

gradle/wrapper/gradle-wrapper.properties

@@ -1,7 +1,7 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionSha256Sum=0d585f69da091fc5b2beced877feab55a3064d43b8a1d46aeb07996b0915e0e0
-distributionUrl=https\://services.gradle.org/distributions/gradle-9.3.0-bin.zip
+distributionSha256Sum=b266d5ff6b90eada6dc3b20cb090e3731302e553a27c5d3e4df1f0d76beaff06
+distributionUrl=https\://services.gradle.org/distributions/gradle-9.3.1-bin.zip
 networkTimeout=10000
 validateDistributionUrl=true
 zipStoreBase=GRADLE_USER_HOME

lib-extra/src/test/java/com/diffplug/spotless/TestP2Provisioner.java

@@ -118,10 +118,12 @@ private static class CacheKey implements Serializable {
 
 		@Override
 		public boolean equals(Object o) {
-			if (this == o)
+			if (this == o) {
 				return true;
-			if (o == null || getClass() != o.getClass())
+			}
+			if (o == null || getClass() != o.getClass()) {
 				return false;
+			}
 			CacheKey cacheKey = (CacheKey) o;
 			return useMavenCentral == cacheKey.useMavenCentral &&
 					Objects.equals(p2Repos, cacheKey.p2Repos) &&

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java

@@ -64,6 +64,7 @@
 import com.diffplug.spotless.SerializedFunction;
 import com.diffplug.spotless.cpp.ClangFormatStep;
 import com.diffplug.spotless.extra.EclipseBasedStepBuilder;
+import com.diffplug.spotless.extra.P2Provisioner;
 import com.diffplug.spotless.extra.wtp.EclipseWtpFormatterStep;
 import com.diffplug.spotless.generic.EndWithNewlineStep;
 import com.diffplug.spotless.generic.FenceStep;
@@ -97,7 +98,7 @@ protected final Provisioner provisioner() {
 		return spotless.getRegisterDependenciesTask().getTaskService().get().provisionerFor(spotless);
 	}
 
-	protected final com.diffplug.spotless.extra.P2Provisioner p2Provisioner() {
+	protected final P2Provisioner p2Provisioner() {
 		return spotless.getRegisterDependenciesTask().getTaskService().get().p2ProvisionerFor(spotless);
 	}
 

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java

@@ -256,7 +256,7 @@ public synchronized List<File> provisionP2Dependencies(
 		private record P2Request(
 				List<String> p2Repos,
 				List<String> installList,
-				java.util.Set<String> filterNames, // Filter names (Filter objects aren't easily comparable)
+				Set<String> filterNames, // Filter names (Filter objects aren't easily comparable)
 				List<String> pureMaven,
 				boolean useMavenCentral,
 				@Nullable File cacheDirectory) {}

plugin-gradle/src/test/java/com/diffplug/gradle/spotless/GradleProvisionerTest.java

@@ -172,35 +172,35 @@ void cacheMissTriggersNewResolution(String scenario, Function<P2ModelWrapper, P2
 
 		static Stream<Arguments> cacheMissScenarios() {
 			return Stream.of(
-					Arguments.of("different P2 repo", (Function<P2ModelWrapper, P2ModelWrapper>) (m) -> createMockModel(
+					Arguments.of("different P2 repo", (Function<P2ModelWrapper, P2ModelWrapper>) m -> createMockModel(
 							List.of("https://download.eclipse.org/eclipse/updates/4.27/"),
 							List.of("org.eclipse.jdt.core"),
 							Set.of(),
 							List.of(),
 							true,
 							null), null),
-					Arguments.of("different install list", (Function<P2ModelWrapper, P2ModelWrapper>) (m) -> createMockModel(
+					Arguments.of("different install list", (Function<P2ModelWrapper, P2ModelWrapper>) m -> createMockModel(
 							List.of("https://download.eclipse.org/eclipse/updates/4.26/"),
 							List.of("org.eclipse.jdt.core", "org.eclipse.jdt.ui"),
 							Set.of(),
 							List.of(),
 							true,
 							null), null),
-					Arguments.of("different filters", (Function<P2ModelWrapper, P2ModelWrapper>) (m) -> createMockModel(
+					Arguments.of("different filters", (Function<P2ModelWrapper, P2ModelWrapper>) m -> createMockModel(
 							List.of("https://download.eclipse.org/eclipse/updates/4.26/"),
 							List.of("org.eclipse.jdt.core"),
 							Set.of("osgiFilter1"),
 							List.of(),
 							true,
 							null), null),
-					Arguments.of("different pure maven", (Function<P2ModelWrapper, P2ModelWrapper>) (m) -> createMockModel(
+					Arguments.of("different pure maven", (Function<P2ModelWrapper, P2ModelWrapper>) m -> createMockModel(
 							List.of("https://download.eclipse.org/eclipse/updates/4.26/"),
 							List.of("org.eclipse.jdt.core"),
 							Set.of(),
 							List.of("com.google:guava:32.0.0-jre"),
 							true,
 							null), null),
-					Arguments.of("different useMavenCentral", (Function<P2ModelWrapper, P2ModelWrapper>) (m) -> createMockModel(
+					Arguments.of("different useMavenCentral", (Function<P2ModelWrapper, P2ModelWrapper>) m -> createMockModel(
 							List.of("https://download.eclipse.org/eclipse/updates/4.26/"),
 							List.of("org.eclipse.jdt.core"),
 							Set.of(),

rewrite.yml

@@ -42,7 +42,7 @@ recipeList:
   - tech.picnic.errorprone.refasterrules.BigDecimalRulesRecipes
   - tech.picnic.errorprone.refasterrules.CharSequenceRulesRecipes
   - tech.picnic.errorprone.refasterrules.ClassRulesRecipes
-  - tech.picnic.errorprone.refasterrules.CollectionRulesRecipes
+  # tech.picnic.errorprone.refasterrules.CollectionRulesRecipes # needs UpgradeToJava21
   - tech.picnic.errorprone.refasterrules.ComparatorRulesRecipes
   - tech.picnic.errorprone.refasterrules.EqualityRulesRecipes
   - tech.picnic.errorprone.refasterrules.FileRulesRecipes