Skip to content

feat: normalize effective getter methods #631

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

Merged
merged 28 commits into from
Apr 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3e0118c
migrate recipes as-is
timo-a Dec 11, 2024
2f1b569
fix year in license
timo-a Dec 15, 2024
50f5b9c
minor polish
timo-a Dec 15, 2024
ccd2181
remove line in recipe spec
timo-a Dec 15, 2024
b4580ff
Update src/test/java/org/openrewrite/java/migrate/lombok/NormalizeGet…
timo-a Dec 15, 2024
5d252e1
fix build
timo-a Dec 15, 2024
4121f30
apply suggestions from review bot
timo-a Jan 1, 2025
8ed2ce9
Merge the `deriveGetterMethodName` methods
timtebeek Jan 3, 2025
f404deb
Inline MethodAcc
timtebeek Jan 3, 2025
9fb20c6
Inline MethodRecorder & rename list of methods not renamed
timtebeek Jan 3, 2025
a84a77e
Remove unnecessary packages
timtebeek Jan 3, 2025
c9eb4a3
Capture current behavior with a running test
timtebeek Jan 3, 2025
5f38f5a
markdown and comments
timo-a Feb 16, 2025
73bc591
add fieldaccess
timo-a Feb 16, 2025
3a37f7d
add document missing behaviour
timo-a Feb 16, 2025
5605d68
deal wth subtypes
timo-a Feb 16, 2025
c1d5850
Merge branch 'main' into lombok/normalize-getter
timtebeek Mar 12, 2025
33aeb94
Merge branch 'main' into lombok/normalize-getter
timtebeek Mar 12, 2025
6468fed
Merge branch 'main' into lombok/normalize-getter
timtebeek Apr 20, 2025
6a8a51f
Apply suggestions from code review
timtebeek Apr 20, 2025
f62c3e0
Group the tests that expect no change
timtebeek Apr 20, 2025
5113216
Use early returns in `isEffectivelyGetter`
timtebeek Apr 20, 2025
8201fe5
Adopt `TypeUtils.isOverride`
timtebeek Apr 20, 2025
927fa72
Use `MethodMatcher.methodPattern(method)` and `getTypesInUse().getDec…
timtebeek Apr 20, 2025
023be35
Update src/main/java/org/openrewrite/java/migrate/lombok/NormalizeGet…
timtebeek Apr 20, 2025
66db380
Expect to fail for a case not covered yet
timtebeek Apr 20, 2025
3fac0f9
Expect partial rename outside of `NoChange` nested class
timtebeek Apr 20, 2025
857d2ec
refactor: rename to sth. less ambiguous
timo-a Apr 21, 2025
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,136 @@
/*
* Copyright 2024 the original author or authors.
* <p>
* Licensed 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
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <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.java.migrate.lombok;

import lombok.EqualsAndHashCode;
import lombok.Value;
import org.jspecify.annotations.Nullable;
import org.openrewrite.ExecutionContext;
import org.openrewrite.ScanningRecipe;
import org.openrewrite.Tree;
import org.openrewrite.TreeVisitor;
import org.openrewrite.java.ChangeMethodName;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.TypeUtils;

import java.util.ArrayList;
import java.util.List;
import java.util.Set;

@Value
@EqualsAndHashCode(callSuper = false)
public class AdoptLombokGetterMethodNames extends ScanningRecipe<List<AdoptLombokGetterMethodNames.RenameRecord>> {

private final static String DO_NOT_RENAME = "DO_NOT_RENAME";

@Override
public String getDisplayName() {
return "Rename getter methods to fit Lombok";
}

@Override
public String getDescription() {
return "Rename methods that are effectively getter to the name Lombok would give them.\n\n" +
"Limitations:\n" +
" - If two methods in a class are effectively the same getter then one's name will be corrected and the others name will be left as it is.\n" +
" - If the correct name for a method is already taken by another method then the name will not be corrected.\n" +
" - Method name swaps or circular renaming within a class cannot be performed because the names block each other.\n" +
"E.g. `int getFoo() { return ba; } int getBa() { return foo; }` stays as it is.";
}

@Value
public static class RenameRecord {
String methodPattern;
String newMethodName;
}

@Override
public List<RenameRecord> getInitialValue(ExecutionContext ctx) {
return new ArrayList<>();
}

@Override
public TreeVisitor<?, ExecutionContext> getScanner(List<RenameRecord> renameRecords) {
return new JavaIsoVisitor<ExecutionContext>() {
@Override
public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) {
// Cheaply collect all declared methods; this also means we do not support clashing nested class methods
Set<JavaType.Method> declaredMethods = cu.getTypesInUse().getDeclaredMethods();
List<String> existingMethodNames = new ArrayList<>();
for (JavaType.Method method : declaredMethods) {
existingMethodNames.add(method.getName());
}
getCursor().putMessage(DO_NOT_RENAME, existingMethodNames);
return super.visitCompilationUnit(cu, ctx);
}

@Override
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
if (method.getMethodType() == null || method.getBody() == null ||
!LombokUtils.isEffectivelyGetter(method) ||
TypeUtils.isOverride(method.getMethodType())) {
return method;
}

String simpleName;
Expression returnExpression = ((J.Return) method.getBody().getStatements().get(0)).getExpression();
if (returnExpression instanceof J.Identifier) {
simpleName = ((J.Identifier) returnExpression).getSimpleName();
} else if (returnExpression instanceof J.FieldAccess) {
simpleName = ((J.FieldAccess) returnExpression).getSimpleName();
} else {
return method;
}

// If method already has the name it should have, then nothing to be done
String expectedMethodName = LombokUtils.deriveGetterMethodName(returnExpression.getType(), simpleName);
if (expectedMethodName.equals(method.getSimpleName())) {
return method;
}

// If the desired method name is already taken by an existing method, the current method cannot be renamed
List<String> doNotRename = getCursor().getNearestMessage(DO_NOT_RENAME);
assert doNotRename != null;
if (doNotRename.contains(expectedMethodName)) {
return method;
}

renameRecords.add(new RenameRecord(MethodMatcher.methodPattern(method), expectedMethodName));
doNotRename.remove(method.getSimpleName()); //actual method name becomes available again
doNotRename.add(expectedMethodName); //expected method name now blocked
return method;
}
};
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor(List<RenameRecord> renameRecords) {
return new TreeVisitor<Tree, ExecutionContext>() {
@Override
public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
for (RenameRecord rr : renameRecords) {
tree = new ChangeMethodName(rr.methodPattern, rr.newMethodName, true, null)
.getVisitor().visit(tree, ctx);
}
return tree;
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,25 @@ private static boolean hasMatchingTypeAndGetterName(J.MethodDeclaration method,
return false;
}

private static String deriveGetterMethodName(@Nullable JavaType type, String fieldName) {
public static boolean isEffectivelyGetter(J.MethodDeclaration method) {
if (!method.getParameters().isEmpty() && !(method.getParameters().get(0) instanceof J.Empty)) {
return false;
}
if (method.getBody() == null ||
method.getBody().getStatements().size() != 1 ||
!(method.getBody().getStatements().get(0) instanceof J.Return)) {
return false;
}
Expression returnExpression = ((J.Return) method.getBody().getStatements().get(0)).getExpression();
if (!(returnExpression instanceof J.Identifier) && !(returnExpression instanceof J.FieldAccess)) {
return false;
}
// compiler already guarantees that the returned variable is a subtype of the method type, but we want an exact match
return method.getType() == returnExpression.getType();
}

public static String deriveGetterMethodName(@Nullable JavaType type, String fieldName) {

if (type == JavaType.Primitive.Boolean) {
boolean alreadyStartsWithIs = fieldName.length() >= 3 &&
fieldName.substring(0, 3).matches("is[A-Z]");
Expand Down
Loading
Loading