Merge pull request #21759 from hvitved/csharp/cfg-params

C#: Include parameters and their defaults in the CFG

Author: hvitved

csharp/downgrades/3cabc77473cbbda95edebafea345c2e3fdfa12d9/old.dbscheme

@@ -0,0 +1,2 @@
+description: Remove `@parameter` from `@control_flow_element`
+compatibility: full

csharp/downgrades/3cabc77473cbbda95edebafea345c2e3fdfa12d9/semmlecode.csharp.dbscheme

@@ -8,6 +8,7 @@
  */
 
 import csharp
+import semmle.code.csharp.controlflow.internal.ControlFlowGraph
 
 external string selectedSourceFile();
 
@@ -22,14 +23,16 @@ external int selectedSourceColumn();
 private predicate selectedSourceColumnAlias = selectedSourceColumn/0;
 
 module ViewCfgQueryInput implements ControlFlow::ViewCfgQueryInputSig<File> {
+  private import semmle.code.csharp.controlflow.ControlFlowGraph
+
   predicate selectedSourceFile = selectedSourceFileAlias/0;
 
   predicate selectedSourceLine = selectedSourceLineAlias/0;
 
   predicate selectedSourceColumn = selectedSourceColumnAlias/0;
 
   predicate cfgScopeSpan(
-    Callable scope, File file, int startLine, int startColumn, int endLine, int endColumn
+    Ast::Callable scope, File file, int startLine, int startColumn, int endLine, int endColumn
   ) {
     file = scope.getFile() and
     scope.getLocation().getStartLine() = startLine and

csharp/downgrades/3cabc77473cbbda95edebafea345c2e3fdfa12d9/upgrade.properties

@@ -271,6 +271,8 @@ module AssignableInternal {
     def = TPatternDefinition(result)
     or
     def = TAssignOperationDefinition(result)
+    or
+    def = TParameterDefaultDefinition(_, result)
   }
 
   /** A local variable declaration at the top-level of a pattern. */
@@ -308,10 +310,17 @@ module AssignableInternal {
         exists(Callable c | p = c.getAParameter() |
           c.hasBody()
           or
-          // Same as `c.(Constructor).hasInitializer()`, but avoids negative recursion warning
-          c.getAChildExpr() instanceof @constructor_init_expr
+          c.(Constructor).hasInitializer()
         )
       } or
+      TParameterDefaultDefinition(Parameter p, Expr default) {
+        exists(Callable c | p = c.getAParameter() |
+          c.hasBody()
+          or
+          c.(Constructor).hasInitializer()
+        ) and
+        default = p.getDefaultValue()
+      } or
       TAddressOfDefinition(AddressOfExpr aoe) or
       TPatternDefinition(TopLevelPatternDecl tlpd) or
       TAssignOperationDefinition(AssignOperation ao) {
@@ -350,6 +359,8 @@ module AssignableInternal {
         any(AssignableDefinitions::PatternDefinition pd | result = pd.getDeclaration().getVariable())
       or
       def = any(AssignableDefinitions::InitializerDefinition init | result = init.getAssignable())
+      or
+      def = TParameterDefaultDefinition(result, _)
     }
 
     // Not defined by dispatch in order to avoid too conservative negative recursion error
@@ -493,11 +504,6 @@ class AssignableDefinition extends TAssignableDefinition {
       exists(Ssa::ExplicitDefinition def | result = def.getAFirstReadAtNode(cfn) |
         this = def.getADefinition()
       )
-      or
-      exists(Ssa::ImplicitParameterDefinition def | result = def.getAFirstReadAtNode(cfn) |
-        this.(AssignableDefinitions::ImplicitParameterDefinition).getParameter() =
-          def.getParameter()
-      )
     )
   }
 
@@ -689,7 +695,33 @@ module AssignableDefinitions {
 
     override string toString() { result = p.toString() }
 
-    override Location getLocation() { result = this.getTarget().getLocation() }
+    override Location getLocation() { result = p.getLocation() }
+  }
+
+  /**
+   * A default value assigned to a parameter.
+   */
+  class ParameterDefaultDefinition extends AssignableDefinition, TParameterDefaultDefinition {
+    Parameter p;
+    Expr default;
+
+    ParameterDefaultDefinition() { this = TParameterDefaultDefinition(p, default) }
+
+    /** Gets the underlying parameter. */
+    Parameter getParameter() { result = p }
+
+    /** Gets the default value expression for the parameter. */
+    Expr getDefaultValue() { result = default }
+
+    override Expr getSource() { result = default }
+
+    override Expr getElement() { result = default }
+
+    override Callable getEnclosingCallable() { result = p.getCallable() }
+
+    override string toString() { result = p.toString() + " = ..." }
+
+    override Location getLocation() { result = default.getLocation() }
   }
 
   /**

csharp/ql/lib/printCfg.ql

@@ -13,7 +13,7 @@ private import internal.Location
  * An element that can have a child statement or expression.
  */
 class ExprOrStmtParent extends Element, @exprorstmt_parent {
-  final override ControlFlowElement getChild(int i) {
+  override ControlFlowElement getChild(int i) {
     result = this.getChildExpr(i) or
     result = this.getChildStmt(i)
   }
@@ -42,14 +42,8 @@ class ExprOrStmtParent extends Element, @exprorstmt_parent {
  *
  * An element that can have a child top-level expression.
  */
-class TopLevelExprParent extends Element, @top_level_expr_parent {
+class TopLevelExprParent extends ExprOrStmtParent, @top_level_expr_parent {
   final override Expr getChild(int i) { result = this.getChildExpr(i) }
-
-  /** Gets the `i`th child expression of this element (zero-based). */
-  final Expr getChildExpr(int i) { expr_parent_top_level_adjusted(result, i, this) }
-
-  /** Gets a child expression of this element, if any. */
-  final Expr getAChildExpr() { result = this.getChildExpr(_) }
 }
 
 /** INTERNAL: Do not use. */

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

@@ -299,7 +299,9 @@ class ControlFlowElementNode extends ElementNode {
     not isNotNeeded(element.getParent+()) and
     // LambdaExpr is both a Callable and a ControlFlowElement,
     // print it with the more specific CallableNode
-    not element instanceof Callable
+    not element instanceof Callable and
+    // Handled in `ParameterNode`
+    not element instanceof Parameter
   }
 
   override PrintAstNode getChild(int childIndex) {

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

@@ -87,7 +87,9 @@ class LocalScopeVariable extends Variable, @local_scope_variable {
  * }
  * ```
  */
-class Parameter extends LocalScopeVariable, Attributable, TopLevelExprParent, @parameter {
+class Parameter extends LocalScopeVariable, Attributable, TopLevelExprParent, ControlFlowElement,
+  @parameter
+{
   /** Gets the raw position of this parameter, including the `this` parameter at index 0. */
   final int getRawPosition() { this = this.getDeclaringElement().getRawParameter(result) }