Skip to content

Commit d31cdea

Browse files
mbienmbien
authored
Merge pull request #9233 from lahodaj/GITHUB-9198
[GITHUB-9198] Do not remove for each variables, even if unused.
2 parents 9737049 + 1567647 commit d31cdea

5 files changed

Lines changed: 256 additions & 11 deletions

File tree

‎java/java.hints/src/org/netbeans/modules/java/hints/Feature.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,12 @@ public enum Feature {
4545
VIRTUAL_THREADS(19, 21),
4646

4747
/// https://openjdk.org/jeps/440
48-
RECORD_PATTERN(19, 21);
48+
RECORD_PATTERN(19, 21),
49+
50+
/// https://openjdk.org/jeps/456
51+
UNNAMED_VARIABLES(21, 22),
52+
53+
;
4954

5055
/// preview begin
5156
public final int PREVIEW;

‎java/java.hints/src/org/netbeans/modules/java/hints/bugs/Unused.java‎

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,25 @@
1919
package org.netbeans.modules.java.hints.bugs;
2020

2121
import com.sun.source.tree.Tree.Kind;
22+
import com.sun.source.util.TreePath;
23+
import java.util.ArrayList;
2224
import java.util.List;
25+
import java.util.Set;
26+
import javax.lang.model.SourceVersion;
2327
import javax.lang.model.element.ElementKind;
28+
import org.netbeans.api.java.source.CompilationInfo;
29+
import org.netbeans.api.java.source.TreeMaker;
30+
import org.netbeans.api.java.source.WorkingCopy;
2431
import org.netbeans.modules.java.editor.base.semantic.UnusedDetector;
2532
import org.netbeans.modules.java.editor.base.semantic.UnusedDetector.UnusedDescription;
33+
import org.netbeans.modules.java.hints.Feature;
2634
import org.netbeans.spi.editor.hints.ErrorDescription;
2735
import org.netbeans.spi.editor.hints.Fix;
2836
import org.netbeans.spi.java.hints.BooleanOption;
2937
import org.netbeans.spi.java.hints.ErrorDescriptionFactory;
3038
import org.netbeans.spi.java.hints.Hint;
3139
import org.netbeans.spi.java.hints.HintContext;
40+
import org.netbeans.spi.java.hints.JavaFix;
3241
import org.netbeans.spi.java.hints.JavaFixUtilities;
3342
import org.netbeans.spi.java.hints.TriggerTreeKind;
3443
import org.openide.util.NbBundle.Messages;
@@ -46,6 +55,8 @@
4655
})
4756
public class Unused {
4857

58+
private static final Set<ElementKind> SUPPORT_UNNAMED = Set.of(ElementKind.BINDING_VARIABLE, ElementKind.EXCEPTION_PARAMETER, ElementKind.LOCAL_VARIABLE, ElementKind.RESOURCE_VARIABLE);
59+
4960
private static final boolean DETECT_UNUSED_PACKAGE_PRIVATE_DEFAULT = true;
5061

5162
@BooleanOption(displayName="#LBL_UnusedPackagePrivate", tooltip="#TP_UnusedPackagePrivate", defaultValue=DETECT_UNUSED_PACKAGE_PRIVATE_DEFAULT)
@@ -108,10 +119,7 @@ private static ErrorDescription convertUnused(HintContext ctx, UnusedDescription
108119
case NOT_WRITTEN: message = Bundle.ERR_NotWritten(name);
109120
break;
110121
case NOT_READ: message = Bundle.ERR_NotRead(name);
111-
//unclear what can be done with unused binding variables currently (before "_"):
112-
if (ud.unusedElementPath().getParentPath().getLeaf().getKind() != Kind.BINDING_PATTERN) {
113-
fix = JavaFixUtilities.safelyRemoveFromParent(ctx, Bundle.FIX_RemoveUsedElement(name), ud.unusedElementPath());
114-
}
122+
fix = JavaFixUtilities.safelyRemoveFromParent(ctx, Bundle.FIX_RemoveUsedElement(name), ud.unusedElementPath());
115123
break;
116124
case NOT_USED:
117125
if (ud.unusedElement().getKind() == ElementKind.CONSTRUCTOR) {
@@ -125,7 +133,39 @@ private static ErrorDescription convertUnused(HintContext ctx, UnusedDescription
125133
default:
126134
throw new IllegalStateException("Unknown unused type: " + ud.reason());
127135
}
128-
return fix != null ? ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath(), message, fix)
129-
: ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath(), message);
136+
137+
List<Fix> fixes = new ArrayList<>();
138+
139+
if (fix != null) {
140+
fixes.add(fix);
141+
}
142+
143+
if (Feature.UNNAMED_VARIABLES.isEnabled(ctx.getInfo()) &&
144+
SUPPORT_UNNAMED.contains(ud.unusedElement().getKind())) {
145+
fixes.add(new RenameToUnderscore(ctx.getInfo(), ctx.getPath()).toEditorFix());
146+
}
147+
148+
return ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath(), message, fixes.toArray(Fix[]::new));
149+
}
150+
151+
private static final class RenameToUnderscore extends JavaFix {
152+
153+
public RenameToUnderscore(CompilationInfo info, TreePath tp) {
154+
super(info, tp);
155+
}
156+
157+
@Override
158+
@Messages("FIX_RenameToUnderscore=Rename the variable to '_'")
159+
protected String getText() {
160+
return Bundle.FIX_RenameToUnderscore();
161+
}
162+
163+
@Override
164+
protected void performRewrite(TransformationContext ctx) throws Exception {
165+
WorkingCopy wc = ctx.getWorkingCopy();
166+
TreeMaker make = wc.getTreeMaker();
167+
wc.rewrite(ctx.getPath().getLeaf(), make.setLabel(ctx.getPath().getLeaf(), "_"));
168+
}
169+
130170
}
131171
}

‎java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/UnusedTest.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,4 +135,74 @@ record Test() {
135135
.assertWarnings();
136136
}
137137

138+
public void testUnusedForEach() throws Exception {
139+
HintTest.create()
140+
.input("""
141+
package test;
142+
public class Test {
143+
public void test(String[] args) {
144+
for (String a : args) {}
145+
}
146+
}
147+
""")
148+
.run(Unused.class)
149+
.assertWarnings("3:20-3:21:verifier:" + Bundle.ERR_NotRead("a"))
150+
.findWarning("3:20-3:21:verifier:" + Bundle.ERR_NotRead("a"))
151+
.assertFixes();
152+
}
153+
154+
public void testToUndescore1() throws Exception {
155+
HintTest.create()
156+
.sourceLevel("22")
157+
.input("""
158+
package test;
159+
public class Test {
160+
public void test(String[] args) {
161+
String u = "";
162+
}
163+
}
164+
""")
165+
.run(Unused.class)
166+
.findWarning("3:15-3:16:verifier:" + Bundle.ERR_NotRead("u"))
167+
.applyFix(Bundle.FIX_RenameToUnderscore())
168+
.assertCompilable()
169+
.assertOutput("""
170+
package test;
171+
public class Test {
172+
public void test(String[] args) {
173+
String _ = "";
174+
}
175+
}
176+
""");
177+
}
178+
179+
public void testToUndescore2() throws Exception {
180+
HintTest.create()
181+
.sourceLevel("22")
182+
.input("""
183+
package test;
184+
public class Test {
185+
String u = "";
186+
}
187+
""")
188+
.run(Unused.class)
189+
.findWarning("2:11-2:12:verifier:" + Bundle.ERR_NotRead("u"))
190+
.assertFixes(Bundle.FIX_RemoveUsedElement("u"));
191+
}
192+
193+
public void testToUndescore3() throws Exception {
194+
HintTest.create()
195+
.sourceLevel("21")
196+
.input("""
197+
package test;
198+
public class Test {
199+
public void test(String[] args) {
200+
String u = "";
201+
}
202+
}
203+
""")
204+
.run(Unused.class)
205+
.findWarning("3:15-3:16:verifier:" + Bundle.ERR_NotRead("u"))
206+
.assertFixes(Bundle.FIX_RemoveUsedElement("u"));
207+
}
138208
}

‎java/spi.java.hints/src/org/netbeans/spi/java/hints/JavaFixUtilities.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.sun.source.tree.CompilationUnitTree;
3232
import com.sun.source.tree.CompoundAssignmentTree;
3333
import com.sun.source.tree.DoWhileLoopTree;
34+
import com.sun.source.tree.EnhancedForLoopTree;
3435
import com.sun.source.tree.ExpressionStatementTree;
3536
import com.sun.source.tree.ExpressionTree;
3637
import com.sun.source.tree.IdentifierTree;
@@ -1835,6 +1836,21 @@ private void doRemoveFromParent(WorkingCopy wc, TreePath what) {
18351836
}
18361837

18371838
private static boolean canSafelyRemove(CompilationInfo info, TreePath tp) {
1839+
if (tp.getParentPath() != null) {
1840+
//cannot remove from some parent trees:
1841+
switch (tp.getParentPath().getLeaf().getKind()) {
1842+
case BINDING_PATTERN -> {
1843+
return false;
1844+
}
1845+
case ENHANCED_FOR_LOOP -> {
1846+
EnhancedForLoopTree loop = (EnhancedForLoopTree) tp.getParentPath().getLeaf();
1847+
1848+
if (loop.getVariable() == tp.getLeaf()) {
1849+
return false;
1850+
}
1851+
}
1852+
}
1853+
}
18381854
AtomicBoolean ret = new AtomicBoolean(true);
18391855
Element el = info.getTrees().getElement(tp);
18401856
if (el != null) {

‎java/spi.java.hints/test/unit/src/org/netbeans/spi/java/hints/JavaFixUtilitiesTest.java‎

Lines changed: 118 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.netbeans.modules.java.hints.spiimpl.TestBase;
4040
import org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker;
4141
import org.netbeans.modules.java.hints.providers.spi.Trigger.PatternDescription;
42+
import org.netbeans.modules.java.hints.spiimpl.SyntheticFix;
4243
import org.netbeans.modules.java.hints.spiimpl.options.HintsSettings;
4344
import org.netbeans.modules.java.hints.spiimpl.pm.PatternCompilerUtilities;
4445
import org.netbeans.modules.java.source.parsing.JavacParser;
@@ -509,6 +510,102 @@ public void testRemoveFromParentExpressionStatement206116() throws Exception {
509510
"}\n");
510511
}
511512

513+
public void testRemovedFromForEach1() throws Exception {
514+
performRemoveFromParentTest("""
515+
package test;
516+
import java.io.InputStream;
517+
public class Test {
518+
private void t(String[] args) throws Exception {
519+
for (String a : args)
520+
System.err.println();
521+
}
522+
}
523+
""",
524+
"System.err.println();",
525+
"""
526+
package test;
527+
import java.io.InputStream;
528+
public class Test {
529+
private void t(String[] args) throws Exception {
530+
for (String a : args) {
531+
}
532+
}
533+
}
534+
""",
535+
true);
536+
}
537+
538+
public void testRemovedFromForEach2() throws Exception {
539+
performRemoveFromParentTest("""
540+
package test;
541+
import java.io.InputStream;
542+
public class Test {
543+
private void t(String[] args) throws Exception {
544+
for (String a : args)
545+
System.err.println();
546+
}
547+
}
548+
""",
549+
"System.err.println();",
550+
"""
551+
package test;
552+
import java.io.InputStream;
553+
public class Test {
554+
private void t(String[] args) throws Exception {
555+
for (String a : args) {
556+
}
557+
}
558+
}
559+
""",
560+
true);
561+
}
562+
563+
public void testRemovedFromForEach3() throws Exception {
564+
performRemoveFromParentTest("""
565+
package test;
566+
import java.io.InputStream;
567+
public class Test {
568+
private void t(String[] args) throws Exception {
569+
for (String a : args)
570+
System.err.println();
571+
}
572+
}
573+
""",
574+
"String a",
575+
null,
576+
true);
577+
}
578+
579+
public void testRemovedFromForEach4() throws Exception {
580+
performRemoveFromParentTest("""
581+
package test;
582+
import java.util.List;
583+
public class Test {
584+
private void t() throws Exception {
585+
for (String a : List.of("")
586+
System.err.println();
587+
}
588+
}
589+
""",
590+
"java.util.List.of($args)",
591+
null,
592+
true);
593+
}
594+
595+
public void testRemoveBindingPattern1() throws Exception {
596+
performRemoveFromParentTest("""
597+
package test;
598+
public class Test {
599+
private void t(Object o) throws Exception {
600+
if (o instanceof String str) {}
601+
}
602+
}
603+
""",
604+
"String str",
605+
null,
606+
true);
607+
}
608+
512609
public void testUnresolvableTarget() throws Exception {
513610
performRewriteTest("package test;\n" +
514611
"public class Test extends java.util.ArrayList {\n" +
@@ -1396,25 +1493,42 @@ public void performRewriteTest(String code, String rule, String golden, String s
13961493
}
13971494

13981495
public void performRemoveFromParentTest(String code, String rule, String golden) throws Exception {
1496+
performRemoveFromParentTest(code, rule, golden, false);
1497+
}
1498+
1499+
public void performRemoveFromParentTest(String code, String rule, String golden, boolean safelyRemove) throws Exception {
13991500
prepareTest("test/Test.java", code);
14001501

14011502
HintDescription hd = HintDescriptionFactory.create()
14021503
.setTrigger(PatternDescription.create(rule, Collections.<String, String>emptyMap()))
14031504
.setWorker(new HintDescription.Worker() {
14041505
@Override public Collection<? extends ErrorDescription> createErrors(HintContext ctx) {
1405-
return Collections.singletonList(ErrorDescriptionFactory.forName(ctx, ctx.getPath(), "", JavaFixUtilities.removeFromParent(ctx, "", ctx.getPath())));
1506+
Fix fix = safelyRemove ? JavaFixUtilities.safelyRemoveFromParent(ctx, "", ctx.getPath())
1507+
: JavaFixUtilities.removeFromParent(ctx, "", ctx.getPath());
1508+
return Collections.singletonList(ErrorDescriptionFactory.forName(ctx, ctx.getPath(), "", fix));
14061509
}
14071510
}).produce();
14081511

14091512
List<ErrorDescription> computeHints = new HintsInvoker(HintsSettings.getGlobalSettings(), new AtomicBoolean()).computeHints(info, Collections.singleton(hd));
14101513

14111514
assertEquals(computeHints.toString(), 1, computeHints.size());
14121515

1413-
Fix fix = computeHints.get(0).getFixes().getFixes().get(0);
1516+
List<Fix> fixes = computeHints.get(0).getFixes().getFixes();
14141517

1415-
fix.implement();
1518+
assertEquals(String.valueOf(fixes), 1, fixes.size());
14161519

1417-
assertEquals(golden, doc.getText(0, doc.getLength()));
1520+
if (golden != null) {
1521+
Fix fix = fixes.get(0);
1522+
1523+
fix.implement();
1524+
1525+
assertEquals(golden, doc.getText(0, doc.getLength()));
1526+
} else {
1527+
Fix fix = fixes.get(0);
1528+
1529+
assertTrue(String.valueOf(fixes),
1530+
fix instanceof SyntheticFix);
1531+
}
14181532
}
14191533

14201534
static {

0 commit comments

Comments
 (0)