You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@netbeans.apache.org by lk...@apache.org on 2020/10/23 01:11:03 UTC
[netbeans] 16/18: Handling of switch expressions and rule cases in
Flow and NPECheck hint. (#2293)
This is an automated email from the ASF dual-hosted git repository.
lkishalmi pushed a commit to branch release122
in repository https://gitbox.apache.org/repos/asf/netbeans.git
commit fc033ea496c648e4ce910695d58e3caf94502c95
Author: Jan Lahoda <jl...@netbeans.org>
AuthorDate: Thu Oct 22 23:24:19 2020 +0200
Handling of switch expressions and rule cases in Flow and NPECheck hint. (#2293)
* Handling of switch expressions and rule cases in Flow and NPECheck hint.
* Using a correct overload if TreeScanner.scan
* Fixing FlowTest on JDK 8.
* Attempting to improve stability of HideFieldByVarTest.
* Using a diamond instead of explicit type parameters.
---
java/java.hints/nbproject/project.xml | 4 +
.../netbeans/modules/java/hints/bugs/NPECheck.java | 97 +++++++++++++++++++---
.../modules/java/hints/introduce/Flow.java | 79 ++++++++++++++++--
.../modules/java/hints/HideFieldByVarTest.java | 15 +++-
.../modules/java/hints/bugs/NPECheckTest.java | 38 +++++++++
.../modules/java/hints/introduce/FlowTest.java | 49 +++++++++++
.../netbeans/api/java/source/TreeUtilities.java | 5 +-
.../netbeans/modules/java/source/TreeShims.java | 25 ++++++
8 files changed, 290 insertions(+), 22 deletions(-)
diff --git a/java/java.hints/nbproject/project.xml b/java/java.hints/nbproject/project.xml
index a328b25..a94078f 100644
--- a/java/java.hints/nbproject/project.xml
+++ b/java/java.hints/nbproject/project.xml
@@ -487,6 +487,10 @@
<compile-dependency/>
</test-dependency>
<test-dependency>
+ <code-name-base>org.netbeans.core.startup</code-name-base>
+ <compile-dependency/>
+ </test-dependency>
+ <test-dependency>
<code-name-base>org.netbeans.insane</code-name-base>
<compile-dependency/>
</test-dependency>
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/bugs/NPECheck.java b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/NPECheck.java
index 1731632..d8bec25 100644
--- a/java/java.hints/src/org/netbeans/modules/java/hints/bugs/NPECheck.java
+++ b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/NPECheck.java
@@ -48,7 +48,8 @@ import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.ElementFilter;
import org.netbeans.api.annotations.common.CheckForNull;
import org.netbeans.api.java.source.CompilationInfo;
-import org.netbeans.api.java.source.support.CancellableTreePathScanner;
+import org.netbeans.api.java.source.support.CancellableTreeScanner;
+import org.netbeans.modules.editor.java.TreeShims;
import org.netbeans.spi.editor.hints.ErrorDescription;
import org.openide.util.NbBundle;
@@ -542,7 +543,7 @@ public class NPECheck {
public Iterable<? extends AnnotationMirror> getAnnotationMirrors(CompilationInfo info, Element el);
}
- private static final class VisitorImpl extends CancellableTreePathScanner<State, Void> {
+ private static final class VisitorImpl extends CancellableTreeScanner<State, Void> {
private final HintContext ctx;
private final CompilationInfo info;
@@ -564,6 +565,7 @@ public class NPECheck {
private Map<TypeMirror, Map<VariableElement, State>> resumeOnExceptionHandler = new IdentityHashMap<>();
private final Map<Tree, State> expressionState = new IdentityHashMap<>();
private final List<TreePath> pendingFinally = new LinkedList<>();
+ private List<State> pendingYields = new ArrayList<>();
private boolean not;
private boolean doNotRecord;
private final TypeElement throwableEl;
@@ -617,11 +619,45 @@ public class NPECheck {
}
+ private TreePath currentPath;
+
+ public TreePath getCurrentPath() {
+ return currentPath;
+ }
+
+ public State scan(TreePath path, Void p) {
+ TreePath oldPath = currentPath;
+ try {
+ currentPath = path;
+ return super.scan(path.getLeaf(), p);
+ } finally {
+ currentPath = oldPath;
+ }
+ }
+
@Override
public State scan(Tree tree, Void p) {
resume(tree, resumeBefore);
- State r = super.scan(tree, p);
+ State r;
+
+ if (tree != null) {
+ TreePath oldPath = currentPath;
+ try {
+ currentPath = new TreePath(currentPath, tree);
+ if (TreeShims.SWITCH_EXPRESSION.equals(tree.getKind().name())) {
+ r = visitSwitchExpression(tree, p);
+ } else if (TreeShims.YIELD.equals(tree.getKind().name())) {
+ r = visitYield(tree, p);
+ } else {
+ r = super.scan(tree, p);
+ }
+ } finally {
+ currentPath = oldPath;
+ }
+ } else {
+ r = null;
+ }
TypeMirror currentType = tree != null ? info.getTrees().getTypeMirror(new TreePath(getCurrentPath(), tree)) : null;
@@ -1223,27 +1259,54 @@ public class NPECheck {
@Override
public State visitSwitch(SwitchTree node, Void p) {
- scan(node.getExpression(), null);
+ handleGeneralizedSwitch(node, node.getExpression(), node.getCases());
+ return null;
+ }
+
+ public State visitSwitchExpression(Tree node, Void p) {
+ List<State> oldPendingYields = pendingYields;
+ try {
+ pendingYields = new ArrayList<>();
+ handleGeneralizedSwitch(node, TreeShims.getExpressions(node).get(0), TreeShims.getCases(node));
+ if (pendingYields.isEmpty()) {
+ //should not happen (for valid source)
+ return State.POSSIBLE_NULL;
+ }
+ State result = pendingYields.get(0);
+ for (State s : pendingYields.subList(1, pendingYields.size())) {
+ result = State.collect(result, s);
+ }
+ return result;
+ } finally {
+ pendingYields = oldPendingYields;
+ }
+ }
+
+ private void handleGeneralizedSwitch(Tree switchTree, ExpressionTree expression, List<? extends CaseTree> cases) {
+ scan(expression, null);
Map<VariableElement, State> origVariable2State = new HashMap<>(variable2State);
boolean exhaustive = false;
- for (CaseTree ct : node.getCases()) {
+ for (CaseTree ct : cases) {
mergeIntoVariable2State(origVariable2State);
if (ct.getExpression() == null) {
exhaustive = true;
}
- scan(ct, null);
+ State caseResult = scan(ct, null);
+
+ if (TreeShims.isRuleCase(ct)) {
+ pendingYields.add(caseResult);
+ breakTo(switchTree);
+ }
}
if (!exhaustive) {
mergeIntoVariable2State(origVariable2State);
}
-
- return null;
}
@Override
@@ -1252,13 +1315,27 @@ public class NPECheck {
Tree target = info.getTreeUtilities().getBreakContinueTargetTree(getCurrentPath());
- resumeAfter(target, variable2State);
+ breakTo(target);
- variable2State = new HashMap<>(); //XXX: fields?
+ return null;
+ }
+
+ public State visitYield(Tree node, Void p) {
+ pendingYields.add(scan(TreeShims.getYieldValue(node), p));
+
+ Tree target = info.getTreeUtilities().getBreakContinueTargetTree(getCurrentPath());
+ breakTo(target);
+
return null;
}
+ private void breakTo(Tree target) {
+ resumeAfter(target, variable2State);
+
+ variable2State = new HashMap<>(); //XXX: fields?
+ }
+
@Override
public State visitTry(TryTree node, Void p) {
Map<TypeMirror, Map<VariableElement, State>> oldResumeOnExceptionHandler = resumeOnExceptionHandler;
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 46df84f..8e52140 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
@@ -101,7 +101,8 @@ import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import org.netbeans.api.java.source.CompilationInfo;
import org.netbeans.api.java.source.CompilationInfo.CacheClearPolicy;
-import org.netbeans.api.java.source.support.CancellableTreePathScanner;
+import org.netbeans.api.java.source.support.CancellableTreeScanner;
+import org.netbeans.modules.editor.java.TreeShims;
import org.netbeans.modules.java.hints.errors.Utilities;
import org.netbeans.spi.java.hints.HintContext;
@@ -353,7 +354,7 @@ public class Flow {
* that are qualified to belong to other scopes are ignored at the moment.
*
*/
- private static final class VisitorImpl extends CancellableTreePathScanner<Boolean, ConstructorData> {
+ private static final class VisitorImpl extends CancellableTreeScanner<Boolean, ConstructorData> {
private final CompilationInfo info;
private final TypeElement undefinedSymbolScope;
@@ -444,11 +445,45 @@ public class Flow {
return cancel.isCanceled();
}
+ private TreePath currentPath;
+
+ public TreePath getCurrentPath() {
+ return currentPath;
+ }
+
+ public Boolean scan(TreePath path, ConstructorData p) {
+ TreePath oldPath = currentPath;
+ try {
+ currentPath = path;
+ return super.scan(path.getLeaf(), p);
+ } finally {
+ currentPath = oldPath;
+ }
+ }
+
@Override
public Boolean scan(Tree tree, ConstructorData p) {
resume(tree, resumeBefore);
- Boolean result = super.scan(tree, p);
+ Boolean result;
+
+ if (tree != null) {
+ TreePath oldPath = currentPath;
+ try {
+ currentPath = new TreePath(currentPath, tree);
+ if (TreeShims.SWITCH_EXPRESSION.equals(tree.getKind().name())) {
+ result = visitSwitchExpression(tree, p);
+ } else if (TreeShims.YIELD.equals(tree.getKind().name())) {
+ result = visitYield(tree, p);
+ } else {
+ result = super.scan(tree, p);
+ }
+ } finally {
+ currentPath = oldPath;
+ }
+ } else {
+ result = null;
+ }
resume(tree, resumeAfter);
@@ -1172,15 +1207,39 @@ public class Flow {
Tree target = info.getTreeUtilities().getBreakContinueTargetTree(getCurrentPath());
- resumeAfter(target, variable2State);
+ breakTo(target);
- variable2State = new HashMap< Element, State>();
+ return null;
+ }
+
+ public Boolean visitYield(Tree node, ConstructorData p) {
+ scan(TreeShims.getYieldValue(node), p);
+
+ Tree target = info.getTreeUtilities().getBreakContinueTargetTree(getCurrentPath());
+ breakTo(target);
+
return null;
}
+ private void breakTo(Tree target) {
+ resumeAfter(target, variable2State);
+
+ variable2State = new HashMap<>();
+ }
+
public Boolean visitSwitch(SwitchTree node, ConstructorData p) {
- scan(node.getExpression(), null);
+ generalizedSwitch(node, node.getExpression(), node.getCases());
+ return null;
+ }
+
+ public Boolean visitSwitchExpression(Tree node, ConstructorData p) {
+ generalizedSwitch(node, TreeShims.getExpressions(node).get(0), TreeShims.getCases(node));
+ return null; //never a constant expression
+ }
+
+ private void generalizedSwitch(Tree switchTree, ExpressionTree expression, List<? extends CaseTree> cases) {
+ scan(expression, null);
Map< Element, State> origVariable2State = new HashMap< Element, State>(variable2State);
@@ -1188,7 +1247,7 @@ public class Flow {
boolean exhaustive = false;
- for (CaseTree ct : node.getCases()) {
+ for (CaseTree ct : cases) {
variable2State = mergeOr(variable2State, origVariable2State);
if (ct.getExpression() == null) {
@@ -1196,13 +1255,15 @@ public class Flow {
}
scan(ct, null);
+
+ if (TreeShims.isRuleCase(ct)) {
+ breakTo(switchTree);
+ }
}
if (!exhaustive) {
variable2State = mergeOr(variable2State, origVariable2State);
}
-
- return null;
}
public Boolean visitEnhancedForLoop(EnhancedForLoopTree node, ConstructorData p) {
diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/HideFieldByVarTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/HideFieldByVarTest.java
index a37c7734..bba18e8 100644
--- a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/HideFieldByVarTest.java
+++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/HideFieldByVarTest.java
@@ -19,6 +19,7 @@
package org.netbeans.modules.java.hints;
import com.sun.source.util.TreePath;
+import java.io.File;
import java.util.List;
import java.util.Locale;
import org.netbeans.api.java.source.CompilationInfo;
@@ -78,9 +79,8 @@ public class HideFieldByVarTest extends TreeRuleTestBase {
"}";
for (int i = 0; i < text.length(); i++) {
- clearWorkDir();
+ workDirIndex = i;
performAnalysisTest("test/Test.java", "// index: " + i + "\n" + text, i);
- SourceUtils.waitScanFinished();
}
}
@RandomlyFails
@@ -119,5 +119,16 @@ public class HideFieldByVarTest extends TreeRuleTestBase {
}
private String sourceLevel = "1.5";
+
+ private int workDirIndex = -1;
+
+ @Override
+ public String getWorkDirPath() {
+ String basePath = super.getWorkDirPath();
+ if (workDirIndex != (-1)) {
+ basePath += File.separator + workDirIndex;
+ }
+ return basePath;
+ }
}
diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/NPECheckTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/NPECheckTest.java
index 6e9dc30..e62a098 100644
--- a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/NPECheckTest.java
+++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/NPECheckTest.java
@@ -1760,6 +1760,44 @@ public class NPECheckTest extends NbTestCase {
.assertWarnings("6:29-6:37:verifier:DN");
}
+ public void testRuleCases() throws Exception {
+ HintTest.create()
+ .sourceLevel("14")
+ .input("package test;\n" +
+ "public class Test {\n" +
+ " public void test(int i) {\n" +
+ " Object o;\n" +
+ " switch (i) {\n" +
+ " case 0 -> o = null;\n" +
+ " default -> { o = 12; }\n" +
+ " }\n" +
+ " o.toString();\n" +
+ " }\n" +
+ "}")
+ .run(NPECheck.class)
+ .assertWarnings("8:6-8:14:verifier:Possibly Dereferencing null");
+ }
+
+ public void testSwitchExpression1() throws Exception {
+ HintTest.create()
+ .sourceLevel("14")
+ .input("package test;\n" +
+ "public class Test {\n" +
+ " public void test(int i) {\n" +
+ " Object o1;\n" +
+ " Object o2 = switch (i) {\n" +
+ " case 0 -> o1 = null;\n" +
+ " default -> { yield o1 = 12; }\n" +
+ " };\n" +
+ " o1.toString();\n" +
+ " o2.toString();\n" +
+ " }\n" +
+ "}")
+ .run(NPECheck.class)
+ .assertWarnings("8:7-8:15:verifier:Possibly Dereferencing null",
+ "9:7-9:15:verifier:Possibly Dereferencing null");
+ }
+
private void performAnalysisTest(String fileName, String code, String... golden) throws Exception {
HintTest.create()
.input(fileName, code)
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 b7fdad3..14973c5 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
@@ -39,6 +39,7 @@ import org.netbeans.api.java.source.JavaSource;
import org.netbeans.api.java.source.JavaSource.Phase;
import org.netbeans.api.java.source.SourceUtilsTestUtil;
import org.netbeans.api.lexer.Language;
+import org.netbeans.core.startup.Main;
import org.netbeans.junit.NbTestCase;
import org.netbeans.modules.java.hints.introduce.Flow.FlowResult;
import org.netbeans.modules.java.hints.spiimpl.TestUtilities;
@@ -62,6 +63,7 @@ public class FlowTest extends NbTestCase {
@Override
protected void setUp() throws Exception {
+ JavacParser.DISABLE_SOURCE_LEVEL_DOWNGRADE = true;
SourceUtilsTestUtil
.prepareTest(new String[0], new Object[0]);
super
@@ -1552,6 +1554,49 @@ public class FlowTest extends NbTestCase {
"di");
}
+ public void testRuleCases() throws Exception {
+ sourceLevel = "14";
+ performTest("package test;\n" +
+ "public class Test {\n" +
+ " public void test(int i) {\n" +
+ " int val;\n" +
+ " switch (i) {\n" +
+ " case 0 -> val = 0;\n" +
+ " case 1 -> val = 1;\n" +
+ " case 2 -> { val = 2; }\n" +
+ " case 3 -> { val = 3; }\n" +
+ " default -> { val = -1; }\n" +
+ " }\n" +
+ " System.err.println(val`);\n" +
+ " }\n" +
+ "}",
+ "0",
+ "1",
+ "2",
+ "3",
+ "-1");
+ }
+
+ public void testSwitchExpression1() throws Exception {
+ sourceLevel = "14";
+ performTest("package test;\n" +
+ "public class Test {\n" +
+ " static void t(int p) {\n" +
+ " int ii;\n" +
+ " int x = switch (p) {\n" +
+ " case 0: ii = 1; yield 0;\n" +
+ " case 1: ii = 2;\n" +
+ " case 2: ii = 3; yield 0;\n" +
+ " default: ii = 4; yield 0;\n" +
+ " };\n" +
+ " System.err.println(i`i);\n" +
+ " }\n" +
+ "}\n",
+ "1",
+ "3",
+ "4");
+ }
+
private void performFinalCandidatesTest(String code, boolean allowErrors, String... finalCandidates) throws Exception {
prepareTest(code, allowErrors);
@@ -1566,4 +1611,8 @@ public class FlowTest extends NbTestCase {
assertEquals(Arrays.asList(finalCandidates), computedCandidates);
}
+
+ static {
+ Main.initializeURLFactory();
+ }
}
diff --git a/java/java.source.base/src/org/netbeans/api/java/source/TreeUtilities.java b/java/java.source.base/src/org/netbeans/api/java/source/TreeUtilities.java
index 944f641..52fc10b 100644
--- a/java/java.source.base/src/org/netbeans/api/java/source/TreeUtilities.java
+++ b/java/java.source.base/src/org/netbeans/api/java/source/TreeUtilities.java
@@ -1385,7 +1385,7 @@ public final class TreeUtilities {
*
* @param breakOrContinue {@link TreePath} to the tree that should be inspected.
* The <code>breakOrContinue.getLeaf().getKind()</code>
- * has to be either {@link Kind#BREAK} or {@link Kind#CONTINUE}, or
+ * has to be one of {@link Kind#BREAK}, {@link Kind#CONTINUE}, or {@link Kind#YIELD}, or
* an IllegalArgumentException is thrown
* @return the tree that is the "target" for the given break or continue statement, or null if there is none. Tree can be of type StatementTree or ExpressionTree
* @throws IllegalArgumentException if the given tree is not a break or continue tree or if the given {@link CompilationInfo}
@@ -1424,6 +1424,9 @@ public final class TreeUtilities {
return target;
}
default:
+ if (TreeShims.YIELD.equals(leaf.getKind().name())) {
+ return TreeShims.getTarget(leaf);
+ }
throw new IllegalArgumentException("Unsupported kind: " + leaf.getKind());
}
}
diff --git a/java/java.source.base/src/org/netbeans/modules/java/source/TreeShims.java b/java/java.source.base/src/org/netbeans/modules/java/source/TreeShims.java
index e7c1e2d..4577f6b 100644
--- a/java/java.source.base/src/org/netbeans/modules/java/source/TreeShims.java
+++ b/java/java.source.base/src/org/netbeans/modules/java/source/TreeShims.java
@@ -29,6 +29,7 @@ import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.JCTree.JCClassDecl;
import com.sun.tools.javac.tree.TreeMaker;
import com.sun.tools.javac.util.ListBuffer;
+import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.ArrayList;
@@ -71,6 +72,17 @@ public class TreeShims {
}
}
+ public static boolean isRuleCase(CaseTree node) {
+ try {
+ Method getCaseKind = CaseTree.class.getDeclaredMethod("getCaseKind");
+ return "RULE".equals(String.valueOf(getCaseKind.invoke(node)));
+ } catch (NoSuchMethodException ex) {
+ return false;
+ } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException ex) {
+ throw TreeShims.<RuntimeException>throwAny(ex);
+ }
+ }
+
public static Tree getPattern(InstanceOfTree node) {
try {
Method getPattern = InstanceOfTree.class.getDeclaredMethod("getPattern");
@@ -265,4 +277,17 @@ public class TreeShims {
}
}
+ public static Tree getTarget(Tree node) {
+ if (!node.getKind().name().equals(YIELD)) {
+ throw new IllegalStateException();
+ }
+ try {
+ Field target = node.getClass().getField("target");
+ return (Tree) target.get(node);
+ } catch (NoSuchFieldException ex) {
+ return null;
+ } catch (IllegalAccessException | IllegalArgumentException ex) {
+ throw TreeShims.<RuntimeException>throwAny(ex);
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@netbeans.apache.org
For additional commands, e-mail: commits-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists