fix: handle non null default values in the rule

Author: baywet

csharp/ql/lib/Linq/Helpers.qll

@@ -25,10 +25,24 @@ private predicate returnsLoopVariable(ForeachStmt fes, Stmt s, ReturnStmt ret) {
   ret.getExpr().stripCasts().(VariableAccess).getTarget() = fes.getVariable()
 }
 
-private predicate returnsDefaultValue(ReturnStmt ret) {
-  ret.getExpr().stripCasts() instanceof NullLiteral
-  or
-  ret.getExpr().stripCasts() instanceof DefaultValueExpr
+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" */
@@ -186,7 +200,7 @@ predicate missedFirstOrDefaultOpportunity(
   not is.getCondition().getAChildExpr*() instanceof AwaitExpr and
   returnsLoopVariable(fes, is.getThen(), ret) and
   // If no element matches, the method returns the same value that FirstOrDefault would.
-  returnsDefaultValue(defaultRet) and
+  returnsDefaultValue(fes, defaultRet) and
   exists(BlockStmt enclosingBlock, int i |
     enclosingBlock.getStmt(i) = fes and
     enclosingBlock.getStmt(i + 1) = defaultRet

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

@@ -119,6 +119,54 @@ public Operation M9(IEnumerable<Operation> operations, string operationId)
         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));
 }

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

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