Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
* Copyright 2025 the original author or authors.
* <p>
* Licensed under the Moderne Source Available License (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* 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.
*/
package org.openrewrite.staticanalysis;

import org.openrewrite.*;
import org.openrewrite.java.*;
import org.openrewrite.java.tree.*;
import org.openrewrite.marker.Markers;

import static java.util.Collections.emptyList;

public class RemovePrivateFieldUnderscores extends Recipe {

@Override
public String getDisplayName() {
return "Remove underscores from private class field names";
}

@Override
public String getDescription() {
return "Removes prefix or suffix underscores from private class field names and adds `this.` to all field accesses for clarity and consistency.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new JavaVisitor<ExecutionContext>() {
@Override
public J visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) {
J.VariableDeclarations mv = (J.VariableDeclarations) super.visitVariableDeclarations(multiVariable, ctx);
if (!mv.hasModifier(J.Modifier.Type.Private)) {
return mv;
}

Cursor parent = getCursor().getParentTreeCursor();
if (!(parent.getValue() instanceof J.Block && parent.getParentTreeCursor().getValue() instanceof J.ClassDeclaration)) {
return mv;
}

for (J.VariableDeclarations.NamedVariable variable : mv.getVariables()) {
String oldName = variable.getSimpleName();
if (oldName.startsWith("_") || oldName.endsWith("_")) {
String newName = oldName.startsWith("_") ? oldName.substring(1) : oldName.substring(0, oldName.length() - 1);
doAfterVisit(new AddThisToFieldUses(variable));
doAfterVisit(new RenameVariable(variable, newName));
}
}
return mv;
}
};
}

private static class AddThisToFieldUses extends JavaVisitor<ExecutionContext> {
private final JavaType.Variable fieldType;

public AddThisToFieldUses(J.VariableDeclarations.NamedVariable field) {
this.fieldType = field.getVariableType();
}

@Override
public J visitIdentifier(J.Identifier identifier, ExecutionContext ctx) {
J.Identifier id = (J.Identifier) super.visitIdentifier(identifier, ctx);
if (id.getFieldType() != null && id.getFieldType().equals(this.fieldType)) {
if (getCursor().getParentTreeCursor().getValue() instanceof J.VariableDeclarations.NamedVariable) {
return id;
}

if (getCursor().getParentTreeCursor().getValue() instanceof J.FieldAccess) {
return id;
}

return new J.FieldAccess(
Tree.randomId(),
identifier.getPrefix(),
Markers.EMPTY,
new J.Identifier(
Tree.randomId(),
Space.EMPTY,
Markers.EMPTY,
emptyList(),
"this",
identifier.getType(),
null
),
JLeftPadded.build(identifier.withPrefix(Space.EMPTY).withSimpleName(id.getSimpleName())),
identifier.getType()
);
}
return id;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
/*
* Copyright 2025 the original author or authors.
* <p>
* Licensed under the Moderne Source Available License (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* 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.
*/
package org.openrewrite.staticanalysis;

import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

import static org.openrewrite.java.Assertions.java;

class RemovePrivateFieldUnderscoresTest implements RewriteTest {

@Override
public void defaults(RecipeSpec spec) {
spec.recipe(new RemovePrivateFieldUnderscores());
}

@DocumentExample
@Test
void removesPrefixUnderscore() {
rewriteRun(
//language=java
java(
"""
public class ParseLocation {
private String _ruleName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to add a test showing either class_ or _class to show we do not end up renaming to restricted keywords.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. I've added the test case!


public String getRuleName() {
return _ruleName;
}

public void setRuleName(String ruleName) {
_ruleName = ruleName;
}
}
""",
"""
public class ParseLocation {
private String ruleName;

public String getRuleName() {
return this.ruleName;
Copy link

@dsgrieve dsgrieve Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unnecessary where there is no ambiguity. I think a recipe should do one thing which, in this case, is remove the underscore. IMO, this behavior should be removed from the visitor.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! Your point about recipes having a single responsibility makes a lot of sense. I'll get this updated shortly.

}

public void setRuleName(String ruleName) {
this.ruleName = ruleName;
}
}
"""
)
);
}

@Test
void removesSuffixUnderscore() {
rewriteRun(
//language=java
java(
"""
public class ParseLocation {
private String ruleName_;

public String getRuleName() {
return ruleName_;
}

public void setRuleName(String ruleName) {
ruleName_ = ruleName;
}
}
""",
"""
public class ParseLocation {
private String ruleName;

public String getRuleName() {
return this.ruleName;
}

public void setRuleName(String ruleName) {
this.ruleName = ruleName;
}
}
"""
)
);
}

@Test
void handlesFieldAccessInOtherMethods() {
rewriteRun(
//language=java
java(
"""
public class Calculator {
private int _operand1;
private int operand2_;

public Calculator(int a, int b) {
_operand1 = a;
operand2_ = b;
}

public int sum() {
return _operand1 + operand2_;
}
}
""",
"""
public class Calculator {
private int operand1;
private int operand2;

public Calculator(int a, int b) {
this.operand1 = a;
this.operand2 = b;
}

public int sum() {
return this.operand1 + this.operand2;
}
}
"""
)
);
}

@Test
void handlesFieldsAlreadyQualifiedWithThis() {
rewriteRun(
//language=java
java(
"""
public class ParseLocation {
private String _ruleName;

public String getRuleName() {
return this._ruleName;
}

public void setRuleName(String ruleName) {
this._ruleName = ruleName;
}
}
""",
"""
public class ParseLocation {
private String ruleName;

public String getRuleName() {
return this.ruleName;
}

public void setRuleName(String ruleName) {
this.ruleName = ruleName;
}
}
"""
)
);
}

@Test
void doesNotChangeNonPrivateFields() {
rewriteRun(
//language=java
java(
"""
public class MyClass {
public String _publicField;
protected String _protectedField;
String packagePrivateField_;
}
"""
)
);
}

@Test
void doesNotChangeLocalVariables() {
rewriteRun(
//language=java
java(
"""
public class MyClass {
public void myMethod() {
String _localVar = "prefixUnderscore";
String localVar_ = "suffixUnderscore";
}
}
"""
)
);
}
}