Skip to content

Commit 297f424

Browse files
author
Vincent Potucek
committed
Introduce Rewrite & PMD covering S1144: Unused "private" methods should be removed
Signed-off-by: Vincent Potucek <[email protected]>
1 parent a033cd1 commit 297f424

File tree

8 files changed

+172
-2
lines changed

8 files changed

+172
-2
lines changed

.github/workflows/precommit.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,8 @@ jobs:
2626
shell: bash
2727
run: |
2828
./gradlew javadoc precommit --parallel
29+
- name: Run Gradle (rewrite)
30+
continue-on-error: ${{ matrix.experimental }}
31+
shell: bash
32+
run: ./gradlew rewriteDryRun -Dorg.gradle.jvmargs=-Xmx8G
33+
timeout-minutes: 90 # dependency lookup takes 15 mins

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1414
- Use S3CrtClient for higher throughput while uploading files to S3 ([#18800](https://github.com/opensearch-project/OpenSearch/pull/18800))
1515
- Add a dynamic setting to change skip_cache_factor and min_frequency for querycache ([#18351](https://github.com/opensearch-project/OpenSearch/issues/18351))
1616
- Add overload constructor for Translog to accept Channel Factory as a parameter ([#18918](https://github.com/opensearch-project/OpenSearch/pull/18918))
17+
- Introduce `Rewrite & PMD` covering S1144: Unused "private" methods should be removed ([#18791](https://github.com/opensearch-project/OpenSearch/pull/18791))
1718

1819
### Changed
1920
- Add CompletionStage variants to methods in the Client Interface and default to ActionListener impl ([#18998](https://github.com/opensearch-project/OpenSearch/pull/18998))

DEVELOPER_GUIDE.md

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ Fork [opensearch-project/OpenSearch](https://github.com/opensearch-project/OpenS
7676

7777
#### JDK
7878

79-
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`.
79+
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`.
8080

81-
Download Java 11 from [here](https://adoptium.net/releases.html?variant=openjdk11).
81+
Download Java 11 from [here](https://adoptium.net/releases.html?variant=openjdk11).
8282

8383

8484
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:
341341
* Note that JavaDoc and block comments i.e. `/* ... */` are not formatted, but line comments i.e `// ...` are.
342342
* 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.
343343

344+
## PMD
345+
346+
As a quick alternative to rewrite, it's quick scanning locally. Build being part of pre-commit executed (optionally) locally but definitely on CI.
347+
348+
### ⚙️ Usage
349+
350+
- **pmdMain (check for compliance):**
351+
- Full project:
352+
- `./gradlew pmdMain`
353+
- Subproject (e.g., `server`):
354+
- `./gradlew server:pmdMain`
355+
- **Apply needed transformations by leveraging rewrite, please see below**
356+
357+
## Rewrite
358+
*(Advanced Refactoring, Modernization, and Bulk Code Changes)*
359+
360+
The OpenSearch build system supports **automated, large-scale code transformations** using [Moderne](https://moderne.io/) and OpenRewrite. These tools enable:
361+
362+
- **Safe Refactoring** (e.g., renaming methods, migrating deprecated APIs)
363+
- **Modernization** (e.g., adopting Java 17 features like `var`, `records`, or `switch` expressions)
364+
- **Anti-Pattern Fixes** (e.g., replacing `Vector` with `ArrayList`, removing redundant `StringBuilder`)
365+
- **Convention Enforcement** (e.g., consistent JUnit test naming, `final` keyword usage)
366+
- **Dependency Upgrades** (e.g., automatic migration when updating library versions)
367+
368+
### ⚙️ Usage
369+
370+
- **Dry-run (check for changes):**
371+
- Full project:
372+
- `./gradlew rewriteDryRun -Dorg.gradle.jvmargs=-Xmx8G`
373+
- Subproject (e.g., `server`):
374+
- `./gradlew server:rewriteDryRun -Dorg.gradle.jvmargs=-Xmx8G`
375+
---
376+
377+
- **Apply transformations:**
378+
- Full project:
379+
- `./gradlew rewriteRun -Dorg.gradle.jvmargs=-Xmx8G`
380+
- Subproject:
381+
- `./gradlew server:rewriteRun -Dorg.gradle.jvmargs=-Xmx8G`
382+
---
383+
### 🛠️ Example Transformations
384+
385+
| **Before** | **After** | **Rule** |
386+
|--------------------------------|------------------------------------|-----------------------------------|
387+
| `new ArrayList<String>()` | `new ArrayList<>()` | Diamond Operator (Java 7+) |
388+
| `if (x == false)` | `if (!x)` | Boolean Simplification |
389+
| `@Test public void testFoo()` | `@Test void foo()` | JUnit 5 Convention |
390+
391+
### ⚠️ Notes
392+
393+
- **Custom Rules**: Add project-specific rewrites in `code-convention.yml`.
394+
- **Exclusions**: Use `// rewrite:off` and `// rewrite:on` to opt out of transformations for specific code blocks.
395+
- **PR Reviews**: Automated rewrites are flagged in CI. Verify changes match intent before merging.
396+
- **javalangoutofmemoryerror**: https://docs.openrewrite.org/reference/faq#im-getting-javalangoutofmemoryerror-java-heap-space-when-running-openrewrite
397+
344398
## Adding Dependencies
345399

346400
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.

build.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ plugins {
5656
id 'opensearch.global-build-info'
5757
id "com.diffplug.spotless" version "6.25.0" apply false
5858
id "org.gradle.test-retry" version "1.6.2" apply false
59+
id "org.openrewrite.rewrite" version "7.15.0" apply false
60+
id "pmd"
5961
id "test-report-aggregation"
6062
id 'jacoco-report-aggregation'
6163
}
@@ -65,6 +67,8 @@ apply from: 'gradle/runtime-jdk-provision.gradle'
6567
apply from: 'gradle/ide.gradle'
6668
apply from: 'gradle/forbidden-dependencies.gradle'
6769
apply from: 'gradle/formatting.gradle'
70+
apply from: 'gradle/pmd.gradle'
71+
apply from: 'gradle/rewrite.gradle'
6872
apply from: 'gradle/local-distribution.gradle'
6973
apply from: 'gradle/run.gradle'
7074
apply from: 'gradle/missing-javadoc.gradle'

gradle/pmd.gradle

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*
8+
* Modifications Copyright OpenSearch Contributors. See
9+
* GitHub history for details.
10+
*/
11+
12+
/*
13+
* Licensed to Elasticsearch under one or more contributor
14+
* license agreements. See the NOTICE file distributed with
15+
* this work for additional information regarding copyright
16+
* ownership. Elasticsearch licenses this file to you under
17+
* the Apache License, Version 2.0 (the "License"); you may
18+
* not use this file except in compliance with the License.
19+
* You may obtain a copy of the License at
20+
*
21+
* http://www.apache.org/licenses/LICENSE-2.0
22+
*
23+
* Unless required by applicable law or agreed to in writing,
24+
* software distributed under the License is distributed on an
25+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
26+
* KIND, either express or implied. See the License for the
27+
* specific language governing permissions and limitations
28+
* under the License.
29+
*/
30+
31+
import org.opensearch.gradle.BuildPlugin
32+
33+
allprojects {
34+
plugins.withType(BuildPlugin).whenPluginAdded {
35+
project.apply plugin: "pmd"
36+
37+
pmd {
38+
consoleOutput = true
39+
ruleSetFiles = files("${rootDir}/pmd.xml")
40+
toolVersion = "7.16.0"
41+
}
42+
43+
precommit.dependsOn('pmdMain')
44+
}
45+
}

gradle/rewrite.gradle

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*
8+
* Modifications Copyright OpenSearch Contributors. See
9+
* GitHub history for details.
10+
*/
11+
12+
/*
13+
* Licensed to Elasticsearch under one or more contributor
14+
* license agreements. See the NOTICE file distributed with
15+
* this work for additional information regarding copyright
16+
* ownership. Elasticsearch licenses this file to you under
17+
* the Apache License, Version 2.0 (the "License"); you may
18+
* not use this file except in compliance with the License.
19+
* You may obtain a copy of the License at
20+
*
21+
* http://www.apache.org/licenses/LICENSE-2.0
22+
*
23+
* Unless required by applicable law or agreed to in writing,
24+
* software distributed under the License is distributed on an
25+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
26+
* KIND, either express or implied. See the License for the
27+
* specific language governing permissions and limitations
28+
* under the License.
29+
*/
30+
31+
project.apply plugin: "org.openrewrite.rewrite"
32+
33+
rewrite {
34+
activeRecipe("DefinitionOfDone")
35+
exclusion("**SearchAfterIT.java")
36+
exclusion("**StarTreeMapper.java")
37+
setExportDatatables(true)
38+
setFailOnDryRunResults(true)
39+
}
40+
41+
dependencies {
42+
rewrite("org.openrewrite.recipe:rewrite-static-analysis:2.15.0")
43+
}

pmd.xml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?xml version="1.0"?>
2+
<ruleset name="DefinitionOfDone"
3+
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
4+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
5+
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.net/ruleset_2_0_0.xsd">
6+
<description>DefinitionOfDone</description>
7+
<exclude-pattern>.*/sfBugs/.*</exclude-pattern>
8+
<exclude-pattern>.*/spotbugsTestCases/.*</exclude-pattern>
9+
<rule ref="category/java/bestpractices.xml/UnusedPrivateMethod"/>
10+
<!--<rule ref="category/java/codestyle.xml/UnnecessaryImport"/>-->
11+
</ruleset>

rewrite.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
type: specs.openrewrite.org/v1beta/recipe
2+
name: DefinitionOfDone
3+
displayName: Definition of Done
4+
description: Automatically cleanup code to apply coding convention.
5+
recipeList:
6+
- org.openrewrite.staticanalysis.RemoveUnusedPrivateMethods
7+
#- org.openrewrite.java.RemoveUnusedImports

0 commit comments

Comments
 (0)