Skip to content

Commit

Permalink
Don't crash on switch expressions or switch statements with arrow cases
Browse files Browse the repository at this point in the history
  • Loading branch information
smillst authored Dec 6, 2021
1 parent 06a0aab commit 17dbf44
Show file tree
Hide file tree
Showing 10 changed files with 418 additions and 5 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ List<String> getJavaFilesToFormat(projectName) {
&& !details.path.contains("returnsreceiverdelomboked")
&& !details.path.contains("build")
&& (isJava17 || !details.path.contains("-records"))
&& (isJava17 || !details.path.contains("java17"))
&& details.name.endsWith('.java')) {
javaFiles.add(details.file)
}
Expand Down
2 changes: 1 addition & 1 deletion checker/bin-devel/git.pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ set -e
# Need to keep checked files in sync with getJavaFilesToFormat in build.gradle.
# Otherwise `./gradlew reformat` might not reformat a file that this
# hook complains about.
CHANGED_JAVA_FILES=$(git diff --staged --name-only --diff-filter=ACM | grep '\.java$' | grep -v '/jdk/' | grep -v 'stubparser/' | grep -v '/nullness-javac-errors/' | grep -v 'dataflow/manual/examples/' ) || true
CHANGED_JAVA_FILES=$(git diff --staged --name-only --diff-filter=ACM | grep '\.java$' | grep -v '/jdk/' | grep -v 'stubparser/' | grep -v '/nullness-javac-errors/' | grep -v 'dataflow/manual/examples/' | grep -v '/java17/') || true
# echo "CHANGED_JAVA_FILES=${CHANGED_JAVA_FILES}"
if [ -n "$CHANGED_JAVA_FILES" ]; then
./gradlew getCodeFormatScripts -q
Expand Down
80 changes: 80 additions & 0 deletions checker/tests/nullness/java17/NullnessSwitchArrows.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// @below-java17-jdk-skip-test
public class NullnessSwitchArrows {
public enum Day {
SUNDAY,
MONDAY,
TUESDAY,
WEDNESDAY,
THURSDAY,
FRIDAY,
SATURDAY;
}

void method1() {
Object o;
Day day = Day.WEDNESDAY;
switch (day) {
case MONDAY, FRIDAY, SUNDAY -> o = "hello";
case TUESDAY -> o = null;
case THURSDAY, SATURDAY -> o = "hello";
case WEDNESDAY -> o = "hello";
default -> throw new IllegalStateException("Invalid day: " + day);
}

// :: error: (dereference.of.nullable)
o.toString();
}

void method2() {
Object o;
Day day = Day.WEDNESDAY;
switch (day) {
case MONDAY, FRIDAY, SUNDAY -> o = "hello";
case TUESDAY -> o = "hello";
case THURSDAY, SATURDAY -> o = "hello";
case WEDNESDAY -> o = "hello";
default -> throw new IllegalStateException("Invalid day: " + day);
}

// TODO: this is a false positive. It works for case: statements; see below.
// :: error: (dereference.of.nullable)
o.toString();
}

void method2b() {
Object o;
Day day = Day.WEDNESDAY;
switch (day) {
case MONDAY, FRIDAY, SUNDAY:
o = "hello";
break;
case TUESDAY:
o = "hello";
break;
case THURSDAY, SATURDAY:
o = "hello";
break;
case WEDNESDAY:
o = "hello";
break;
default:
throw new IllegalStateException("Invalid day: " + day);
}

o.toString();
}

void method3() {
Object o;
Day day = Day.WEDNESDAY;
switch (day) {
case MONDAY, FRIDAY, SUNDAY -> o = "hello";
case TUESDAY -> o = "hello";
case THURSDAY, SATURDAY -> o = "hello";
case WEDNESDAY -> o = "hello";
default -> o = "hello";
}

o.toString();
}
}
67 changes: 67 additions & 0 deletions checker/tests/nullness/java17/NullnessSwitchExpressions.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// @below-java17-jdk-skip-test
public class NullnessSwitchExpressions {
public enum Day {
SUNDAY,
MONDAY,
TUESDAY,
WEDNESDAY,
THURSDAY,
FRIDAY,
SATURDAY;
}

void method1() {
Day day = Day.WEDNESDAY;
Object o =
switch (day) {
case MONDAY, FRIDAY, SUNDAY -> "hello";
case TUESDAY -> null;
case THURSDAY, SATURDAY -> "hello";
case WEDNESDAY -> "hello";
default -> throw new IllegalStateException("Invalid day: " + day);
};

// :: error: (dereference.of.nullable)
o.toString();
}

void method2() {
Day day = Day.WEDNESDAY;
Object o =
switch (day) {
case MONDAY, FRIDAY, SUNDAY -> "hello";
case TUESDAY -> "hello";
case THURSDAY, SATURDAY -> "hello";
case WEDNESDAY -> "hello";
default -> throw new IllegalStateException("Invalid day: " + day);
};

// TODO: This is a false positive.
// :: error: (dereference.of.nullable)
o.toString();
}

void method3() {
Day day = Day.WEDNESDAY;
Object o =
switch (day) {
case MONDAY, FRIDAY, SUNDAY -> "hello";
case TUESDAY -> "hello";
case THURSDAY, SATURDAY -> {
String s = null;
if (day == Day.THURSDAY) {
s = "hello";
// TODO: This is a false positive.
// :: error: (dereference.of.nullable)
s.toString();
}
yield s;
}
case WEDNESDAY -> "hello";
default -> throw new IllegalStateException("Invalid day: " + day);
};

// :: error: (dereference.of.nullable)
o.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,46 @@ public PhaseOneResult process(CompilationUnitTree root, UnderlyingAST underlying
*/
public void handleArtificialTree(Tree tree) {}

@Override
public Node scan(Tree tree, Void p) {
if (tree == null) {
return null;
}
// Must use String comparison to support compiling on JDK 11 and earlier.
if (tree.getKind().name().equals("SWITCH_EXPRESSION")) {
return visitSwitchExpression17(tree, p);
}
return super.scan(tree, p);

// TODO: Do we need to support yield trees and binding patterns to?
// Features added between JDK 12 and JDK 17 inclusive.
// switch (tree.getKind().name()) {
// case "BINDING_PATTERN":
// return visitBindingPattern17(path.getLeaf(), p);
// case "SWITCH_EXPRESSION":
// return visitSwitchExpression17(tree, p);
// case "YIELD":
// return visitYield17(path.getLeaf(), p);
// default:
// return super.scan(tree, p);
// }
}

/**
* Visit a SwitchExpressionTree
*
* @param switchExpressionTree a SwitchExpressionTree, typed as Tree to be backward-compatible
* @param p parameter
* @return the result of visiting the switch expression tree
*/
public Node visitSwitchExpression17(Tree switchExpressionTree, Void p) {
// TODO: Analyze switch expressions properly.
return new MarkerNode(
switchExpressionTree,
"switch expression tree; not analyzed #" + TreeUtils.treeUids.get(switchExpressionTree),
env.getTypeUtils());
}

/* --------------------------------------------------------- */
/* Nodes and Labels Management */
/* --------------------------------------------------------- */
Expand Down Expand Up @@ -2190,8 +2230,12 @@ private void buildCase(CaseTree tree, int index) {
extendWithExtendedNode(new ConditionalJump(thisBodyL, nextCaseL));
}
addLabelForNextNode(thisBodyL);
for (StatementTree stmt : tree.getStatements()) {
scan(stmt, null);
if (tree.getStatements() != null) {
for (StatementTree stmt : tree.getStatements()) {
scan(stmt, null);
}
} else {
scan(TreeUtils.caseTreeGetBody(tree), null);
}
extendWithExtendedNode(new UnconditionalJump(nextBodyL));
addLabelForNextNode(nextCaseL);
Expand Down
13 changes: 11 additions & 2 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
Version 3.20.0 (December 1, 2021)
---------------------------------
Version 3.20.1 (December 6, 2021)
-------------------------------

**User-visible changes:**

The Checker Framework now runs on code that contains switch expressions and
switch statements that use the new `->` case syntax, but treats them
conservatively. A future version will improve precision.

**Implementation details:**

The dataflow framework can be run on code that contains switch expressions and
switch statements that use the new `->` case syntax, but it does not yet
analyze the cases in a switch expression and it treats `->` as `:`.. A future
version will do so.

Removed methods and classes that have been deprecated for more than one year:
* Old way of constructing qualifier hierarchies
* `@SuppressWarningsKeys`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.ElementFilter;
import javax.tools.Diagnostic.Kind;
import javax.tools.JavaFileObject;
import org.checkerframework.checker.compilermsgs.qual.CompilerMessageKey;
import org.checkerframework.checker.interning.qual.FindDistinct;
import org.checkerframework.checker.nullness.qual.NonNull;
Expand Down Expand Up @@ -359,6 +360,11 @@ protected void testJointJavacJavaParserVisitor() {
}

Map<Tree, com.github.javaparser.ast.Node> treePairs = new HashMap<>();
JavaFileObject f = root.getSourceFile();
if (f.toUri().getPath().contains("java17")) {
// Skip java17 files because they may contain switch expressions which aren't supported.
return;
}
try (InputStream reader = root.getSourceFile().openInputStream()) {
CompilationUnit javaParserRoot = JavaParserUtil.parseCompilationUnit(reader);
JavaParserUtil.concatenateAddedStringLiterals(javaParserRoot);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,29 @@ public AnnotatedTypeMirror visitConditionalExpression(
return AnnotatedTypes.leastUpperBound(f, trueType, falseType, alub);
}

@Override
public AnnotatedTypeMirror defaultAction(Tree tree, AnnotatedTypeFactory f) {
if (tree.getKind().name().equals("SWITCH_EXPRESSION")) {
return visitSwitchExpressionTree17(tree, f);
}
return super.defaultAction(tree, f);
}

/**
* Compute the type of the switch expression tree.
*
* @param switchExpressionTree SwitchExpressionTree; typed as Tree to be backward-compatible
* @param f AnnotatedTypeFactory
* @return the type of the switch expression
*/
public AnnotatedTypeMirror visitSwitchExpressionTree17(
Tree switchExpressionTree, AnnotatedTypeFactory f) {
// TODO: Properly compute the type from the cases.
AnnotatedTypeMirror result = f.type(switchExpressionTree);
result.addAnnotations(f.getQualifierHierarchy().getTopAnnotations());
return result;
}

@Override
public AnnotatedTypeMirror visitIdentifier(IdentifierTree node, AnnotatedTypeFactory f) {
if (node.getName().contentEquals("this") || node.getName().contentEquals("super")) {
Expand Down
Loading

0 comments on commit 17dbf44

Please sign in to comment.