Skip to content

Commit 6a7ddb7

Browse files
author
Vincent Potucek
committed
Introduce Rewrite & PMD fixing S1144: Unused "private" methods should be removed
1 parent a033cd1 commit 6a7ddb7

File tree

9 files changed

+159
-4
lines changed

9 files changed

+159
-4
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
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` fixing `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: 45 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,49 @@ 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+
## 🚀 Code Transformation & Automated Rewriting
345+
*(Advanced Refactoring, Modernization, and Bulk Code Changes)*
346+
347+
### 🔧 Automated Rewriting of Source Code
348+
The OpenSearch build system supports **automated, large-scale code transformations** using [Moderne](https://moderne.io/) and OpenRewrite. These tools enable:
349+
350+
- **Safe Refactoring** (e.g., renaming methods, migrating deprecated APIs)
351+
- **Modernization** (e.g., adopting Java 17 features like `var`, `records`, or `switch` expressions)
352+
- **Anti-Pattern Fixes** (e.g., replacing `Vector` with `ArrayList`, removing redundant `StringBuilder`)
353+
- **Convention Enforcement** (e.g., consistent JUnit test naming, `final` keyword usage)
354+
- **Dependency Upgrades** (e.g., automatic migration when updating library versions)
355+
356+
357+
### ⚙️ Usage
358+
359+
- **Dry-run (check for changes):**
360+
- Full project:
361+
- `./gradlew rewriteDryRun`
362+
- Subproject (e.g., `server`):
363+
- `./gradlew server: rewriteDryRun`
364+
---
365+
366+
- **Apply transformations:**
367+
- Full project:
368+
- `./gradlew rewriteRun`
369+
- Subproject:
370+
- `./gradlew server:rewriteRun`
371+
---
372+
### 🛠️ Example Transformations
373+
374+
| **Before** | **After** | **Rule** |
375+
|--------------------------------|------------------------------------|-----------------------------------|
376+
| `new ArrayList<String>()` | `new ArrayList<>()` | Diamond Operator (Java 7+) |
377+
| `if (x == false)` | `if (!x)` | Boolean Simplification |
378+
| `@Test public void testFoo()` | `@Test void foo()` | JUnit 5 Convention |
379+
380+
### ⚠️ Notes
381+
382+
- **Custom Rules**: Add project-specific rewrites in `code-convention.yml`.
383+
- **Exclusions**: Use `// rewrite:off` and `// rewrite:on` to opt out of transformations for specific code blocks.
384+
- **PR Reviews**: Automated rewrites are flagged in CI. Verify changes match intent before merging.
385+
- **javalangoutofmemoryerror**: https://docs.openrewrite.org/reference/faq#im-getting-javalangoutofmemoryerror-java-heap-space-when-running-openrewrite
386+
344387
## Adding Dependencies
345388

346389
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" apply false
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.properties

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@
1313
org.gradle.caching=true
1414
org.gradle.warning.mode=none
1515
org.gradle.parallel=true
16-
org.gradle.jvmargs=-Xmx3g -XX:+HeapDumpOnOutOfMemoryError -Xss2m \
16+
org.gradle.jvmargs=-Xmx8g -XX:+HeapDumpOnOutOfMemoryError -Xss2m \
1717
--add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
1818
--add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
1919
--add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
2020
--add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
2121
--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
22-
options.forkOptions.memoryMaximumSize=3g
22+
options.forkOptions.memoryMaximumSize=8g
2323

2424
# Disable duplicate project id detection
2525
# See https://docs.gradle.org/current/userguide/upgrading_version_6.html#duplicate_project_names_may_cause_publication_to_fail

gradle/pmd.gradle

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
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: "pmd"
32+
33+
pmd {
34+
consoleOutput = true
35+
ruleSetFiles = files("${rootDir}/pmd.xml")
36+
toolVersion = "7.16.0"
37+
}
38+
39+
tasks.check.dependsOn(pmdMain)

gradle/rewrite.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+
project.apply plugin: "org.openrewrite.rewrite"
32+
33+
rewrite {
34+
activeRecipe("org.opensearch.openrewrite.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+
}
44+
45+
tasks.check.dependsOn(rewriteDryRun)

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: org.opensearch.openrewrite.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)