Skip to content

Fix false positive in MissedSelectOpportunity when foreach body uses await#21708

Open
Copilot wants to merge 3 commits intomainfrom
copilot/fix-missed-opportunity-to-use-select
Open

Fix false positive in MissedSelectOpportunity when foreach body uses await#21708
Copilot wants to merge 3 commits intomainfrom
copilot/fix-missed-opportunity-to-use-select

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 14, 2026

The cs/linq/missed-select query incorrectly flagged foreach loops whose first statement used await — loops that cannot be refactored to .Select(...) since LINQ's Select operates on IEnumerable<T> and does not support async lambdas.

// Previously flagged — cannot use Select because of await
foreach (var locator in contactSelectors.Select(s => Page.Locator(s)))
{
    var elements = await locator.CountAsync(); // ← await here blocks Select refactor
    if (elements > 0) { ... }
}

Changes

  • csharp/ql/lib/Linq/Helpers.qll: Added exclusion to missedSelectOpportunity — the predicate no longer holds when the first statement's initializer contains an AwaitExpr. Updated doc comment accordingly.

  • csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/: New test covering:

    • BAD: plain variable mapping (should be flagged)
    • GOOD: variable mapping whose initializer uses await (should not be flagged)

Copilot AI changed the title [WIP] Fix false positive for missed opportunity to use Select with C# Fix false positive in MissedSelectOpportunity when foreach body uses await Apr 14, 2026
Copilot AI requested a review from hvitved April 14, 2026 08:19
@github-actions github-actions bot added the C# label Apr 14, 2026
@@ -0,0 +1 @@
Linq/MissedSelectOpportunity.ql
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use inline test expectations, i.e.

postprocess:
 - utils/test/InlineExpectationsTestQuery.ql

This also means adding // $ Alert marker comment in the test file.

@hvitved hvitved marked this pull request as ready for review April 14, 2026 12:43
@hvitved hvitved requested a review from a team as a code owner April 14, 2026 12:43
Copilot AI review requested due to automatic review settings April 14, 2026 12:43
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Apr 14, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts the cs/linq/missed-select query helper logic to avoid flagging foreach loops whose first statement’s initializer contains an await, since these loops can’t be straightforwardly refactored into a synchronous .Select(...) mapping.

Changes:

  • Updated missedSelectOpportunity to exclude cases where the first statement’s initializer contains an AwaitExpr.
  • Added a new query-test folder for Linq/MissedSelectOpportunity.ql covering one flagged (“BAD”) and one non-flagged (“GOOD”) scenario.
  • Added test harness files (options, .qlref, .expected) for the new test.
Show a summary per file
File Description
csharp/ql/lib/Linq/Helpers.qll Refines missedSelectOpportunity to ignore await-based initializers; updates the predicate’s doc comment.
csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/options Adds extractor options/stub project loading for the new test.
csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.qlref Adds a .qlref pointing at Linq/MissedSelectOpportunity.ql with inline-expectations postprocessing.
csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.expected Captures the expected single result (only the “BAD” case).
csharp/ql/test/query-tests/Linq/MissedSelectOpportunity/MissedSelectOpportunity.cs Adds “BAD” and “GOOD” C# examples to validate the updated behavior.

Copilot's findings

  • Files reviewed: 5/5 changed files
  • Comments generated: 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C# no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False positive: Missed opportunity to use Select with C#

4 participants