Skip to content

UnusedHint: defer expensive checks until rewrite is performed #8485

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+41 −29
Draft

UnusedHint: defer expensive checks until rewrite is performed #8485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 31 additions & 23 deletions java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/UnusedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,17 @@ public UnusedTest(String name) {
public void testUnused() throws Exception {
HintTest
.create()
.input("package test;\n" +
"public class Test {\n" +
" private class UnusedClass {}\n" +
" private void unusedMethod() {}\n" +
" private int unusedField;\n" +
" private void test(int unusedParam) {}\n" +
" public void test2() {test(1);}\n" +
" private Test() {}\n" +
"}\n")
.input("""
package test;
public class Test {
private class UnusedClass {}
private void unusedMethod() {}
private int unusedField;
private void test(int unusedParam) {}
public void test2() {test(1);}
private Test() {}
}
""")
.run(Unused.class)
.assertWarnings("2:18-2:29:verifier:" + Bundle.ERR_NotUsed("UnusedClass"),
"3:17-3:29:verifier:" + Bundle.ERR_NotUsed("unusedMethod"),
Expand All @@ -55,12 +57,14 @@ public void testNoFixForBindings() throws Exception {
HintTest
.create()
.sourceLevel("17")
.input("package test;\n" +
"public class Test {\n" +
" boolean test(Object o) {\n" +
" return o instanceof String s;\n" +
" }\n" +
"}\n")
.input("""
package test;
public class Test {
boolean test(Object o) {
return o instanceof String s;
}
}
""")
.run(Unused.class)
.findWarning("3:35-3:36:verifier:Variable s is never read")
.assertFixes();
Expand All @@ -69,19 +73,23 @@ public void testNoFixForBindings() throws Exception {
public void testUnusedNoPackagePrivate() throws Exception {
HintTest
.create()
.input("package test;\n" +
"public class Test {\n" +
" void packagePrivate() {}\n" +
"}\n")
.input("""
package test;
public class Test {
void packagePrivate() {}
}
""")
.run(Unused.class)
.assertWarnings("2:9-2:23:verifier:" + Bundle.ERR_NotUsed("packagePrivate"));
HintTest
.create()
.preference(Unused.DETECT_UNUSED_PACKAGE_PRIVATE, false)
.input("package test;\n" +
"public class Test {\n" +
" void packagePrivate() {}\n" +
"}\n")
.input("""
package test;
public class Test {
void packagePrivate() {}
}
""")
.run(Unused.class)
.assertWarnings();
}
Expand Down
16 changes: 10 additions & 6 deletions java/spi.java.hints/src/org/netbeans/spi/java/hints/JavaFixUtilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ static Fix rewriteFix(CompilationInfo info, String displayName, TreePath what, f
}
}
params.put(e.getKey(), TreePathHandle.create(e.getValue(), info));
if (e.getValue() instanceof Callable callable) {
if (e.getValue() instanceof Callable<?> callable) {
try {
extraParamsData.put(e.getKey(), callable.call());
} catch (Exception ex) {
Expand Down Expand Up @@ -252,9 +252,9 @@ public static Fix removeFromParent(HintContext ctx, String displayName, TreePath
* @since 1.48
*/
public static Fix safelyRemoveFromParent(HintContext ctx, String displayName, TreePath what) {
return RemoveFromParent.canSafelyRemove(ctx.getInfo(), what) ? new RemoveFromParent(displayName, ctx.getInfo(), what, true).toEditorFix() : null;
return new RemoveFromParent(displayName, ctx.getInfo(), what, true).toEditorFix();
}

@SuppressWarnings("AssignmentToMethodParameter")
private static String defaultFixDisplayName(Map<String, TreePath> variables, Map<String, Collection<? extends TreePath>> parametersMulti, String replaceTarget) {
Map<String, String> stringsForVariables = new LinkedHashMap<>();
Expand Down Expand Up @@ -1259,7 +1259,7 @@ public Number visitModifiers(ModifiersTree node, Void p) {
Set<Modifier> actualFlags = EnumSet.noneOf(Modifier.class);
boolean[] actualAnnotationsMask = new boolean[0];

if (actualContent instanceof Object[] objects && objects[0] instanceof Set set) {
if (actualContent instanceof Object[] objects && objects[0] instanceof Set<?> set) {
actualFlags.addAll(NbCollections.checkedSetByFilter(set, Modifier.class, false));
}

Expand Down Expand Up @@ -1558,6 +1558,7 @@ public static boolean isPrimary(@NonNull Tree tree) {
* @return true if and only if inner needs to be wrapped using {@link TreeMaker#Parenthesized(com.sun.source.tree.ExpressionTree) }
* to keep the original meaning.
*/
@SuppressWarnings("element-type-mismatch")
public static boolean requiresParenthesis(Tree inner, Tree original, Tree outter) {
if (!ExpressionTree.class.isAssignableFrom(inner.getKind().asInterface()) || outter == null) return false;
if (!ExpressionTree.class.isAssignableFrom(outter.getKind().asInterface())) {
Expand Down Expand Up @@ -1650,7 +1651,9 @@ protected String getText() {
protected void performRewrite(TransformationContext ctx) {
WorkingCopy wc = ctx.getWorkingCopy();
TreePath tp = ctx.getPath();

if (safely && !canSafelyRemove(wc, tp)) { // TODO is this needed? UnusedDetector seems to cover this already.
return;
}
doRemoveFromParent(wc, tp);
if (safely) {
Element el = wc.getTrees().getElement(tp);
Expand All @@ -1672,6 +1675,7 @@ public Void scan(Tree tree, Void p) {
}
}

@SuppressWarnings("element-type-mismatch")
private void doRemoveFromParent(WorkingCopy wc, TreePath what) {
TreeMaker make = wc.getTreeMaker();
Tree leaf = what.getLeaf();
Expand Down Expand Up @@ -1834,7 +1838,7 @@ private void doRemoveFromParent(WorkingCopy wc, TreePath what) {
}
}

private static boolean canSafelyRemove(CompilationInfo info, TreePath tp) {
private static boolean canSafelyRemove(WorkingCopy info, TreePath tp) {
AtomicBoolean ret = new AtomicBoolean(true);
Element el = info.getTrees().getElement(tp);
if (el != null) {
Expand Down
Loading