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

[baywet]

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]

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

[michaelnebel]

Thank you very much! I was just thinking about the same thing, when I reviewed the other PR about the missed-where false positives 😄 .
I have added some initial comments.

[github-actions]

QHelp previews:

csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.qhelp

Missed opportunity to use FirstOrDefault

Programmers sometimes search a sequence by iterating over each element, testing it, and returning the first element that satisfies the test. If the loop completes without finding a match, the method then returns a default value such as null or default.

Recommendation

This pattern is directly available as the FirstOrDefault method in LINQ. Using the library method makes the search intent explicit and avoids manually spelling out the loop and fallback return.

Example

In this example the method searches a list of operations for the first operation with a matching identifier, returning null if no match is found.

using System;
using System.Collections.Generic;

class MissedFirstOrDefaultOpportunity
{
    public static Operation FindOperation(IEnumerable<Operation> operations, string operationId)
    {
        foreach (var operation in operations)
        {
            if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
                return operation;
        }

        return null;
    }
}

class Operation
{
    public string OperationId { get; set; }
}

The LINQ FirstOrDefault method can express this search more directly.

using System;
using System.Collections.Generic;
using System.Linq;

class MissedFirstOrDefaultOpportunityFix
{
    public static Operation FindOperation(IEnumerable<Operation> operations, string operationId)
    {
        return operations.FirstOrDefault(operation =>
            string.Equals(operation.OperationId, operationId, StringComparison.Ordinal));
    }
}

The following examples should not use FirstOrDefault, because they do more than return the matching element or because the fallback value is not the default value.

using System;
using System.Collections.Generic;

class MissedFirstOrDefaultOpportunityGood
{
    public static Operation FindOperationOrThrow(IEnumerable<Operation> operations, string operationId)
    {
        foreach (var operation in operations)
        {
            if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
                throw new InvalidOperationException("Unexpected operation.");
        }

        return null;
    }

    public static Operation FindReplacementOperation(IEnumerable<Operation> operations, string operationId)
    {
        foreach (var operation in operations)
        {
            if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
                return operation;
        }

        return new Operation();
    }

    public static string FindOperationId(IEnumerable<Operation> operations, string operationId)
    {
        foreach (var operation in operations)
        {
            if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
                return operation.OperationId;
        }

        return null;
    }
}

References

• MSDN: Enumerable.FirstOrDefault Method.

[michaelnebel]

Thank you for addressing the comments!
It looks like the last commit rolls back some of the other changes.
I have opened a PR, which adds some commits on top of these great work you have been doing here. It is the last three commits of the PR seen here. It should solve potential performance problems, update integration test expected output and add a change note.
Feel free to cherry-pick those comments (if you agree with the changes) - then we can run DCA (automated large scale testing) of the new query.

[baywet]

@michaelnebel thanks for putting these together, I've cherry picked all of them. b96bb46 was creating conflicts but I think I got all the changes you intended to apply.

Let me know if you have any additional comments or questions.

[Copilot]

🟡 Changes recommended

Explicit conversions are stripped, causing false positives where replacing the loop changes behavior or fails to compile.

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

Review details

Suppressed comments (2)

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

• This strips explicit/user-defined conversions around null, so a fallback such as return (Result)(Source)null; can be classified as a null default even when the conversion operator returns a non-null value or throws. FirstOrDefault is not equivalent in that case; only implicit compiler conversions should be ignored.

    ret.getExpr().stripCasts() instanceof NullLiteral and

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

• Explicit/user-defined conversions around a default expression are erased here as well. Such a conversion can execute arbitrary code and need not preserve the default value, so reporting the loop as replaceable by FirstOrDefault is a false positive. Preserve explicit casts and strip only implicit conversions.

      defaultValue = ret.getExpr().stripCasts() and

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The query can produce behavior-changing recommendations for converted iteration variables and asynchronous foreach loops.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

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

• Checking only for an awaited condition still allows await foreach loops when the target type also implements IEnumerable<T>. Such a loop uses its asynchronous enumerator, while FirstOrDefault uses the synchronous enumerable, which may produce different elements or effects. Exclude asynchronous foreach statements as well.

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

• elementType is currently the converted iteration-variable type, not the element type whose default FirstOrDefault returns. For example, IEnumerable<int> with foreach (object value in values) and a return null fallback passes this check, although FirstOrDefault returns 0 (then boxes it), so the replacement changes behavior. Compare the fallback against the foreach element type instead, and cover explicit iteration conversions in the tests.

    elementType = fes.getVariable().getType()

• Files reviewed: 12/12 changed files
• Comments generated: 0 new
• Review effort level: Balanced