JointJavacJavaParserVisitor now handles switch expressions (#4992)

JavaParsers crashes on some yields and also parses some to objects other than YieldStmts. ( javaparser/javaparser#3364)
typetools · Dec 17, 2021 · cd9a2a3 · cd9a2a3
1 parent 689b033
commit cd9a2a3
Show file tree

Hide file tree

Showing 6 changed files with 132 additions and 14 deletions.
diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java
@@ -76,7 +76,6 @@
 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;
@@ -366,11 +365,6 @@ 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);

diff --git a/framework/src/main/java/org/checkerframework/framework/ajava/DefaultJointVisitor.java b/framework/src/main/java/org/checkerframework/framework/ajava/DefaultJointVisitor.java
@@ -37,6 +37,7 @@
 import com.github.javaparser.ast.expr.SimpleName;
 import com.github.javaparser.ast.expr.SingleMemberAnnotationExpr;
 import com.github.javaparser.ast.expr.SuperExpr;
+import com.github.javaparser.ast.expr.SwitchExpr;
 import com.github.javaparser.ast.expr.ThisExpr;
 import com.github.javaparser.ast.expr.UnaryExpr;
 import com.github.javaparser.ast.modules.ModuleDeclaration;
@@ -65,6 +66,7 @@
 import com.github.javaparser.ast.stmt.ThrowStmt;
 import com.github.javaparser.ast.stmt.TryStmt;
 import com.github.javaparser.ast.stmt.WhileStmt;
+import com.github.javaparser.ast.stmt.YieldStmt;
 import com.github.javaparser.ast.type.ArrayType;
 import com.github.javaparser.ast.type.ClassOrInterfaceType;
 import com.github.javaparser.ast.type.IntersectionType;
@@ -347,6 +349,9 @@ public void processReturn(ReturnTree javacTree, ReturnStmt javaParserNode) {}
   @Override
   public void processSwitch(SwitchTree javacTree, SwitchStmt javaParserNode) {}
 
+  @Override
+  public void processSwitchExpression(Tree javacTree, SwitchExpr javaParserNode) {}
+
   @Override
   public void processSynchronized(SynchronizedTree javacTree, SynchronizedStmt javaParserNode) {}
 
@@ -389,6 +394,9 @@ public void processWhileLoop(WhileLoopTree javacTree, WhileStmt javaParserNode)
   @Override
   public void processWildcard(WildcardTree javacTree, WildcardType javaParserNode) {}
 
+  @Override
+  public void processYield(Tree javacTree, YieldStmt javaParserNode) {}
+
   @Override
   public void processCompoundAssignment(
       CompoundAssignmentTree javacTree, AssignExpr javaParserNode) {}

diff --git a/framework/src/main/java/org/checkerframework/framework/ajava/ExpectedTreesVisitor.java b/framework/src/main/java/org/checkerframework/framework/ajava/ExpectedTreesVisitor.java
@@ -203,6 +203,14 @@ public Void visitSwitch(SwitchTree tree, Void p) {
     return null;
   }
 
+  @Override
+  public Void visitSwitchExpression17(Tree tree, Void p) {
+    super.visitSwitchExpression17(tree, p);
+    // javac surrounds switch expression in a ParenthesizedTree but JavaParser does not.
+    trees.remove(TreeUtils.switchExpressionTreeGetExpression(tree));
+    return null;
+  }
+
   @Override
   public Void visitSynchronized(SynchronizedTree tree, Void p) {
     super.visitSynchronized(tree, p);
@@ -374,6 +382,14 @@ public Void visitVariable(VariableTree tree, Void p) {
     return super.visitVariable(tree, p);
   }
 
+  @Override
+  public Void visitYield17(Tree tree, Void p) {
+    // JavaParser does not parse yields correctly:
+    // https://github.com/javaparser/javaparser/issues/3364
+    // So skip yields.
+    return null;
+  }
+
   /**
    * Calls the correct visit method for {@code tree} if {@code tree} is non-null.
    *

diff --git a/...ework/src/main/java/org/checkerframework/framework/ajava/JointJavacJavaParserVisitor.java b/...ework/src/main/java/org/checkerframework/framework/ajava/JointJavacJavaParserVisitor.java
@@ -41,6 +41,7 @@
 import com.github.javaparser.ast.expr.SimpleName;
 import com.github.javaparser.ast.expr.SingleMemberAnnotationExpr;
 import com.github.javaparser.ast.expr.SuperExpr;
+import com.github.javaparser.ast.expr.SwitchExpr;
 import com.github.javaparser.ast.expr.ThisExpr;
 import com.github.javaparser.ast.expr.TypeExpr;
 import com.github.javaparser.ast.expr.UnaryExpr;
@@ -74,6 +75,7 @@
 import com.github.javaparser.ast.stmt.ThrowStmt;
 import com.github.javaparser.ast.stmt.TryStmt;
 import com.github.javaparser.ast.stmt.WhileStmt;
+import com.github.javaparser.ast.stmt.YieldStmt;
 import com.github.javaparser.ast.type.ArrayType;
 import com.github.javaparser.ast.type.ClassOrInterfaceType;
 import com.github.javaparser.ast.type.IntersectionType;
@@ -140,6 +142,7 @@
 import com.sun.source.tree.SynchronizedTree;
 import com.sun.source.tree.ThrowTree;
 import com.sun.source.tree.Tree;
+import com.sun.source.tree.Tree.Kind;
 import com.sun.source.tree.TryTree;
 import com.sun.source.tree.TypeCastTree;
 import com.sun.source.tree.TypeParameterTree;
@@ -317,6 +320,7 @@ && isDefaultSuperConstructorCall(javacIter.peek())
       // instances. In javaParser this is one VariableDeclarationExpr with two nested
       // VariableDeclarators. Match the declarators with the VariableTrees.
       if (javaParserIter.hasNext()
+          && javacIter.peek().getKind() == Tree.Kind.VARIABLE
           && javaParserIter.peek().isExpressionStmt()
           && javaParserIter.peek().asExpressionStmt().getExpression().isVariableDeclarationExpr()) {
         for (VariableDeclarator decl :
@@ -327,7 +331,6 @@ && isDefaultSuperConstructorCall(javacIter.peek())
                 .asVariableDeclarationExpr()
                 .getVariables()) {
           assert javacIter.hasNext();
-          assert javacIter.peek().getKind() == Tree.Kind.VARIABLE;
           javacIter.next().accept(this, decl);
         }
 
@@ -409,8 +412,20 @@ public Void visitCase(CaseTree javacTree, Node javaParserNode) {
     for (int i = 0; i < treeExpressions.size(); i++) {
       treeExpressions.get(i).accept(this, labels.get(i));
     }
+    if (javacTree.getStatements() == null) {
+      Tree javacBody = TreeUtils.caseTreeGetBody(javacTree);
+      Statement nodeBody = node.getStatement(0);
+      if (javacBody.getKind() == Kind.EXPRESSION_STATEMENT) {
+        javacBody.accept(this, node.getStatement(0));
+      } else if (nodeBody.isExpressionStmt()) {
+        javacBody.accept(this, nodeBody.asExpressionStmt().getExpression());
+      } else {
+        javacBody.accept(this, nodeBody);
+      }
+    } else {
+      processStatements(javacTree.getStatements(), node.getStatements());
+    }
 
-    processStatements(javacTree.getStatements(), node.getStatements());
     return null;
   }
 
@@ -1228,14 +1243,23 @@ public Void visitSwitch(SwitchTree javacTree, Node javaParserNode) {
   }
 
   /**
-   * Visit a SwitchExpressionTree
+   * Visit a switch expression.
    *
-   * @param tree a SwitchExpressionTree, typed as Tree to be backward-compatible
-   * @param node a SwitchExpr, typed as Node to be backward-compatible
-   * @return nothing
+   * @param javacTree switch expression tree
+   * @param javaParserNode java parser node
+   * @return null
    */
-  public Void visitSwitchExpression17(Tree tree, Node node) {
-    // TODO
+  public Void visitSwitchExpression17(Tree javacTree, Node javaParserNode) {
+    SwitchExpr node = castNode(SwitchExpr.class, javaParserNode, javacTree);
+    processSwitchExpression(javacTree, node);
+
+    // Switch expressions are always parenthesized in javac but never in JavaParser.
+    ExpressionTree expression =
+        ((ParenthesizedTree) TreeUtils.switchExpressionTreeGetExpression(javacTree))
+            .getExpression();
+    expression.accept(this, node.getSelector());
+
+    visitLists(TreeUtils.switchExpressionTreeGetCases(javacTree), node.getEntries());
     return null;
   }
 
@@ -1452,6 +1476,16 @@ public Void visitWildcard(WildcardTree javacTree, Node javaParserNode) {
    * @return nothing
    */
   public Void visitYield17(Tree tree, Node node) {
+    if (node instanceof YieldStmt) {
+      YieldStmt yieldStmt = castNode(YieldStmt.class, node, tree);
+      processYield(tree, yieldStmt);
+
+      TreeUtils.yieldTreeGetValue(tree).accept(this, yieldStmt.getExpression());
+      return null;
+    }
+    // JavaParser does not parse yields correctly:
+    // https://github.com/javaparser/javaparser/issues/3364
+    // So skip yields that aren't matched with a YieldStmt.
     return null;
   }
 
@@ -2036,6 +2070,14 @@ public abstract void processRequires(
    */
   public abstract void processSwitch(SwitchTree javacTree, SwitchStmt javaParserNode);
 
+  /**
+   * Process a {@code SwitchExpressionTree}.
+   *
+   * @param javacTree tree to process
+   * @param javaParserNode corresponding JavaParser node
+   */
+  public abstract void processSwitchExpression(Tree javacTree, SwitchExpr javaParserNode);
+
   /**
    * Process a {@code SynchronizedTree}.
    *
@@ -2152,6 +2194,14 @@ public abstract void processVariable(
    */
   public abstract void processWildcard(WildcardTree javacTree, WildcardType javaParserNode);
 
+  /**
+   * Process a {@code YieldTree}.
+   *
+   * @param javacTree tree to process
+   * @param javaParserNode corresponding Javaparser node
+   */
+  public abstract void processYield(Tree javacTree, YieldStmt javaParserNode);
+
   /**
    * Process a {@code CompoundAssignmentTree}.
    *

diff --git a/...ork/src/main/java/org/checkerframework/framework/ajava/JointVisitorWithDefaultAction.java b/...ork/src/main/java/org/checkerframework/framework/ajava/JointVisitorWithDefaultAction.java
@@ -37,6 +37,7 @@
 import com.github.javaparser.ast.expr.SimpleName;
 import com.github.javaparser.ast.expr.SingleMemberAnnotationExpr;
 import com.github.javaparser.ast.expr.SuperExpr;
+import com.github.javaparser.ast.expr.SwitchExpr;
 import com.github.javaparser.ast.expr.ThisExpr;
 import com.github.javaparser.ast.expr.UnaryExpr;
 import com.github.javaparser.ast.modules.ModuleDeclaration;
@@ -65,6 +66,7 @@
 import com.github.javaparser.ast.stmt.ThrowStmt;
 import com.github.javaparser.ast.stmt.TryStmt;
 import com.github.javaparser.ast.stmt.WhileStmt;
+import com.github.javaparser.ast.stmt.YieldStmt;
 import com.github.javaparser.ast.type.ArrayType;
 import com.github.javaparser.ast.type.ClassOrInterfaceType;
 import com.github.javaparser.ast.type.IntersectionType;
@@ -490,6 +492,11 @@ public void processSwitch(SwitchTree javacTree, SwitchStmt javaParserNode) {
     defaultJointAction(javacTree, javaParserNode);
   }
 
+  @Override
+  public void processSwitchExpression(Tree javacTree, SwitchExpr javaParserNode) {
+    defaultJointAction(javacTree, javaParserNode);
+  }
+
   @Override
   public void processSynchronized(SynchronizedTree javacTree, SynchronizedStmt javaParserNode) {
     defaultJointAction(javacTree, javaParserNode);
@@ -560,6 +567,11 @@ public void processWildcard(WildcardTree javacTree, WildcardType javaParserNode)
     defaultJointAction(javacTree, javaParserNode);
   }
 
+  @Override
+  public void processYield(Tree javacTree, YieldStmt javaParserNode) {
+    defaultJointAction(javacTree, javaParserNode);
+  }
+
   @Override
   public void processCompoundAssignment(
       CompoundAssignmentTree javacTree, AssignExpr javaParserNode) {

diff --git a/framework/src/main/java/org/checkerframework/framework/ajava/TreeScannerWithDefaults.java b/framework/src/main/java/org/checkerframework/framework/ajava/TreeScannerWithDefaults.java
@@ -75,6 +75,20 @@ public abstract class TreeScannerWithDefaults extends TreeScanner<Void, Void> {
    */
   public abstract void defaultAction(Tree tree);
 
+  @Override
+  public Void scan(Tree tree, Void unused) {
+    if (tree != null) {
+      if (tree.getKind().name().equals("SWITCH_EXPRESSION")) {
+        visitSwitchExpression17(tree, unused);
+        return null;
+      } else if (tree.getKind().name().equals("YIELD")) {
+        visitYield17(tree, unused);
+        return null;
+      }
+    }
+    return super.scan(tree, unused);
+  }
+
   @Override
   public Void visitAnnotatedType(AnnotatedTypeTree tree, Void p) {
     defaultAction(tree);
@@ -369,6 +383,18 @@ public Void visitSwitch(SwitchTree tree, Void p) {
     return super.visitSwitch(tree, p);
   }
 
+  /**
+   * Visit a switch expression tree.
+   *
+   * @param tree switch expression tree
+   * @param p null
+   * @return null
+   */
+  public Void visitSwitchExpression17(Tree tree, Void p) {
+    defaultAction(tree);
+    return super.scan(tree, p);
+  }
+
   @Override
   public Void visitSynchronized(SynchronizedTree tree, Void p) {
     defaultAction(tree);
@@ -434,4 +460,16 @@ public Void visitWildcard(WildcardTree tree, Void p) {
     defaultAction(tree);
     return super.visitWildcard(tree, p);
   }
+
+  /**
+   * Visit a yield tree.
+   *
+   * @param tree a yield tree
+   * @param p null
+   * @return null
+   */
+  public Void visitYield17(Tree tree, Void p) {
+    defaultAction(tree);
+    return super.scan(tree, p);
+  }
 }