- 
                Notifications
    You must be signed in to change notification settings 
- Fork 89
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 34 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
        
          
          
            255 changes: 255 additions & 0 deletions
          
          255 
        
  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,255 @@ | ||
| /* | ||
| * 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.jetbrains.annotations.Nullable; | ||
|         
                  iddeepak marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||
| 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.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); | ||
| } else if (!decl.getLeadingAnnotations().isEmpty()) { | ||
|         
                  timtebeek marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||
| 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) { | ||
| J.MethodDeclaration candidate = original.withParameters(pruned) | ||
| .withMethodType( | ||
| original.getMethodType() | ||
| .withParameterTypes(collectParameterTypes(pruned)) | ||
| ); | ||
|  | ||
| String fullSig = MethodMatcher.methodPattern(candidate); | ||
| if (acc.overrideSignatures.contains(fullSig) | ||
| || acc.originalSignatures.get(fullSig.substring(0,fullSig.indexOf(' '))).contains(fullSig) | ||
| || conflictsWithSuperClassMethods(original, candidate, | ||
|         
                  iddeepak marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||
| fullSig.substring(fullSig.indexOf(' ') + 1)) != null) { | ||
| return original; | ||
|         
                  iddeepak marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||
| } | ||
| return candidate; | ||
| } | ||
|  | ||
| @Nullable | ||
| 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.
        
    
  
      
      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.