Skip to content

Commit

Permalink
Enhance unused local to unname variable (#1791)
Browse files Browse the repository at this point in the history 
* Enhance unused to unnamed var quick fix to support try with resources and for loops

- modify RenameUnusedVariableFixCore to add new method that will
  check if rename to unnamed variable is available
  and add logic for try with resources and enhanced for loops
- modify UnusedCodeCleanUpCore and UnusedCodeFixCore to use new
  method to check whether we can rename to unnamed variable
- add new tests to QuickFixTest22
- fixes #1762
  • Loading branch information
@jjohnstn
jjohnstn authored Nov 8, 2024
1 parent fcf7f0e commit 0871ea2
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 47 deletions.
7 changes: 1 addition & 6 deletions ....manipulation/common/org/eclipse/jdt/internal/ui/fix/RenameUnusedVariableCleanUpCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,11 @@
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.Pattern;
import org.eclipse.jdt.core.dom.SimpleName;
import org.eclipse.jdt.core.dom.SingleVariableDeclaration;

import org.eclipse.jdt.internal.corext.fix.CleanUpConstants;
import org.eclipse.jdt.internal.corext.fix.RenameUnusedVariableFixCore;
import org.eclipse.jdt.internal.corext.fix.UnusedCodeFixCore;
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;

import org.eclipse.jdt.ui.cleanup.CleanUpRequirements;
import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
Expand Down Expand Up @@ -150,9 +147,7 @@ public int computeNumberOfFixes(CompilationUnit compilationUnit) {
if (id == IProblem.LocalVariableIsNeverUsed) {
ProblemLocation location= new ProblemLocation(problem);
SimpleName name= UnusedCodeFixCore.getUnusedName(compilationUnit, location);
if (JavaModelUtil.is22OrHigher(compilationUnit.getJavaElement().getJavaProject()) &&
name.getParent() instanceof SingleVariableDeclaration nameParent &&
nameParent.getParent() instanceof Pattern) {
if (RenameUnusedVariableFixCore.canRenameToUnnamedVariable(compilationUnit, name)) {
result++;
}
} else if (id == IProblem.LambdaParameterIsNeverUsed) {
Expand Down
8 changes: 2 additions & 6 deletions ...e.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/UnusedCodeCleanUpCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,11 @@
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.Pattern;
import org.eclipse.jdt.core.dom.SimpleName;
import org.eclipse.jdt.core.dom.SingleVariableDeclaration;

import org.eclipse.jdt.internal.corext.fix.CleanUpConstants;
import org.eclipse.jdt.internal.corext.fix.RenameUnusedVariableFixCore;
import org.eclipse.jdt.internal.corext.fix.UnusedCodeFixCore;
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;

import org.eclipse.jdt.ui.cleanup.CleanUpRequirements;
import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
Expand Down Expand Up @@ -277,9 +275,7 @@ public int computeNumberOfFixes(CompilationUnit compilationUnit) {
if (id == IProblem.LocalVariableIsNeverUsed) {
ProblemLocation location= new ProblemLocation(problem);
SimpleName name= UnusedCodeFixCore.getUnusedName(compilationUnit, location);
if (!JavaModelUtil.is22OrHigher(compilationUnit.getJavaElement().getJavaProject()) ||
!(name.getParent() instanceof SingleVariableDeclaration nameParent) ||
!(nameParent.getParent() instanceof Pattern)) {
if (!RenameUnusedVariableFixCore.canRenameToUnnamedVariable(compilationUnit, name)) {
result++;
}
}
Expand Down
48 changes: 26 additions & 22 deletions ...ation/core extension/org/eclipse/jdt/internal/corext/fix/RenameUnusedVariableFixCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,19 @@
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.EnhancedForStatement;
import org.eclipse.jdt.core.dom.ForStatement;
import org.eclipse.jdt.core.dom.IBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.LambdaExpression;
import org.eclipse.jdt.core.dom.MethodDeclaration;
import org.eclipse.jdt.core.dom.Pattern;
import org.eclipse.jdt.core.dom.SimpleName;
import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
import org.eclipse.jdt.core.dom.TryStatement;
import org.eclipse.jdt.core.dom.VariableDeclarationExpression;
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;

import org.eclipse.jdt.internal.corext.dom.ASTNodes;
import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;

Expand Down Expand Up @@ -78,15 +80,10 @@ public static RenameUnusedVariableFixCore createRenameToUnnamedFix(CompilationUn
if (name != null) {
IBinding binding= name.resolveBinding();
if (binding != null) {
if (JavaModelUtil.is22OrHigher(compilationUnit.getJavaElement().getJavaProject())) {
if (name.getParent() instanceof SingleVariableDeclaration parent &&
parent.getParent() instanceof Pattern ||
name.getParent() instanceof VariableDeclarationFragment parent2 &&
parent2.getParent() instanceof LambdaExpression) {
String label= FixMessages.UnusedCodeFix_RenameToUnnamedVariable_description;
RenameToUnnamedVariableOperation operation= new RenameToUnnamedVariableOperation(name);
return new RenameUnusedVariableFixCore(label, compilationUnit, new CompilationUnitRewriteOperation[] { operation }, getCleanUpOptions());
}
if (canRenameToUnnamedVariable(compilationUnit, name)) {
String label= FixMessages.UnusedCodeFix_RenameToUnnamedVariable_description;
RenameToUnnamedVariableOperation operation= new RenameToUnnamedVariableOperation(name);
return new RenameUnusedVariableFixCore(label, compilationUnit, new CompilationUnitRewriteOperation[] { operation }, getCleanUpOptions());
}
}
}
Expand Down Expand Up @@ -131,16 +128,8 @@ public static ICleanUpFix createCleanUp(CompilationUnit compilationUnit, IProble
if (name != null) {
IBinding binding= name.resolveBinding();
if (binding instanceof IVariableBinding) {
VariableDeclarationFragment parent= ASTNodes.getParent(name, VariableDeclarationFragment.class);
if (parent == null || id == IProblem.LambdaParameterIsNeverUsed) {
if (JavaModelUtil.is22OrHigher(compilationUnit.getJavaElement().getJavaProject())) {
if (name.getParent() instanceof SingleVariableDeclaration nameParent &&
nameParent.getParent() instanceof Pattern ||
name.getParent() instanceof VariableDeclarationFragment varFragment &&
varFragment.getParent() instanceof LambdaExpression) {
result.add(new RenameToUnnamedVariableOperation(name));
}
}
if (canRenameToUnnamedVariable(compilationUnit, name)) {
result.add(new RenameToUnnamedVariableOperation(name));
}
}
}
Expand All @@ -153,8 +142,23 @@ public static ICleanUpFix createCleanUp(CompilationUnit compilationUnit, IProble
return new RenameUnusedVariableFixCore(FixMessages.UnusedCodeFix_change_name, compilationUnit, result.toArray(new CompilationUnitRewriteOperation[result.size()]));
}

public static boolean isFormalParameterInEnhancedForStatement(SimpleName name) {
return name.getParent() instanceof SingleVariableDeclaration && name.getParent().getLocationInParent() == EnhancedForStatement.PARAMETER_PROPERTY;
public static boolean canRenameToUnnamedVariable(CompilationUnit compilationUnit, SimpleName name) {
if (JavaModelUtil.is22OrHigher(compilationUnit.getJavaElement().getJavaProject())) {
if (name.getParent() instanceof SingleVariableDeclaration nameParent) {
if (nameParent.getParent() instanceof Pattern || nameParent.getParent() instanceof EnhancedForStatement) {
return true;
}
} else if (name.getParent() instanceof VariableDeclarationFragment varFragment) {
if (varFragment.getParent() instanceof LambdaExpression) {
return true;
} else if (varFragment.getParent() instanceof VariableDeclarationExpression varFragmentParent) {
if (varFragmentParent.getParent() instanceof TryStatement || varFragmentParent.getParent() instanceof ForStatement) {
return true;
}
}
}
}
return false;
}

public static SimpleName getUnusedName(CompilationUnit compilationUnit, IProblemLocation problem) {
Expand Down
15 changes: 2 additions & 13 deletions ...re.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/UnusedCodeFixCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
import org.eclipse.jdt.core.dom.Modifier;
import org.eclipse.jdt.core.dom.NodeFinder;
import org.eclipse.jdt.core.dom.ParenthesizedExpression;
import org.eclipse.jdt.core.dom.Pattern;
import org.eclipse.jdt.core.dom.PostfixExpression;
import org.eclipse.jdt.core.dom.PrefixExpression;
import org.eclipse.jdt.core.dom.QualifiedName;
Expand All @@ -90,7 +89,6 @@
import org.eclipse.jdt.internal.corext.dom.ReplaceRewrite;
import org.eclipse.jdt.internal.corext.dom.StatementRewrite;
import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
import org.eclipse.jdt.internal.corext.util.Messages;

import org.eclipse.jdt.ui.cleanup.CleanUpOptions;
Expand Down Expand Up @@ -930,7 +928,7 @@ public static ICleanUpFix createCleanUp(CompilationUnit compilationUnit, IProble

if ((removeUnusedLocalVariables && id == IProblem.LocalVariableIsNeverUsed) || (removeUnusedPrivateFields && id == IProblem.UnusedPrivateField)) {
SimpleName name= getUnusedName(compilationUnit, problem);
if (name != null) {
if (name != null && !RenameUnusedVariableFixCore.canRenameToUnnamedVariable(compilationUnit, name)) {
IBinding binding= name.resolveBinding();
if (binding instanceof IVariableBinding && !isFormalParameterInEnhancedForStatement(name) && (!((IVariableBinding) binding).isField() || isSideEffectFree(name, compilationUnit))) {
VariableDeclarationFragment parent= ASTNodes.getParent(name, VariableDeclarationFragment.class);
Expand All @@ -941,16 +939,7 @@ public static ICleanUpFix createCleanUp(CompilationUnit compilationUnit, IProble
}
variableDeclarations.get(varDecl).add(name);
} else {
if (id == IProblem.LocalVariableIsNeverUsed) {
SimpleName nameNode= UnusedCodeFixCore.getUnusedName(compilationUnit, problem);
if (!JavaModelUtil.is22OrHigher(compilationUnit.getJavaElement().getJavaProject()) ||
!(nameNode.getParent() instanceof SingleVariableDeclaration nameParent) ||
!(nameParent.getParent() instanceof Pattern)) {
result.add(new RemoveUnusedMemberOperation(new SimpleName[] { name }, false));
}
} else {
result.add(new RemoveUnusedMemberOperation(new SimpleName[] { name }, false));
}
result.add(new RemoveUnusedMemberOperation(new SimpleName[] { name }, false));
}
}
}
Expand Down
157 changes: 157 additions & 0 deletions org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/QuickFixTest22.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,161 @@ case R(_, long _) -> {}
assertEqualString(preview1, expected1);
}

@Test
public void testRenameToUnnamedProposal3() throws Exception {
fJProject1= JavaProjectHelper.createJavaProject("TestProject1", "bin");
fJProject1.setRawClasspath(projectsetup.getDefaultClasspath(), null);
JavaProjectHelper.set22CompilerOptions(fJProject1);

fSourceFolder= JavaProjectHelper.addSourceContainer(fJProject1, "src");
IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);

String test= """
package test;
public class Unused {
public static void main(String[] args) {
for (String arg : args) {
System.out.println("abc");
}
}
}
""";

ICompilationUnit cu= pack1.createCompilationUnit("Unused.java", test, false, null);

CompilationUnit astRoot= getASTRoot(cu);
ArrayList<IJavaCompletionProposal> proposals= collectCorrections(cu, astRoot, 1);
assertCorrectLabels(proposals);

CUCorrectionProposal proposal1= (CUCorrectionProposal) proposals.get(0);
String preview1= getPreviewContent(proposal1);

String expected1= """
package test;
public class Unused {
public static void main(String[] args) {
for (String _ : args) {
System.out.println("abc");
}
}
}
""";

assertEqualString(preview1, expected1);
}

@Test
public void testRenameToUnnamedProposal4() throws Exception {
fJProject1= JavaProjectHelper.createJavaProject("TestProject1", "bin");
fJProject1.setRawClasspath(projectsetup.getDefaultClasspath(), null);
JavaProjectHelper.set22CompilerOptions(fJProject1);

fSourceFolder= JavaProjectHelper.addSourceContainer(fJProject1, "src");
IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);

String test= """
package test;
import java.io.FileInputStream;
public class Unused {
public static void main(String[] args) {
try (FileInputStream x = new FileInputStream("a.b")) {
System.out.println("abc");
} catch (Exception e) {
}
}
}
""";

ICompilationUnit cu= pack1.createCompilationUnit("Unused.java", test, false, null);

CompilationUnit astRoot= getASTRoot(cu);
ArrayList<IJavaCompletionProposal> proposals= collectCorrections(cu, astRoot, 1);
assertCorrectLabels(proposals);

CUCorrectionProposal proposal1= (CUCorrectionProposal) proposals.get(0);
String preview1= getPreviewContent(proposal1);

String expected1= """
package test;
import java.io.FileInputStream;
public class Unused {
public static void main(String[] args) {
try (FileInputStream _ = new FileInputStream("a.b")) {
System.out.println("abc");
} catch (Exception e) {
}
}
}
""";

assertEqualString(preview1, expected1);
}

@Test
public void testRenameToUnnamedProposal5() throws Exception {
fJProject1= JavaProjectHelper.createJavaProject("TestProject1", "bin");
fJProject1.setRawClasspath(projectsetup.getDefaultClasspath(), null);
JavaProjectHelper.set22CompilerOptions(fJProject1);

fSourceFolder= JavaProjectHelper.addSourceContainer(fJProject1, "src");
IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);

String test= """
package test;
public class Unused {
private static int x = 1;
private static int sideEffect() {
return ++x;
}
public static void main(String[] args) {
for (int i = 0, se = sideEffect(); i < 9; ++i) {
System.out.println("abc");
}
}
}
""";

ICompilationUnit cu= pack1.createCompilationUnit("Unused.java", test, false, null);

CompilationUnit astRoot= getASTRoot(cu);
ArrayList<IJavaCompletionProposal> proposals= collectCorrections(cu, astRoot, 1);
assertCorrectLabels(proposals);

CUCorrectionProposal proposal1= (CUCorrectionProposal) proposals.get(0);
String preview1= getPreviewContent(proposal1);

String expected1= """
package test;
public class Unused {
private static int x = 1;
private static int sideEffect() {
return ++x;
}
public static void main(String[] args) {
for (int i = 0, _ = sideEffect(); i < 9; ++i) {
System.out.println("abc");
}
}
}
""";

assertEqualString(preview1, expected1);
}

}

0 comments on commit 0871ea2

Please sign in to comment.