C#: Extract underlying methods of foreach statements

tamasvajk · tamasvajk · commit 7c506f445ce8 · 2021-02-07T09:06:11.000+01:00
diff --git a/csharp/change-notes/2021-02-02-foreach-underlying-methods.md b/csharp/change-notes/2021-02-02-foreach-underlying-methods.md
@@ -0,0 +1,3 @@
+lgtm,codescanning
+* The underlying methods of `foreach` statements are now explicitly extracted and
+they are made available on the `ForeachStmt` class.
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Statements/ForEach.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Statements/ForEach.cs
@@ -18,18 +18,28 @@ public static ForEach Create(Context cx, ForEachStatementSyntax node, IStatement
             return ret;
         }
 
-        protected override void PopulateStatement(TextWriter _)
+        protected override void PopulateStatement(TextWriter trapFile)
         {
             Expression.Create(cx, Stmt.Expression, this, 1);
 
-            var typeSymbol = cx.GetModel(Stmt).GetDeclaredSymbol(Stmt);
+            var semanticModel = cx.GetModel(Stmt);
+            var typeSymbol = semanticModel.GetDeclaredSymbol(Stmt);
             var type = typeSymbol.GetAnnotatedType();
 
             var location = cx.Create(Stmt.Identifier.GetLocation());
 
             Expressions.VariableDeclaration.Create(cx, typeSymbol, type, Stmt.Type, location, Stmt.Type.IsVar, this, 0);
 
             Statement.Create(cx, Stmt.Statement, this, 2);
+
+            var info = semanticModel.GetForEachStatementInfo(Stmt);
+            var getEnumerator = Method.Create(cx, info.GetEnumeratorMethod);
+            var currentProp = Property.Create(cx, info.CurrentProperty);
+            var moveNext = Method.Create(cx, info.MoveNextMethod);
+            var dispose = Method.Create(cx, info.DisposeMethod);
+            var elementType = Type.Create(cx, info.ElementType);
+
+            trapFile.foreach_stmt_info(this, elementType, getEnumerator, moveNext, dispose, currentProp, info.IsAsynchronous);
         }
     }
 
diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Tuples.cs b/csharp/extractor/Semmle.Extraction.CSharp/Tuples.cs
@@ -53,6 +53,12 @@ internal static void catch_type(this TextWriter trapFile, Entities.Statements.Ca
             trapFile.WriteTuple("catch_type", @catch, type, explicityCaught ? 1 : 2);
         }
 
+        internal static void foreach_stmt_info(this TextWriter trapFile, Entities.Statements.ForEach @foreach, Type element, Method getEnumerator,
+            Method moveNext, Method dispose, Property current, bool isAsync)
+        {
+            trapFile.WriteTuple("foreach_stmt_info", @foreach, element, getEnumerator, moveNext, dispose, current, isAsync ? 2 : 1);
+        }
+
         internal static void commentblock(this TextWriter trapFile, CommentBlock k)
         {
             trapFile.WriteTuple("commentblock", k);
diff --git a/csharp/ql/src/Dead Code/DeadCode.qll b/csharp/ql/src/Dead Code/DeadCode.qll
@@ -43,6 +43,8 @@ Expr getAMethodAccess(Method m) {
 predicate potentiallyAccessedByForEach(Method m) {
   m.hasName("GetEnumerator") and
   m.getDeclaringType().getABaseType+().hasQualifiedName("System.Collections.IEnumerable")
+  or
+  foreach_stmt_info(_, _, m, _, _, _, _)
 }
 
 predicate isRecursivelyLiveExpression(Expr e) {
diff --git a/csharp/ql/src/experimental/ir/implementation/raw/internal/desugar/Foreach.qll b/csharp/ql/src/experimental/ir/implementation/raw/internal/desugar/Foreach.qll
@@ -171,11 +171,7 @@ private class TranslatedForeachMoveNext extends TranslatedCompilerGeneratedCall,
 
   override Callable getInstructionFunction(InstructionTag tag) {
     tag = CallTargetTag() and
-    exists(Callable internal |
-      internal.getName() = "MoveNext" and
-      internal.getReturnType() instanceof BoolType and
-      result = internal
-    )
+    result = generatedBy.getMoveNext()
   }
 
   override Type getCallResultType() { result instanceof BoolType }
@@ -205,28 +201,9 @@ private class TranslatedForeachGetEnumerator extends TranslatedCompilerGenerated
     result = getInstructionFunction(CallTargetTag()).getReturnType()
   }
 
-  private Callable getEnumeratorCallable() {
-    if exists(generatedBy.getIterableExpr().getType().(ValueOrRefType).getAMember("GetEnumerator"))
-    then
-      result = generatedBy.getIterableExpr().getType().(ValueOrRefType).getAMember("GetEnumerator")
-    else
-      exists(Interface inter |
-        inter =
-          generatedBy
-              .getIterableExpr()
-              .getType()
-              .(ValueOrRefType)
-              // There could be some abstract base types until we reach `IEnumerable` (eg. `Array`)
-              .getABaseType*()
-              .getABaseInterface() and
-        inter.getName() = "IEnumerable" and
-        result = inter.getAMember("GetEnumerator")
-      )
-  }
-
   override Callable getInstructionFunction(InstructionTag tag) {
     tag = CallTargetTag() and
-    result = getEnumeratorCallable()
+    result = generatedBy.getGetEnumerator()
   }
 
   override TranslatedExpr getArgument(int id) { none() }
@@ -247,7 +224,7 @@ private class TranslatedForeachCurrent extends TranslatedCompilerGeneratedCall,
 
   TranslatedForeachCurrent() { this = TTranslatedCompilerGeneratedElement(generatedBy, 5) }
 
-  override Type getCallResultType() { result = generatedBy.getAVariable().getType() }
+  override Type getCallResultType() { result = generatedBy.getElementType() }
 
   override TranslatedExpr getArgument(int id) { none() }
 
@@ -262,10 +239,7 @@ private class TranslatedForeachCurrent extends TranslatedCompilerGeneratedCall,
 
   override Callable getInstructionFunction(InstructionTag tag) {
     tag = CallTargetTag() and
-    exists(Property prop |
-      prop.getName() = "Current" and
-      result = prop.getGetter()
-    )
+    result = generatedBy.getCurrent().getGetter()
   }
 }
 
@@ -280,10 +254,7 @@ private class TranslatedForeachDispose extends TranslatedCompilerGeneratedCall,
 
   override Callable getInstructionFunction(InstructionTag tag) {
     tag = CallTargetTag() and
-    exists(Callable dispose |
-      dispose.getName() = "Dispose" and
-      result = dispose
-    )
+    result = generatedBy.getDispose()
   }
 
   final override Type getCallResultType() { result instanceof VoidType }
diff --git a/csharp/ql/src/semmle/code/csharp/Stmt.qll b/csharp/ql/src/semmle/code/csharp/Stmt.qll
@@ -582,6 +582,27 @@ class ForeachStmt extends LoopStmt, @foreach_stmt {
    */
   Expr getIterableExpr() { result = this.getChild(1) }
 
+  /** Gets the called `GetEnumerator` method. */
+  Method getGetEnumerator() { foreach_stmt_info(this, _, result, _, _, _, _) }
+
+  /** Gets the called `MoveNext` or `MoveNextAsync` method. */
+  Method getMoveNext() { foreach_stmt_info(this, _, _, result, _, _, _) }
+
+  /** Gets the called `Dispose` or `DisposeAsync` method. */
+  Method getDispose() { foreach_stmt_info(this, _, _, _, result, _, _) }
+
+  /** Gets the called `Current` property. */
+  Property getCurrent() { foreach_stmt_info(this, _, _, _, _, result, _) }
+
+  /**
+   * Gets the intermediate type to which the `Current` property is converted before
+   * being converted to the iteration variable type.
+   */
+  Type getElementType() { foreach_stmt_info(this, result, _, _, _, _, _) }
+
+  /** Holds if this `foreach` statement is asynchronous. */
+  predicate isAsync() { foreach_stmt_info(this, _, _, _, _, _, 2) }
+
   override string toString() { result = "foreach (... ... in ...) ..." }
 
   override string getAPrimaryQlClass() { result = "ForeachStmt" }
diff --git a/csharp/ql/src/semmlecode.csharp.dbscheme b/csharp/ql/src/semmlecode.csharp.dbscheme
@@ -982,6 +982,15 @@ catch_type(
   int type_id: @type_or_ref ref,
   int kind: int ref /* explicit = 1, implicit = 2 */);
 
+foreach_stmt_info(
+  unique int id: @foreach_stmt ref,
+  int element_type_id: @type_or_ref ref,
+  int getenumerator_id: @method ref,
+  int movenext_id: @method ref,
+  int dispose_id: @method ref,
+  int current_id: @property ref,
+  int kind: int ref /* non-async = 1, async = 2 */);
+
 /** EXPRESSIONS **/
 
 expressions(
diff --git a/csharp/ql/test/library-tests/csharp9/ForeachExtension.cs b/csharp/ql/test/library-tests/csharp9/ForeachExtension.cs
@@ -0,0 +1,50 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
+
+static class Extensions
+{
+    public static IEnumerator<T> GetEnumerator<T>(this IEnumerator<T> enumerator) => enumerator;
+    public static IAsyncEnumerator<T> GetAsyncEnumerator<T>(this IAsyncEnumerator<T> enumerator) => enumerator;
+    public static IEnumerator<int> GetEnumerator(this int count)
+    {
+        for (int i = 0; i < count; i++)
+        {
+            yield return i;
+        }
+    }
+}
+
+class Program
+{
+    async Task Main()
+    {
+        IEnumerator<int> enumerator1 = Enumerable.Range(0, 10).GetEnumerator();
+        foreach (var item in enumerator1)
+        {
+        }
+
+        IAsyncEnumerator<int> enumerator2 = GetAsyncEnumerator();
+        await foreach (var item in enumerator2)
+        {
+        }
+
+        foreach (var item in 42)
+        {
+        }
+
+        foreach (var i in new int[] { 1, 2, 3 }) // not extension
+        {
+        }
+    }
+
+    static async IAsyncEnumerator<int> GetAsyncEnumerator()
+    {
+        yield return 0;
+        await Task.Delay(1);
+        yield return 1;
+    }
+}
+
+// semmle-extractor-options: /r:System.Linq.dll
diff --git a/csharp/ql/test/library-tests/csharp9/PrintAst.expected b/csharp/ql/test/library-tests/csharp9/PrintAst.expected
@@ -166,6 +166,109 @@ Discard.cs:
 #   10|             4: [BlockStmt] {...}
 #   10|               0: [ReturnStmt] return ...;
 #   10|                 0: [IntLiteral] 0
+ForeachExtension.cs:
+#    6| [Class] Extensions
+#    8|   4: [ExtensionMethod] GetEnumerator
+#    8|     -1: [TypeMention] IEnumerator<T>
+#    8|       1: [TypeMention] T
+#-----|     1: (Type parameters)
+#    8|       0: [TypeParameter] T
+#-----|     2: (Parameters)
+#    8|       0: [Parameter] enumerator
+#    8|         -1: [TypeMention] IEnumerator<T>
+#    8|           1: [TypeMention] T
+#    8|     4: [ParameterAccess] access to parameter enumerator
+#    9|   6: [ExtensionMethod] GetAsyncEnumerator
+#    9|     -1: [TypeMention] IAsyncEnumerator<T>
+#    9|       1: [TypeMention] T
+#-----|     1: (Type parameters)
+#    9|       0: [TypeParameter] T
+#-----|     2: (Parameters)
+#    9|       0: [Parameter] enumerator
+#    9|         -1: [TypeMention] IAsyncEnumerator<T>
+#    9|           1: [TypeMention] T
+#    9|     4: [ParameterAccess] access to parameter enumerator
+#   10|   8: [ExtensionMethod] GetEnumerator
+#   10|     -1: [TypeMention] IEnumerator<Int32>
+#   10|       1: [TypeMention] int
+#-----|     2: (Parameters)
+#   10|       0: [Parameter] count
+#   10|         -1: [TypeMention] int
+#   11|     4: [BlockStmt] {...}
+#   12|       0: [ForStmt] for (...;...;...) ...
+#   12|         -1: [LocalVariableDeclAndInitExpr] Int32 i = ...
+#   12|           -1: [TypeMention] int
+#   12|           0: [LocalVariableAccess] access to local variable i
+#   12|           1: [IntLiteral] 0
+#   12|         0: [LTExpr] ... < ...
+#   12|           0: [LocalVariableAccess] access to local variable i
+#   12|           1: [ParameterAccess] access to parameter count
+#   12|         1: [PostIncrExpr] ...++
+#   12|           0: [LocalVariableAccess] access to local variable i
+#   13|         2: [BlockStmt] {...}
+#   14|           0: [YieldReturnStmt] yield return ...;
+#   14|             0: [LocalVariableAccess] access to local variable i
+#   19| [Class] Program
+#   21|   5: [Method] Main
+#   21|     -1: [TypeMention] Task
+#   22|     4: [BlockStmt] {...}
+#   23|       0: [LocalVariableDeclStmt] ... ...;
+#   23|         0: [LocalVariableDeclAndInitExpr] IEnumerator<Int32> enumerator1 = ...
+#   23|           -1: [TypeMention] IEnumerator<Int32>
+#   23|             1: [TypeMention] int
+#   23|           0: [LocalVariableAccess] access to local variable enumerator1
+#   23|           1: [MethodCall] call to method GetEnumerator
+#   23|             -1: [MethodCall] call to method Range
+#   23|               -1: [TypeAccess] access to type Enumerable
+#   23|                 0: [TypeMention] Enumerable
+#   23|               0: [IntLiteral] 0
+#   23|               1: [IntLiteral] 10
+#   24|       1: [ForeachStmt] foreach (... ... in ...) ...
+#   24|         0: [LocalVariableDeclExpr] Int32 item
+#   24|           0: [TypeMention] int
+#   24|         1: [LocalVariableAccess] access to local variable enumerator1
+#   25|         2: [BlockStmt] {...}
+#   28|       2: [LocalVariableDeclStmt] ... ...;
+#   28|         0: [LocalVariableDeclAndInitExpr] IAsyncEnumerator<Int32> enumerator2 = ...
+#   28|           -1: [TypeMention] IAsyncEnumerator<Int32>
+#   28|             1: [TypeMention] int
+#   28|           0: [LocalVariableAccess] access to local variable enumerator2
+#   28|           1: [MethodCall] call to method GetAsyncEnumerator
+#   29|       3: [ForeachStmt] foreach (... ... in ...) ...
+#   29|         0: [LocalVariableDeclExpr] Int32 item
+#   29|           0: [TypeMention] int
+#   29|         1: [LocalVariableAccess] access to local variable enumerator2
+#   30|         2: [BlockStmt] {...}
+#   33|       4: [ForeachStmt] foreach (... ... in ...) ...
+#   33|         0: [LocalVariableDeclExpr] Int32 item
+#   33|           0: [TypeMention] int
+#   33|         1: [IntLiteral] 42
+#   34|         2: [BlockStmt] {...}
+#   37|       5: [ForeachStmt] foreach (... ... in ...) ...
+#   37|         0: [LocalVariableDeclExpr] Int32 i
+#   37|           0: [TypeMention] int
+#   37|         1: [ArrayCreation] array creation of type Int32[]
+#   37|           -2: [TypeMention] Int32[]
+#   37|             1: [TypeMention] int
+#   37|           -1: [ArrayInitializer] { ..., ... }
+#   37|             0: [IntLiteral] 1
+#   37|             1: [IntLiteral] 2
+#   37|             2: [IntLiteral] 3
+#   38|         2: [BlockStmt] {...}
+#   42|   6: [Method] GetAsyncEnumerator
+#   42|     -1: [TypeMention] IAsyncEnumerator<Int32>
+#   42|       1: [TypeMention] int
+#   43|     4: [BlockStmt] {...}
+#   44|       0: [YieldReturnStmt] yield return ...;
+#   44|         0: [IntLiteral] 0
+#   45|       1: [ExprStmt] ...;
+#   45|         0: [AwaitExpr] await ...
+#   45|           0: [MethodCall] call to method Delay
+#   45|             -1: [TypeAccess] access to type Task
+#   45|               0: [TypeMention] Task
+#   45|             0: [IntLiteral] 1
+#   46|       2: [YieldReturnStmt] yield return ...;
+#   46|         0: [IntLiteral] 1
 FunctionPointer.cs:
 #    5| [Class] FnPointer
 #    7|   5: [Class] Program
diff --git a/csharp/ql/test/library-tests/csharp9/foreach.expected b/csharp/ql/test/library-tests/csharp9/foreach.expected
@@ -0,0 +1,4 @@
+| ForeachExtension.cs:24:9:26:9 | foreach (... ... in ...) ... | Int32 | sync | Extensions | ForeachExtension.cs:8:34:8:49 | System.Collections.Generic.IEnumerator<Int32> | - | System.Collections.IEnumerator | - |
+| ForeachExtension.cs:29:9:31:9 | foreach (... ... in ...) ... | Int32 | async | Extensions | ForeachExtension.cs:9:39:9:59 | System.Collections.Generic.IAsyncEnumerator<Int32> | - | System.Collections.Generic.IAsyncEnumerator<Int32> | - |
+| ForeachExtension.cs:33:9:35:9 | foreach (... ... in ...) ... | Int32 | sync | Extensions | ForeachExtension.cs:10:36:10:48 | System.Collections.Generic.IEnumerator<Int32> | - | System.Collections.IEnumerator | - |
+| ForeachExtension.cs:37:9:39:9 | foreach (... ... in ...) ... | Int32 | sync | System.Collections.IEnumerable | - | System.Collections.IEnumerator | - | System.Collections.IEnumerator | - |
diff --git a/csharp/ql/test/library-tests/csharp9/foreach.ql b/csharp/ql/test/library-tests/csharp9/foreach.ql
@@ -0,0 +1,15 @@
+import csharp
+
+private string getLocation(Member m) {
+  if m.fromSource() then result = m.getALocation().(SourceLocation).toString() else result = "-"
+}
+
+private string getIsAsync(ForeachStmt f) {
+  if f.isAsync() then result = "async" else result = "sync"
+}
+
+from ForeachStmt f
+select f, f.getElementType().toString(), getIsAsync(f),
+  f.getGetEnumerator().getDeclaringType().getQualifiedName(), getLocation(f.getGetEnumerator()),
+  f.getCurrent().getDeclaringType().getQualifiedName(), getLocation(f.getCurrent()),
+  f.getMoveNext().getDeclaringType().getQualifiedName(), getLocation(f.getMoveNext())
diff --git a/csharp/ql/test/library-tests/csharp9/typeParameterNullability.expected b/csharp/ql/test/library-tests/csharp9/typeParameterNullability.expected
@@ -10,8 +10,6 @@ valueType
 | TypeParameterNullability.cs:19:29:19:29 | T |
 | TypeParameterNullability.cs:24:29:24:29 | T |
 valueOrRefType
-| FunctionPointer.cs:22:24:22:24 | T |
-| FunctionPointer.cs:34:24:34:24 | T |
 | TypeParameterNullability.cs:5:28:5:28 | T |
 | TypeParameterNullability.cs:9:28:9:28 | T |
 | TypeParameterNullability.cs:15:29:15:29 | T |
diff --git a/csharp/ql/test/library-tests/csharp9/typeParameterNullability.ql b/csharp/ql/test/library-tests/csharp9/typeParameterNullability.ql
@@ -1,17 +1,17 @@
 import csharp
 
 query predicate refType(TypeParameter tp) {
-  tp.fromSource() and
+  tp.getFile().getStem() = "TypeParameterNullability" and
   tp.isRefType()
 }
 
 query predicate valueType(TypeParameter tp) {
-  tp.fromSource() and
+  tp.getFile().getStem() = "TypeParameterNullability" and
   tp.isValueType()
 }
 
 query predicate valueOrRefType(TypeParameter tp) {
-  tp.fromSource() and
+  tp.getFile().getStem() = "TypeParameterNullability" and
   not tp.isRefType() and
   not tp.isValueType()
 }
diff --git a/csharp/upgrades/TO_CHANGE/upgrade.properties b/csharp/upgrades/TO_CHANGE/upgrade.properties
@@ -0,0 +1,2 @@
+description: Add 'foreach_stmt_info' relation.
+compatibility: backwards

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+lgtm,codescanning`
	`2`	+* The underlying methods of `foreach` statements are now explicitly extracted and
	`3`	+they are made available on the `ForeachStmt` class.
Original file line number	Diff line number	Diff line change
`@@ -53,6 +53,12 @@ internal static void catch_type(this TextWriter trapFile, Entities.Statements.Ca`
`53`	`53`	`trapFile.WriteTuple("catch_type", @catch, type, explicityCaught ? 1 : 2);`
`54`	`54`	`}`
`55`	`55`
	`56`	`+ internal static void foreach_stmt_info(this TextWriter trapFile, Entities.Statements.ForEach @foreach, Type element, Method getEnumerator,`
	`57`	`+ Method moveNext, Method dispose, Property current, bool isAsync)`
	`58`	`+ {`
	`59`	`+ trapFile.WriteTuple("foreach_stmt_info", @foreach, element, getEnumerator, moveNext, dispose, current, isAsync ? 2 : 1);`
	`60`	`+ }`
	`61`	`+`
`56`	`62`	`internal static void commentblock(this TextWriter trapFile, CommentBlock k)`
`57`	`63`	`{`
`58`	`64`	`trapFile.WriteTuple("commentblock", k);`
Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,8 @@ Expr getAMethodAccess(Method m) {`
`43`	`43`	`predicate potentiallyAccessedByForEach(Method m) {`
`44`	`44`	`m.hasName("GetEnumerator") and`
`45`	`45`	`m.getDeclaringType().getABaseType+().hasQualifiedName("System.Collections.IEnumerable")`
	`46`	`+ or`
	`47`	`+ foreach_stmt_info(_, _, m, _, _, _, _)`
`46`	`48`	`}`
`47`	`49`
`48`	`50`	`predicate isRecursivelyLiveExpression(Expr e) {`