[rewrite] Add GradleBestPractices

.github/workflows/precommit.yml

@@ -1,4 +1,4 @@
-name: Gradle Precommit
+name: Gradle PreCommit
 on: [pull_request]
 
 jobs:
@@ -21,8 +21,11 @@ jobs:
           java-version: ${{ matrix.java }}
           distribution: temurin
           cache: gradle
-      - name: Run Gradle (precommit)
+      - name: Run Gradle (pre-commit)
         continue-on-error: ${{ matrix.experimental }}
         shell: bash
-        run: |
-          ./gradlew javadoc precommit --parallel
+        run: ./gradlew javadoc precommit --parallel
+      - name: Run Gradle (sanity-check)
+        continue-on-error: ${{ matrix.experimental }}
+        shell: bash
+        run: ./gradlew rewriteDryRun -Dorg.gradle.jvmargs=-Xmx8G

CHANGELOG.md

@@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add build-tooling to run in FIPS environment ([#18921](https://github.com/opensearch-project/OpenSearch/pull/18921))
 - Add SMILE/CBOR/YAML document format support to Bulk GRPC endpoint ([#19744](https://github.com/opensearch-project/OpenSearch/pull/19744))
 - Implement GRPC ConstantScoreQuery, FuzzyQuery, MatchBoolPrefixQuery, MatchPhrasePrefix, PrefixQuery, MatchQuery ([#19854](https://github.com/opensearch-project/OpenSearch/pull/19854))
+- [rewrite] Add `GradleBestPractices` ([#19885](https://github.com/opensearch-project/OpenSearch/pull/19885))
 
 ### Changed
 - Faster `terms` query creation for `keyword` field with index and docValues enabled ([#19350](https://github.com/opensearch-project/OpenSearch/pull/19350))

build.gradle

@@ -59,6 +59,7 @@ plugins {
   id "org.gradle.test-retry" version "1.6.2" apply false
   id "test-report-aggregation"
   id 'jacoco-report-aggregation'
+  id 'org.openrewrite.rewrite' version '7.19.0' apply false
 }
 
 apply from: 'gradle/build-complete.gradle'
@@ -70,6 +71,7 @@ apply from: 'gradle/local-distribution.gradle'
 apply from: 'gradle/run.gradle'
 apply from: 'gradle/missing-javadoc.gradle'
 apply from: 'gradle/code-coverage.gradle'
+apply from: 'gradle/rewrite.gradle'
 
 // Apply FIPS configuration to all projects
 allprojects {

distribution/archives/build.gradle

@@ -28,7 +28,7 @@
  * under the License.
  */
 
-import org.opensearch.gradle.JavaPackageType 
+import org.opensearch.gradle.JavaPackageType
 
 apply plugin: 'opensearch.internal-distribution-archive-setup'
 
@@ -193,7 +193,7 @@ distribution_archives {
     }
   }
 
-  
+
   linuxPpc64leTar {
     archiveClassifier = 'linux-ppc64le'
     content {

gradle/rewrite.gradle

@@ -0,0 +1,28 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+project.apply plugin: 'org.openrewrite.rewrite'
+
+rewrite {
+  activeRecipe('org.opensearch.openrewrite.SanityCheck')
+  exclusion('**ActionListener.java')
+  exclusion('**SearchAfterIT.java')
+  exclusion('**StarTreeMapper.java')
+  exclusion('**package-info.java')
+  setExportDatatables(true)
+  setFailOnDryRunResults(true)
+}
+
+dependencies {
+  rewrite(platform('org.openrewrite.recipe:rewrite-recipe-bom:3.17.0'))
+  rewrite('org.openrewrite.recipe:rewrite-migrate-java:3.20.0')
+  rewrite('org.openrewrite.recipe:rewrite-java-security:3.19.2')
+  rewrite('org.openrewrite.recipe:rewrite-rewrite:0.14.1')
+  rewrite('org.openrewrite.recipe:rewrite-static-analysis:2.20.0')
+  rewrite('org.openrewrite.recipe:rewrite-third-party:0.30.0')
+}

gradle/wrapper/gradle-wrapper.properties

@@ -1,8 +1,8 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-9.2.0-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-9.2.0-bin.zip
 networkTimeout=10000
 validateDistributionUrl=true
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
-distributionSha256Sum=16f2b95838c1ddcf7242b1c39e7bbbb43c842f1f1a1a0dc4959b6d4d68abcac3
+distributionSha256Sum=df67a32e86e3276d011735facb1535f64d0d88df84fa87521e90becc2d735444

gradlew

@@ -1,7 +1,7 @@
 #!/bin/sh
 
 #
-# Copyright © 2015-2021 the original authors.
+# Copyright © 2015 the original authors.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -202,7 +202,7 @@ fi
 
 
 # Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
-DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
+DEFAULT_JVM_OPTS=
 
 # Collect all arguments for the java command:
 #   * DEFAULT_JVM_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments,

gradlew.bat

@@ -36,7 +36,7 @@ set APP_HOME=%DIRNAME%
 for %%i in ("%APP_HOME%") do set APP_HOME=%%~fi
 
 @rem Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
-set DEFAULT_JVM_OPTS="-Xmx64m" "-Xms64m"
+set DEFAULT_JVM_OPTS=
 
 @rem Find java.exe
 if defined JAVA_HOME goto findJavaFromJavaHome
@@ -70,11 +70,10 @@ goto fail
 :execute
 @rem Setup the command line
 
-set CLASSPATH=
 
 
 @rem Execute Gradle
-"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %*
+"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %*
 
 :end
 @rem End local scope for the variables with windows NT shell

modules/build.gradle

@@ -32,7 +32,7 @@ configure(subprojects.findAll { it.parent.path == project.path }) {
   group = 'org.opensearch.plugin' // for modules which publish client jars
   apply plugin: 'opensearch.testclusters'
   apply plugin: 'opensearch.opensearchplugin'
-  
+
   opensearchplugin {
     // for local OpenSearch plugins, the name of the plugin is the same as the directory
     name = project.name

plugins/arrow-flight-rpc/src/main/java/org/apache/arrow/flight/OSFlightClient.java

@@ -47,7 +47,7 @@ public final static class Builder {
         private ExecutorService executorService;
         private Class<? extends io.netty.channel.Channel> channelType;
         private SslContext sslContext;
-        
+
         private Builder() {}
 
         Builder(BufferAllocator allocator, Location location) {
@@ -131,7 +131,7 @@ public FlightClient build() {
                             builder.channelType(
                                 Class.forName("io.netty.channel.epoll.EpollDomainSocketChannel")
                                     .asSubclass(ServerChannel.class));
-                            final EventLoopGroup elg =  
+                            final EventLoopGroup elg =
                                 Class.forName("io.netty.channel.epoll.EpollEventLoopGroup")
                                     .asSubclass(EventLoopGroup.class)
                                     .getDeclaredConstructor()
@@ -142,7 +142,7 @@ public FlightClient build() {
                             builder.channelType(
                                 Class.forName("io.netty.channel.kqueue.KQueueDomainSocketChannel")
                                     .asSubclass(ServerChannel.class));
-                            final EventLoopGroup elg =  
+                            final EventLoopGroup elg =
                                 Class.forName("io.netty.channel.kqueue.KQueueEventLoopGroup")
                                     .asSubclass(EventLoopGroup.class)
                                     .getDeclaredConstructor()
@@ -179,7 +179,7 @@ public FlightClient build() {
                     builder.sslContext(sslContext);
                 } else {
                     final SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient();
-    
+
                     if (!this.verifyServer) {
                         sslContextBuilder.trustManager(InsecureTrustManagerFactory.INSTANCE);
                     } else if (this.trustedCertificates != null
@@ -215,11 +215,11 @@ public FlightClient build() {
             if (channelType != null) {
                 builder.channelType(channelType);
             }
- 
+
             if (workerELG != null) {
                 builder.eventLoopGroup(workerELG);
             }
- 
+
             return new FlightClient(allocator, builder.build(), middleware);
         }
 
@@ -232,7 +232,7 @@ public Builder channelType(Class<? extends io.netty.channel.Channel> channelType
             this.channelType = channelType;
             return this;
         }
-        
+
         public Builder eventLoopGroup(EventLoopGroup workerELG) {
             this.workerELG = workerELG;
             return this;

plugins/arrow-flight-rpc/src/main/java/org/apache/arrow/flight/OSFlightServer.java

@@ -121,10 +121,10 @@ public FlightServer build() {
             this.middleware(FlightConstants.HEADER_KEY, new ServerHeaderMiddleware.Factory());
 
             final NettyServerBuilder builder;
-            
+
             // Use primary location for initial setup
             Location primaryLocation = location != null ? location : listenAddresses.get(0);
-            
+
             switch (primaryLocation.getUri().getScheme()) {
                 case LocationSchemes.GRPC_DOMAIN_SOCKET:
                 {
@@ -268,7 +268,7 @@ public FlightServer build() {
                     builder.addListenAddress(listenAddress.toSocketAddress());
                 }
             }
-            
+
             builder.intercept(new ServerInterceptorAdapter(interceptors));
 
             try {
@@ -472,7 +472,7 @@ public Builder location(Location location) {
             this.location = Preconditions.checkNotNull(location);
             return this;
         }
-        
+
         public Builder addListenAddress(Location location) {
             this.listenAddresses.add(Preconditions.checkNotNull(location));
             return this;

qa/remote-clusters/build.gradle

@@ -54,8 +54,8 @@ opensearch_distributions {
 tasks.named("preProcessFixture").configure {
   dependsOn opensearch_distributions.docker
   // always run the task, otherwise the folders won't be created
-  outputs.upToDateWhen { 
-    false 
+  outputs.upToDateWhen {
+    false
   }
   doLast {
     // tests expect to have an empty repo

qa/systemd-test/src/test/java/org/opensearch/systemdinteg/SystemdIntegTests.java

@@ -124,14 +124,14 @@ public void testReadWritePaths() throws IOException, InterruptedException {
 
     public void testMaxProcesses() throws IOException, InterruptedException {
         String limits = executeCommand("sudo su -c 'cat /proc/" + opensearchPid + "/limits'", "Failed to read process limits");
-        assertTrue("Max processes limit should be 4096 or unlimited", 
+        assertTrue("Max processes limit should be 4096 or unlimited",
                 limits.contains("Max processes             4096                 4096") ||
                 limits.contains("Max processes             unlimited            unlimited"));
     }
 
     public void testFileDescriptorLimit() throws IOException, InterruptedException {
         String limits = executeCommand("sudo su -c 'cat /proc/" + opensearchPid + "/limits'", "Failed to read process limits");
-        assertTrue("File descriptor limit should be at least 65535", 
+        assertTrue("File descriptor limit should be at least 65535",
                 limits.contains("Max open files            65535                65535") ||
                 limits.contains("Max open files            unlimited            unlimited"));
     }

rewrite.yml

@@ -0,0 +1,69 @@
+---
+type: specs.openrewrite.org/v1beta/recipe
+name: org.opensearch.openrewrite.SanityCheck
+displayName: Apply all Java & Gradle best practices
+description: Comprehensive code quality recipe combining modernization, security, and best practices.
+tags:
+  - java
+  - gradle
+  - static-analysis
+  - cleanup
+recipeList:
+  - org.openrewrite.gradle.EnableGradleBuildCache
+  - org.openrewrite.gradle.EnableGradleParallelExecution
+  - org.openrewrite.gradle.GradleBestPractices
+  - org.openrewrite.java.format.NormalizeLineBreaks
+  - org.openrewrite.java.format.RemoveTrailingWhitespace
+  - org.openrewrite.staticanalysis.RemoveUnusedPrivateMethods
+  # TBD
+  # - org.openrewrite.java.RemoveUnusedImports
+  # - org.openrewrite.java.format.NormalizeFormat
+  # - org.openrewrite.java.migrate.UpgradeToJava17
+  # - org.openrewrite.java.migrate.lang.FindVirtualThreadOpportunities # don't want to use: https://github.com/diffplug/spotless/pull/2684#discussion_r2433831887
+  # - org.openrewrite.java.migrate.lang.StringRulesRecipes
+  # - org.openrewrite.java.migrate.util.JavaLangAPIs
+  # - org.openrewrite.java.migrate.util.JavaUtilAPIs
+  # - org.openrewrite.java.migrate.util.MigrateInflaterDeflaterToClose
+  # - org.openrewrite.java.migrate.util.ReplaceStreamCollectWithToList
+  # - org.openrewrite.java.migrate.util.SequencedCollection
+  # - org.openrewrite.java.recipes.JavaRecipeBestPractices
+  # - org.openrewrite.java.recipes.RecipeTestingBestPractices
+  # - org.openrewrite.java.security.JavaSecurityBestPractices
+  # - org.openrewrite.staticanalysis.BufferedWriterCreationRecipes
+  # - org.openrewrite.staticanalysis.CommonStaticAnalysis
+  # - org.openrewrite.staticanalysis.EqualsAvoidsNull
+  # - org.openrewrite.staticanalysis.JavaApiBestPractices
+  # - org.openrewrite.staticanalysis.LowercasePackage
+  # - org.openrewrite.staticanalysis.MissingOverrideAnnotation
+  # - org.openrewrite.staticanalysis.ModifierOrder
+  # - org.openrewrite.staticanalysis.NoFinalizer
+  # - org.openrewrite.staticanalysis.NoToStringOnStringType
+  # - org.openrewrite.staticanalysis.NoValueOfOnStringType
+  # - org.openrewrite.staticanalysis.RemoveUnusedLocalVariables
+  # - org.openrewrite.staticanalysis.RemoveUnusedPrivateFields
+  # - org.openrewrite.staticanalysis.ReplaceApacheCommonsLang3ValidateNotNullWithObjectsRequireNonNull
+  # - org.openrewrite.staticanalysis.SimplifyTernaryRecipes
+  # - org.openrewrite.staticanalysis.URLEqualsHashCodeRecipes
+  # - org.openrewrite.staticanalysis.UnnecessaryCloseInTryWithResources
+  # - org.openrewrite.staticanalysis.UnnecessaryExplicitTypeArguments
+  # - org.openrewrite.staticanalysis.UnnecessaryParentheses
+  # - org.openrewrite.staticanalysis.UnnecessaryReturnAsLastStatement
+  # - tech.picnic.errorprone.refasterrules.BigDecimalRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.CharSequenceRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.ClassRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.CollectionRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.ComparatorRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.EqualityRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.FileRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.MapRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.MicrometerRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.MockitoRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.NullRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.OptionalRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.PatternRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.PreconditionsRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.PrimitiveRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.StreamRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.StringRulesRecipes
+  # - tech.picnic.errorprone.refasterrules.TimeRulesRecipes
+---

test/fixtures/azure-fixture/build.gradle

@@ -41,8 +41,8 @@ dependencies {
 preProcessFixture {
   dependsOn jar, configurations.runtimeClasspath
   // always run the task, otherwise the folders won't be created
-  outputs.upToDateWhen { 
-    false 
+  outputs.upToDateWhen {
+    false
   }
   doLast {
     file("${testFixturesDir}/shared").mkdirs()

test/fixtures/gcs-fixture/build.gradle

@@ -41,8 +41,8 @@ group = 'fixture.gcs'
 preProcessFixture {
   dependsOn jar, configurations.runtimeClasspath
   // always run the task, otherwise the folders won't be created
-  outputs.upToDateWhen { 
-    false 
+  outputs.upToDateWhen {
+    false
   }
   doLast {
     file("${testFixturesDir}/shared").mkdirs()

test/fixtures/s3-fixture/build.gradle

@@ -41,8 +41,8 @@ dependencies {
 preProcessFixture {
   dependsOn jar, configurations.runtimeClasspath
   // always run the task, otherwise the folders won't be created
-  outputs.upToDateWhen { 
-    false 
+  outputs.upToDateWhen {
+    false
   }
   doLast {
     file("${testFixturesDir}/shared").mkdirs()