Adding enhanced switch support to ThrowableNotThrown
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/bugs/ThrowableNotThrown.java b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/ThrowableNotThrown.java
index da87859..d14747d 100644
--- a/java/java.hints/src/org/netbeans/modules/java/hints/bugs/ThrowableNotThrown.java
+++ b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/ThrowableNotThrown.java
@@ -19,16 +19,22 @@
package org.netbeans.modules.java.hints.bugs;
import com.sun.source.tree.AssignmentTree;
+import com.sun.source.tree.BindingPatternTree;
+import com.sun.source.tree.CaseTree;
import com.sun.source.tree.ConditionalExpressionTree;
import com.sun.source.tree.ForLoopTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.NewClassTree;
+import com.sun.source.tree.PatternCaseLabelTree;
+import com.sun.source.tree.SwitchExpressionTree;
+import com.sun.source.tree.SwitchTree;
import com.sun.source.tree.Tree;
+import com.sun.source.tree.Tree.Kind;
import com.sun.source.tree.VariableTree;
import com.sun.source.util.TreePath;
import java.util.Collection;
-import java.util.EnumSet;
import java.util.HashSet;
+import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.lang.model.element.Element;
@@ -90,13 +96,6 @@
return null;
}
- private static final EnumSet<ElementKind> LOCAL_VARIABLES = EnumSet.of(
- ElementKind.LOCAL_VARIABLE,
- ElementKind.PARAMETER,
- ElementKind.RESOURCE_VARIABLE,
- ElementKind.EXCEPTION_PARAMETER
- );
-
private static TreePath findEnclosingMethodPath(TreePath path) {
TreePath enclosingMethodPath = path;
TreePath nextPath = enclosingMethodPath.getParentPath();
@@ -303,7 +302,7 @@
Element el = info.getTrees().getElement(new TreePath(excPath, var));
if (el == null || el.getKind() == ElementKind.FIELD) {
return true;
- } else if (LOCAL_VARIABLES.contains(el.getKind())) {
+ } else if (Flow.LOCAL_VARIABLES.contains(el.getKind())) {
varAssignments.add(as.getExpression());
}
process = true;
@@ -359,10 +358,40 @@
break;
}
+ case SWITCH: {
+ SwitchTree st = (SwitchTree) leaf;
+
+ if (st.getExpression() == prevLeaf) {
+ collectCaseBindings(st.getCases());
+ }
+
+ break;
+ }
+ case SWITCH_EXPRESSION: {
+ SwitchExpressionTree st = (SwitchExpressionTree) leaf;
+
+ if (st.getExpression() == prevLeaf) {
+ collectCaseBindings(st.getCases());
+ process = true;
+ }
+ break;
+ }
}
prevLeaf = excPath.getLeaf();
} while (process);
return varAssignments.isEmpty() ? Boolean.FALSE : null;
}
+
+ private void collectCaseBindings(List<? extends CaseTree> cases) {
+ //all binding patterns should be considered as new target variables for the selector value:
+ cases.stream()
+ .flatMap(cse -> cse.getLabels().stream())
+ .filter(label -> label.getKind() == Kind.PATTERN_CASE_LABEL)
+ .map(label -> ((PatternCaseLabelTree) label).getPattern())
+ .filter(pattern -> pattern.getKind() == Kind.BINDING_PATTERN)
+ .map(pattern -> ((BindingPatternTree) pattern).getVariable())
+ .filter(var -> !var.getName().isEmpty())
+ .forEach(varAssignments::add);
+ }
}
}
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/introduce/Flow.java b/java/java.hints/src/org/netbeans/modules/java/hints/introduce/Flow.java
index 860bd8b..cb499aa 100644
--- a/java/java.hints/src/org/netbeans/modules/java/hints/introduce/Flow.java
+++ b/java/java.hints/src/org/netbeans/modules/java/hints/introduce/Flow.java
@@ -114,6 +114,26 @@
*/
public class Flow {
+ public static final Set<ElementKind> LOCAL_VARIABLES =
+ Set.of(ElementKind.BINDING_VARIABLE,
+ ElementKind.EXCEPTION_PARAMETER,
+ ElementKind.LOCAL_VARIABLE,
+ ElementKind.PARAMETER,
+ ElementKind.RESOURCE_VARIABLE
+ );
+ private static final Set<ElementKind> IMPLICITLY_INITIALIZED_VARIABLES =
+ Set.of(ElementKind.BINDING_VARIABLE,
+ ElementKind.EXCEPTION_PARAMETER,
+ ElementKind.PARAMETER
+ );
+ private static final Set<ElementKind> SUPPORTED_VARIABLES;
+
+ static {
+ SUPPORTED_VARIABLES = EnumSet.noneOf(ElementKind.class);
+ SUPPORTED_VARIABLES.addAll(LOCAL_VARIABLES);
+ SUPPORTED_VARIABLES.add(ElementKind.FIELD);
+ }
+
public static FlowResult assignmentsForUse(CompilationInfo info, AtomicBoolean cancel) {
return assignmentsForUse(info, new AtomicBooleanCancel(cancel));
}
@@ -382,7 +402,6 @@
private Map<Tree, Collection<Map<Element, State>>> resumeBefore = new IdentityHashMap<Tree, Collection<Map<Element, State>>>();
private Map<Tree, Collection<Map<Element, State>>> resumeAfter = new IdentityHashMap<Tree, Collection<Map<Element, State>>>();
private Map<TypeMirror, Map<Element, State>> resumeOnExceptionHandler = new IdentityHashMap<TypeMirror, Map<Element, State>>();
- private boolean inParameters;
private Tree nearestMethod;
private Set<Element> currentMethodVariables = Collections.newSetFromMap(new IdentityHashMap<Element, Boolean>());
private final Set<Tree> deadBranches = new HashSet<Tree>();
@@ -645,17 +664,32 @@
Element e = info.getTrees().getElement(getCurrentPath());
if (e != null && SUPPORTED_VARIABLES.contains(e.getKind())) {
- // note: if the variable does not have an initializer and is not a parameter (inParameters = true),
+ // note: if the variable does not have an initializer and is not an implicitly initialized variable,
// the State will receive null as an assignment to indicate an uninitializer
+ boolean implicitlyInitialized = IMPLICITLY_INITIALIZED_VARIABLES.contains(e.getKind());
TreePath tp =
node.getInitializer() != null ?
new TreePath(getCurrentPath(), node.getInitializer()) :
- inParameters ? getCurrentPath() : null;
+ implicitlyInitialized ? getCurrentPath() : null;
recordVariableState(e, tp);
currentMethodVariables.add((Element) e);
- TreePath pp = getCurrentPath().getParentPath();
- if (pp != null) {
- addScopedVariable(pp.getLeaf(), e);
+ TreePath scopePath;
+ if (e.getKind() == ElementKind.BINDING_VARIABLE) {
+ //a wider scope does not matter too much:
+ scopePath = getCurrentPath().getParentPath();
+ SCOPE_SEARCH: while (scopePath != null) {
+ switch (scopePath.getLeaf().getKind()) {
+ case VARIABLE, BLOCK: break SCOPE_SEARCH;
+ default:
+ scopePath = scopePath.getParentPath();
+ continue SCOPE_SEARCH;
+ }
+ }
+ } else {
+ scopePath = getCurrentPath().getParentPath();
+ }
+ if (scopePath != null) {
+ addScopedVariable(scopePath.getLeaf(), e);
}
}
@@ -921,15 +955,7 @@
scan(node.getModifiers(), null);
scan(node.getReturnType(), null);
scan(node.getTypeParameters(), null);
-
- inParameters = true;
-
- try {
- scan(node.getParameters(), null);
- } finally {
- inParameters = false;
- }
-
+ scan(node.getParameters(), null);
scan(node.getThrows(), null);
List<Tree> additionalTrees = p != null ? p.initializers : Collections.<Tree>emptyList();
@@ -1393,13 +1419,8 @@
}
try {
- inParameters = true;
+ scan(node.getParameters(), null);
- try {
- scan(node.getParameters(), null);
- } finally {
- inParameters = false;
- }
b = scan(node.getBody(), p);
Set<Element> assigned = new HashSet<Element>();
@@ -1723,9 +1744,7 @@
}
public Boolean visitCatch(CatchTree node, ConstructorData p) {
- inParameters = true;
scan(node.getParameter(), p);
- inParameters = false;
scan(node.getBlock(), p);
return null;
}
@@ -1810,9 +1829,6 @@
}
}
- private static final Set<ElementKind> SUPPORTED_VARIABLES = EnumSet.of(ElementKind.EXCEPTION_PARAMETER, ElementKind.LOCAL_VARIABLE, ElementKind.PARAMETER, ElementKind.FIELD);
- private static final Set<ElementKind> LOCAL_VARIABLES = EnumSet.of(ElementKind.EXCEPTION_PARAMETER, ElementKind.LOCAL_VARIABLE, ElementKind.PARAMETER);
-
static class State {
private final Set<TreePath> assignments;
private final boolean reassigned;
diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/ThrowableNotThrownTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/ThrowableNotThrownTest.java
index 482c8bc..a7a6641 100644
--- a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/ThrowableNotThrownTest.java
+++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/ThrowableNotThrownTest.java
@@ -347,4 +347,130 @@
.run(ThrowableNotThrown.class)
.assertWarnings();
}
+
+ public void testEnhancedSwitch1() throws Exception {
+ HintTest.create()
+ .sourceLevel(25)
+ .input("""
+ package test;
+ import java.io.FileInputStream;
+ import java.io.IOException;
+
+ public class Test {
+ void test() throws Throwable {
+ try {
+ new FileInputStream("/foo");
+ } catch (IOException ex) {
+ switch (ex.getCause()) {
+ case IOException ioe -> throw ioe;
+ case Throwable _ -> throw ex;
+ }
+ }
+ }
+ }
+ """)
+ .run(ThrowableNotThrown.class)
+ .assertWarnings();
+ }
+
+ public void testEnhancedSwitch2() throws Exception {
+ HintTest.create()
+ .sourceLevel(25)
+ .input("""
+ package test;
+ import java.io.FileInputStream;
+ import java.io.IOException;
+
+ public class Test {
+ void test() throws Throwable {
+ try {
+ new FileInputStream("/foo");
+ } catch (IOException ex) {
+ switch (ex.getCause()) {
+ case IOException ioe -> throw ex;
+ case Throwable _ -> throw ex;
+ }
+ }
+ }
+ }
+ """)
+ .run(ThrowableNotThrown.class)
+ .assertWarnings("9:20-9:33:verifier:Throwable method result is ignored");
+ }
+
+ public void testEnhancedSwitchExpressionNoWarn1() throws Exception {
+ HintTest.create()
+ .sourceLevel(25)
+ .input("""
+ package test;
+ import java.io.FileInputStream;
+ import java.io.IOException;
+
+ public class Test {
+ void test() throws Throwable {
+ try {
+ new FileInputStream("/foo");
+ } catch (IOException ex) {
+ throw switch (ex.getCause()) {
+ case IOException ioe -> ioe;
+ case Throwable _ -> ex;
+ };
+ }
+ }
+ }
+ """)
+ .run(ThrowableNotThrown.class)
+ .assertWarnings();
+ }
+
+ public void testEnhancedSwitchExpressionNoWarn2() throws Exception {
+ HintTest.create()
+ .sourceLevel(25)
+ .input("""
+ package test;
+ import java.io.FileInputStream;
+ import java.io.IOException;
+
+ public class Test {
+ void test() throws Throwable {
+ try {
+ new FileInputStream("/foo");
+ } catch (IOException ex) {
+ Throwable t = switch (ex.getCause()) {
+ case IOException ioe -> ioe;
+ case Throwable tt -> throw tt;
+ };
+ }
+ }
+ }
+ """)
+ .run(ThrowableNotThrown.class)
+ .assertWarnings();
+ }
+
+ public void testEnhancedSwitchExpressionWarn() throws Exception {
+ HintTest.create()
+ .sourceLevel(25)
+ .input("""
+ package test;
+ import java.io.FileInputStream;
+ import java.io.IOException;
+
+ public class Test {
+ void test() throws Throwable {
+ try {
+ new FileInputStream("/foo");
+ } catch (IOException ex) {
+ Throwable t = switch (ex.getCause()) {
+ case IOException ioe -> ioe;
+ case Throwable tt -> tt;
+ };
+ }
+ }
+ }
+ """)
+ .run(ThrowableNotThrown.class)
+ .assertWarnings("9:34-9:47:verifier:Throwable method result is ignored");
+ }
+
}
diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/introduce/FlowTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/introduce/FlowTest.java
index 14973c5..50535c7 100644
--- a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/introduce/FlowTest.java
+++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/introduce/FlowTest.java
@@ -1597,6 +1597,61 @@
"4");
}
+ public void testParameters() throws Exception {
+ performTest("""
+ package test;
+ public class Test {
+ static void t(int ii) {
+ System.err.println(i`i);
+ }
+ }
+ """,
+ "int ii");
+ }
+
+ public void testLambdaParameters() throws Exception {
+ performTest("""
+ package test;
+ public class Test {
+ static void t() {
+ I i = ii -> System.err.println(i`i);
+ }
+ interface I {
+ public void consume(int ii);
+ }
+ }
+ """,
+ "int ii");
+ }
+
+ public void testCatchParameters() throws Exception {
+ performTest("""
+ package test;
+ public class Test {
+ static void t() {
+ try {
+ } catch (Throwable tt) {
+ System.err.println(t`t);
+ }
+ }
+ }
+ """,
+ "Throwable tt");
+ }
+
+ public void testBindingVariables() throws Exception {
+ sourceLevel = "21";
+ performTest("""
+ package test;
+ public class Test {
+ static void t(Object o) {
+ boolean b = o instanceof Integer ii && i`i == 0;
+ }
+ }
+ """,
+ "Integer ii");
+ }
+
private void performFinalCandidatesTest(String code, boolean allowErrors, String... finalCandidates) throws Exception {
prepareTest(code, allowErrors);
