Merge pull request #1392 from calumgrant/cs/cs8/static-using-null

C#: More C# 8 features

Author: hvitved

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