diff --git a/.github/workflows/precommit.yml b/.github/workflows/precommit.yml index 541b75c73fbdc..e9bdccc861603 100644 --- a/.github/workflows/precommit.yml +++ b/.github/workflows/precommit.yml @@ -26,3 +26,8 @@ jobs: shell: bash run: | ./gradlew javadoc precommit --parallel + - name: Run Gradle (rewrite) + continue-on-error: ${{ matrix.experimental }} + shell: bash + run: ./gradlew rewriteDryRun -Dorg.gradle.jvmargs=-Xmx8G + timeout-minutes: 90 # dependency lookup takes 15 mins diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cc2e517d844c..0847cc88d7fc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Use S3CrtClient for higher throughput while uploading files to S3 ([#18800](https://github.com/opensearch-project/OpenSearch/pull/18800)) - Add a dynamic setting to change skip_cache_factor and min_frequency for querycache ([#18351](https://github.com/opensearch-project/OpenSearch/issues/18351)) - Add overload constructor for Translog to accept Channel Factory as a parameter ([#18918](https://github.com/opensearch-project/OpenSearch/pull/18918)) +- Introduce `Rewrite & PMD` covering S1144: Unused "private" methods should be removed ([#18791](https://github.com/opensearch-project/OpenSearch/pull/18791)) ### Changed - Add CompletionStage variants to methods in the Client Interface and default to ActionListener impl ([#18998](https://github.com/opensearch-project/OpenSearch/pull/18998)) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 2d7125b241af7..1d1e057a5d9f6 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -76,9 +76,9 @@ Fork [opensearch-project/OpenSearch](https://github.com/opensearch-project/OpenS #### JDK -OpenSearch recommends building with the [Temurin/Adoptium](https://adoptium.net/temurin/releases/) distribution. JDK 11 is the minimum supported, and JDK-24 is the newest supported. You must have a supported JDK installed with the environment variable `JAVA_HOME` referencing the path to Java home for your JDK installation, e.g. `JAVA_HOME=/usr/lib/jvm/jdk-21`. +OpenSearch recommends building with the [Temurin/Adoptium](https://adoptium.net/temurin/releases/) distribution. JDK 11 is the minimum supported, and JDK-24 is the newest supported. You must have a supported JDK installed with the environment variable `JAVA_HOME` referencing the path to Java home for your JDK installation, e.g. `JAVA_HOME=/usr/lib/jvm/jdk-21`. -Download Java 11 from [here](https://adoptium.net/releases.html?variant=openjdk11). +Download Java 11 from [here](https://adoptium.net/releases.html?variant=openjdk11). In addition, certain backward compatibility tests check out and compile the previous major version of OpenSearch, and therefore require installing [JDK 11](https://adoptium.net/temurin/releases/?version=11) and [JDK 17](https://adoptium.net/temurin/releases/?version=17) and setting the `JAVA11_HOME` and `JAVA17_HOME` environment variables. More to that, since 8.10 release, Gradle has deprecated the usage of the any JDKs below JDK-16. For smooth development experience, the recommendation is to install at least [JDK 17](https://adoptium.net/temurin/releases/?version=17) or [JDK 21](https://adoptium.net/temurin/releases/?version=21). If you still want to build with JDK-11 only, please add `-Dorg.gradle.warning.mode=none` when invoking any Gradle build task from command line, for example: @@ -341,6 +341,60 @@ Please follow these formatting guidelines: * Note that JavaDoc and block comments i.e. `/* ... */` are not formatted, but line comments i.e `// ...` are. * There is an implicit rule that negative boolean expressions should use the form `foo == false` instead of `!foo` for better readability of the code. While this isn't strictly enforced, it might get called out in PR reviews as something to change. +## PMD + +As a quick alternative to rewrite, it's quick scanning locally. Build being part of pre-commit executed (optionally) locally but definitely on CI. + +### ⚙️ Usage + +- **pmdMain (check for compliance):** + - Full project: + - `./gradlew pmdMain` + - Subproject (e.g., `server`): + - `./gradlew server:pmdMain` +- **Apply needed transformations by leveraging rewrite, please see below** + +## Rewrite +*(Advanced Refactoring, Modernization, and Bulk Code Changes)* + +The OpenSearch build system supports **automated, large-scale code transformations** using [Moderne](https://moderne.io/) and OpenRewrite. These tools enable: + +- **Safe Refactoring** (e.g., renaming methods, migrating deprecated APIs) +- **Modernization** (e.g., adopting Java 17 features like `var`, `records`, or `switch` expressions) +- **Anti-Pattern Fixes** (e.g., replacing `Vector` with `ArrayList`, removing redundant `StringBuilder`) +- **Convention Enforcement** (e.g., consistent JUnit test naming, `final` keyword usage) +- **Dependency Upgrades** (e.g., automatic migration when updating library versions) + +### ⚙️ Usage + +- **Dry-run (check for changes):** + - Full project: + - `./gradlew rewriteDryRun -Dorg.gradle.jvmargs=-Xmx8G` + - Subproject (e.g., `server`): + - `./gradlew server:rewriteDryRun -Dorg.gradle.jvmargs=-Xmx8G` +--- + +- **Apply transformations:** + - Full project: + - `./gradlew rewriteRun -Dorg.gradle.jvmargs=-Xmx8G` + - Subproject: + - `./gradlew server:rewriteRun -Dorg.gradle.jvmargs=-Xmx8G` +--- +### 🛠️ Example Transformations + +| **Before** | **After** | **Rule** | +|--------------------------------|------------------------------------|-----------------------------------| +| `new ArrayList()` | `new ArrayList<>()` | Diamond Operator (Java 7+) | +| `if (x == false)` | `if (!x)` | Boolean Simplification | +| `@Test public void testFoo()` | `@Test void foo()` | JUnit 5 Convention | + +### ⚠️ Notes + +- **Custom Rules**: Add project-specific rewrites in `code-convention.yml`. +- **Exclusions**: Use `// rewrite:off` and `// rewrite:on` to opt out of transformations for specific code blocks. +- **PR Reviews**: Automated rewrites are flagged in CI. Verify changes match intent before merging. +- **javalangoutofmemoryerror**: https://docs.openrewrite.org/reference/faq#im-getting-javalangoutofmemoryerror-java-heap-space-when-running-openrewrite + ## Adding Dependencies When adding a new dependency or removing an existing dependency via any `build.gradle` (that are not in the test scope), update the dependency LICENSE and library SHAs. diff --git a/build.gradle b/build.gradle index 1422495926f3f..8a8868eeaecf0 100644 --- a/build.gradle +++ b/build.gradle @@ -56,6 +56,8 @@ plugins { id 'opensearch.global-build-info' id "com.diffplug.spotless" version "6.25.0" apply false id "org.gradle.test-retry" version "1.6.2" apply false + id "org.openrewrite.rewrite" version "7.15.0" apply false + id "pmd" id "test-report-aggregation" id 'jacoco-report-aggregation' } @@ -65,6 +67,8 @@ apply from: 'gradle/runtime-jdk-provision.gradle' apply from: 'gradle/ide.gradle' apply from: 'gradle/forbidden-dependencies.gradle' apply from: 'gradle/formatting.gradle' +apply from: 'gradle/pmd.gradle' +apply from: 'gradle/rewrite.gradle' apply from: 'gradle/local-distribution.gradle' apply from: 'gradle/run.gradle' apply from: 'gradle/missing-javadoc.gradle' diff --git a/gradle/pmd.gradle b/gradle/pmd.gradle new file mode 100644 index 0000000000000..756b252482382 --- /dev/null +++ b/gradle/pmd.gradle @@ -0,0 +1,45 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import org.opensearch.gradle.BuildPlugin + +allprojects { + plugins.withType(BuildPlugin).whenPluginAdded { + project.apply plugin: "pmd" + + pmd { + consoleOutput = true + ruleSetFiles = files("${rootDir}/pmd.xml") + toolVersion = "7.16.0" + } + + precommit.dependsOn('pmdMain') + } +} diff --git a/gradle/rewrite.gradle b/gradle/rewrite.gradle new file mode 100644 index 0000000000000..264bbd93eb85c --- /dev/null +++ b/gradle/rewrite.gradle @@ -0,0 +1,43 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +project.apply plugin: "org.openrewrite.rewrite" + +rewrite { + activeRecipe("DefinitionOfDone") + exclusion("**SearchAfterIT.java") + exclusion("**StarTreeMapper.java") + setExportDatatables(true) + setFailOnDryRunResults(true) +} + +dependencies { + rewrite("org.openrewrite.recipe:rewrite-static-analysis:2.15.0") +} diff --git a/pmd.xml b/pmd.xml new file mode 100644 index 0000000000000..a9d82922c47c4 --- /dev/null +++ b/pmd.xml @@ -0,0 +1,11 @@ + + + DefinitionOfDone + .*/sfBugs/.* + .*/spotbugsTestCases/.* + + + diff --git a/rewrite.yml b/rewrite.yml new file mode 100644 index 0000000000000..8c4631f407449 --- /dev/null +++ b/rewrite.yml @@ -0,0 +1,7 @@ +type: specs.openrewrite.org/v1beta/recipe +name: DefinitionOfDone +displayName: Definition of Done +description: Automatically cleanup code to apply coding convention. +recipeList: + - org.openrewrite.staticanalysis.RemoveUnusedPrivateMethods + #- org.openrewrite.java.RemoveUnusedImports