C#: More C# 8 features

change-notes/1.21/analysis-csharp.md

@@ -13,13 +13,18 @@
 * The following C# 8 features are now extracted:
     - Range expressions
     - Recursive patterns
+    - Using declaration statements
+    - `static` modifiers on local functions
+    - Null-coalescing assignment expressions
+
 * The `unmanaged` type parameter constraint is now extracted.
 
 ## Changes to QL libraries
 
 * The class `Attribute` has two new predicates: `getConstructorArgument()` and `getNamedArgument()`. The first predicate returns arguments to the underlying constructor call and the latter returns named arguments for initializing fields and properties.
 * The class `TypeParameterConstraints` has a new predicate `hasUnmanagedTypeConstraint()`, indicating that the type parameter has the `unmanaged` constraint.
 * The following QL classes have been added to model C# 8 features:
+    - Class `AssignCoalesceExpr` models null-coalescing assignment, for example `x ??= y`
     - Class `IndexExpr` models from-end index expressions, for example `^1`
     - Class `PatternExpr` is an `Expr` that appears in a pattern. It has the new subclasses `DiscardPatternExpr`, `LabeledPatternExpr`, `RecursivePatternExpr`, `TypeAccessPatternExpr`, `TypePatternExpr`, and `VariablePatternExpr`.
     - Class `PatternMatch` models a pattern being matched. It has the subclasses `Case` and `IsExpr`.
@@ -31,5 +36,6 @@
     - Classes `IsConstantExpr`, `IsTypeExpr` and `IsPatternExpr` are deprecated in favour of `IsExpr`
     - Class `Switch` models both `SwitchExpr` and `SwitchStmt`
     - Class `Case` models both `CaseStmt` and `SwitchCaseExpr`
+    - Class `UsingStmt` models both `UsingBlockStmt` and `UsingDeclStmt`
 
 ## Changes to autobuilder

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

@@ -71,6 +71,8 @@ static ExprKind GetAssignmentOperation(Context cx, AssignmentExpressionSyntax sy
                     return ExprKind.ASSIGN_LSHIFT;
                 case SyntaxKind.GreaterThanGreaterThanEqualsToken:
                     return ExprKind.ASSIGN_RSHIFT;
+                case SyntaxKind.QuestionQuestionEqualsToken:
+                    return ExprKind.ASSIGN_COALESCE;
                 default:
                     cx.ModelError(syntax, "Unrecognised assignment type " + GetKind(cx, syntax));
                     return ExprKind.UNKNOWN;
@@ -142,6 +144,8 @@ static ExprKind GetKind(Context cx, AssignmentExpressionSyntax syntax)
                         return ExprKind.SUB;
                     case ExprKind.ASSIGN_XOR:
                         return ExprKind.BIT_XOR;
+                    case ExprKind.ASSIGN_COALESCE:
+                        return ExprKind.NULL_COALESCING;
                     default:
                         cx.ModelError(Syntax, "Couldn't unfold assignment of type " + kind);
                         return ExprKind.UNKNOWN;

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

@@ -80,6 +80,7 @@ internal static Expression Create(ExpressionNodeInfo info)
                     case SyntaxKind.RightShiftAssignmentExpression:
                     case SyntaxKind.DivideAssignmentExpression:
                     case SyntaxKind.ModuloAssignmentExpression:
+                    case SyntaxKind.CoalesceAssignmentExpression:
                         return Assignment.Create(info);
 
                     case SyntaxKind.ObjectCreationExpression:

csharp/extractor/Semmle.Extraction.CSharp/Entities/LocalFunction.cs

@@ -1,4 +1,6 @@
 using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using System.Linq;
 
 namespace Semmle.Extraction.CSharp.Entities
 {
@@ -39,6 +41,18 @@ class LocalFunctionFactory : ICachedEntityFactory<IMethodSymbol, LocalFunction>
         public override void Populate()
         {
             PopulateMethod();
+
+            // There is a "bug" in Roslyn whereby the IMethodSymbol associated with the local function symbol
+            // is always static, so we need to go to the syntax reference of the local function to see whether
+            // the "static" modifier is present.
+            if (symbol.DeclaringSyntaxReferences.SingleOrDefault().GetSyntax() is LocalFunctionStatementSyntax fn)
+            {
+                foreach(var modifier in fn.Modifiers)
+                {
+                    Modifier.HasModifier(Context, this, modifier.Text);
+                }
+            }
+
             var originalDefinition = IsSourceDeclaration ? this : Create(Context, symbol.OriginalDefinition);
             var returnType = Type.Create(Context, symbol.ReturnType);
             Context.Emit(Tuples.local_functions(this, symbol.Name, returnType, originalDefinition));

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

@@ -6,8 +6,19 @@ namespace Semmle.Extraction.CSharp.Entities.Statements
 {
     class LocalDeclaration : Statement<LocalDeclarationStatementSyntax>
     {
+        static StmtKind GetKind(LocalDeclarationStatementSyntax declStmt)
+        {
+            if (declStmt.UsingKeyword.RawKind != 0)
+                return StmtKind.USING_DECL;
+
+            if (declStmt.IsConst)
+                return StmtKind.CONST_DECL;
+
+            return StmtKind.VAR_DECL;
+        }
+
         LocalDeclaration(Context cx, LocalDeclarationStatementSyntax declStmt, IStatementParentEntity parent, int child)
-            : base(cx, declStmt, declStmt.IsConst ? StmtKind.CONST_DECL : StmtKind.VAR_DECL, parent, child) { }
+            : base(cx, declStmt, GetKind(declStmt), parent, child) { }
 
         public static LocalDeclaration Create(Context cx, LocalDeclarationStatementSyntax node, IStatementParentEntity parent, int child)
         {

csharp/extractor/Semmle.Extraction.CSharp/Kinds/ExprKind.cs

@@ -112,6 +112,7 @@ public enum ExprKind
         RECURSIVE_PATTERN = 115,
         PROPERTY_PATTERN = 116,
         POSITIONAL_PATTERN = 117,
-        SWITCH_CASE = 118
+        SWITCH_CASE = 118,
+        ASSIGN_COALESCE = 119 
     }
 }

csharp/extractor/Semmle.Extraction.CSharp/Kinds/StmtKind.cs

@@ -34,6 +34,7 @@ public enum StmtKind
         LABEL = 27,
         CATCH = 28,
         CASE = 29,
-        LOCAL_FUNCTION = 30
+        LOCAL_FUNCTION = 30,
+        USING_DECL = 31
     }
 }

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

@@ -903,7 +903,7 @@ class ExplicitConversionOperator extends ConversionOperator {
  * }
  * ```
  */
-class LocalFunction extends Callable, @local_function {
+class LocalFunction extends Callable, Modifiable, @local_function {
   override string getName() { local_functions(this, result, _, _) }
 
   override LocalFunction getSourceDeclaration() { local_functions(this, _, _, result) }

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

@@ -1115,40 +1115,16 @@ class LockStmt extends Stmt, @lock_stmt {
 }
 
 /**
- * A `using` statement, for example
- *
- * ```
- * using (FileStream f = File.Open("settings.xml")) {
- *   ...
- * }
- * ```
+ * A using block or declaration. Either a using declaration (`UsingDeclStmt`) or
+ * a using block (`UsingBlockStmt`).
  */
 class UsingStmt extends Stmt, @using_stmt {
-  /** Gets the `i`th local variable of this `using` statement. */
-  LocalVariable getVariable(int i) { result = this.getVariableDeclExpr(i).getVariable() }
-
-  /** Gets a local variable of this `using` statement. */
-  LocalVariable getAVariable() { result = this.getVariable(_) }
-
   /** Gets the `i`th local variable declaration of this `using` statement. */
-  LocalVariableDeclExpr getVariableDeclExpr(int i) { result = this.getChild(-i - 1) }
+  LocalVariableDeclExpr getVariableDeclExpr(int i) { none() }
 
   /** Gets a local variable declaration of this `using` statement. */
   LocalVariableDeclExpr getAVariableDeclExpr() { result = this.getVariableDeclExpr(_) }
 
-  /**
-   * Gets the expression directly used by this `using` statement, if any. For
-   * example, `f` on line 2 in
-   *
-   * ```
-   * var f = File.Open("settings.xml");
-   * using (f) {
-   *   ...
-   * }
-   * ```
-   */
-  Expr getExpr() { result = this.getChild(0) }
-
   /**
    * Gets an expression that is used in this `using` statement. Either an
    * expression assigned to a variable, for example `File.Open("settings.xml")`
@@ -1169,14 +1145,69 @@ class UsingStmt extends Stmt, @using_stmt {
    * }
    * ```
    */
-  Expr getAnExpr() {
+  Expr getAnExpr() { none() }
+
+  /**
+   * DEPRECATED: Use UsingBlockStmt.getExpr() instead.
+   * Gets the expression directly used by this `using` statement, if any. For
+   * example, `f` on line 2 in
+   *
+   * ```
+   * var f = File.Open("settings.xml");
+   * using (f) {
+   *   ...
+   * }
+   * ```
+   */
+  deprecated Expr getExpr() { none() }
+
+  /**
+   * DEPRECATED: Use UsingBlockStmt.getBody() instead.
+   * Gets the body of this `using` statement.
+   */
+  deprecated Stmt getBody() { none() }
+}
+
+/**
+ * A `using` block statement, for example
+ *
+ * ```
+ * using (FileStream f = File.Open("settings.xml")) {
+ *   ...
+ * }
+ * ```
+ */
+class UsingBlockStmt extends UsingStmt, @using_block_stmt {
+  /** Gets the `i`th local variable of this `using` statement. */
+  LocalVariable getVariable(int i) { result = this.getVariableDeclExpr(i).getVariable() }
+
+  /** Gets a local variable of this `using` statement. */
+  LocalVariable getAVariable() { result = this.getVariable(_) }
+
+  /** Gets the `i`th local variable declaration of this `using` statement. */
+  override LocalVariableDeclExpr getVariableDeclExpr(int i) { result = this.getChild(-i - 1) }
+
+  /**
+   * Gets the expression directly used by this `using` statement, if any. For
+   * example, `f` on line 2 in
+   *
+   * ```
+   * var f = File.Open("settings.xml");
+   * using (f) {
+   *   ...
+   * }
+   * ```
+   */
+  override Expr getExpr() { result = this.getChild(0) }
+
+  override Expr getAnExpr() {
     result = this.getAVariableDeclExpr().getInitializer()
     or
     result = this.getExpr()
   }
 
   /** Gets the body of this `using` statement. */
-  Stmt getBody() { result.getParent() = this }
+  override Stmt getBody() { result.getParent() = this }
 
   override string toString() { result = "using (...) {...}" }
 }
@@ -1254,6 +1285,29 @@ class LocalConstantDeclStmt extends LocalVariableDeclStmt, @const_decl_stmt {
   override string toString() { result = "const ... ...;" }
 }
 
+/**
+ * A `using` declaration statement, for example
+ *
+ * ```
+ * using FileStream f = File.Open("settings.xml");
+ * ```
+ */
+class UsingDeclStmt extends LocalVariableDeclStmt, UsingStmt, @using_decl_stmt {
+  override string toString() { result = "using ... ...;" }
+
+  override LocalVariableDeclExpr getAVariableDeclExpr() {
+    result = LocalVariableDeclStmt.super.getAVariableDeclExpr()
+  }
+
+  override LocalVariableDeclExpr getVariableDeclExpr(int n) {
+    result = LocalVariableDeclStmt.super.getVariableDeclExpr(n)
+  }
+
+  override Expr getAnExpr() {
+    result = this.getAVariableDeclExpr().getInitializer()
+  }
+}
+
 /**
  * An empty statement, for example line 2 in
  *

csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowGraph.qll

@@ -468,7 +468,7 @@ module ControlFlow {
         override ControlFlowElement getChildElement(int i) {
           not this instanceof GeneralCatchClause and
           not this instanceof FixedStmt and
-          not this instanceof UsingStmt and
+          not this instanceof UsingBlockStmt and
           result = this.getChild(i)
           or
           this = any(GeneralCatchClause gcc | i = 0 and result = gcc.getBlock())
@@ -480,7 +480,7 @@ module ControlFlow {
               i = max(int j | exists(fs.getVariableDeclExpr(j))) + 1
             )
           or
-          this = any(UsingStmt us |
+          this = any(UsingBlockStmt us |
               if exists(us.getExpr())
               then (
                 result = us.getExpr() and

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

@@ -230,3 +230,10 @@ class AddEventExpr extends AddOrRemoveEventExpr, @add_event_expr {
 class RemoveEventExpr extends AddOrRemoveEventExpr, @remove_event_expr {
   override string toString() { result = "... -= ..." }
 }
+
+/**
+ * A null-coalescing assignment operation, for example `x ??= y`.
+ */
+class AssignCoalesceExpr extends AssignOperation, @assign_coalesce_expr {
+  override string toString() { result = "... ??= ..." }
+}

csharp/ql/src/semmlecode.csharp.dbscheme

@@ -526,7 +526,7 @@ specific_type_parameter_constraints(
 
 @modifiable = @modifiable_direct | @event_accessor;
 
-@modifiable_direct = @member | @accessor;
+@modifiable_direct = @member | @accessor | @local_function;
 
 modifiers(
   unique int id: @modifier,
@@ -798,7 +798,7 @@ case @stmt.kind of
 | 18 = @checked_stmt
 | 19 = @unchecked_stmt
 | 20 = @lock_stmt
-| 21 = @using_stmt
+| 21 = @using_block_stmt
 | 22 = @var_decl_stmt
 | 23 = @const_decl_stmt
 | 24 = @empty_stmt
@@ -808,11 +808,14 @@ case @stmt.kind of
 | 28 = @catch
 | 29 = @case_stmt
 | 30 = @local_function_stmt
+| 31 = @using_decl_stmt
   ;
 
+@using_stmt = @using_block_stmt | @using_decl_stmt;
+
 @labeled_stmt = @label_stmt | @case;
 
-@decl_stmt = @var_decl_stmt | @const_decl_stmt;
+@decl_stmt = @var_decl_stmt | @const_decl_stmt | @using_decl_stmt;
 
 @cond_stmt = @if_stmt | @switch_stmt;
 
@@ -983,6 +986,7 @@ case @expr.kind of
 | 116 = @property_pattern_expr
 | 117 = @positional_pattern_expr
 | 118 = @switch_case_expr
+| 119 = @assign_coalesce_expr
 ;
 
 @switch = @switch_stmt | @switch_expr;
@@ -995,7 +999,7 @@ case @expr.kind of
               | @string_literal_expr | @null_literal_expr;
 
 @assign_expr = @simple_assign_expr | @assign_op_expr | @local_var_decl_expr;
-@assign_op_expr =  @assign_arith_expr | @assign_bitwise_expr | @assign_event_expr;
+@assign_op_expr =  @assign_arith_expr | @assign_bitwise_expr | @assign_event_expr | @assign_coalesce_expr;
 @assign_event_expr = @add_event_expr | @remove_event_expr;
 
 @assign_arith_expr = @assign_add_expr | @assign_sub_expr | @assign_mul_expr | @assign_div_expr

csharp/ql/src/semmlecode.csharp.dbscheme.stats

@@ -360,10 +360,14 @@
 <v>1854</v>
 </e>
 <e>
-<k>@using_stmt</k>
+<k>@using_block_stmt</k>
 <v>5815</v>
 </e>
 <e>
+<k>@using_decl_stmt</k>
+<v>1000</v>
+</e>
+<e>
 <k>@var_decl_stmt</k>
 <v>243866</v>
 </e>
@@ -688,6 +692,10 @@
 <v>116</v>
 </e>
 <e>
+<k>@assign_coalesce_expr</k>
+<v>116</v>
+</e>
+<e>
 <k>@object_init_expr</k>
 <v>5320</v>
 </e>

csharp/ql/test/library-tests/assignables/AssignableAccess.expected

@@ -104,6 +104,7 @@
 | Assignables.cs:133:15:133:15 | access to local variable x | Assignables.cs:132:13:132:13 | x | read |
 | Assignables.cs:133:15:133:15 | access to local variable x | Assignables.cs:132:13:132:13 | x | write |
 | Assignables.cs:133:29:133:29 | String s | Assignables.cs:133:29:133:29 | s | write |
+| Assignables.cs:138:19:138:19 | access to local variable x | Assignables.cs:138:19:138:19 | x | write |
 | Discards.cs:7:9:7:9 | access to parameter x | Discards.cs:5:30:5:30 | x | write |
 | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | z | write |
 | Discards.cs:25:27:25:30 | access to parameter args | Discards.cs:23:27:23:30 | args | read |

csharp/ql/test/library-tests/assignables/AssignableDefinition.expected

@@ -87,6 +87,7 @@
 | Assignables.cs:132:13:132:13 | x | Assignables.cs:132:13:132:17 | Int32 x = ... | Assignables.cs:132:13:132:13 | access to local variable x | Assignables.cs:132:17:132:17 | 0 | certain |
 | Assignables.cs:132:13:132:13 | x | Assignables.cs:133:15:133:15 | access to local variable x | Assignables.cs:133:15:133:15 | access to local variable x | Assignables.cs:133:15:133:15 | <none> | certain |
 | Assignables.cs:133:29:133:29 | s | Assignables.cs:133:29:133:29 | String s | Assignables.cs:133:29:133:29 | String s | Assignables.cs:133:29:133:29 | <none> | certain |
+| Assignables.cs:138:19:138:19 | x | Assignables.cs:138:19:138:50 | MemoryStream x = ... | Assignables.cs:138:19:138:19 | access to local variable x | Assignables.cs:138:23:138:50 | object creation of type MemoryStream | certain |
 | Discards.cs:5:30:5:30 | x | Discards.cs:5:30:5:30 | x | Discards.cs:5:30:5:30 | <none> | Discards.cs:5:30:5:30 | <none> | certain |
 | Discards.cs:5:30:5:30 | x | Discards.cs:7:9:7:17 | ... = ... | Discards.cs:7:9:7:9 | access to parameter x | Discards.cs:7:13:7:17 | false | certain |
 | Discards.cs:19:14:19:14 | x | Discards.cs:19:9:19:29 | ... = ... | Discards.cs:19:9:19:29 | <none> | Discards.cs:19:9:19:29 | <none> | certain |