Skip to content

Commit 3483050

Browse files
Copilothvitved
andauthored
Fix false positive in MissedSelectOpportunity for async/await loops
Agent-Logs-Url: https://github.com/github/codeql/sessions/3e8f4320-2bf4-45f5-b9ea-dad41d522d84 Co-authored-by: hvitved <3667920+hvitved@users.noreply.github.com>
1 parent 0e66555 commit 3483050

File tree

5 files changed

+41
-3
lines changed

5 files changed

+41
-3
lines changed

csharp/ql/lib/Linq/Helpers.qll

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,15 +121,17 @@ predicate missedOfTypeOpportunity(ForeachStmtEnumerable fes, LocalVariableDeclSt
121121
/**
122122
* Holds if `foreach` statement `fes` can be converted to a `.Select()` call.
123123
* That is, the loop variable is accessed only in the first statement of the
124-
* block, the access is not a cast, and the first statement is a
125-
* local variable declaration statement `s`.
124+
* block, the access is not a cast, the first statement is a
125+
* local variable declaration statement `s`, and the initializer does not
126+
* contain an `await` expression (since `Select` does not support async lambdas).
126127
*/
127128
predicate missedSelectOpportunity(ForeachStmtGenericEnumerable fes, LocalVariableDeclStmt s) {
128129
s = firstStmt(fes) and
129130
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
130131
va = s.getAVariableDeclExpr().getAChildExpr*()
131132
) and
132-
not s.getAVariableDeclExpr().getInitializer() instanceof Cast
133+
not s.getAVariableDeclExpr().getInitializer() instanceof Cast and
134+
not s.getAVariableDeclExpr().getInitializer().getAChildExpr*() instanceof AwaitExpr
133135
}
134136

135137
/**
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
using System;
2+
using System.Linq;
3+
using System.Collections.Generic;
4+
using System.Threading.Tasks;
5+
6+
class MissedSelectOpportunity
7+
{
8+
public void M1(List<int> lst)
9+
{
10+
// BAD: Can be replaced with lst.Select(i => i * i)
11+
foreach (int i in lst)
12+
{
13+
int j = i * i;
14+
Console.WriteLine(j);
15+
}
16+
}
17+
18+
public async Task M2(IEnumerable<ICounter> counters)
19+
{
20+
// GOOD: Cannot use Select because the initializer contains an await expression
21+
foreach (var counter in counters)
22+
{
23+
var count = await counter.CountAsync();
24+
Console.WriteLine(count);
25+
}
26+
}
27+
28+
public interface ICounter
29+
{
30+
Task<int> CountAsync();
31+
}
32+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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 |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Linq/MissedSelectOpportunity.ql
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
semmle-extractor-options: /nostdlib /noconfig
2+
semmle-extractor-options: --load-sources-from-project:${testdir}/../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj

0 commit comments

Comments
 (0)