-
Notifications
You must be signed in to change notification settings - Fork 88
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
44
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 5 commits
Commits
Show all changes
44 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 1ee3249
Reduce warnings
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
182 changes: 182 additions & 0 deletions
182
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,182 @@ | ||
/* | ||
* 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.ExecutionContext; | ||
import org.openrewrite.Preconditions; | ||
import org.openrewrite.Repeat; | ||
import org.openrewrite.ScanningRecipe; | ||
import org.openrewrite.TreeVisitor; | ||
import org.openrewrite.java.JavaIsoVisitor; | ||
import org.openrewrite.java.NoMissingTypes; | ||
import org.openrewrite.java.tree.J; | ||
import org.openrewrite.java.tree.Statement; | ||
|
||
import java.time.Duration; | ||
import java.util.ArrayDeque; | ||
iddeepak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import java.util.ArrayList; | ||
import java.util.Deque; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Set; | ||
iddeepak marked this conversation as resolved.
Show resolved
Hide resolved
iddeepak marked this conversation as resolved.
Show resolved
Hide resolved
iddeepak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import java.util.stream.Collectors; | ||
|
||
public class RemoveUnusedParams extends ScanningRecipe<RemoveUnusedParams.Accumulator> { | ||
|
||
static class Accumulator { | ||
final Set<String> OVERRIDE_SIGNATURES = new HashSet<>(); | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
@Override | ||
public String getDisplayName() { | ||
return "Remove unused method parameters"; | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
@Override | ||
public String getDescription() { | ||
return "Removes parameters from methods that are declared but never used in the method body."; | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
@Override | ||
public Duration getEstimatedEffortPerOccurrence() { | ||
return Duration.ofMinutes(5); | ||
} | ||
|
||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@Override | ||
public Accumulator getInitialValue(ExecutionContext ctx) { | ||
return new Accumulator(); | ||
} | ||
|
||
private String buildSignature(J.MethodDeclaration m){ | ||
return m.getSimpleName() + "#" + | ||
m.getMethodType().getParameterTypes() | ||
.stream().map(p->p.toString()) | ||
.collect(Collectors.joining(",")); | ||
} | ||
|
||
@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); | ||
for (J.Annotation ann : m.getLeadingAnnotations()) { | ||
if ("Override".equals(ann.getSimpleName())) { | ||
acc.OVERRIDE_SIGNATURES.add(buildSignature(m)); | ||
break; | ||
} | ||
} | ||
return m; | ||
} | ||
}; | ||
} | ||
|
||
@Override | ||
public TreeVisitor<?, ExecutionContext> getVisitor(Accumulator acc) { | ||
return Preconditions.check(new NoMissingTypes(), | ||
Repeat.repeatUntilStable(new JavaIsoVisitor<ExecutionContext>() { | ||
|
||
private boolean skipAnyExplicitOverride(J.MethodDeclaration m) { | ||
return m.getMethodType() != null && m.getMethodType().isOverride(); | ||
} | ||
|
||
private boolean skipIfOverriddenElsewhere(String signature) { | ||
return acc.OVERRIDE_SIGNATURES.contains(signature); | ||
} | ||
|
||
@Override | ||
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { | ||
J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx); | ||
|
||
if (skipAnyExplicitOverride(m)) { | ||
return m; | ||
} | ||
|
||
if (skipIfOverriddenElsewhere(buildSignature(m))) { | ||
return m; | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if (m.getBody() == null || | ||
m.hasModifier(J.Modifier.Type.Native) || | ||
!m.getLeadingAnnotations().isEmpty()) { | ||
return m; | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
Set<String> params = m.getParameters().stream() | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.filter(p -> p instanceof J.VariableDeclarations) | ||
.flatMap(p -> ((J.VariableDeclarations) p).getVariables().stream()) | ||
.map(J.VariableDeclarations.NamedVariable::getSimpleName) | ||
.collect(Collectors.toSet()); | ||
|
||
Set<String> used = new HashSet<>(); | ||
new JavaIsoVisitor<Set<String>>() { | ||
Deque<Set<String>> shadowed = new ArrayDeque<>(); | ||
|
||
@Override | ||
public J.Block visitBlock(J.Block block, Set<String> u) { | ||
shadowed.push(new HashSet<>()); | ||
J.Block b = super.visitBlock(block, u); | ||
shadowed.pop(); | ||
return b; | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
@Override | ||
public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations vars, Set<String> u) { | ||
vars.getVariables().forEach(v -> shadowed.peek().add(v.getSimpleName())); | ||
return super.visitVariableDeclarations(vars, u); | ||
} | ||
|
||
@Override | ||
public J.Identifier visitIdentifier(J.Identifier ident, Set<String> u) { | ||
for (Set<String> scope : shadowed) { | ||
if (scope.contains(ident.getSimpleName())) { | ||
return ident; | ||
} | ||
} | ||
if (params.contains(ident.getSimpleName())) { | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
u.add(ident.getSimpleName()); | ||
} | ||
return ident; | ||
} | ||
}.visit(m.getBody(), used); | ||
|
||
List<Statement> newParams = new ArrayList<>(); | ||
for (Statement p : m.getParameters()) { | ||
if (!(p instanceof J.VariableDeclarations)) { | ||
newParams.add(p); | ||
continue; | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
J.VariableDeclarations vd = (J.VariableDeclarations) p; | ||
if (!vd.getLeadingAnnotations().isEmpty()) { | ||
newParams.add(vd); | ||
continue; | ||
} | ||
List<J.VariableDeclarations.NamedVariable> keep = vd.getVariables().stream() | ||
.filter(v -> used.contains(v.getSimpleName())) | ||
.collect(Collectors.toList()); | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!keep.isEmpty()) { | ||
newParams.add(vd.withVariables(keep)); | ||
} | ||
} | ||
|
||
return newParams.equals(m.getParameters()) ? | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
m : | ||
m.withParameters(newParams); | ||
} | ||
}) | ||
); | ||
} | ||
} |
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.