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

csharp/ql/lib/Linq/Helpers.qll

@@ -20,6 +20,32 @@ private int numStmts(ForeachStmt fes) {
   else result = 1
 }
 
+private predicate returnsLoopVariable(ForeachStmt fes, Stmt s, ReturnStmt ret) {
+  ret = s.stripSingletonBlocks() and
+  ret.getExpr().stripCasts().(VariableAccess).getTarget() = fes.getVariable()
+}
+
+private predicate hasNullDefault(Type t) { t.isRefType() or t instanceof NullableType }
+
+private predicate returnsDefaultValue(ForeachStmt fes, ReturnStmt ret) {
+  exists(Type elementType |
+    elementType = fes.getVariable().getType()
+  |
+    ret.getExpr().stripCasts() instanceof NullLiteral and
+    hasNullDefault(elementType)
+    or
+    exists(DefaultValueExpr defaultValue |
+      defaultValue = ret.getExpr().stripCasts() and
+      (
+        defaultValue.getType() = elementType
+        or
+        hasNullDefault(elementType) and
+        hasNullDefault(defaultValue.getType())
+      )
+    )
+  )
+}
+
 /** Holds if the type's qualified name is "System.Linq.Enumerable" */
 predicate isEnumerableType(ValueOrRefType t) {
   t.hasFullyQualifiedName("System.Linq", "Enumerable")
@@ -156,6 +182,30 @@ 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
+  exists(VariableAccess va |
+    va.getTarget() = fes.getVariable() and
+    va = is.getCondition().getAChildExpr*()
+  ) and
+  not is.getCondition().getAChildExpr*() instanceof AwaitExpr and
+  exists(ReturnStmt ret, ReturnStmt defaultRet, BlockStmt enclosingBlock, int i |
+    returnsLoopVariable(fes, is.getThen(), ret) and
+    // If no element matches, the method returns the same value that FirstOrDefault would.
+    returnsDefaultValue(fes, defaultRet) and
+    enclosingBlock.getStmt(i) = fes and
+    enclosingBlock.getStmt(i + 1) = defaultRet
+  )
+}
+
 //#################### 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/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.cs

@@ -0,0 +1,177 @@
+using System;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+
+class MissedFirstOrDefaultOpportunity
+{
+    public Operation M1(IEnumerable<Operation> operations, string operationId)
+    {
+        // BAD: Can be replaced with operations.FirstOrDefault(operation => ...).
+        foreach (var operation in operations)
+        {
+            if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
+                return operation;
+        } // $ Alert
+
+        return null;
+    }
+
+    public int M2(IEnumerable<int> values)
+    {
+        // BAD: Can be replaced with values.FirstOrDefault(value => ...).
+        foreach (var value in values)
+        {
+            if (value > 0)
+            {
+                return value;
+            }
+        } // $ Alert
+
+        return default;
+    }
+
+    public int? M3(List<int> values)
+    {
+        // BAD: Can be replaced with values.FirstOrDefault(value => ...).
+        foreach (var value in values)
+        {
+            if (value > 0)
+                return value;
+        } // $ Alert
+
+        return default(int);
+    }
+
+    public Operation M4(IEnumerable<Operation> operations, string operationId)
+    {
+        // GOOD: FirstOrDefault does not throw when a match is found.
+        foreach (var operation in operations)
+        {
+            if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
+                throw new InvalidOperationException();
+        }
+
+        return null;
+    }
+
+    public Operation M5(IEnumerable<Operation> operations, string operationId)
+    {
+        // GOOD: FirstOrDefault would return null/default when no match is found.
+        foreach (var operation in operations)
+        {
+            if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
+                return operation;
+        }
+
+        return new Operation();
+    }
+
+    public string M6(IEnumerable<Operation> operations, string operationId)
+    {
+        // GOOD: FirstOrDefault would return the matching operation, not one of its properties.
+        foreach (var operation in operations)
+        {
+            if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
+                return operation.OperationId;
+        }
+
+        return null;
+    }
+
+    public Operation M7(IEnumerable<Operation> operations, string operationId)
+    {
+        // GOOD: The matched case has an additional side effect.
+        foreach (var operation in operations)
+        {
+            if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
+            {
+                Console.WriteLine(operation.OperationId);
+                return operation;
+            }
+        }
+
+        return null;
+    }
+
+    public async Task<Operation> M8(IEnumerable<Operation> operations, string operationId)
+    {
+        // GOOD: FirstOrDefault does not support an async predicate.
+        foreach (var operation in operations)
+        {
+            if (await IsMatch(operation, operationId))
+                return operation;
+        }
+
+        return null;
+    }
+
+    public Operation M9(IEnumerable<Operation> operations, string operationId)
+    {
+        // GOOD: FirstOrDefault does not have an equivalent for an else branch in the loop.
+        foreach (var operation in operations)
+        {
+            if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
+                return operation;
+            else
+                return null;
+        }
+
+        return null;
+    }
+
+    public object M10(IEnumerable<int> values)
+    {
+        // GOOD: FirstOrDefault would return boxed 0 when no match is found, not null.
+        foreach (var value in values)
+        {
+            if (value > 0)
+                return value;
+        }
+
+        return null;
+    }
+
+    public object M11(IEnumerable<int> values)
+    {
+        // GOOD: FirstOrDefault would return boxed 0 when no match is found, not default(object).
+        foreach (var value in values)
+        {
+            if (value > 0)
+                return value;
+        }
+
+        return default(object);
+    }
+
+    public object M12(IEnumerable<string> values)
+    {
+        // BAD: FirstOrDefault returns null for missing reference-type elements, matching the fallback.
+        foreach (var value in values)
+        {
+            if (value.Length > 0)
+                return value;
+        } // $ Alert
+
+        return null;
+    }
+
+    public object M13(IEnumerable<int> values)
+    {
+        // BAD: FirstOrDefault returns 0 for missing int elements, matching the fallback before boxing.
+        foreach (var value in values)
+        {
+            if (value > 0)
+                return value;
+        } // $ Alert
+
+        return default(int);
+    }
+
+    private static Task<bool> IsMatch(Operation operation, string operationId) =>
+        Task.FromResult(string.Equals(operation.OperationId, operationId, StringComparison.Ordinal));
+}
+
+class Operation
+{
+    public string OperationId { get; set; }
+}

csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.expected

@@ -0,0 +1,5 @@
+| MissedFirstOrDefaultOpportunity.cs:10:9:14:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:12:17:12:91 | call to method Equals | predicate |
+| MissedFirstOrDefaultOpportunity.cs:22:9:28:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:24:17:24:25 | ... > ... | predicate |
+| MissedFirstOrDefaultOpportunity.cs:36:9:40:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:38:17:38:25 | ... > ... | predicate |
+| MissedFirstOrDefaultOpportunity.cs:149:9:153:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:151:17:151:32 | ... > ... | predicate |
+| MissedFirstOrDefaultOpportunity.cs:161:9:165:9 | foreach (... ... in ...) ... | This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'. | MissedFirstOrDefaultOpportunity.cs:163:17:163:25 | ... > ... | predicate |

csharp/ql/test/query-tests/Linq/MissedFirstOrDefaultOpportunity/MissedFirstOrDefaultOpportunity.qlref

@@ -0,0 +1,2 @@
+query: Linq/MissedFirstOrDefaultOpportunity.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql