Merge pull request #22485 from baywet/feat/csharp-missed-firstordefault-opprtunity

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

Author: michaelnebel

csharp/ql/integration-tests/posix/query-suite/csharp-code-quality-extended.qls.expected

@@ -93,6 +93,7 @@ ql/csharp/ql/src/Likely Bugs/UncheckedCastInEquals.ql
 ql/csharp/ql/src/Linq/BadMultipleIteration.ql
 ql/csharp/ql/src/Linq/MissedAllOpportunity.ql
 ql/csharp/ql/src/Linq/MissedCastOpportunity.ql
+ql/csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.ql
 ql/csharp/ql/src/Linq/MissedOfTypeOpportunity.ql
 ql/csharp/ql/src/Linq/MissedSelectOpportunity.ql
 ql/csharp/ql/src/Linq/MissedWhereOpportunity.ql

csharp/ql/integration-tests/posix/query-suite/csharp-code-quality.qls.expected

@@ -55,6 +55,7 @@ ql/csharp/ql/src/Likely Bugs/StringBuilderCharInit.ql
 ql/csharp/ql/src/Likely Bugs/UncheckedCastInEquals.ql
 ql/csharp/ql/src/Linq/MissedAllOpportunity.ql
 ql/csharp/ql/src/Linq/MissedCastOpportunity.ql
+ql/csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.ql
 ql/csharp/ql/src/Linq/MissedOfTypeOpportunity.ql
 ql/csharp/ql/src/Linq/MissedSelectOpportunity.ql
 ql/csharp/ql/src/Linq/MissedWhereOpportunity.ql

csharp/ql/lib/Linq/Helpers.qll

@@ -20,6 +20,36 @@ private int numStmts(ForEachStmt fes) {
   else result = 1
 }
 
+private predicate returnsLoopVariable(ForEachStmt fes, Stmt s) {
+  exists(ReturnStmt ret |
+    ret = s.stripSingletonBlocks() and
+    ret.getExpr().stripImplicit().(VariableAccess).getTarget() = fes.getVariable()
+  )
+}
+
+private predicate hasNullDefault(Type t) { t.isRefType() or t instanceof NullableType }
+
+private predicate returnsDefaultValueAfterForeach(ForEachStmt fes) {
+  exists(BlockStmt enclosingBlock, int i, Type elementType, ReturnStmt ret |
+    enclosingBlock.getStmt(i) = fes and
+    enclosingBlock.getStmt(i + 1) = ret and
+    elementType = fes.getVariable().getType()
+  |
+    ret.getExpr().stripImplicit() instanceof NullLiteral and
+    hasNullDefault(elementType)
+    or
+    exists(DefaultValueExpr defaultValue |
+      defaultValue = ret.getExpr().stripImplicit() and
+      (
+        defaultValue.getType() = elementType
+        or
+        hasNullDefault(elementType) and
+        hasNullDefault(defaultValue.getType())
+      )
+    )
+  )
+}
+
 private predicate terminatesCallable(Stmt s) {
   exists(Stmt stripped | stripped = s.stripSingletonBlocks() |
     stripped instanceof ReturnStmt
@@ -183,6 +213,29 @@ predicate missedWhereOpportunity(ForEachStmtGenericEnumerable fes, IfStmt is) {
   )
 }
 
+/**
+ * Holds if `foreach` statement `fes` could be converted to a `.FirstOrDefault()` call.
+ * That is, the loop contains a single `if` statement that accesses the loop variable,
+ * returns the loop variable when the condition matches, and is followed by a default return.
+ */
+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
+  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
+  fes.getElementType() = fes.getVariable().getType() and
+  returnsDefaultValueAfterForeach(fes)
+}
+
 //#################### CLASSES ####################
 /** A LINQ Any(...) call. */
 class AnyCall extends MethodCall {

csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.cs

@@ -0,0 +1,21 @@
+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; }
+}

csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.qhelp

@@ -0,0 +1,36 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>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 <code>null</code> or <code>default</code>.</p>
+
+</overview>
+<recommendation>
+<p>This pattern is directly available as the <code>FirstOrDefault</code> method in LINQ. Using the
+library method makes the search intent explicit and avoids manually spelling out the loop and
+fallback return.</p>
+
+</recommendation>
+<example>
+<p>In this example the method searches a list of operations for the first operation with a matching
+identifier, returning <code>null</code> if no match is found.</p>
+<sample src="MissedFirstOrDefaultOpportunity.cs" />
+
+<p>The LINQ <code>FirstOrDefault</code> method can express this search more directly.</p>
+<sample src="MissedFirstOrDefaultOpportunityFix.cs" />
+
+<p>The following examples should not use <code>FirstOrDefault</code>, because they do more than
+return the matching element or because the fallback value is not the default value.</p>
+<sample src="MissedFirstOrDefaultOpportunityGood.cs" />
+
+</example>
+<references>
+
+  <li>MSDN: <a href="https://learn.microsoft.com/dotnet/api/system.linq.enumerable.firstordefault">Enumerable.FirstOrDefault Method</a>.</li>
+
+
+</references>
+</qhelp>

csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.ql

@@ -0,0 +1,22 @@
+/**
+ * @name Missed opportunity to use FirstOrDefault
+ * @description The intent of a foreach loop that returns the first sequence element satisfying a predicate, or a default value otherwise,
+ *              can often be better expressed using LINQ's 'FirstOrDefault' method.
+ * @kind problem
+ * @problem.severity recommendation
+ * @precision high
+ * @id cs/linq/missed-firstordefault
+ * @tags quality
+ *       maintainability
+ *       readability
+ *       language-features
+ */
+
+import csharp
+import Linq.Helpers
+
+from ForEachStmtGenericEnumerable fes, IfStmt is
+where missedFirstOrDefaultOpportunity(fes, is)
+select fes,
+  "This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'.",
+  is.getCondition(), "predicate"

csharp/ql/src/Linq/MissedFirstOrDefaultOpportunityFix.cs

@@ -0,0 +1,12 @@
+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));
+    }
+}

csharp/ql/src/Linq/MissedFirstOrDefaultOpportunityGood.cs

@@ -0,0 +1,38 @@
+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;
+    }
+}

csharp/ql/src/change-notes/2026-09-03-missed-firstordefault.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `cs/linq/missed-firstordefault`, that detects `foreach` loops that can be expressed more clearly using LINQ's `FirstOrDefault` method.