-
Notifications
You must be signed in to change notification settings - Fork 110
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
Changes from 24 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
3e0118c
migrate recipes as-is
timo-a 2f1b569
fix year in license
timo-a 50f5b9c
minor polish
timo-a ccd2181
remove line in recipe spec
timo-a b4580ff
Update src/test/java/org/openrewrite/java/migrate/lombok/NormalizeGet…
timo-a 5d252e1
fix build
timo-a 4121f30
apply suggestions from review bot
timo-a 8ed2ce9
Merge the `deriveGetterMethodName` methods
timtebeek f404deb
Inline MethodAcc
timtebeek 9fb20c6
Inline MethodRecorder & rename list of methods not renamed
timtebeek a84a77e
Remove unnecessary packages
timtebeek c9eb4a3
Capture current behavior with a running test
timtebeek 5f38f5a
markdown and comments
timo-a 73bc591
add fieldaccess
timo-a 3a37f7d
add document missing behaviour
timo-a 5605d68
deal wth subtypes
timo-a c1d5850
Merge branch 'main' into lombok/normalize-getter
timtebeek 33aeb94
Merge branch 'main' into lombok/normalize-getter
timtebeek 6468fed
Merge branch 'main' into lombok/normalize-getter
timtebeek 6a8a51f
Apply suggestions from code review
timtebeek f62c3e0
Group the tests that expect no change
timtebeek 5113216
Use early returns in `isEffectivelyGetter`
timtebeek 8201fe5
Adopt `TypeUtils.isOverride`
timtebeek 927fa72
Use `MethodMatcher.methodPattern(method)` and `getTypesInUse().getDec…
timtebeek 023be35
Update src/main/java/org/openrewrite/java/migrate/lombok/NormalizeGet…
timtebeek 66db380
Expect to fail for a case not covered yet
timtebeek 3fac0f9
Expect partial rename outside of `NoChange` nested class
timtebeek 857d2ec
refactor: rename to sth. less ambiguous
timo-a File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
139 changes: 139 additions & 0 deletions
139
src/main/java/org/openrewrite/java/migrate/lombok/NormalizeGetter.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| /* | ||
| * 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; | ||
|
|
||
| import static java.util.stream.Collectors.toList; | ||
|
|
||
| @Value | ||
| @EqualsAndHashCode(callSuper = false) | ||
| public class NormalizeGetter extends ScanningRecipe<List<NormalizeGetter.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; | ||
| } | ||
| }; | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.