Skip to content

feat(csharp): adds a missed first or default opprtunity rule - #22485

Open
baywet wants to merge 2 commits into
github:mainfrom
baywet:feat/csharp-missed-firstordefault-opprtunity
Open

feat(csharp): adds a missed first or default opprtunity rule#22485
baywet wants to merge 2 commits into
github:mainfrom
baywet:feat/csharp-missed-firstordefault-opprtunity

Conversation

@baywet

@baywet baywet commented Sep 1, 2026

Copy link
Copy Markdown

there are already rules for missed where/all/oftype/select/cast opportunities. It only makes sense to have FirstOrDefault which is a very common use case. Related #22484

Copilot AI balanced review requested due to automatic review settings September 1, 2026 15:04
@baywet
baywet requested a review from a team as a code owner September 1, 2026 15:04

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.

🟡 Changes recommended

The detection currently produces behavior-changing recommendations for converted iteration types, incompatible defaults, and asynchronous enumeration.

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

Pull request overview

Adds a C# quality query detecting loops that can use LINQ’s FirstOrDefault.

Changes:

  • Implements and registers the new query.
  • Adds documentation, examples, and tests.
  • Extends shared LINQ detection helpers.
File summaries
File Description
csharp/ql/lib/Linq/Helpers.qll Adds detection logic.
csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.ql Defines the query.
csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.qhelp Documents the recommendation.
csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.cs Provides a flagged example.
csharp/ql/src/Linq/MissedFirstOrDefaultOpportunityFix.cs Provides the recommended fix.
csharp/ql/src/Linq/MissedFirstOrDefaultOpportunityGood.cs Provides non-alerting examples.
csharp/ql/src/codeql-suites/csharp-security-and-quality.qls Enables the query in the quality suite.
csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.cs Adds test cases.
csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.qlref Configures the query test.
csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.expected Records expected results.
Review details

Suppressed comments (2)

csharp/ql/lib/Linq/Helpers.qll:184

  • This only checks that the condition accesses the iteration variable, but foreach may implicitly convert each source element to a different variable type. For example, foreach (string value in IEnumerable<object>) can match here, while FirstOrDefault receives and returns object, so the proposed replacement neither exposes string members in the predicate nor returns the same type. Require an identity conversion between fes.getElementType() and the iteration-variable type (or account for the required Cast<T>()).
  exists(VariableAccess va |
    va.getTarget() = fes.getVariable() and
    va = is.getCondition().getAChildExpr*()

csharp/ql/lib/Linq/Helpers.qll:179

  • Exclude asynchronous foreach statements here. A type can implement both IAsyncEnumerable<T> and IEnumerable<T>, so it satisfies ForeachStmtGenericEnumerable, but replacing its await foreach with FirstOrDefault switches to synchronous enumeration and can change behavior.
  is = firstStmt(fes) and
  • Files reviewed: 10/10 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 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
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.

2 participants