From 6963d9635fcb541cea60f1e97c5f8f5b2186255a Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Wed, 25 Feb 2026 09:36:48 +0100 Subject: [PATCH] [GITHUB-9198] Do not remove for each variables, even if unused. --- .../netbeans/modules/java/hints/Feature.java | 7 +- .../modules/java/hints/bugs/Unused.java | 53 +++++++- .../modules/java/hints/bugs/UnusedTest.java | 70 ++++++++++ .../spi/java/hints/JavaFixUtilities.java | 16 +++ .../spi/java/hints/JavaFixUtilitiesTest.java | 122 +++++++++++++++++- 5 files changed, 257 insertions(+), 11 deletions(-) diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/Feature.java b/java/java.hints/src/org/netbeans/modules/java/hints/Feature.java index 950b79d52d09..4176236793e5 100644 --- a/java/java.hints/src/org/netbeans/modules/java/hints/Feature.java +++ b/java/java.hints/src/org/netbeans/modules/java/hints/Feature.java @@ -45,7 +45,12 @@ public enum Feature { VIRTUAL_THREADS(19, 21), /// https://openjdk.org/jeps/440 - RECORD_PATTERN(19, 21); + RECORD_PATTERN(19, 21), + + /// https://openjdk.org/jeps/456 + UNNAMED_VARIABLES(21, 22), + + ; /// preview begin public final int PREVIEW; diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/bugs/Unused.java b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/Unused.java index b7806ffede04..d019deb0b406 100644 --- a/java/java.hints/src/org/netbeans/modules/java/hints/bugs/Unused.java +++ b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/Unused.java @@ -19,16 +19,25 @@ package org.netbeans.modules.java.hints.bugs; import com.sun.source.tree.Tree.Kind; +import com.sun.source.util.TreePath; +import java.util.ArrayList; import java.util.List; +import java.util.Set; +import javax.lang.model.SourceVersion; import javax.lang.model.element.ElementKind; +import org.netbeans.api.java.source.CompilationInfo; +import org.netbeans.api.java.source.TreeMaker; +import org.netbeans.api.java.source.WorkingCopy; import org.netbeans.modules.java.editor.base.semantic.UnusedDetector; import org.netbeans.modules.java.editor.base.semantic.UnusedDetector.UnusedDescription; +import org.netbeans.modules.java.hints.Feature; import org.netbeans.spi.editor.hints.ErrorDescription; import org.netbeans.spi.editor.hints.Fix; import org.netbeans.spi.java.hints.BooleanOption; import org.netbeans.spi.java.hints.ErrorDescriptionFactory; import org.netbeans.spi.java.hints.Hint; import org.netbeans.spi.java.hints.HintContext; +import org.netbeans.spi.java.hints.JavaFix; import org.netbeans.spi.java.hints.JavaFixUtilities; import org.netbeans.spi.java.hints.TriggerTreeKind; import org.openide.util.NbBundle.Messages; @@ -46,6 +55,9 @@ }) public class Unused { + private static final SourceVersion UNNAMED_VARIABLES = SourceVersion.RELEASE_22; + private static final Set SUPPORT_UNNAMED = Set.of(ElementKind.BINDING_VARIABLE, ElementKind.EXCEPTION_PARAMETER, ElementKind.LOCAL_VARIABLE, ElementKind.RESOURCE_VARIABLE); + private static final boolean DETECT_UNUSED_PACKAGE_PRIVATE_DEFAULT = true; @BooleanOption(displayName="#LBL_UnusedPackagePrivate", tooltip="#TP_UnusedPackagePrivate", defaultValue=DETECT_UNUSED_PACKAGE_PRIVATE_DEFAULT) @@ -108,10 +120,7 @@ private static ErrorDescription convertUnused(HintContext ctx, UnusedDescription case NOT_WRITTEN: message = Bundle.ERR_NotWritten(name); break; case NOT_READ: message = Bundle.ERR_NotRead(name); - //unclear what can be done with unused binding variables currently (before "_"): - if (ud.unusedElementPath().getParentPath().getLeaf().getKind() != Kind.BINDING_PATTERN) { - fix = JavaFixUtilities.safelyRemoveFromParent(ctx, Bundle.FIX_RemoveUsedElement(name), ud.unusedElementPath()); - } + fix = JavaFixUtilities.safelyRemoveFromParent(ctx, Bundle.FIX_RemoveUsedElement(name), ud.unusedElementPath()); break; case NOT_USED: if (ud.unusedElement().getKind() == ElementKind.CONSTRUCTOR) { @@ -125,7 +134,39 @@ private static ErrorDescription convertUnused(HintContext ctx, UnusedDescription default: throw new IllegalStateException("Unknown unused type: " + ud.reason()); } - return fix != null ? ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath(), message, fix) - : ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath(), message); + + List fixes = new ArrayList<>(); + + if (fix != null) { + fixes.add(fix); + } + + if (Feature.UNNAMED_VARIABLES.isEnabled(ctx.getInfo()) && + SUPPORT_UNNAMED.contains(ud.unusedElement().getKind())) { + fixes.add(new RenameToUnderscore(ctx.getInfo(), ctx.getPath()).toEditorFix()); + } + + return ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath(), message, fixes.toArray(Fix[]::new)); + } + + private static final class RenameToUnderscore extends JavaFix { + + public RenameToUnderscore(CompilationInfo info, TreePath tp) { + super(info, tp); + } + + @Override + @Messages("FIX_RenameToUnderscore=Rename the variable to '_'") + protected String getText() { + return Bundle.FIX_RenameToUnderscore(); + } + + @Override + protected void performRewrite(TransformationContext ctx) throws Exception { + WorkingCopy wc = ctx.getWorkingCopy(); + TreeMaker make = wc.getTreeMaker(); + wc.rewrite(ctx.getPath().getLeaf(), make.setLabel(ctx.getPath().getLeaf(), "_")); + } + } } diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/UnusedTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/UnusedTest.java index 9e5daf4b21c4..d9008203555c 100644 --- a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/UnusedTest.java +++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/UnusedTest.java @@ -135,4 +135,74 @@ record Test() { .assertWarnings(); } + public void testUnusedForEach() throws Exception { + HintTest.create() + .input(""" + package test; + public class Test { + public void test(String[] args) { + for (String a : args) {} + } + } + """) + .run(Unused.class) + .assertWarnings("3:20-3:21:verifier:" + Bundle.ERR_NotRead("a")) + .findWarning("3:20-3:21:verifier:" + Bundle.ERR_NotRead("a")) + .assertFixes(); + } + + public void testToUndescore1() throws Exception { + HintTest.create() + .sourceLevel("22") + .input(""" + package test; + public class Test { + public void test(String[] args) { + String u = ""; + } + } + """) + .run(Unused.class) + .findWarning("3:15-3:16:verifier:" + Bundle.ERR_NotRead("u")) + .applyFix(Bundle.FIX_RenameToUnderscore()) + .assertCompilable() + .assertOutput(""" + package test; + public class Test { + public void test(String[] args) { + String _ = ""; + } + } + """); + } + + public void testToUndescore2() throws Exception { + HintTest.create() + .sourceLevel("22") + .input(""" + package test; + public class Test { + String u = ""; + } + """) + .run(Unused.class) + .findWarning("2:11-2:12:verifier:" + Bundle.ERR_NotRead("u")) + .assertFixes(Bundle.FIX_RemoveUsedElement("u")); + } + + public void testToUndescore3() throws Exception { + HintTest.create() + .sourceLevel("21") + .input(""" + package test; + public class Test { + public void test(String[] args) { + String u = ""; + } + } + """) + .run(Unused.class) + .findWarning("3:15-3:16:verifier:" + Bundle.ERR_NotRead("u")) + .assertFixes(Bundle.FIX_RemoveUsedElement("u")); + } } diff --git a/java/spi.java.hints/src/org/netbeans/spi/java/hints/JavaFixUtilities.java b/java/spi.java.hints/src/org/netbeans/spi/java/hints/JavaFixUtilities.java index 7e8ef5f5ad89..2bc33215afa6 100644 --- a/java/spi.java.hints/src/org/netbeans/spi/java/hints/JavaFixUtilities.java +++ b/java/spi.java.hints/src/org/netbeans/spi/java/hints/JavaFixUtilities.java @@ -31,6 +31,7 @@ import com.sun.source.tree.CompilationUnitTree; import com.sun.source.tree.CompoundAssignmentTree; import com.sun.source.tree.DoWhileLoopTree; +import com.sun.source.tree.EnhancedForLoopTree; import com.sun.source.tree.ExpressionStatementTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.IdentifierTree; @@ -1835,6 +1836,21 @@ private void doRemoveFromParent(WorkingCopy wc, TreePath what) { } private static boolean canSafelyRemove(CompilationInfo info, TreePath tp) { + if (tp.getParentPath() != null) { + //cannot remove from some parent trees: + switch (tp.getParentPath().getLeaf().getKind()) { + case BINDING_PATTERN -> { + return false; + } + case ENHANCED_FOR_LOOP -> { + EnhancedForLoopTree loop = (EnhancedForLoopTree) tp.getParentPath().getLeaf(); + + if (loop.getVariable() == tp.getLeaf()) { + return false; + } + } + } + } AtomicBoolean ret = new AtomicBoolean(true); Element el = info.getTrees().getElement(tp); if (el != null) { diff --git a/java/spi.java.hints/test/unit/src/org/netbeans/spi/java/hints/JavaFixUtilitiesTest.java b/java/spi.java.hints/test/unit/src/org/netbeans/spi/java/hints/JavaFixUtilitiesTest.java index 3957df6cc587..8c92e0982e3e 100644 --- a/java/spi.java.hints/test/unit/src/org/netbeans/spi/java/hints/JavaFixUtilitiesTest.java +++ b/java/spi.java.hints/test/unit/src/org/netbeans/spi/java/hints/JavaFixUtilitiesTest.java @@ -39,6 +39,7 @@ import org.netbeans.modules.java.hints.spiimpl.TestBase; import org.netbeans.modules.java.hints.spiimpl.hints.HintsInvoker; import org.netbeans.modules.java.hints.providers.spi.Trigger.PatternDescription; +import org.netbeans.modules.java.hints.spiimpl.SyntheticFix; import org.netbeans.modules.java.hints.spiimpl.options.HintsSettings; import org.netbeans.modules.java.hints.spiimpl.pm.PatternCompilerUtilities; import org.netbeans.modules.java.source.parsing.JavacParser; @@ -509,6 +510,102 @@ public void testRemoveFromParentExpressionStatement206116() throws Exception { "}\n"); } + public void testRemovedFromForEach1() throws Exception { + performRemoveFromParentTest(""" + package test; + import java.io.InputStream; + public class Test { + private void t(String[] args) throws Exception { + for (String a : args) + System.err.println(); + } + } + """, + "System.err.println();", + """ + package test; + import java.io.InputStream; + public class Test { + private void t(String[] args) throws Exception { + for (String a : args) { + } + } + } + """, + true); + } + + public void testRemovedFromForEach2() throws Exception { + performRemoveFromParentTest(""" + package test; + import java.io.InputStream; + public class Test { + private void t(String[] args) throws Exception { + for (String a : args) + System.err.println(); + } + } + """, + "System.err.println();", + """ + package test; + import java.io.InputStream; + public class Test { + private void t(String[] args) throws Exception { + for (String a : args) { + } + } + } + """, + true); + } + + public void testRemovedFromForEach3() throws Exception { + performRemoveFromParentTest(""" + package test; + import java.io.InputStream; + public class Test { + private void t(String[] args) throws Exception { + for (String a : args) + System.err.println(); + } + } + """, + "String a", + null, + true); + } + + public void testRemovedFromForEach4() throws Exception { + performRemoveFromParentTest(""" + package test; + import java.util.List; + public class Test { + private void t() throws Exception { + for (String a : List.of("") + System.err.println(); + } + } + """, + "java.util.List.of($args)", + null, + true); + } + + public void testRemoveBindingPattern1() throws Exception { + performRemoveFromParentTest(""" + package test; + public class Test { + private void t(Object o) throws Exception { + if (o instanceof String str) {} + } + } + """, + "String str", + null, + true); + } + public void testUnresolvableTarget() throws Exception { performRewriteTest("package test;\n" + "public class Test extends java.util.ArrayList {\n" + @@ -1396,13 +1493,19 @@ public void performRewriteTest(String code, String rule, String golden, String s } public void performRemoveFromParentTest(String code, String rule, String golden) throws Exception { + performRemoveFromParentTest(code, rule, golden, false); + } + + public void performRemoveFromParentTest(String code, String rule, String golden, boolean safelyRemove) throws Exception { prepareTest("test/Test.java", code); HintDescription hd = HintDescriptionFactory.create() .setTrigger(PatternDescription.create(rule, Collections.emptyMap())) .setWorker(new HintDescription.Worker() { @Override public Collection createErrors(HintContext ctx) { - return Collections.singletonList(ErrorDescriptionFactory.forName(ctx, ctx.getPath(), "", JavaFixUtilities.removeFromParent(ctx, "", ctx.getPath()))); + Fix fix = safelyRemove ? JavaFixUtilities.safelyRemoveFromParent(ctx, "", ctx.getPath()) + : JavaFixUtilities.removeFromParent(ctx, "", ctx.getPath()); + return Collections.singletonList(ErrorDescriptionFactory.forName(ctx, ctx.getPath(), "", fix)); } }).produce(); @@ -1410,11 +1513,22 @@ public void performRemoveFromParentTest(String code, String rule, String golden) assertEquals(computeHints.toString(), 1, computeHints.size()); - Fix fix = computeHints.get(0).getFixes().getFixes().get(0); + List fixes = computeHints.get(0).getFixes().getFixes(); - fix.implement(); + assertEquals(String.valueOf(fixes), 1, fixes.size()); - assertEquals(golden, doc.getText(0, doc.getLength())); + if (golden != null) { + Fix fix = fixes.get(0); + + fix.implement(); + + assertEquals(golden, doc.getText(0, doc.getLength())); + } else { + Fix fix = fixes.get(0); + + assertTrue(String.valueOf(fixes), + fix instanceof SyntheticFix); + } } static {