Merge pull request #226 from smowton/smowton/fix/remove-spurious-cfg-edge-from-expressionless-switch

Remove spurious control-flow edge around switch block without a test

Author: max-schaefer

ql/src/semmle/go/Stmt.qll

@@ -882,6 +882,9 @@ class TypeSwitchStmt extends @typeswitchstmt, SwitchStmt {
   /** Gets the assign statement of this type-switch statement. */
   SimpleAssignStmt getAssign() { result = getChildStmt(1) }
 
+  /** Gets the test statement of this type-switch statement. This is a `SimpleAssignStmt` or `ExprStmt`. */
+  Stmt getTest() { result = getChildStmt(1) }
+
   /** Gets the expression whose type is examined by this `switch` statement. */
   Expr getExpr() { result = getAssign().getRhs() or result = getChildStmt(1).(ExprStmt).getExpr() }
 

ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll

@@ -1761,7 +1761,7 @@ module CFG {
         or
         first = MkImplicitTrue(this)
         or
-        firstNode(this.(TypeSwitchStmt).getChildStmt(1), first)
+        firstNode(this.(TypeSwitchStmt).getTest(), first)
       )
     }
 
@@ -1772,17 +1772,18 @@ module CFG {
       (
         lastNode(this.(ExpressionSwitchStmt).getExpr(), last, cmpl)
         or
-        last = MkImplicitTrue(this) and
-        cmpl = Bool(true)
-        or
-        lastNode(this.(TypeSwitchStmt).getChildStmt(1), last, cmpl)
+        lastNode(this.(TypeSwitchStmt).getTest(), last, cmpl)
       ) and
       (
         not cmpl.isNormal()
         or
         not exists(this.getDefault())
       )
       or
+      last = MkImplicitTrue(this) and
+      cmpl = Bool(true) and
+      this.getNumCase() = 0
+      or
       exists(CaseClause cc, int i, Completion inner |
         cc = this.getCase(i) and lastNode(cc, last, inner)
       |
@@ -1811,13 +1812,13 @@ module CFG {
       (
         firstNode(this.(ExpressionSwitchStmt).getExpr(), succ) or
         succ = MkImplicitTrue(this) or
-        firstNode(this.(TypeSwitchStmt).getChildStmt(1), succ)
+        firstNode(this.(TypeSwitchStmt).getTest(), succ)
       )
       or
       (
         lastNode(this.(ExpressionSwitchStmt).getExpr(), pred, normalCompletion()) or
         pred = MkImplicitTrue(this) or
-        lastNode(this.(TypeSwitchStmt).getChildStmt(1), pred, normalCompletion())
+        lastNode(this.(TypeSwitchStmt).getTest(), pred, normalCompletion())
       ) and
       (
         firstNode(getNonDefaultCase(0), succ)

ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected

@@ -1206,7 +1206,6 @@
 | tst.go:3:12:3:12 | argument corresponding to x | tst.go:3:12:3:12 | initialization of x |
 | tst.go:3:12:3:12 | initialization of x | tst.go:4:2:4:2 | true |
 | tst.go:4:2:4:2 | true | tst.go:5:7:5:7 | x |
-| tst.go:4:2:4:2 | true | tst.go:12:1:12:1 | exit |
 | tst.go:5:2:5:13 | skip | tst.go:12:1:12:1 | exit |
 | tst.go:5:7:5:7 | x | tst.go:5:11:5:12 | 23 |
 | tst.go:5:7:5:12 | ...<... | tst.go:5:7:5:12 | case ...<... |
@@ -1232,12 +1231,11 @@
 | tst.go:9:12:9:12 | ...<... is false | tst.go:12:1:12:1 | exit |
 | tst.go:9:12:9:12 | ...<... is true | tst.go:9:2:9:13 | skip |
 | tst.go:14:1:14:1 | entry | tst.go:14:13:14:17 | argument corresponding to value |
-| tst.go:14:1:21:1 | function declaration | tst.go:0:0:0:0 | exit |
+| tst.go:14:1:21:1 | function declaration | tst.go:23:6:23:11 | skip |
 | tst.go:14:6:14:11 | skip | tst.go:14:1:21:1 | function declaration |
 | tst.go:14:13:14:17 | argument corresponding to value | tst.go:14:13:14:17 | initialization of value |
 | tst.go:14:13:14:17 | initialization of value | tst.go:15:2:15:2 | true |
 | tst.go:15:2:15:2 | true | tst.go:16:7:16:11 | value |
-| tst.go:15:2:15:2 | true | tst.go:21:1:21:1 | exit |
 | tst.go:16:2:16:34 | skip | tst.go:21:1:21:1 | exit |
 | tst.go:16:7:16:11 | value | tst.go:16:15:16:33 | ...*... |
 | tst.go:16:7:16:33 | ...<... | tst.go:16:7:16:33 | case ...<... |
@@ -1254,3 +1252,12 @@
 | tst.go:18:15:18:38 | ...*... | tst.go:18:7:18:38 | ...<... |
 | tst.go:18:38:18:38 | ...<... is false | tst.go:21:1:21:1 | exit |
 | tst.go:18:38:18:38 | ...<... is true | tst.go:18:2:18:39 | skip |
+| tst.go:23:1:23:1 | entry | tst.go:24:2:24:2 | true |
+| tst.go:23:1:25:1 | function declaration | tst.go:27:6:27:11 | skip |
+| tst.go:23:6:23:11 | skip | tst.go:23:1:25:1 | function declaration |
+| tst.go:24:2:24:2 | true | tst.go:25:1:25:1 | exit |
+| tst.go:27:1:27:1 | entry | tst.go:28:2:28:2 | true |
+| tst.go:27:1:31:1 | function declaration | tst.go:0:0:0:0 | exit |
+| tst.go:27:6:27:11 | skip | tst.go:27:1:31:1 | function declaration |
+| tst.go:28:2:28:2 | true | tst.go:29:2:29:9 | skip |
+| tst.go:29:2:29:9 | skip | tst.go:31:1:31:1 | exit |

ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/tst.go

@@ -19,3 +19,13 @@ func check2(value int64) {
 
 	}
 }
+
+func check3() {
+	switch { }
+}
+
+func check4() {
+	switch {
+	default:
+	}
+}