Handle 'case null, default:'

java/ql/examples/snippets/switchcase.ql

@@ -14,5 +14,5 @@ where
   switch.getExpr().getType() = enum and
   missing.getDeclaringType() = enum and
   not switch.getAConstCase().getValue() = missing.getAnAccess() and
-  not exists(switch.getDefaultCase())
+  not exists(switch.getDefaultOrNullDefaultCase())
 select switch

java/ql/lib/config/semmlecode.dbscheme

@@ -992,6 +992,10 @@ providesWith(
   string serviceImpl: string ref
 );
 
+isNullDefaultCase(
+  int id: @case ref
+);
+
 /*
  * Javadoc
  */

java/ql/lib/semmle/code/java/Expr.qll

@@ -1514,7 +1514,8 @@ class SwitchExpr extends Expr, StmtParent, @switchexpr {
    * which may be either a normal `case` or a `default`.
    */
   SwitchCase getCase(int i) {
-    result = rank[i + 1](SwitchCase case, int idx | case.isNthChildOf(this, idx) | case order by idx)
+    result =
+      rank[i + 1](SwitchCase case, int idx | case.isNthChildOf(this, idx) | case order by idx)
   }
 
   /**
@@ -1532,6 +1533,9 @@ class SwitchExpr extends Expr, StmtParent, @switchexpr {
   /** Gets the `default` case of this switch expression, if any. */
   DefaultCase getDefaultCase() { result = this.getACase() }
 
+  /** Gets the `default` or `case null, default` case of this switch statement, if any. */
+  SwitchCase getDefaultOrNullDefaultCase() { result = this.getACase() and result.hasDefaultLabel() }
+
   /** Gets the expression of this `switch` expression. */
   Expr getExpr() { result.getParent() = this }
 
@@ -1543,9 +1547,7 @@ class SwitchExpr extends Expr, StmtParent, @switchexpr {
   }
 
   /** Holds if this switch has a case handling a null literal. */
-  predicate hasNullCase() {
+  predicate hasNullCase() { this.getAConstCase().getValue(_) instanceof NullLiteral }
-    this.getAConstCase().getValue(_) instanceof NullLiteral
-  }
 
   /** Gets a printable representation of this expression. */
   override string toString() { result = "switch (...)" }
@@ -1638,19 +1640,13 @@ class LocalVariableDeclExpr extends Expr, @localvariabledeclexpr {
   string getName() { result = this.getVariable().getName() }
 
   /** Gets the switch statement or expression whose pattern declares this identifier, if any. */
-  StmtParent getAssociatedSwitch() {
+  StmtParent getAssociatedSwitch() { result = this.getParent().(PatternCase).getParent() }
-    result = this.getParent().(PatternCase).getParent()
-  }
 
   /** Holds if this is a declaration stemming from a pattern switch case. */
-  predicate hasAssociatedSwitch() {
+  predicate hasAssociatedSwitch() { exists(this.getAssociatedSwitch()) }
-    exists(this.getAssociatedSwitch())
-  }
 
   /** Gets the initializer expression of this local variable declaration expression, if any. */
-  Expr getInit() {
+  Expr getInit() { result.isNthChildOf(this, 0) }
-    result.isNthChildOf(this, 0)
-  }
 
   /** Holds if this variable declaration implicitly initializes the variable. */
   predicate hasImplicitInit() {

java/ql/lib/semmle/code/java/PrettyPrintAst.qll

@@ -746,7 +746,9 @@ private class PpSwitchCase extends PpAst, SwitchCase {
   override string getPart(int i) {
     i = 0 and result = "default" and this instanceof DefaultCase
     or
-    i = 0 and result = "case " and this instanceof ConstCase
+    i = 0 and result = "case " and not this instanceof DefaultCase
+    or
+    i = this.lastConstCaseValueIndex() and result = "default" and this instanceof NullDefaultCase
     or
     exists(int j | i = 2 * j and j != 0 and result = ", " and exists(this.(ConstCase).getValue(j)))
     or
@@ -757,8 +759,13 @@ private class PpSwitchCase extends PpAst, SwitchCase {
     i = 3 + this.lastConstCaseValueIndex() and result = ";" and exists(this.getRuleExpression())
   }
 
+  private int getCaseDefaultOffset() {
+    if this instanceof NullDefaultCase then result = 1 else result = 0
+  }
+
   private int lastConstCaseValueIndex() {
-    result = 1 + 2 * max(int j | j = 0 or exists(this.(ConstCase).getValue(j)))
+    result =
+      1 + 2 * (max(int j | j = 0 or exists(this.(ConstCase).getValue(j))) + this.getCaseDefaultOffset())
   }
 
   override PpAst getChild(int i) {

java/ql/lib/semmle/code/java/Statement.qll

@@ -406,6 +406,9 @@ class SwitchStmt extends Stmt, @switchstmt {
   /** Gets the `default` case of this switch statement, if any. */
   DefaultCase getDefaultCase() { result = this.getACase() }
 
+  /** Gets the `default` or `case null, default` case of this switch statement, if any. */
+  SwitchCase getDefaultOrNullDefaultCase() { result = this.getACase() and result.hasDefaultLabel() }
+
   /** Gets the expression of this `switch` statement. */
   Expr getExpr() { result.getParent() = this }
 
@@ -487,14 +490,28 @@ class SwitchCase extends Stmt, @case {
   Stmt getRuleStatementOrExpressionStatement() {
     result.getParent() = this and result.getIndex() = -1
   }
+
+  /**
+   * Holds if this case statement includes the default label, i.e. it is either `default`
+   * or `case null, default`.
+   */
+  predicate hasDefaultLabel() { this instanceof DefaultCase or this instanceof NullDefaultCase }
 }
 
-/** A constant `case` of a switch statement. */
+/**
+ * A constant `case` of a switch statement.
+ *
+ * Note this excludes `case null, default` even though that includes a null constant. It
+ * does however include plain `case null`.
+ */
 class ConstCase extends SwitchCase {
   ConstCase() {
     exists(Expr e |
       e.getParent() = this and e.getIndex() >= 0 and not e instanceof LocalVariableDeclExpr
     )
+    // For backward compatibility, we don't include `case null, default:` here, on the assumption
+    // this will come as a surprise to CodeQL that predates that statement's validity.
+    and not isNullDefaultCase(this)
   }
 
   /** Gets the `case` constant at index 0. */
@@ -535,7 +552,11 @@ class PatternCase extends SwitchCase {
   override string getAPrimaryQlClass() { result = "PatternCase" }
 }
 
-/** A `default` case of a `switch` statement */
+/**
+ * A `default` case of a `switch` statement.
+ *
+ * Note this does not include `case null, default` -- for that, see `NullDefaultCase`.
+ */
 class DefaultCase extends SwitchCase {
   DefaultCase() { not exists(Expr e | e.getParent() = this | e.getIndex() >= 0) }
 
@@ -548,6 +569,19 @@ class DefaultCase extends SwitchCase {
   override string getAPrimaryQlClass() { result = "DefaultCase" }
 }
 
+/** A `case null, default` statement of a `switch` statement or expression. */
+class NullDefaultCase extends SwitchCase {
+  NullDefaultCase() { isNullDefaultCase(this) }
+
+  override string pp() { result = "case null, default" }
+
+  override string toString() { result = "case null, default" }
+
+  override string getHalsteadID() { result = "NullDefaultCase" }
+
+  override string getAPrimaryQlClass() { result = "NullDefaultCase" }
+}
+
 /** A `synchronized` statement. */
 class SynchronizedStmt extends Stmt, @synchronizedstmt {
   /** Gets the expression on which this `synchronized` statement synchronizes. */

java/ql/lib/semmle/code/java/controlflow/UnreachableBlocks.qll

@@ -170,10 +170,11 @@ class ConstSwitchStmt extends SwitchStmt {
   /** Gets the matching case, if it can be deduced. */
   SwitchCase getMatchingCase() {
     // Must be a value we can deduce
+    // TODO: handle other known constants (enum constants, String constants)
     exists(this.getExpr().(ConstantExpr).getIntValue()) and
     if exists(this.getMatchingConstCase())
     then result = this.getMatchingConstCase()
-    else result = this.getDefaultCase()
+    else result = this.getDefaultOrNullDefaultCase()
   }
 
   /**

java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll

@@ -55,9 +55,13 @@ predicate implies_v1(Guard g1, boolean b1, Guard g2, boolean b2) {
     )
   )
   or
-  g1.(DefaultCase).getSwitch().getAConstCase() = g2 and b1 = true and b2 = false
+  exists(SwitchCase sc | g1 = sc and sc.hasDefaultLabel() |
+    sc.getSwitch().getAConstCase() = g2 and b1 = true and b2 = false
+  )
   or
-  g1.(DefaultCase).getSwitchExpr().getAConstCase() = g2 and b1 = true and b2 = false
+  exists(SwitchCase sc | g1 = sc and sc.hasDefaultLabel() |
+    sc.getSwitchExpr().getAConstCase() = g2 and b1 = true and b2 = false
+  )
   or
   exists(MethodCall check, int argIndex | check = g1 |
     conditionCheckArgument(check, argIndex, _) and

java/ql/src/Advisory/Statements/MissingDefaultInSwitch.ql

@@ -15,5 +15,5 @@ import java
 from SwitchStmt switch
 where
   not switch.getExpr().getType() instanceof EnumType and
-  not exists(switch.getDefaultCase())
+  not exists(switch.getDefaultOrNullDefaultCase())
 select switch, "Switch statement does not have a default case."

java/ql/src/Likely Bugs/Statements/MissingEnumInSwitch.ql

@@ -17,6 +17,6 @@ from SwitchStmt switch, EnumType enum, EnumConstant missing
 where
   switch.getExpr().getType() = enum and
   missing.getDeclaringType() = enum and
-  not exists(switch.getDefaultCase()) and
+  not exists(switch.getDefaultOrNullDefaultCase()) and
   not switch.getAConstCase().getValue() = missing.getAnAccess()
 select switch, "Switch statement does not have a case for $@.", missing, missing.getName()

java/ql/test/library-tests/pattern-switch/cfg/Test.java

@@ -52,6 +52,24 @@ public class Test {
        default -> { }
      }
 
+     switch((String)thing) {
+       case "Const1":
+         System.out.println("It's Const1!");
+       case "Const2":
+         System.out.println("It's Const1 or Const2!");
+         break;
+       case String s when s.length() <= 6:
+         System.out.println("It's <= 6 chars long, and neither Const1 nor Const2");
+       case "Const3":
+         System.out.println("It's (<= 6 chars long, and neither Const1 nor Const2), or Const3");
+         break;
+       case "Const30":
+         System.out.println("It's Const30");
+         break;
+       case null, default:
+         System.out.println("It's null, or something else");
+     }
+
   }
 
 }

java/ql/test/library-tests/printAst/A.java

@@ -88,6 +88,10 @@ class A {
                    case String s when s.length() == 5 -> "it's 5 letters long";
                    default -> "It's something else";
                };
+               var nullDefaultTest = switch(thing) {
+                   case String s -> "It's a string";
+                   case null, default -> "It's something else";
+               };
            }
        }
        catch (RuntimeException rte) {

java/ql/test/library-tests/printAst/PrintAst.expected

@@ -246,36 +246,48 @@ A.java:
 #   88|                           1: [LocalVariableDeclExpr] s
 #   89|                       3: [DefaultCase] default
 #   89|                         -1: [StringLiteral] "It's something else"
-#   93|           0: [CatchClause] catch (...)
+#   91|                 8: [LocalVariableDeclStmt] var ...;
+#   91|                   1: [LocalVariableDeclExpr] nullDefaultTest
+#   91|                     0: [SwitchExpr] switch (...)
+#   91|                       -1: [VarAccess] thing
+#   92|                       0: [PatternCase] case T t ...
+#   92|                         -1: [StringLiteral] "It's a string"
+#-----|                         0: (Single Local Variable Declaration)
+#   92|                           0: [TypeAccess] String
+#   92|                           1: [LocalVariableDeclExpr] s
+#   93|                       1: [NullDefaultCase] case null, default
+#   93|                         -1: [StringLiteral] "It's something else"
+#   93|                         0: [NullLiteral] null
+#   97|           0: [CatchClause] catch (...)
 #-----|             0: (Single Local Variable Declaration)
-#   93|               0: [TypeAccess] RuntimeException
+#   97|               0: [TypeAccess] RuntimeException
-#   93|               1: [LocalVariableDeclExpr] rte
+#   97|               1: [LocalVariableDeclExpr] rte
-#   93|             1: [BlockStmt] { ... }
+#   97|             1: [BlockStmt] { ... }
-#   94|               0: [ReturnStmt] return ...
+#   98|               0: [ReturnStmt] return ...
-#   98|     10: [Class] E
+#  102|     10: [Class] E
-#  102|       3: [FieldDeclaration] E A;
+#  106|       3: [FieldDeclaration] E A;
 #-----|         -3: (Javadoc)
-#   99|           1: [Javadoc] /** Javadoc for enum constant */
+#  103|           1: [Javadoc] /** Javadoc for enum constant */
-#  100|             0: [JavadocText] Javadoc for enum constant
+#  104|             0: [JavadocText] Javadoc for enum constant
-#  102|         -1: [TypeAccess] E
+#  106|         -1: [TypeAccess] E
-#  102|         0: [ClassInstanceExpr] new E(...)
+#  106|         0: [ClassInstanceExpr] new E(...)
-#  102|           -3: [TypeAccess] E
+#  106|           -3: [TypeAccess] E
-#  103|       4: [FieldDeclaration] E B;
+#  107|       4: [FieldDeclaration] E B;
 #-----|         -3: (Javadoc)
-#   99|           1: [Javadoc] /** Javadoc for enum constant */
+#  103|           1: [Javadoc] /** Javadoc for enum constant */
-#  100|             0: [JavadocText] Javadoc for enum constant
+#  104|             0: [JavadocText] Javadoc for enum constant
-#  103|         -1: [TypeAccess] E
+#  107|         -1: [TypeAccess] E
-#  103|         0: [ClassInstanceExpr] new E(...)
+#  107|         0: [ClassInstanceExpr] new E(...)
-#  103|           -3: [TypeAccess] E
+#  107|           -3: [TypeAccess] E
-#  104|       5: [FieldDeclaration] E C;
+#  108|       5: [FieldDeclaration] E C;
 #-----|         -3: (Javadoc)
-#   99|           1: [Javadoc] /** Javadoc for enum constant */
+#  103|           1: [Javadoc] /** Javadoc for enum constant */
-#  100|             0: [JavadocText] Javadoc for enum constant
+#  104|             0: [JavadocText] Javadoc for enum constant
-#  104|         -1: [TypeAccess] E
+#  108|         -1: [TypeAccess] E
-#  104|         0: [ClassInstanceExpr] new E(...)
+#  108|         0: [ClassInstanceExpr] new E(...)
-#  104|           -3: [TypeAccess] E
+#  108|           -3: [TypeAccess] E
-#  110|     11: [FieldDeclaration] int i, ...;
+#  114|     11: [FieldDeclaration] int i, ...;
 #-----|       -3: (Javadoc)
-#  107|         1: [Javadoc] /** Javadoc for fields */
+#  111|         1: [Javadoc] /** Javadoc for fields */
-#  108|           0: [JavadocText] Javadoc for fields
+#  112|           0: [JavadocText] Javadoc for fields
-#  110|       -1: [TypeAccess] int
+#  114|       -1: [TypeAccess] int