Merge pull request github#4628 from tamasvajk/feature/csharp9-foreach

C#: Extract underlying methods of foreach statements

Author: tamasvajk

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.

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

@@ -8,6 +8,15 @@ namespace Semmle.Extraction.CSharp.Entities.Statements
 {
     internal class ForEach : Statement<ForEachStatementSyntax>
     {
+        internal enum ForeachSymbolType
+        {
+            GetEnumeratorMethod = 1,
+            CurrentProperty,
+            MoveNextMethod,
+            DisposeMethod,
+            ElementType
+        }
+
         private ForEach(Context cx, ForEachStatementSyntax stmt, IStatementParentEntity parent, int child)
             : base(cx, stmt, StmtKind.FOREACH, parent, child) { }
 
@@ -18,18 +27,59 @@ 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);
+
+            if (info.Equals(default))
+            {
+                cx.ExtractionError("Could not get foreach statement info", null, Location.Create(cx, this.ReportingLocation), severity: Util.Logging.Severity.Info);
+                return;
+            }
+
+            trapFile.foreach_stmt_info(this, info.IsAsynchronous);
+
+            if (info.GetEnumeratorMethod != null)
+            {
+                var m = Method.Create(cx, info.GetEnumeratorMethod);
+                trapFile.foreach_stmt_desugar(this, m, ForeachSymbolType.GetEnumeratorMethod);
+            }
+
+            if (info.MoveNextMethod != null)
+            {
+                var m = Method.Create(cx, info.MoveNextMethod);
+                trapFile.foreach_stmt_desugar(this, m, ForeachSymbolType.MoveNextMethod);
+            }
+
+            if (info.DisposeMethod != null)
+            {
+                var m = Method.Create(cx, info.DisposeMethod);
+                trapFile.foreach_stmt_desugar(this, m, ForeachSymbolType.DisposeMethod);
+            }
+
+            if (info.CurrentProperty != null)
+            {
+                var p = Property.Create(cx, info.CurrentProperty);
+                trapFile.foreach_stmt_desugar(this, p, ForeachSymbolType.CurrentProperty);
+            }
+
+            if (info.ElementType != null)
+            {
+                var t = Type.Create(cx, info.ElementType);
+                trapFile.foreach_stmt_desugar(this, t, ForeachSymbolType.ElementType);
+            }
         }
     }
 

csharp/extractor/Semmle.Extraction.CSharp/Tuples.cs

@@ -53,6 +53,17 @@ 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, bool isAsync)
+        {
+            trapFile.WriteTuple("foreach_stmt_info", @foreach, isAsync ? 2 : 1);
+        }
+
+        internal static void foreach_stmt_desugar(this TextWriter trapFile, Entities.Statements.ForEach @foreach, IEntity entity,
+            Entities.Statements.ForEach.ForeachSymbolType type)
+        {
+            trapFile.WriteTuple("foreach_stmt_desugar", @foreach, entity, (int)type);
+        }
+
         internal static void commentblock(this TextWriter trapFile, CommentBlock k)
         {
             trapFile.WriteTuple("commentblock", k);

csharp/extractor/Semmle.Extraction/Entities/File.cs

@@ -62,7 +62,7 @@ public override void Populate(TextWriter trapFile)
                 }
                 catch (Exception exc)
                 {
-                    Context.ExtractionError($"Couldn't read file: {originalPath}", null, null, exc.StackTrace);
+                    Context.ExtractionError($"Couldn't read file: {originalPath}. {exc.Message}", null, null, exc.StackTrace);
                 }
             }
 

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_desugar(_, m, 1)
 }
 
 predicate isRecursivelyLiveExpression(Expr e) {

csharp/ql/src/Likely Bugs/NestedLoopsSameVariable.ql

@@ -27,6 +27,13 @@ class NestedForConditions extends SC::StructuralComparisonConfiguration {
   }
 }
 
+private predicate hasChild(Stmt outer, Element child) {
+  outer = child.getParent() and
+  (outer instanceof ForStmt or outer = any(ForStmt f).getBody())
+  or
+  hasChild(outer, child.getParent())
+}
+
 /** A nested `for` statement that shares the same iteration variable as an outer `for` statement. */
 class NestedForLoopSameVariable extends ForStmt {
   ForStmt outer;
@@ -35,7 +42,7 @@ class NestedForLoopSameVariable extends ForStmt {
   MutatorOperation outerUpdate;
 
   NestedForLoopSameVariable() {
-    outer = this.getParent+() and
+    hasChild(outer, this) and
     innerUpdate = this.getAnUpdate() and
     outerUpdate = outer.getAnUpdate() and
     innerUpdate.getOperand() = iteration.getAnAccess() and
@@ -88,7 +95,7 @@ class NestedForLoopSameVariable extends ForStmt {
 
   /** Finds elements inside the outer loop that are no longer guarded by the loop invariant. */
   private ControlFlow::Node getAnUnguardedNode() {
-    result.getElement().getParent+() = getOuterForStmt().getBody() and
+    hasChild(getOuterForStmt().getBody(), result.getElement()) and
     (
       result =
         this.getCondition().(ControlFlowElement).getAControlFlowExitNode().getAFalseSuccessor()

csharp/ql/src/Likely Bugs/UncheckedCastInEquals.ql

@@ -12,12 +12,18 @@
 import csharp
 import semmle.code.csharp.frameworks.System
 
+private predicate equalsMethodChild(EqualsMethod equals, Element child) {
+  child = equals
+  or
+  equalsMethodChild(equals, child.getParent())
+}
+
 predicate nodeBeforeParameterAccess(ControlFlow::Node node) {
   exists(EqualsMethod equals | equals.getBody() = node.getElement())
   or
   exists(EqualsMethod equals, Parameter param, ControlFlow::Node mid |
     equals.getParameter(0) = param and
-    equals.getAChild*() = mid.getElement() and
+    equalsMethodChild(equals, mid.getElement()) and
     nodeBeforeParameterAccess(mid) and
     not param.getAnAccess() = mid.getElement() and
     mid.getASuccessor() = node

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 }

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_desugar(this, result, 1) }
+
+  /** Gets the called `MoveNext` or `MoveNextAsync` method. */
+  Method getMoveNext() { foreach_stmt_desugar(this, result, 3) }
+
+  /** Gets the called `Dispose` or `DisposeAsync` method, if any. */
+  Method getDispose() { foreach_stmt_desugar(this, result, 4) }
+
+  /** Gets the called `Current` property. */
+  Property getCurrent() { foreach_stmt_desugar(this, result, 2) }
+
+  /**
+   * Gets the intermediate type to which the `Current` property is converted before
+   * being converted to the iteration variable type.
+   */
+  Type getElementType() { foreach_stmt_desugar(this, result, 5) }
+
+  /** 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" }

csharp/ql/src/semmlecode.csharp.dbscheme

@@ -982,6 +982,18 @@ 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 kind: int ref /* non-async = 1, async = 2 */);
+
+@foreach_symbol =  @method | @property | @type_or_ref;
+
+#keyset[id, kind]
+foreach_stmt_desugar(
+  int id: @foreach_stmt ref,
+  int symbol: @foreach_symbol ref,
+  int kind: int ref /* GetEnumeratorMethod = 1, CurrentProperty = 2, MoveNextMethod = 3, DisposeMethod = 4, ElementType = 5 */);
+
 /** EXPRESSIONS **/
 
 expressions(