Skip to content

Fix #5953: [java] PoC: Add rewrite support for errorprone.refasterrules #5956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 33 additions & 5 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
enableCrossOsArchive: true
- name: Fast Build with Maven
run: |
./mvnw --show-version --errors --batch-mode \
./mvnw --show-version --errors --batch-mode --no-transfer-progress \
-Dmaven.repo.local=.m2/repository \
verify -PfastSkip -DskipTests -Dcyclonedx.skip=false \
deploy:deploy -DaltDeploymentRepository="dogfood::file://$(pwd)/target/staging"
Expand Down Expand Up @@ -97,7 +97,7 @@ jobs:
name: compile-artifact
- name: Full Build with Maven
run: |
./mvnw --show-version --errors --batch-mode \
./mvnw --show-version --errors --batch-mode --no-transfer-progress \
-Dmaven.repo.local=.m2/repository \
verify -DskipTests
- uses: actions/upload-artifact@v4
Expand Down Expand Up @@ -154,14 +154,42 @@ jobs:
name: compile-artifact
- name: Build with Maven and run unit tests
run: |
./mvnw --show-version --errors --batch-mode \
./mvnw --show-version --errors --batch-mode --no-transfer-progress \
-Dmaven.repo.local=.m2/repository \
verify \
-PfastSkip -Dcyclonedx.skip=false \
-Djava8.home="${JAVA_HOME_8_X64}" \
-Djava17.home="${JAVA_HOME_17_X64}" \
-Djava21.home="${JAVA_HOME_21_X64}"

rewrite:
needs: compile
timeout-minutes: 90
runs-on: ubuntu-latest
defaults:
run:
shell: bash
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
with:
distribution: 'temurin'
java-version: '11'
- uses: actions/cache@v4
with:
key: maven-${{ hashFiles('**/pom.xml') }}
restore-keys: maven-
path: .m2/repository
enableCrossOsArchive: true
- uses: actions/download-artifact@v4
with:
name: compile-artifact
- name: Rewrite
run: |
./mvnw --show-version --errors --batch-mode --no-transfer-progress \
-Dmaven.repo.local=.m2/repository \
clean rewrite:dryRun

dogfood:
needs: compile
timeout-minutes: 30
Expand Down Expand Up @@ -232,7 +260,7 @@ jobs:
echo "::endgroup::"

echo "::group::Run ./mvnw verify"
./mvnw --show-version --errors --batch-mode \
./mvnw --show-version --errors --batch-mode --no-transfer-progress \
--settings "${maven_settings_file}" \
-Dmaven.repo.local=.m2/repository \
verify \
Expand Down Expand Up @@ -272,7 +300,7 @@ jobs:
name: compile-artifact
- name: Generate rule docs
run: |
./mvnw --show-version --errors --batch-mode \
./mvnw --show-version --errors --batch-mode --no-transfer-progress \
-Dmaven.repo.local=.m2/repository \
verify \
-Pgenerate-rule-docs,fastSkip \
Expand Down
16 changes: 16 additions & 0 deletions docs/pages/pmd/devdocs/contributing/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,20 @@ There are various channels, on which you can ask questions:

* Ask your question our [PMD Guru at Gurubase](https://gurubase.io/g/pmd).

## 🚀 Code Transformation & Automated Rewriting
*(Advanced Refactoring, Modernization, and Bulk Code Changes)*

#### 🔧 Automated rewriting of source code to
- **Refactor** safely (e.g., rename methods, migrate APIs)
- **Modernize** (e.g., Java 8 → Java 17 features)
- **Patterns** (e.g., replace `Vector` with `ArrayList`)
- **Enforce conventions** (e.g., JUnit's naming rules)

The build system incorporates [OpenRewrite](https://docs.openrewrite.org/) capabilities for automated code transformations.

- Check for compliance with `mvn rewrite:dryRun`.
- Apply suggestions with `mvn rewrite:run`.

## Code Style

PMD uses [Checkstyle](https://checkstyle.org/) to enforce a common code style.
Expand All @@ -89,6 +103,8 @@ See [pmd-checkstyle-config.xml](https://github.com/pmd/build-tools/blob/main/src
[the eclipse configuration files](https://github.com/pmd/build-tools/tree/main/eclipse) that can
be imported into a fresh workspace.

Rewrite will automatically apply all rules, by using the CodeCleanup recipe like in this [showcase](https://docs.openrewrite.org/running-recipes/popular-recipe-guides/automatically-fix-checkstyle-violations).

## Add yourself as contributor

We use [All Contributors](https://allcontributors.org/en) - all our contributors are listed on the page [Credits](pmd_projectdocs_credits.html).
Expand Down
6 changes: 3 additions & 3 deletions pmd-ant/src/main/java/net/sourceforge/pmd/ant/Formatter.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,16 @@ private void start(String baseDir) {
Charset charset;
{
String s = (String) properties.get("encoding");
if (null == s) {
if (s == null) {

if (toConsole) {
s = getConsoleEncoding();
if (null == s) {
if (s == null) {
s = System.getProperty("file.encoding");
}
}

if (null == s) {
if (s == null) {
charset = StandardCharsets.UTF_8;
} else {
charset = Charset.forName(s);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ protected <P, R> R acceptApexVisitor(ApexVisitor<? super P, ? extends R> visitor
* The type name does NOT include type arguments.
*/
public String getSuperInterfaceName() {
return node.getExtendsTypes().stream().map(TypeRef::asTypeErasedString).findFirst().orElse("");
return node.getExtendsTypes().stream().findFirst().map(TypeRef::asTypeErasedString).orElse("");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public static void reconfigureDefaultLogLevel(Level level) {
Map<String, Logger> loggerMap = (Map<String, Logger>) loggerMapField.get(loggerFactory);
for (Logger logger : loggerMap.values()) {
if (logger.getName().startsWith(PMD_ROOT_LOGGER)
&& simpleLoggerClass.isAssignableFrom(logger.getClass())) {
&& simpleLoggerClass.isInstance(logger)) {
String newConfiguredLevel = (String) levelStringMethod.invoke(logger);
int newLogLevel = newDefaultLogLevel;
if (newConfiguredLevel != null) {
Expand Down Expand Up @@ -132,7 +132,7 @@ public static boolean isSimpleLogger() {
try {
ILoggerFactory loggerFactory = LoggerFactory.getILoggerFactory();
Class<?> loggerFactoryClass = loggerFactory.getClass().getClassLoader().loadClass(SIMPLE_LOGGER_FACTORY_CLASS);
return loggerFactoryClass.isAssignableFrom(loggerFactory.getClass());
return loggerFactoryClass.isInstance(loggerFactory);
} catch (ClassNotFoundException e) {
// not slf4j simple logger
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import java.util.LinkedHashSet;
import java.util.Set;
import java.util.stream.Collectors;

import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
Expand Down Expand Up @@ -94,7 +93,7 @@ private static String makeMessage(@NonNull JavaccToken currentToken,
expectedBranches.add(expected.toString());
}

String expected = expectedBranches.stream().collect(Collectors.joining(System.lineSeparator() + " "));
String expected = String.join(System.lineSeparator() + " ", expectedBranches);

StringBuilder retval = new StringBuilder("Encountered ");
if (maxSize > 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import java.util.DoubleSummaryStatistics;
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

import net.sourceforge.pmd.lang.ast.Node;
Expand Down Expand Up @@ -65,8 +64,7 @@ public static <O extends Node> DoubleSummaryStatistics computeStatistics(Metric<
Objects.requireNonNull(ops, NULL_NODE_MESSAGE);

return StreamSupport.stream(ops.spliterator(), false)
.filter(key::supports)
.collect(Collectors.summarizingDouble(op -> computeMetric(key, op, options).doubleValue()));
.filter(key::supports).mapToDouble(op -> computeMetric(key, op, options).doubleValue()).summaryStatistics();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public boolean equals(Object o) {

AbstractRule that = (AbstractRule) o;
return Objects.equals(getName(), that.getName())
&& Objects.equals(getPriority(), that.getPriority())
&& getPriority() == that.getPriority()
&& super.equals(o);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private boolean isConsideredReturnType(Method method) {
try {
// ignore type variables, such as List<N>… we could check all bounds, but probably it's overkill
Type actualTypeArgument = ((ParameterizedType) t).getActualTypeArguments()[0];
if (!TypeVariable.class.isAssignableFrom(actualTypeArgument.getClass())) {
if (!TypeVariable.class.isInstance(actualTypeArgument)) {
Class<?> elementKlass = Class.forName(actualTypeArgument.getTypeName());
return CONSIDERED_RETURN_TYPES.contains(elementKlass) || elementKlass.isEnum();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public String getStringValue() {
// potentially text nodes
return node.descendants(TextNode.class).toStream()
.map(TextNode::getText)
.collect(Collectors.joining(""));
.collect(Collectors.joining());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void addDeclaration(NameDeclaration declaration) {
public <T extends Scope> T getEnclosingScope(Class<T> clazz) {
Scope current = this;
while (current != null) {
if (clazz.isAssignableFrom(current.getClass())) {
if (clazz.isInstance(current)) {
return (T) current;
}
current = current.getParent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ public java.io.Reader getSourceCode(String objectType, String name, String schem
* Only define callableStatement once and reuse it for subsequent calls
* to getSourceCode()
*/
if (null == callableStatement) {
if (callableStatement == null) {
LOG.trace("getSourceCode: returnSourceCodeStatement=\"{}\"", returnSourceCodeStatement);
LOG.trace("getSourceCode: returnType=\"{}\"", returnType);
callableStatement = getConnection().prepareCall(returnSourceCodeStatement);
Expand Down Expand Up @@ -282,7 +282,7 @@ public java.io.Reader getSourceCode(String objectType, String name, String schem
*/
public List<SourceObject> getSourceObjectList() {

if (null == dburi) {
if (dburi == null) {
LOG.warn("No dbUri defined - no further action possible");
return Collections.emptyList();
} else {
Expand Down Expand Up @@ -328,36 +328,36 @@ public List<SourceObject> getSourceObjectList(List<String> languages, List<Strin
* explicit parameter dburi field wildcard list
*
*/
if (null == searchLanguages) {
List<String> dbURIList = (null == dburi) ? null : dburi.getLanguagesList();
if (null == dbURIList || dbURIList.isEmpty()) {
if (searchLanguages == null) {
List<String> dbURIList = (dburi == null) ? null : dburi.getLanguagesList();
if (dbURIList == null || dbURIList.isEmpty()) {
searchLanguages = wildcardList;
} else {
searchLanguages = dbURIList;
}
}

if (null == searchSchemas) {
List<String> dbURIList = (null == dburi) ? null : dburi.getSchemasList();
if (null == dbURIList || dbURIList.isEmpty()) {
if (searchSchemas == null) {
List<String> dbURIList = (dburi == null) ? null : dburi.getSchemasList();
if (dbURIList == null || dbURIList.isEmpty()) {
searchSchemas = wildcardList;
} else {
searchSchemas = dbURIList;
}
}

if (null == searchSourceCodeTypes) {
List<String> dbURIList = (null == dburi) ? null : dburi.getSourceCodeTypesList();
if (null == dbURIList || dbURIList.isEmpty()) {
if (searchSourceCodeTypes == null) {
List<String> dbURIList = (dburi == null) ? null : dburi.getSourceCodeTypesList();
if (dbURIList == null || dbURIList.isEmpty()) {
searchSourceCodeTypes = wildcardList;
} else {
searchSourceCodeTypes = dbURIList;
}
}

if (null == searchSourceCodeNames) {
List<String> dbURIList = (null == dburi) ? null : dburi.getSourceCodeNamesList();
if (null == dbURIList || dbURIList.isEmpty()) {
if (searchSourceCodeNames == null) {
List<String> dbURIList = (dburi == null) ? null : dburi.getSourceCodeNamesList();
if (dbURIList == null || dbURIList.isEmpty()) {
searchSourceCodeNames = wildcardList;
} else {
searchSourceCodeNames = dbURIList;
Expand All @@ -366,7 +366,7 @@ public List<SourceObject> getSourceObjectList(List<String> languages, List<Strin

try {

if (null != returnSourceCodeObjectsStatement) {
if (returnSourceCodeObjectsStatement != null) {
LOG.debug("Have bespoke returnSourceCodeObjectsStatement from DBURI: \"{}\"",
returnSourceCodeObjectsStatement);
try (PreparedStatement sourceCodeObjectsStatement = getConnection()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public DBType(String subProtocol, String subnamePrefix) throws IOException {

LOG.debug("subProtocol={}, subnamePrefix={}", subProtocol, subnamePrefix);

if (null == subProtocol && null == subnamePrefix) {
if (subProtocol == null && subnamePrefix == null) {
throw new RuntimeException("subProtocol and subnamePrefix cannot both be null");
} else {

Expand Down Expand Up @@ -211,7 +211,7 @@ private Properties loadDBProperties(String matchString) throws IOException {
* any previous properties with the matched properties.
*/
String extendedPropertyFile = (String) matchedProperties.remove("extends");
if (null != extendedPropertyFile && !"".equals(extendedPropertyFile.trim())) {
if (extendedPropertyFile != null && !"".equals(extendedPropertyFile.trim())) {
Properties extendedProperties = loadDBProperties(extendedPropertyFile.trim());

// Overwrite extended properties with properties in the matched
Expand Down Expand Up @@ -378,27 +378,27 @@ public void setProperties(Properties properties) {
this.properties = properties;

// Driver Class
if (null != this.properties.getProperty("driver")) {
if (this.properties.getProperty("driver") != null) {
this.driverClass = this.properties.getProperty("driver");
}

// Database CharacterSet
if (null != this.properties.getProperty("characterset")) {
if (this.properties.getProperty("characterset") != null) {
this.characterSet = this.properties.getProperty("characterset");
}

// String to get objects
if (null != this.properties.getProperty("sourcecodetypes")) {
if (this.properties.getProperty("sourcecodetypes") != null) {
this.sourceCodeTypes = this.properties.getProperty("sourcecodetypes");
}

// Languages to process
if (null != this.properties.getProperty("languages")) {
if (this.properties.getProperty("languages") != null) {
this.languages = this.properties.getProperty("languages");
}

// Return class for source code
if (null != this.properties.getProperty("returnType")) {
if (this.properties.getProperty("returnType") != null) {
LOG.trace("returnType={}", this.properties.getProperty("returnType"));
this.sourceCodeReturnType = Integer.parseInt(this.properties.getProperty("returnType"));
}
Expand Down
Loading
Loading