From 612e515f48a23cf23569505fef8da98ff0ba3c2a Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 9 Sep 2026 10:46:17 +0200 Subject: [PATCH 1/6] C#: Add some tests for cs/linq/missed-all. --- .../MissedAllOpportunity.cs | 39 +++++++++++++++++++ .../MissedAllOpportunity.expected | 1 + .../MissedAllOpportunity.qlref | 2 + .../Linq/MissedAllOpportunity/options | 2 + 4 files changed, 44 insertions(+) create mode 100644 csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.cs create mode 100644 csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected create mode 100644 csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.qlref create mode 100644 csharp/ql/test/query-tests/Linq/MissedAllOpportunity/options diff --git a/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.cs b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.cs new file mode 100644 index 000000000000..e92ad7e91212 --- /dev/null +++ b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.cs @@ -0,0 +1,39 @@ +using System; +using System.Linq; +using System.Collections.Generic; + +class MissedAllOpportunity +{ + public void M1(List lst) + { + // BAD: Can be replaced with lst.All(e => e % 2 == 0) + var allEven = true; + foreach (int i in lst) + { + if (i % 2 != 0) + { + allEven = false; + break; + } + } // $ Alert + } + + public void M2(NonEnumerableClass nec) + { + // GOOD: Linq can't be used here. + var allEven = true; + foreach (int i in nec) + { + if (i % 2 != 0) + { + allEven = false; + break; + } + } + } + + public class NonEnumerableClass + { + public IEnumerator GetEnumerator() => throw null; + } +} diff --git a/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected new file mode 100644 index 000000000000..b4300f8caf07 --- /dev/null +++ b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected @@ -0,0 +1 @@ +| MissedAllOpportunity.cs:11:9:18:9 | foreach (... ... in ...) ... | This foreach loop looks as if it might be testing whether every sequence element satisfies a predicate - consider using '.All(...)'. | diff --git a/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.qlref b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.qlref new file mode 100644 index 000000000000..689d5fbb60a4 --- /dev/null +++ b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.qlref @@ -0,0 +1,2 @@ +query: Linq/MissedAllOpportunity.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/options b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/options new file mode 100644 index 000000000000..75c39b4541ba --- /dev/null +++ b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/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 From e2ebb0d5dfa3c776decd7470bafddfc342f77983 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 9 Sep 2026 11:38:24 +0200 Subject: [PATCH 2/6] C#: Add test cases that uses in/out or ref parameters. --- .../MissedAllOpportunity/MissedAllOpportunity.cs | 14 ++++++++++++++ .../MissedAllOpportunity.expected | 4 ++++ .../MissedFirstOrDefaultOpportunity.cs | 14 ++++++++++++++ .../MissedFirstOrDefaultOpportunity.expected | 4 ++++ .../MissedSelectOpportunity.cs | 11 +++++++++++ .../MissedSelectOpportunity.expected | 4 ++++ .../MissedWhereOpportunity.cs | 12 ++++++++++++ .../MissedWhereOpportunity.expected | 4 ++++ 8 files changed, 67 insertions(+) diff --git a/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.cs b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.cs index e92ad7e91212..4b09d8bd6409 100644 --- a/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.cs +++ b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.cs @@ -32,6 +32,20 @@ public void M2(NonEnumerableClass nec) } } + public void M3(List lst, ref int x) + { + // GOOD: Linq can't be used here because the condition uses a ref parameter. + var allEven = true; + foreach (int i in lst) + { + if (i % 2 != x) + { + allEven = false; + break; + } + } + } + public class NonEnumerableClass { public IEnumerator GetEnumerator() => throw null; diff --git a/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected index b4300f8caf07..67203c735f1e 100644 --- a/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected +++ b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected @@ -1 +1,5 @@ +#select | MissedAllOpportunity.cs:11:9:18:9 | foreach (... ... in ...) ... | This foreach loop looks as if it might be testing whether every sequence element satisfies a predicate - consider using '.All(...)'. | +| MissedAllOpportunity.cs:39:9:46:9 | foreach (... ... in ...) ... | This foreach loop looks as if it might be testing whether every sequence element satisfies a predicate - consider using '.All(...)'. | +testFailures +| MissedAllOpportunity.cs:39:9:46:9 | This foreach loop looks as if it might be testing whether every sequence element satisfies a predicate - consider using '.All(...)'. | Unexpected result: Alert | diff --git a/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.cs b/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.cs index 8120a70414e5..84824efee4bc 100644 --- a/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.cs +++ b/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.cs @@ -179,6 +179,20 @@ public Operation M14(IEnumerable operations, Func[] pre return null; } + public int M15(IEnumerable values, ref readonly int x) + { + // GOOD: FirstOrDefault does not support a predicate that captures a ref parameter. + foreach (var value in values) + { + if (value > x) + { + return value; + } + } + + return default; + } + private static Task IsMatch(Operation operation, string operationId) => Task.FromResult(string.Equals(operation.OperationId, operationId, StringComparison.Ordinal)); } diff --git a/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.expected b/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.expected index b4cfdff5fdd9..aba54e7933cb 100644 --- a/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.expected +++ b/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.expected @@ -1,5 +1,9 @@ +#select | MissedFirstOrDefaultOpportunity.cs:10:9:14:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:12:17:12:91 | call to method Equals | predicate | | MissedFirstOrDefaultOpportunity.cs:22:9:28:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:24:17:24:25 | ... > ... | predicate | | MissedFirstOrDefaultOpportunity.cs:36:9:40:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:38:17:38:25 | ... > ... | predicate | | MissedFirstOrDefaultOpportunity.cs:149:9:153:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:151:17:151:32 | ... > ... | predicate | | MissedFirstOrDefaultOpportunity.cs:161:9:165:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:163:17:163:25 | ... > ... | predicate | +| MissedFirstOrDefaultOpportunity.cs:185:9:191:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:187:17:187:25 | ... > ... | predicate | +testFailures +| MissedFirstOrDefaultOpportunity.cs:185:9:191:9 | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | Unexpected result: Alert | diff --git a/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.cs b/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.cs index 9655a5a0fa9c..0a958d3e50d8 100644 --- a/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.cs +++ b/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.cs @@ -25,6 +25,17 @@ public async Task M2(IEnumerable counters) } } + public void M3(List lst, out int x) + { + // GOOD: Linq can't be used here as the Select would capture an out parameter. + x = 2; + foreach (int i in lst) + { + int j = i * x; + Console.WriteLine(j); + } + } + public interface ICounter { Task CountAsync(); diff --git a/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.expected b/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.expected index bc6d464fa3b9..b84ec32441f4 100644 --- a/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.expected +++ b/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.expected @@ -1 +1,5 @@ +#select | 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 | +| MissedSelectOpportunity.cs:32:9:36:9 | foreach (... ... in ...) ... | This foreach loop immediately $@ - consider mapping the sequence explicitly using '.Select(...)'. | MissedSelectOpportunity.cs:34:13:34:26 | ... ...; | maps its iteration variable to another variable | +testFailures +| MissedSelectOpportunity.cs:32:9:36:9 | This foreach loop immediately $@ - consider mapping the sequence explicitly using '.Select(...)'. | Unexpected result: Alert | diff --git a/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.cs b/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.cs index 7b9d35821299..b3575473eab9 100644 --- a/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.cs +++ b/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.cs @@ -174,6 +174,18 @@ public void M12(IEnumerable elements) } } + public void M13(List lst, in int x) + { + // GOOD: Linq can't be used here because the condition uses an in parameter. + foreach (int i in lst) + { + if (i % 2 != x) + continue; + Console.WriteLine(i); + Console.WriteLine((i / 2)); + } + } + public class NonEnumerableClass { public IEnumerator GetEnumerator() => throw null; diff --git a/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.expected b/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.expected index 8b9398292ef2..20c2cece7605 100644 --- a/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.expected +++ b/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.expected @@ -1,5 +1,9 @@ +#select | MissedWhereOpportunity.cs:10:9:16:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:12:17:12:26 | ... != ... | implicitly filters its target sequence | | MissedWhereOpportunity.cs:19:9:26:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:21:17:21:26 | ... == ... | implicitly filters its target sequence | | MissedWhereOpportunity.cs:45:9:52:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:47:17:47:26 | ... == ... | implicitly filters its target sequence | | MissedWhereOpportunity.cs:70:9:76:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:72:17:72:46 | ... == ... | implicitly filters its target sequence | | MissedWhereOpportunity.cs:120:9:126:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:122:17:122:46 | ... == ... | implicitly filters its target sequence | +| MissedWhereOpportunity.cs:180:9:186:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:182:17:182:26 | ... != ... | implicitly filters its target sequence | +testFailures +| MissedWhereOpportunity.cs:180:9:186:9 | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | Unexpected result: Alert | From e8440b77de421f176954a3db39611f1827c428ac Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 9 Sep 2026 13:29:58 +0200 Subject: [PATCH 3/6] C#: Re-factor. --- csharp/ql/lib/Linq/Helpers.qll | 169 +++++++++++++++++++++------------ 1 file changed, 110 insertions(+), 59 deletions(-) diff --git a/csharp/ql/lib/Linq/Helpers.qll b/csharp/ql/lib/Linq/Helpers.qll index 052b5fe6c2e6..43083c90a2d2 100644 --- a/csharp/ql/lib/Linq/Helpers.qll +++ b/csharp/ql/lib/Linq/Helpers.qll @@ -116,6 +116,46 @@ class ForEachStmtEnumerable extends ForEachStmt { } } +private signature predicate linqCandidateSig(Stmt s, Expr e); + +private module LinqFilterOpportunity { + predicate missed(ForEachStmtGenericEnumerable fes, Stmt s) { + s = firstStmt(fes) and + // The linq candidate expression accesses the loop variable, and the + // candidate doesn't access an in, out, or ref parameter. + exists(Expr candidate | linqCandidate(s, candidate) | + fes.getVariable().getAnAccess() = candidate.getAChildExpr*() + ) + } +} + +private module LinqMapOpportunity { + predicate missed(ForEachStmt fes, Stmt s) { + s = firstStmt(fes) and + // The linq candidate (and only the candidate) expression accesses the loop variable and the + // candidate doesn't access an in, out, or ref parameter. + exists(Expr candidate | linqCandidate(s, candidate) | + forex(VariableAccess va | va = fes.getVariable().getAnAccess() | + va = candidate.getAChildExpr*() + ) + ) + } +} + +private predicate linqAllCandidate(Stmt s, Expr e) { + s = + any(IfStmt is | + e = is.getCondition() and + not exists(is.getElse()) and // The then case of the if assigns false to something and breaks out of the loop. + exists(Assignment a, BoolLiteral bl | + a = is.getThen().getAChild*() and + bl = a.getRightOperand() and + bl.toString() = "false" + ) and + is.getThen().getAChild*() instanceof BreakStmt + ) +} + /** * Holds if `foreach` statement `fes` could be converted to a `.All()` call. * That is, the `ForEachStmt` contains a single `if` with a condition that @@ -123,21 +163,20 @@ class ForEachStmtEnumerable extends ForEachStmt { * and `break`s out of the `foreach`. */ predicate missedAllOpportunity(ForEachStmtGenericEnumerable fes) { - exists(IfStmt is | - // The loop contains an if statement with no else case, and nothing else. - is = firstStmt(fes) and - numStmts(fes) = 1 and - not exists(is.getElse()) and - // The if statement accesses the loop variable. - is.getCondition().getAChildExpr*() = fes.getVariable().getAnAccess() and - // The then case of the if assigns false to something and breaks out of the loop. - exists(Assignment a, BoolLiteral bl | - a = is.getThen().getAChild*() and - bl = a.getRightOperand() and - bl.toString() = "false" - ) and - is.getThen().getAChild*() instanceof BreakStmt - ) + // The loop contains an if statement with no else case, and nothing else. + LinqFilterOpportunity::missed(fes, _) and + numStmts(fes) = 1 +} + +private predicate linqCastCandidate(Stmt s, Expr e) { + s = + any(LocalVariableDeclStmt lvds | + exists(CastExpr ce | + ce = lvds.getAVariableDeclExpr().getInitializer() and + e = ce.getExpr() and + e instanceof LocalVariableAccess + ) + ) } /** @@ -147,14 +186,18 @@ predicate missedAllOpportunity(ForEachStmtGenericEnumerable fes) { * local variable declaration statement `s`. */ predicate missedCastOpportunity(ForEachStmtEnumerable fes, LocalVariableDeclStmt s) { - s = firstStmt(fes) and - forex(VariableAccess va | va = fes.getVariable().getAnAccess() | - va = s.getAVariableDeclExpr().getAChildExpr*() - ) and - exists(CastExpr ce | - ce = s.getAVariableDeclExpr().getInitializer() and - ce.getExpr() = fes.getVariable().getAnAccess() - ) + LinqMapOpportunity::missed(fes, s) +} + +private predicate linqOfTypeCandidate(Stmt s, Expr e) { + s = + any(LocalVariableDeclStmt lvds | + exists(AsExpr ae | + ae = lvds.getAVariableDeclExpr().getInitializer() and + e = ae.getExpr() and + e instanceof LocalVariableAccess + ) + ) } /** @@ -164,14 +207,16 @@ predicate missedCastOpportunity(ForEachStmtEnumerable fes, LocalVariableDeclStmt * is a local variable declaration statement `s`. */ predicate missedOfTypeOpportunity(ForEachStmtEnumerable fes, LocalVariableDeclStmt s) { - s = firstStmt(fes) and - forex(VariableAccess va | va = fes.getVariable().getAnAccess() | - va = s.getAVariableDeclExpr().getAChildExpr*() - ) and - exists(AsExpr ae | - ae = s.getAVariableDeclExpr().getInitializer() and - ae.getExpr() = fes.getVariable().getAnAccess() - ) + LinqMapOpportunity::missed(fes, s) +} + +private predicate linqSelectCandidate(Stmt s, Expr e) { + s = + any(LocalVariableDeclStmt lvds | + e = lvds.getAVariableDeclExpr().getInitializer() and + not e instanceof Cast and + not e.getAChildExpr*() instanceof AwaitExpr + ) } /** @@ -182,12 +227,24 @@ predicate missedOfTypeOpportunity(ForEachStmtEnumerable fes, LocalVariableDeclSt * 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 and - not s.getAVariableDeclExpr().getInitializer().getAChildExpr*() instanceof AwaitExpr + LinqMapOpportunity::missed(fes, s) +} + +private predicate linqWhereCandidateCase1(Stmt s, Expr e) { + s = + any(IfStmt is | + e = is.getCondition() and + is.getThen() instanceof ContinueStmt + ) +} + +private predicate linqWhereCandidateCase2(Stmt s, Expr e) { + s = + any(IfStmt is | + e = is.getCondition() and + not exists(is.getElse()) and + not terminatesCallable(is.getThen()) + ) } /** @@ -197,20 +254,21 @@ predicate missedSelectOpportunity(ForEachStmtGenericEnumerable fes, LocalVariabl * else in the loop than the `if`. */ predicate missedWhereOpportunity(ForEachStmtGenericEnumerable fes, IfStmt is) { - // The very first thing the foreach loop does is test its iteration variable. - is = firstStmt(fes) and - exists(VariableAccess va | - va.getTarget() = fes.getVariable() and - va = is.getCondition().getAChildExpr*() - ) and - // It then either (a) continues, or (b) performs the entire body of the loop within the condition. - ( - is.getThen() instanceof ContinueStmt - or - not exists(is.getElse()) and - numStmts(fes) = 1 and - not terminatesCallable(is.getThen()) - ) + // The body of the `if` is a continue. + LinqFilterOpportunity::missed(fes, is) + or + // There's nothing else in the loop than the `if`. + LinqFilterOpportunity::missed(fes, is) and + numStmts(fes) = 1 +} + +private predicate linqFirstOrDefaultCandidate(Stmt s, Expr e) { + s = + any(IfStmt is | + e = is.getCondition() and + not exists(is.getElse()) and + not e.getAChildExpr*() instanceof AwaitExpr + ) } /** @@ -220,15 +278,8 @@ predicate missedWhereOpportunity(ForEachStmtGenericEnumerable fes, IfStmt is) { */ predicate missedFirstOrDefaultOpportunity(ForEachStmtGenericEnumerable fes, IfStmt is) { // The loop only checks whether the current element is the first match. - is = firstStmt(fes) and - not exists(is.getElse()) and + LinqFilterOpportunity::missed(fes, is) and numStmts(fes) = 1 and - // Condition relies on loop variable. - exists(VariableAccess va | - va.getTarget() = fes.getVariable() and - va = is.getCondition().getAChildExpr*() - ) and - not is.getCondition().getAChildExpr*() instanceof AwaitExpr and not fes.isAsync() and not fes.getVariable().isCaptured() and returnsLoopVariable(fes, is.getThen()) and From a61705322e3a981825c29e2abcc0741005ea4456 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 10 Sep 2026 14:12:38 +0200 Subject: [PATCH 4/6] C#: Remove cases where suggestion would result in capturing expressions that access in/out or ref parameters. --- csharp/ql/lib/Linq/Helpers.qll | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/csharp/ql/lib/Linq/Helpers.qll b/csharp/ql/lib/Linq/Helpers.qll index 43083c90a2d2..66ade5db5387 100644 --- a/csharp/ql/lib/Linq/Helpers.qll +++ b/csharp/ql/lib/Linq/Helpers.qll @@ -116,6 +116,14 @@ class ForEachStmtEnumerable extends ForEachStmt { } } +bindingset[e] +private predicate acceptableForLinqCapture(Expr e) { + not exists(ParameterAccess pa, Parameter p | p = pa.getTarget() | + pa = e.getAChildExpr*() and + (p.isOutOrRef() or p.isIn() or p.isReadonlyRef()) + ) +} + private signature predicate linqCandidateSig(Stmt s, Expr e); private module LinqFilterOpportunity { @@ -124,7 +132,8 @@ private module LinqFilterOpportunity { // The linq candidate expression accesses the loop variable, and the // candidate doesn't access an in, out, or ref parameter. exists(Expr candidate | linqCandidate(s, candidate) | - fes.getVariable().getAnAccess() = candidate.getAChildExpr*() + fes.getVariable().getAnAccess() = candidate.getAChildExpr*() and + acceptableForLinqCapture(candidate) ) } } @@ -137,7 +146,8 @@ private module LinqMapOpportunity { exists(Expr candidate | linqCandidate(s, candidate) | forex(VariableAccess va | va = fes.getVariable().getAnAccess() | va = candidate.getAChildExpr*() - ) + ) and + acceptableForLinqCapture(candidate) ) } } From 722a5509aa5f0abb3cf5c334468cbbdc8d631bb8 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 10 Sep 2026 14:14:34 +0200 Subject: [PATCH 5/6] C#: Update test expected output. --- .../Linq/MissedAllOpportunity/MissedAllOpportunity.expected | 4 ---- .../MissedFirstOrDefaultOpportunity.expected | 4 ---- .../MissedSelectOpportunity/MissedSelectOpportunity.expected | 4 ---- .../MissedWhereOpportunity/MissedWhereOpportunity.expected | 4 ---- 4 files changed, 16 deletions(-) diff --git a/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected index 67203c735f1e..b4300f8caf07 100644 --- a/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected +++ b/csharp/ql/test/query-tests/Linq/MissedAllOpportunity/MissedAllOpportunity.expected @@ -1,5 +1 @@ -#select | MissedAllOpportunity.cs:11:9:18:9 | foreach (... ... in ...) ... | This foreach loop looks as if it might be testing whether every sequence element satisfies a predicate - consider using '.All(...)'. | -| MissedAllOpportunity.cs:39:9:46:9 | foreach (... ... in ...) ... | This foreach loop looks as if it might be testing whether every sequence element satisfies a predicate - consider using '.All(...)'. | -testFailures -| MissedAllOpportunity.cs:39:9:46:9 | This foreach loop looks as if it might be testing whether every sequence element satisfies a predicate - consider using '.All(...)'. | Unexpected result: Alert | diff --git a/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.expected b/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.expected index aba54e7933cb..b4cfdff5fdd9 100644 --- a/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.expected +++ b/csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.expected @@ -1,9 +1,5 @@ -#select | MissedFirstOrDefaultOpportunity.cs:10:9:14:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:12:17:12:91 | call to method Equals | predicate | | MissedFirstOrDefaultOpportunity.cs:22:9:28:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:24:17:24:25 | ... > ... | predicate | | MissedFirstOrDefaultOpportunity.cs:36:9:40:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:38:17:38:25 | ... > ... | predicate | | MissedFirstOrDefaultOpportunity.cs:149:9:153:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:151:17:151:32 | ... > ... | predicate | | MissedFirstOrDefaultOpportunity.cs:161:9:165:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:163:17:163:25 | ... > ... | predicate | -| MissedFirstOrDefaultOpportunity.cs:185:9:191:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:187:17:187:25 | ... > ... | predicate | -testFailures -| MissedFirstOrDefaultOpportunity.cs:185:9:191:9 | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | Unexpected result: Alert | diff --git a/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.expected b/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.expected index b84ec32441f4..bc6d464fa3b9 100644 --- a/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.expected +++ b/csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.expected @@ -1,5 +1 @@ -#select | 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 | -| MissedSelectOpportunity.cs:32:9:36:9 | foreach (... ... in ...) ... | This foreach loop immediately $@ - consider mapping the sequence explicitly using '.Select(...)'. | MissedSelectOpportunity.cs:34:13:34:26 | ... ...; | maps its iteration variable to another variable | -testFailures -| MissedSelectOpportunity.cs:32:9:36:9 | This foreach loop immediately $@ - consider mapping the sequence explicitly using '.Select(...)'. | Unexpected result: Alert | diff --git a/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.expected b/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.expected index 20c2cece7605..8b9398292ef2 100644 --- a/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.expected +++ b/csharp/ql/test/query-tests/Linq/MissedWhereOpportunity/MissedWhereOpportunity.expected @@ -1,9 +1,5 @@ -#select | MissedWhereOpportunity.cs:10:9:16:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:12:17:12:26 | ... != ... | implicitly filters its target sequence | | MissedWhereOpportunity.cs:19:9:26:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:21:17:21:26 | ... == ... | implicitly filters its target sequence | | MissedWhereOpportunity.cs:45:9:52:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:47:17:47:26 | ... == ... | implicitly filters its target sequence | | MissedWhereOpportunity.cs:70:9:76:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:72:17:72:46 | ... == ... | implicitly filters its target sequence | | MissedWhereOpportunity.cs:120:9:126:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:122:17:122:46 | ... == ... | implicitly filters its target sequence | -| MissedWhereOpportunity.cs:180:9:186:9 | foreach (... ... in ...) ... | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | MissedWhereOpportunity.cs:182:17:182:26 | ... != ... | implicitly filters its target sequence | -testFailures -| MissedWhereOpportunity.cs:180:9:186:9 | This foreach loop $@ - consider filtering the sequence explicitly using '.Where(...)'. | Unexpected result: Alert | From 2a5ebb85921ffe83ed284fe89d2297765a31f15d Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 10 Sep 2026 14:55:33 +0200 Subject: [PATCH 6/6] C#: Add change note. --- csharp/ql/src/change-notes/2026-09-10-missed-linq-inoutref.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 csharp/ql/src/change-notes/2026-09-10-missed-linq-inoutref.md diff --git a/csharp/ql/src/change-notes/2026-09-10-missed-linq-inoutref.md b/csharp/ql/src/change-notes/2026-09-10-missed-linq-inoutref.md new file mode 100644 index 000000000000..400e0bcd2ed0 --- /dev/null +++ b/csharp/ql/src/change-notes/2026-09-10-missed-linq-inoutref.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `cs/linq/missed-*` queries no longer suggest rewrites that would capture `in`, `out`, or `ref` parameters in a lambda, fixing false-positive results for transformations that would not compile.