Skip to content

C#: Don't suggest LINQ re-writes that would capture in, out, or ref parameters in a lambda. - #22541

Open
michaelnebel wants to merge 6 commits into
github:mainfrom
michaelnebel:csharp/linqinoutret
Open

C#: Don't suggest LINQ re-writes that would capture in, out, or ref parameters in a lambda.#22541
michaelnebel wants to merge 6 commits into
github:mainfrom
michaelnebel:csharp/linqinoutret

Conversation

@michaelnebel

@michaelnebel michaelnebel commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

In this PR we

  • Re-factor parts of the Helpers.qll for the cs/linq/missed-* queries to enable sharing of logic.
  • Remove FPs where a LINQ re-write that would capture an in, out or ref parameter is suggested.

Comment thread csharp/ql/lib/Linq/Helpers.qll Fixed

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

The shared check omits separately modeled ref readonly parameters, allowing another uncompilable lambda rewrite.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Medium severity · 1 Low severity

New issues introduced by this change (2)
Severity Finding
Medium severity csharp/​ql/​lib/​Linq/​Helpers.qllref readonly parameters are modeled separately by Parameter.isReadonlyRef()
Low severity csharp/​ql/​lib/​Linq/​Helpers.qll — Correct the typo and subject-verb agreement in this comment.
What changed in this PR

Refactors shared LINQ opportunity detection and suppresses invalid lambda rewrites involving by-reference parameters.

Changes:

  • Centralizes candidate-expression validation in reusable QL modules.
  • Adds regression coverage and a change note.
File Description
csharp/​ql/​lib/​Linq/​Helpers.qll Refactors detection and excludes by-reference captures.
csharp/​ql/​src/​change-notes/​2026-09-10-missed-linq-inoutref.md Documents the analysis improvement.
csharp/​ql/​test/​query-tests/​Linq/​MissedAllOpportunity/​MissedAllOpportunity.cs Adds All regression cases.
csharp/​ql/​test/​query-tests/​Linq/​MissedAllOpportunity/​MissedAllOpportunity.expected Records expected All result.
csharp/​ql/​test/​query-tests/​Linq/​MissedAllOpportunity/​MissedAllOpportunity.qlref Configures the All query test.
csharp/​ql/​test/​query-tests/​Linq/​MissedAllOpportunity/​options Configures All test extraction.
csharp/​ql/​test/​query-tests/​Linq/​MissedFirstOrDefaultOpportunity/​MissedFirstOrDefaultOpportunity.cs Tests ref-parameter suppression.
csharp/​ql/​test/​query-tests/​Linq/​MissedSelectOpportunity/​MissedSelectOpportunity.cs Tests out-parameter suppression.
csharp/​ql/​test/​query-tests/​Linq/​MissedWhereOpportunity/​MissedWhereOpportunity.cs Tests in-parameter suppression.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread csharp/ql/lib/Linq/Helpers.qll Outdated
Comment thread csharp/ql/lib/Linq/Helpers.qll Outdated
@michaelnebel
michaelnebel force-pushed the csharp/linqinoutret branch 2 times, most recently from 65d7e71 to 2a43fc9 Compare September 11, 2026 08:25
@michaelnebel
michaelnebel marked this pull request as ready for review September 11, 2026 13:25
@michaelnebel
michaelnebel requested a review from a team as a code owner September 11, 2026 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants