-
Notifications
You must be signed in to change notification settings - Fork 86
Add recipe to remove unused constructor and method parameters #560
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
iddeepak
wants to merge
43
commits into
openrewrite:main
Choose a base branch
from
iddeepak:feature/RemoveUnusedParams
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
0973eaa
Add RemoveUnusedParams recipe
iddeepak d93a14c
Apply suggestions from code review
timtebeek 84d8671
Updated review comments
iddeepak 6365a31
Updated review comments
iddeepak e718033
Update src/main/java/org/openrewrite/staticanalysis/RemoveUnusedParam…
iddeepak c735b6e
Updated review comments
iddeepak afff3de
Merge branch 'main' into feature/RemoveUnusedParams
timtebeek e6a633b
Update src/main/java/org/openrewrite/staticanalysis/RemoveUnusedParam…
iddeepak 674a465
Update src/main/java/org/openrewrite/staticanalysis/RemoveUnusedParam…
iddeepak e656fa2
Updated review comments
iddeepak 11ef2ac
Updated review comments
iddeepak f88dd77
Updated review comments
iddeepak 6d989cc
Updated review comments
iddeepak 80b57e8
Apply suggestions from code review
timtebeek 95628c3
Updated review comments
iddeepak a6acdef
Added Test for nested inheritance override chain
iddeepak a4e7e3d
Update src/main/java/org/openrewrite/staticanalysis/RemoveUnusedParam…
iddeepak 8e8953f
Update src/main/java/org/openrewrite/staticanalysis/RemoveUnusedParam…
iddeepak e03e540
Merge branch 'main' into feature/RemoveUnusedParams
timtebeek 328984d
Merge branch 'openrewrite:main' into feature/RemoveUnusedParams
iddeepak c306682
Merge branch 'main' into feature/RemoveUnusedParams
timtebeek 298ee1f
Show two cases missed with conflicts after removal
timtebeek 9f689ab
Show another case missed: callers should be updated too
timtebeek 28beff4
Remove unnecessary `final` to make intentional ones stand out
timtebeek 0b77aa9
Inline `collectOverrideSignature` only used once
timtebeek c9c21ca
Move methods into named visitor for logical grouping
timtebeek d1bf138
Add missing braces
timtebeek 25ece10
Use `reduce` in `collectUsedParameters`
timtebeek 6bd0405
use ListUtils for referential comparison
iddeepak 459d327
use SemanticallyEqual comparison
iddeepak 12ccb6c
fix avoidDirectConflict and avoidInheritedConflict
iddeepak c527506
fix avoidDirectConflict
iddeepak dd23526
fix avoidInheritiedConflict
iddeepak 03056a5
clean up
iddeepak f4a0ac0
clean up
iddeepak e1f414a
Update src/main/java/org/openrewrite/staticanalysis/RemoveUnusedParam…
iddeepak 2cdeff6
fix cascadeRemove
iddeepak 4f6b346
Update src/main/java/org/openrewrite/staticanalysis/RemoveUnusedParam…
iddeepak 5e8d863
Merge branch 'main' into feature/RemoveUnusedParams
iddeepak ec388d6
fix build
iddeepak 00817b3
Merge branch 'main' into feature/RemoveUnusedParams
timtebeek 83157dc
Merge branch 'main' into feature/RemoveUnusedParams
timtebeek 5be9333
Update src/main/java/org/openrewrite/staticanalysis/RemoveUnusedParam…
timtebeek 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
309 changes: 309 additions & 0 deletions
309
src/main/java/org/openrewrite/staticanalysis/RemoveUnusedParams.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,309 @@ | ||
/* | ||
* 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 lombok.RequiredArgsConstructor; | ||
import org.openrewrite.*; | ||
import org.openrewrite.internal.ListUtils; | ||
import org.openrewrite.java.JavaIsoVisitor; | ||
import org.openrewrite.java.MethodMatcher; | ||
import org.openrewrite.java.NoMissingTypes; | ||
import org.openrewrite.java.search.SemanticallyEqual; | ||
import org.openrewrite.java.tree.Expression; | ||
import org.openrewrite.java.tree.J; | ||
import org.openrewrite.java.tree.JavaType; | ||
import org.openrewrite.java.tree.Statement; | ||
|
||
import java.util.*; | ||
|
||
public class RemoveUnusedParams extends ScanningRecipe<RemoveUnusedParams.Accumulator> { | ||
public static class Accumulator { | ||
/** | ||
* Signatures of all methods that override or implement a supertype method. | ||
* Each entry is a string of the form | ||
* <code>"fully.qualified.ClassName#methodName(paramType1,paramType2,...)"</code>. | ||
* Parameters of these methods are considered part of the public API | ||
* and will not be removed even if they appear unused. | ||
*/ | ||
private final Set<String> overrideSignatures = new HashSet<>(); | ||
|
||
private final Map<String,Set<String>> originalSignatures = new HashMap<>(); | ||
} | ||
|
||
@Override | ||
public String getDisplayName() { | ||
return "Remove obsolete constructor and method parameters"; | ||
} | ||
|
||
@Override | ||
public String getDescription() { | ||
return "Removes obsolete method parameters from signature, not used in body."; | ||
} | ||
|
||
@Override | ||
public Accumulator getInitialValue(ExecutionContext ctx) { | ||
return new Accumulator(); | ||
} | ||
|
||
@Override | ||
public TreeVisitor<?, ExecutionContext> getScanner(Accumulator acc) { | ||
return new JavaIsoVisitor<ExecutionContext>() { | ||
@Override | ||
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { | ||
J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx); | ||
JavaType.Method mt = m.getMethodType(); | ||
String className = mt.getDeclaringType().toString(); | ||
acc.originalSignatures.computeIfAbsent(className, k -> new HashSet<>()) | ||
.add(MethodMatcher.methodPattern(mt)); | ||
if (mt != null && mt.isOverride()) { | ||
while (mt != null) { | ||
acc.overrideSignatures.add(MethodMatcher.methodPattern(mt)); | ||
mt = mt.getOverride(); | ||
} | ||
} | ||
return m; | ||
} | ||
}; | ||
} | ||
|
||
@Override | ||
public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) { | ||
return Preconditions.check( | ||
new NoMissingTypes(), | ||
Repeat.repeatUntilStable(new RemoveUnusedParametersVisitor(acc))); | ||
} | ||
|
||
@RequiredArgsConstructor | ||
private static class RemoveUnusedParametersVisitor extends JavaIsoVisitor<ExecutionContext> { | ||
private final Accumulator acc; | ||
|
||
@Override | ||
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { | ||
J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx); | ||
if (shouldPruneParameters(m)) { | ||
List<Statement> prunedParams = filterUnusedParameters(m, collectUsedParameters(m)); | ||
return prunedParams == m.getParameters() ? m : applyPrunedSignature(m, prunedParams); | ||
} | ||
return m; | ||
} | ||
|
||
private boolean shouldPruneParameters(J.MethodDeclaration m) { | ||
if (m.getBody() == null || | ||
m.getMethodType() == null || | ||
m.hasModifier(J.Modifier.Type.Native) || | ||
!m.getLeadingAnnotations().isEmpty()) { | ||
return false; | ||
} | ||
return !acc.overrideSignatures.contains(MethodMatcher.methodPattern(m.getMethodType())); | ||
} | ||
|
||
private Set<String> collectUsedParameters(J.MethodDeclaration m) { | ||
Deque<Set<J.Identifier>> shadowStack = new ArrayDeque<>(); | ||
return new JavaIsoVisitor<Set<String>>() { | ||
@Override | ||
public J.Block visitBlock(J.Block block, Set<String> u) { | ||
shadowStack.push(new HashSet<>()); | ||
try { | ||
return super.visitBlock(block, u); | ||
} finally { | ||
shadowStack.pop(); | ||
} | ||
} | ||
|
||
@Override | ||
public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations decl, Set<String> u) { | ||
decl.getVariables().forEach(v -> shadowStack.peek().add(v.getName())); | ||
return super.visitVariableDeclarations(decl, u); | ||
} | ||
|
||
@Override | ||
public J.Identifier visitIdentifier(J.Identifier id, Set<String> u) { | ||
if (isVisibleParameter(id, m, shadowStack)) { | ||
u.add(id.getSimpleName()); | ||
} | ||
return id; | ||
} | ||
}.reduce(m.getBody(), new HashSet<>()); | ||
} | ||
|
||
private boolean isVisibleParameter(J.Identifier id, J.MethodDeclaration m, Deque<Set<J.Identifier>> shadowStack) { | ||
return !isShadowed(id, shadowStack) && isDeclaredAsParameter(id, m); | ||
} | ||
|
||
private boolean isShadowed(J.Identifier id, Deque<Set<J.Identifier>> shadowStack) { | ||
for (Set<J.Identifier> scope : shadowStack) { | ||
for (J.Identifier local : scope) { | ||
if (SemanticallyEqual.areEqual(id, local)) { | ||
return true; | ||
} | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
private boolean isDeclaredAsParameter(J.Identifier id, J.MethodDeclaration m) { | ||
for (Statement p : m.getParameters()) { | ||
if (p instanceof J.VariableDeclarations) { | ||
for (J.VariableDeclarations.NamedVariable v : ((J.VariableDeclarations) p).getVariables()) { | ||
if (v.getSimpleName().equals(id.getSimpleName())) { | ||
return true; | ||
} | ||
} | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
private List<Statement> filterUnusedParameters(J.MethodDeclaration m, Set<String> usedParams) { | ||
return ListUtils.map( | ||
m.getParameters(), | ||
p -> { | ||
if (!(p instanceof J.VariableDeclarations)) { | ||
return p; | ||
} | ||
return processVariableDeclaration((J.VariableDeclarations) p, usedParams); | ||
} | ||
); | ||
} | ||
|
||
private Statement processVariableDeclaration(J.VariableDeclarations decl, Set<String> usedParams) { | ||
List<J.VariableDeclarations.NamedVariable> kept = keepUsedVariables(decl, usedParams); | ||
if (!kept.isEmpty()) { | ||
return decl.withVariables(kept); | ||
} | ||
if (!decl.getLeadingAnnotations().isEmpty()) { | ||
return decl; | ||
} | ||
return null; | ||
} | ||
|
||
private List<J.VariableDeclarations.NamedVariable> keepUsedVariables(J.VariableDeclarations decl, Set<String> usedParams) { | ||
List<J.VariableDeclarations.NamedVariable> kept = new ArrayList<>(decl.getVariables().size()); | ||
for (J.VariableDeclarations.NamedVariable v : decl.getVariables()) { | ||
if (usedParams.contains(v.getSimpleName())) { | ||
kept.add(v); | ||
} | ||
} | ||
return kept; | ||
} | ||
|
||
private J.MethodDeclaration applyPrunedSignature(J.MethodDeclaration original, | ||
List<Statement> pruned) { | ||
// Identify exactly which parameter positions were removed | ||
List<Statement> originalParams = original.getParameters(); | ||
Set<Integer> removedIndexes = new HashSet<>(); | ||
for (int i = 0; i < originalParams.size(); i++) { | ||
if (!pruned.contains(originalParams.get(i))) { | ||
removedIndexes.add(i); | ||
} | ||
} | ||
|
||
// Build the pruned method declaration | ||
JavaType.Method originalType = original.getMethodType(); | ||
List<JavaType> prunedParamTypes = collectParameterTypes(pruned); | ||
J.MethodDeclaration candidate = original | ||
.withParameters(pruned) | ||
.withMethodType(originalType.withParameterTypes(prunedParamTypes)); | ||
|
||
// Do override/original‐signature/superclass conflict checks | ||
String fullSignature = MethodMatcher.methodPattern(candidate); | ||
int split = fullSignature.indexOf(' '); | ||
String qualifier = fullSignature.substring(0, split); | ||
String signatureTail = fullSignature.substring(split + 1); | ||
|
||
if (acc.overrideSignatures.contains(fullSignature) || | ||
acc.originalSignatures | ||
.getOrDefault(qualifier, Collections.emptySet()) | ||
.contains(fullSignature) || | ||
conflictsWithSuperClassMethods(original, candidate, signatureTail) != null) { | ||
return original; | ||
} | ||
|
||
// Schedule a one‐off visitor to prune matching call‐site arguments | ||
String oldSignature = MethodMatcher.methodPattern(originalType); | ||
doAfterVisit(new JavaIsoVisitor<ExecutionContext>() { | ||
private final MethodMatcher matcher = new MethodMatcher(oldSignature); | ||
|
||
@Override | ||
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation invocation, | ||
ExecutionContext ctx) { | ||
J.MethodInvocation m = super.visitMethodInvocation(invocation, ctx); | ||
if (matcher.matches(m) && m.getArguments().size() != prunedParamTypes.size()) { | ||
// Trim the argument list | ||
List<Expression> keptArgs = new ArrayList<>(); | ||
for (int i = 0; i < m.getArguments().size(); i++) { | ||
if (!removedIndexes.contains(i)) { | ||
keptArgs.add(m.getArguments().get(i)); | ||
} | ||
} | ||
// Trim the MethodType parameter list | ||
JavaType.Method mt = m.getMethodType(); | ||
List<JavaType> keptTypes = new ArrayList<>(); | ||
for (int i = 0; i < mt.getParameterTypes().size(); i++) { | ||
if (!removedIndexes.contains(i)) { | ||
keptTypes.add(mt.getParameterTypes().get(i)); | ||
} | ||
} | ||
JavaType.Method updatedType = mt.withParameterTypes(keptTypes); | ||
// Update the name identifier to carry the same type instance | ||
J.Identifier newName = m.getName().withType(updatedType); | ||
return m.withArguments(keptArgs) | ||
.withMethodType(updatedType) | ||
.withName(newName); | ||
} | ||
return m; | ||
} | ||
}); | ||
|
||
return candidate; | ||
} | ||
|
||
private J.MethodDeclaration conflictsWithSuperClassMethods(J.MethodDeclaration original, | ||
J.MethodDeclaration candidate, String tail) { | ||
JavaType.Method mt = candidate.getMethodType(); | ||
if (mt != null && mt.getDeclaringType() instanceof JavaType.Class) { | ||
JavaType.Class cls = (JavaType.Class) mt.getDeclaringType(); | ||
JavaType.Class superCls = (JavaType.Class) cls.getSupertype(); | ||
if (superCls != null) { | ||
String superKey = superCls.getFullyQualifiedName() + " " + tail; | ||
Set<String> superSigs = acc.originalSignatures | ||
.getOrDefault(superCls.getFullyQualifiedName(), | ||
Collections.emptySet()); | ||
if (superSigs.contains(superKey)) { | ||
return original; | ||
} | ||
} | ||
} | ||
return null; | ||
} | ||
|
||
private static List<JavaType> collectParameterTypes(List<Statement> prunedParams) { | ||
List<JavaType> newParamTypes = new ArrayList<>(); | ||
for (Statement stmt : prunedParams) { | ||
if (stmt instanceof J.VariableDeclarations) { | ||
J.VariableDeclarations decl = (J.VariableDeclarations) stmt; | ||
for (J.VariableDeclarations.NamedVariable v : decl.getVariables()) { | ||
JavaType t = v.getType(); | ||
if (t != null) { | ||
newParamTypes.add(t); | ||
} | ||
} | ||
} | ||
} | ||
return newParamTypes; | ||
} | ||
} | ||
} |
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.