Skip to content

Commit 6755dba

Browse files
jjohnstnjjohnstn
authored
Fix unused variable cleanup to work with unused pattern/lambda vars (#1784)
- add checks to UnusedCodeFixCore to not create a remove member operation in the case of unused pattern variables and unused lambda parameters when Java is 22 and up - add RenameUnusedVariableCleanUpCore to JDT UI plug.xml - add new CleanUpTest22 - fixes #1782
1 parent 0734ebc commit 6755dba

File tree

6 files changed

+128
-6
lines changed

6 files changed

+128
-6
lines changed

‎org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/RenameUnusedVariableCleanUpCore.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ case R(_, _) -> {}
134134

135135
@Override
136136
public boolean canFix(ICompilationUnit compilationUnit, IProblemLocation problem) {
137-
if (UnusedCodeFixCore.isUnusedMember(problem) || UnusedCodeFixCore.isUnusedLambdaParameter(problem))
137+
if (RenameUnusedVariableFixCore.isUnusedMember(problem) || RenameUnusedVariableFixCore.isUnusedLambdaParameter(problem))
138138
return isEnabled(CleanUpConstants.REMOVE_UNUSED_CODE_LOCAL_VARIABLES);
139139

140140
return false;

‎org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/UnusedCodeCleanUpCore.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ public boolean canFix(ICompilationUnit compilationUnit, IProblemLocation problem
237237
if (UnusedCodeFixCore.isUnusedImport(problem))
238238
return isEnabled(CleanUpConstants.REMOVE_UNUSED_CODE_IMPORTS);
239239

240-
if (UnusedCodeFixCore.isUnusedMember(problem) || UnusedCodeFixCore.isUnusedLambdaParameter(problem))
240+
if (UnusedCodeFixCore.isUnusedMember(problem))
241241
return isEnabled(CleanUpConstants.REMOVE_UNUSED_CODE_PRIVATE_MEMBERS) && isEnabled(CleanUpConstants.REMOVE_UNUSED_CODE_PRIVATE_METHODS) ||
242242
isEnabled(CleanUpConstants.REMOVE_UNUSED_CODE_PRIVATE_MEMBERS) && isEnabled(CleanUpConstants.REMOVE_UNUSED_CODE_PRIVATE_CONSTRUCTORS) ||
243243
isEnabled(CleanUpConstants.REMOVE_UNUSED_CODE_PRIVATE_MEMBERS) && isEnabled(CleanUpConstants.REMOVE_UNUSED_CODE_PRIVATE_TYPES) ||

‎org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/UnusedCodeFixCore.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import org.eclipse.jdt.core.dom.Modifier;
6666
import org.eclipse.jdt.core.dom.NodeFinder;
6767
import org.eclipse.jdt.core.dom.ParenthesizedExpression;
68+
import org.eclipse.jdt.core.dom.Pattern;
6869
import org.eclipse.jdt.core.dom.PostfixExpression;
6970
import org.eclipse.jdt.core.dom.PrefixExpression;
7071
import org.eclipse.jdt.core.dom.QualifiedName;
@@ -89,6 +90,7 @@
8990
import org.eclipse.jdt.internal.corext.dom.ReplaceRewrite;
9091
import org.eclipse.jdt.internal.corext.dom.StatementRewrite;
9192
import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
93+
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
9294
import org.eclipse.jdt.internal.corext.util.Messages;
9395

9496
import org.eclipse.jdt.ui.cleanup.CleanUpOptions;
@@ -926,21 +928,29 @@ public static ICleanUpFix createCleanUp(CompilationUnit compilationUnit, IProble
926928
}
927929
}
928930

929-
if ((removeUnusedLocalVariables && id == IProblem.LocalVariableIsNeverUsed) || (removeUnusedPrivateFields && id == IProblem.UnusedPrivateField)
930-
|| (removeUnusedLocalVariables && id == IProblem.LambdaParameterIsNeverUsed)) {
931+
if ((removeUnusedLocalVariables && id == IProblem.LocalVariableIsNeverUsed) || (removeUnusedPrivateFields && id == IProblem.UnusedPrivateField)) {
931932
SimpleName name= getUnusedName(compilationUnit, problem);
932933
if (name != null) {
933934
IBinding binding= name.resolveBinding();
934935
if (binding instanceof IVariableBinding && !isFormalParameterInEnhancedForStatement(name) && (!((IVariableBinding) binding).isField() || isSideEffectFree(name, compilationUnit))) {
935936
VariableDeclarationFragment parent= ASTNodes.getParent(name, VariableDeclarationFragment.class);
936-
if (parent != null && id != IProblem.LambdaParameterIsNeverUsed) {
937+
if (parent != null) {
937938
ASTNode varDecl= parent.getParent();
938939
if (!variableDeclarations.containsKey(varDecl)) {
939940
variableDeclarations.put(varDecl, new ArrayList<>());
940941
}
941942
variableDeclarations.get(varDecl).add(name);
942943
} else {
943-
result.add(new RemoveUnusedMemberOperation(new SimpleName[] { name }, false));
944+
if (id == IProblem.LocalVariableIsNeverUsed) {
945+
SimpleName nameNode= UnusedCodeFixCore.getUnusedName(compilationUnit, problem);
946+
if (!JavaModelUtil.is22OrHigher(compilationUnit.getJavaElement().getJavaProject()) ||
947+
!(nameNode.getParent() instanceof SingleVariableDeclaration nameParent) ||
948+
!(nameParent.getParent() instanceof Pattern)) {
949+
result.add(new RemoveUnusedMemberOperation(new SimpleName[] { name }, false));
950+
}
951+
} else {
952+
result.add(new RemoveUnusedMemberOperation(new SimpleName[] { name }, false));
953+
}
944954
}
945955
}
946956
}

‎org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest22.java‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2020, 2024 Red Hat Inc. and others.
3+
*
4+
* This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Public License 2.0
6+
* which accompanies this distribution, and is available at
7+
* https://www.eclipse.org/legal/epl-2.0/
8+
*
9+
* SPDX-License-Identifier: EPL-2.0
10+
*
11+
* Contributors:
12+
* Red Hat Inc. - initial API and implementation
13+
*******************************************************************************/
14+
package org.eclipse.jdt.ui.tests.quickfix;
15+
16+
import java.util.Hashtable;
17+
18+
import org.junit.Rule;
19+
import org.junit.Test;
20+
21+
import org.eclipse.core.runtime.CoreException;
22+
23+
import org.eclipse.jdt.core.IClasspathEntry;
24+
import org.eclipse.jdt.core.ICompilationUnit;
25+
import org.eclipse.jdt.core.IJavaProject;
26+
import org.eclipse.jdt.core.IPackageFragment;
27+
import org.eclipse.jdt.core.JavaCore;
28+
29+
import org.eclipse.jdt.internal.corext.fix.CleanUpConstants;
30+
31+
import org.eclipse.jdt.ui.tests.core.rules.Java22ProjectTestSetup;
32+
import org.eclipse.jdt.ui.tests.core.rules.ProjectTestSetup;
33+
34+
/**
35+
* Tests the cleanup features related to Java 22.
36+
*/
37+
public class CleanUpTest22 extends CleanUpTestCase {
38+
@Rule
39+
public ProjectTestSetup projectSetup= new Java22ProjectTestSetup();
40+
41+
@Override
42+
protected IJavaProject getProject() {
43+
return projectSetup.getProject();
44+
}
45+
46+
@Override
47+
protected IClasspathEntry[] getDefaultClasspath() throws CoreException {
48+
return projectSetup.getDefaultClasspath();
49+
}
50+
51+
@Test
52+
public void testUnusedCleanUpUnnamedVariable() throws Exception {
53+
Hashtable<String, String> options= JavaCore.getOptions();
54+
options.put(JavaCore.COMPILER_PB_UNUSED_LAMBDA_PARAMETER, JavaCore.WARNING);
55+
options.put(JavaCore.COMPILER_PB_UNUSED_LOCAL, JavaCore.WARNING);
56+
JavaCore.setOptions(options);
57+
58+
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
59+
String sample= """
60+
package test1;
61+
62+
public class E {
63+
private interface J {
64+
public void run(String a, String b);
65+
}
66+
record R(int i, long l) {}
67+
public void test () {
68+
J j = (a, b) -> System.out.println(a);
69+
j.run("a", "b");
70+
R r = new R(1, 1);
71+
switch (r) {
72+
case R(_, long l) -> {}
73+
case R r2 -> {}
74+
}
75+
}
76+
}
77+
""";
78+
ICompilationUnit cu1= pack1.createCompilationUnit("E.java", sample, false, null);
79+
80+
enable(CleanUpConstants.REMOVE_UNUSED_CODE_LOCAL_VARIABLES);
81+
82+
sample= """
83+
package test1;
84+
85+
public class E {
86+
private interface J {
87+
public void run(String a, String b);
88+
}
89+
record R(int i, long l) {}
90+
public void test () {
91+
J j = (a, _) -> System.out.println(a);
92+
j.run("a", "b");
93+
R r = new R(1, 1);
94+
switch (r) {
95+
case R(_, long _) -> {}
96+
case R _ -> {}
97+
}
98+
}
99+
}
100+
""";
101+
String expected1= sample;
102+
103+
assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { expected1 }, null);
104+
}
105+
106+
}

‎org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTestCaseSuite.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
CleanUpTest14.class,
3434
CleanUpTest15.class,
3535
CleanUpTest16.class,
36+
CleanUpTest22.class,
3637
CleanUpAnnotationTest.class,
3738
SaveParticipantTest.class,
3839
CleanUpActionTest.class,

‎org.eclipse.jdt.ui/plugin.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7550,6 +7550,11 @@
75507550
id="org.eclipse.jdt.ui.cleanup.unboxing"
75517551
runAfter="org.eclipse.jdt.ui.cleanup.autoboxing">
75527552
</cleanUp>
7553+
<cleanUp
7554+
class="org.eclipse.jdt.internal.ui.fix.RenameUnusedVariableCleanUpCore"
7555+
id="org.eclipse.jdt.ui.cleanup.rename_unused"
7556+
runAfter="org.eclipse.jdt.ui.cleanup.unboxing">
7557+
</cleanUp>
75537558
</extension>
75547559

75557560
<extension

0 commit comments

Comments
 (0)