C#: Simplify extraction of is expressions and case statements

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/IsPattern.cs

@@ -18,13 +18,20 @@ public static Expression CreatePattern(this Context cx, PatternSyntax syntax, IE
                 case DeclarationPatternSyntax declPattern:
                     // Creates a single local variable declaration.
                     {
-                        if (declPattern.Designation is VariableDesignationSyntax designation && cx.Model(syntax).GetDeclaredSymbol(designation) is ILocalSymbol symbol)
+                        if (declPattern.Designation is VariableDesignationSyntax designation)
                         {
-                            var type = Type.Create(cx, symbol.Type);
-
-                            return VariableDeclaration.Create(cx, symbol, type, cx.Create(syntax.GetLocation()), cx.Create(designation.GetLocation()), false, parent, child);
+                            if (cx.Model(syntax).GetDeclaredSymbol(designation) is ILocalSymbol symbol)
+                            {
+                                var type = Type.Create(cx, symbol.Type);
+                                return VariableDeclaration.Create(cx, symbol, type, declPattern.Type, cx.Create(syntax.GetLocation()), cx.Create(designation.GetLocation()), false, parent, child);
+                            }
+                            if (designation is DiscardDesignationSyntax)
+                            {
+                                return Expressions.TypeAccess.Create(cx, declPattern.Type, parent, child);
+                            }
+                            throw new InternalError(designation, "Designation pattern not handled");
                         }
-                        throw new InternalError(syntax, "Is pattern not handled");
+                        throw new InternalError(declPattern, "Declaration pattern not handled");
                     }
 
                 case RecursivePatternSyntax recPattern:
@@ -40,7 +47,7 @@ public static Expression CreatePattern(this Context cx, PatternSyntax syntax, IE
                             {
                                 var type = Type.Create(cx, symbol.Type);
 
-                                return VariableDeclaration.Create(cx, symbol, type, cx.Create(syntax.GetLocation()), cx.Create(varDesignation.GetLocation()), false, parent, child);
+                                return VariableDeclaration.Create(cx, symbol, type, null, cx.Create(syntax.GetLocation()), cx.Create(varDesignation.GetLocation()), false, parent, child);
                             }
                             else
                             {
@@ -54,7 +61,7 @@ public static Expression CreatePattern(this Context cx, PatternSyntax syntax, IE
                     return new Discard(cx, dp, parent, child);
 
                 default:
-                    throw new InternalError(syntax, "Is pattern not handled");
+                    throw new InternalError(syntax, "Pattern not handled");
             }
         }
     }
@@ -108,7 +115,7 @@ public RecursivePattern(Context cx, RecursivePatternSyntax syntax, IExpressionPa
             {
                 var type = Type.Create(cx, symbol.Type);
 
-                VariableDeclaration.Create(cx, symbol, type, cx.Create(syntax.GetLocation()), cx.Create(designation.GetLocation()), false, this, 0);
+                VariableDeclaration.Create(cx, symbol, type, null, cx.Create(syntax.GetLocation()), cx.Create(designation.GetLocation()), false, this, 0);
             }
 
             if (syntax.PositionalPatternClause is PositionalPatternClauseSyntax posPc)
@@ -131,19 +138,14 @@ private IsPattern(ExpressionNodeInfo info) : base(info.SetKind(ExprKind.IS))
 
         private void PopulatePattern(PatternSyntax pattern, TypeSyntax optionalType, SyntaxToken varKeyword, VariableDesignationSyntax designation)
         {
-            bool isVar = optionalType is null;
-            if (!isVar)
-                Expressions.TypeAccess.Create(cx, optionalType, this, 1);
-
+            var isVar = optionalType is null;
             if (!(designation is null) && cx.Model(pattern).GetDeclaredSymbol(designation) is ILocalSymbol symbol)
             {
                 var type = Type.Create(cx, symbol.Type);
-
-                if (isVar)
-                    new Expression(new ExpressionInfo(cx, type, cx.Create(varKeyword.GetLocation()), ExprKind.TYPE_ACCESS, this, 1, false, null));
-
-                VariableDeclaration.Create(cx, symbol, type, cx.Create(pattern.GetLocation()), cx.Create(designation.GetLocation()), isVar, this, 2);
+                VariableDeclaration.Create(cx, symbol, type, optionalType, cx.Create(pattern.GetLocation()), cx.Create(designation.GetLocation()), isVar, this, 1);
             }
+            else if (!isVar)
+                Expressions.TypeAccess.Create(cx, optionalType, this, 1);
         }
 
         protected override void Populate()
@@ -152,7 +154,7 @@ protected override void Populate()
             switch (Syntax.Pattern)
             {
                 case ConstantPatternSyntax constantPattern:
-                    Create(cx, constantPattern.Expression, this, 3);
+                    Create(cx, constantPattern.Expression, this, 1);
                     return;
                 case VarPatternSyntax varPattern:
                     PopulatePattern(varPattern, null, varPattern.VarKeyword, varPattern.Designation);
@@ -161,7 +163,7 @@ protected override void Populate()
                     PopulatePattern(declPattern, declPattern.Type, default(SyntaxToken), declPattern.Designation);
                     return;
                 case RecursivePatternSyntax recPattern:
-                    new RecursivePattern(cx, recPattern, this, 3);
+                    new RecursivePattern(cx, recPattern, this, 1);
                     return;
                 default:
                     throw new InternalError(Syntax, "Is pattern not handled");

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Query.cs

@@ -66,19 +66,21 @@ protected Expression DeclareRangeVariable(Context cx, IExpressionParentEntity pa
                 Extraction.Entities.Location nameLoc;
 
                 Type declType;
+                TypeSyntax declTypeSyntax = null;
                 if (getElement)
                 {
                     var from = node as FromClauseSyntax;
-                    declType = from != null && from.Type != null
-                         ? Type.Create(cx, cx.GetType(from.Type))
-                         : type.ElementType;
+                    (declType, declTypeSyntax) = from != null && from.Type != null
+                         ? (Type.Create(cx, cx.GetType(from.Type)), from.Type)
+                         : (type.ElementType, null);
                 }
                 else
                     declType = type;
 
                 var decl = VariableDeclaration.Create(cx,
                     variableSymbol,
                     declType,
+                    declTypeSyntax,
                     cx.Create(node.GetLocation()),
                     nameLoc = cx.Create(name.GetLocation()),
                     true,

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Switch.cs

@@ -18,7 +18,7 @@ protected override void Populate()
         {
             SwitchedExpr = Expression.Create(cx, Syntax.GoverningExpression, this, -1);
             int child = 0;
-            foreach(var arm in Syntax.Arms)
+            foreach (var arm in Syntax.Arms)
             {
                 new SwitchCase(cx, arm, this, child++);
             }

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/VariableDeclaration.cs

@@ -11,10 +11,15 @@ class VariableDeclaration : Expression
     {
         VariableDeclaration(IExpressionInfo info) : base(info) { }
 
-        public static VariableDeclaration Create(Context cx, ISymbol symbol, Type type, Extraction.Entities.Location exprLocation, Extraction.Entities.Location declLocation, bool isVar, IExpressionParentEntity parent, int child)
+        public static VariableDeclaration Create(Context cx, ISymbol symbol, Type type, TypeSyntax optionalSyntax, Extraction.Entities.Location exprLocation, Extraction.Entities.Location declLocation, bool isVar, IExpressionParentEntity parent, int child)
         {
             var ret = new VariableDeclaration(new ExpressionInfo(cx, type, exprLocation, ExprKind.LOCAL_VAR_DECL, parent, child, false, null));
-            cx.Try(null, null, () => LocalVariable.Create(cx, symbol, ret, isVar, declLocation));
+            cx.Try(null, null, () =>
+            {
+                LocalVariable.Create(cx, symbol, ret, isVar, declLocation);
+                if (optionalSyntax != null)
+                    TypeMention.Create(cx, optionalSyntax, parent, type);
+            });
             return ret;
         }
 
@@ -65,7 +70,7 @@ public static Expression CreateParenthesized(Context cx, VarPatternSyntax varPat
             {
                 var child0 = 0;
                 foreach (var variable in designation.Variables)
-                    switch(variable)
+                    switch (variable)
                     {
                         case ParenthesizedVariableDesignationSyntax paren:
                             CreateParenthesized(cx, varPattern, paren, tuple, child0++);
@@ -97,7 +102,7 @@ public static Expression CreateParenthesized(Context cx, VarPatternSyntax varPat
 
         static Expression Create(Context cx, DeclarationExpressionSyntax node, VariableDesignationSyntax designation, IExpressionParentEntity parent, int child)
         {
-            switch(designation)
+            switch (designation)
             {
                 case SingleVariableDesignationSyntax single:
                     return CreateSingle(cx, node, single, parent, child);

csharp/extractor/Semmle.Extraction.CSharp/Entities/Statements/Case.cs

@@ -67,27 +67,23 @@ class CasePattern : Case<CasePatternSwitchLabelSyntax>
         private CasePattern(Context cx, CasePatternSwitchLabelSyntax node, Switch parent, int child)
             : base(cx, node, parent, child) { }
 
-        private void PopulatePattern(PatternSyntax pattern, TypeSyntax optionalType, SyntaxToken varKeyword, VariableDesignationSyntax designation)
+        private void PopulatePattern(PatternSyntax pattern, TypeSyntax optionalType, VariableDesignationSyntax designation)
         {
             var isVar = optionalType is null;
-            if (!isVar)
-                Expressions.TypeAccess.Create(cx, optionalType, this, 1);
-
             switch (designation)
             {
                 case SingleVariableDesignationSyntax _:
                     if (cx.Model(pattern).GetDeclaredSymbol(designation) is ILocalSymbol symbol)
                     {
                         var type = Type.Create(cx, symbol.Type);
-
-                        if (isVar)
-                            new Expression(new ExpressionInfo(cx, type, cx.Create(varKeyword.GetLocation()), ExprKind.TYPE_ACCESS, this, 1, false, null));
-
-                        Expressions.VariableDeclaration.Create(cx, symbol, type, cx.Create(pattern.GetLocation()), cx.Create(designation.GetLocation()), isVar, this, 0);
+                        Expressions.VariableDeclaration.Create(cx, symbol, type, optionalType, cx.Create(pattern.GetLocation()), cx.Create(designation.GetLocation()), isVar, this, 0);
                     }
                     break;
                 case DiscardDesignationSyntax discard:
-                    new Expressions.Discard(cx, discard, this, 0);
+                    if (isVar)
+                        new Expressions.Discard(cx, discard, this, 0);
+                    else
+                        Expressions.TypeAccess.Create(cx, optionalType, this, 0);
                     break;
                 case null:
                     break;
@@ -104,10 +100,10 @@ protected override void Populate()
             switch (Stmt.Pattern)
             {
                 case VarPatternSyntax varPattern:
-                    PopulatePattern(varPattern, null, varPattern.VarKeyword, varPattern.Designation);
+                    PopulatePattern(varPattern, null, varPattern.Designation);
                     break;
                 case DeclarationPatternSyntax declarationPattern:
-                    PopulatePattern(declarationPattern, declarationPattern.Type, default(SyntaxToken), declarationPattern.Designation);
+                    PopulatePattern(declarationPattern, declarationPattern.Type, declarationPattern.Designation);
                     break;
                 case ConstantPatternSyntax pattern:
                     Expression.Create(cx, pattern.Expression, this, 0);
@@ -121,7 +117,7 @@ protected override void Populate()
 
             if (Stmt.WhenClause != null)
             {
-                Expression.Create(cx, Stmt.WhenClause.Condition, this, 2);
+                Expression.Create(cx, Stmt.WhenClause.Condition, this, 1);
             }
         }
 

csharp/extractor/Semmle.Extraction.CSharp/Entities/Statements/ForEach.cs

@@ -27,8 +27,9 @@ protected override void Populate()
             var location = cx.Create(Stmt.Identifier.GetLocation());
 
             if (typeSymbol.Name != "_")
-                Expressions.VariableDeclaration.Create(cx, typeSymbol, type, location, location, Stmt.Type.IsVar, this, 0);
-            TypeMention.Create(cx, Stmt.Type, this, type);
+                Expressions.VariableDeclaration.Create(cx, typeSymbol, type, Stmt.Type, location, location, Stmt.Type.IsVar, this, 0);
+            else
+                TypeMention.Create(cx, Stmt.Type, this, type);
 
             Statement.Create(cx, Stmt.Statement, this, 2);
         }

csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql

@@ -72,7 +72,8 @@ class ConstantNullnessCondition extends ConstantCondition {
   ConstantNullnessCondition() {
     forex(ControlFlow::Node cfn | cfn = this.getAControlFlowNode() |
       exists(ControlFlow::SuccessorTypes::NullnessSuccessor t, ControlFlow::Node s |
-        s = cfn.getASuccessorByType(t) |
+        s = cfn.getASuccessorByType(t)
+      |
         b = t.getValue() and
         not s.isJoin()
       ) and

csharp/ql/src/semmle/code/csharp/Assignable.qll

@@ -306,9 +306,8 @@ module AssignableInternal {
   /** A local variable declaration at the top-level of a pattern. */
   class TopLevelPatternDecl extends LocalVariableDeclExpr {
     private PatternMatch pm;
-    TopLevelPatternDecl() {
-      this = pm.getPattern().(BindingPatternExpr).getVariableDeclExpr()
-    }
+
+    TopLevelPatternDecl() { this = pm.getPattern().(BindingPatternExpr).getVariableDeclExpr() }
 
     PatternMatch getMatch() { result = pm }
   }

csharp/ql/src/semmle/code/csharp/ExprOrStmtParent.qll

@@ -19,17 +19,8 @@ predicate expr_parent_top_level_adjusted(Expr child, int i, @top_level_exprorstm
 }
 
 /**
- * Holds if `case` statement `cs` is a type check of the form `case int _`,
- * where `ta` is the type access `int`.
- */
-private predicate discardTypeCaseStmt(CaseStmt cs, TypeAccess ta) {
-  expr_parent(ta, 1, cs) and
-  expr_parent(any(DiscardExpr de), 0, cs)
-}
-
-/**
- * The `expr_parent()` relation adjusted for expandable assignments, `is` expressions,
- * and `case` statements. For example, the assignment `x += y` is extracted as
+ * The `expr_parent()` relation adjusted for expandable assignments. For example,
+ * the assignment `x += y` is extracted as
  *
  * ```
  *          +=
@@ -78,35 +69,7 @@ private predicate expr_parent_adjusted(Expr child, int i, ControlFlowElement par
         not ao.hasExpandedAssignment() and
         expr_parent(child, i, parent)
       )
-  else
-    if parent instanceof IsExpr
-    then
-      i = 0 and
-      expr_parent(child, i, parent)
-      or
-      // e.g. `x is string s` or `x is null`
-      i = 1 and
-      expr_parent(child, any(int j | j in [2 .. 3]), parent)
-      or
-      // e.g. `x is string`
-      i = 1 and
-      not expr_parent(_, any(int j | j in [2 .. 3]), parent) and
-      expr_parent(child, i, parent)
-    else
-      if parent instanceof CaseStmt
-      then
-        // e.g. `case string s:` or `case 5:`
-        i = 0 and
-        expr_parent(child, i, parent) and
-        not discardTypeCaseStmt(parent, _)
-        or
-        // e.g. `case string _`
-        i = 0 and
-        discardTypeCaseStmt(parent, child)
-        or
-        i = 1 and
-        expr_parent(child, 2, parent)
-      else expr_parent(child, i, parent)
+  else expr_parent(child, i, parent)
 }
 
 /**

csharp/ql/src/semmle/code/csharp/controlflow/internal/Completion.qll

@@ -417,9 +417,7 @@ predicate switchMatching(Switch s, Case c, PatternExpr pe) {
   pe = c.getPattern()
 }
 
-private predicate mustHaveMatchingCompletion(Switch s, PatternExpr pe) {
-  switchMatching(s, _, pe)
-}
+private predicate mustHaveMatchingCompletion(Switch s, PatternExpr pe) { switchMatching(s, _, pe) }
 
 /**
  * Holds if a normal completion of `cfe` must be a matching completion. Thats is,

csharp/ql/src/semmle/code/csharp/exprs/Expr.qll

@@ -313,6 +313,7 @@ class ConstantPatternExpr extends PatternExpr {
  */
 class TypePatternExpr extends PatternExpr {
   private Type t;
+
   TypePatternExpr() {
     t = this.(TypeAccess).getTarget() or
     t = this.(LocalVariableDeclExpr).getVariable().getType()
@@ -517,7 +518,7 @@ class Case extends PatternMatch, @case {
   /**
    * Gets the `when` expression of this case, if any. For example, `s.Length < 10`
    * in `string s when s.Length < 10 => s`
-    */
+   */
   Expr getCondition() { none() }
 
   /** Gets the body of this `case`. */

csharp/ql/test/library-tests/controlflow/graph/EntryElement.expected

@@ -1241,11 +1241,9 @@
 | Switch.cs:46:17:46:17 | access to parameter o | Switch.cs:46:17:46:17 | access to parameter o |
 | Switch.cs:48:13:48:23 | case ...: | Switch.cs:48:13:48:23 | case ...: |
 | Switch.cs:48:18:48:20 | access to type Int32 | Switch.cs:48:18:48:20 | access to type Int32 |
-| Switch.cs:48:22:48:22 | _ | Switch.cs:48:22:48:22 | _ |
 | Switch.cs:49:17:49:22 | break; | Switch.cs:49:17:49:22 | break; |
 | Switch.cs:50:13:50:39 | case ...: | Switch.cs:50:13:50:39 | case ...: |
 | Switch.cs:50:18:50:21 | access to type Boolean | Switch.cs:50:18:50:21 | access to type Boolean |
-| Switch.cs:50:23:50:23 | _ | Switch.cs:50:23:50:23 | _ |
 | Switch.cs:50:30:50:30 | access to parameter o | Switch.cs:50:30:50:30 | access to parameter o |
 | Switch.cs:50:30:50:38 | ... != ... | Switch.cs:50:30:50:30 | access to parameter o |
 | Switch.cs:50:35:50:38 | null | Switch.cs:50:35:50:38 | null |
@@ -1267,7 +1265,6 @@
 | Switch.cs:68:25:68:25 | access to parameter s | Switch.cs:68:25:68:25 | access to parameter s |
 | Switch.cs:70:13:70:24 | case ...: | Switch.cs:70:13:70:24 | case ...: |
 | Switch.cs:70:18:70:20 | access to type Int32 | Switch.cs:70:18:70:20 | access to type Int32 |
-| Switch.cs:70:22:70:22 | _ | Switch.cs:70:22:70:22 | _ |
 | Switch.cs:71:15:71:20 | break; | Switch.cs:71:15:71:20 | break; |
 | Switch.cs:72:13:72:21 | case ...: | Switch.cs:72:13:72:21 | case ...: |
 | Switch.cs:72:18:72:19 | "" | Switch.cs:72:18:72:19 | "" |
@@ -1295,7 +1292,6 @@
 | Switch.cs:93:17:93:17 | access to parameter o | Switch.cs:93:17:93:17 | access to parameter o |
 | Switch.cs:95:13:95:24 | case ...: | Switch.cs:95:13:95:24 | case ...: |
 | Switch.cs:95:18:95:20 | access to type Int32 | Switch.cs:95:18:95:20 | access to type Int32 |
-| Switch.cs:95:22:95:22 | _ | Switch.cs:95:22:95:22 | _ |
 | Switch.cs:96:15:96:26 | return ...; | Switch.cs:96:22:96:25 | true |
 | Switch.cs:96:22:96:25 | true | Switch.cs:96:22:96:25 | true |
 | Switch.cs:98:9:98:21 | return ...; | Switch.cs:98:16:98:20 | false |