Merge pull request #21708 from github/copilot/fix-missed-opportunity-to-use-select

Fix false positive in `MissedSelectOpportunity` when foreach body uses `await`

Author: hvitved

csharp/ql/lib/Linq/Helpers.qll

@@ -121,15 +121,17 @@ predicate missedOfTypeOpportunity(ForeachStmtEnumerable fes, LocalVariableDeclSt
 /**
  * Holds if `foreach` statement `fes` can be converted to a `.Select()` call.
  * That is, the loop variable is accessed only in the first statement of the
- * block, the access is not a cast, and the first statement is a
- * local variable declaration statement `s`.
+ * block, the access is not a cast, the first statement is a
+ * local variable declaration statement `s`, and the initializer does not
+ * contain an `await` expression (since `Select` does not support async lambdas).
  */
 predicate missedSelectOpportunity(ForeachStmtGenericEnumerable fes, LocalVariableDeclStmt s) {
   s = firstStmt(fes) and
   forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
     va = s.getAVariableDeclExpr().getAChildExpr*()
   ) and
-  not s.getAVariableDeclExpr().getInitializer() instanceof Cast
+  not s.getAVariableDeclExpr().getInitializer() instanceof Cast and
+  not s.getAVariableDeclExpr().getInitializer().getAChildExpr*() instanceof AwaitExpr
 }
 
 /**

csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.cs

@@ -0,0 +1,32 @@
+using System;
+using System.Linq;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+
+class MissedSelectOpportunity
+{
+    public void M1(List<int> lst)
+    {
+        // BAD: Can be replaced with lst.Select(i => i * i)
+        foreach (int i in lst)
+        {
+            int j = i * i;
+            Console.WriteLine(j);
+        } // $ Alert
+    }
+
+    public async Task M2(IEnumerable<ICounter> counters)
+    {
+        // GOOD: Cannot use Select because the initializer contains an await expression
+        foreach (var counter in counters)
+        {
+            var count = await counter.CountAsync();
+            Console.WriteLine(count);
+        }
+    }
+
+    public interface ICounter
+    {
+        Task<int> CountAsync();
+    }
+}

csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.expected

@@ -0,0 +1 @@
+| MissedSelectOpportunity.cs:11:9:15:9 | foreach (... ... in ...) ... | This foreach loop immediately $@ - consider mapping the sequence explicitly using '.Select(...)'. | MissedSelectOpportunity.cs:13:13:13:26 | ... ...; | maps its iteration variable to another variable |

csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.qlref

@@ -0,0 +1,2 @@
+query: Linq/MissedSelectOpportunity.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/options

@@ -0,0 +1,2 @@
+semmle-extractor-options: /nostdlib /noconfig
+semmle-extractor-options: --load-sources-from-project:${testdir}/../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj