Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: handle non null default values in the rule
  • Loading branch information
baywet committed Sep 1, 2026
commit 41a9a16b757559819b29c2a8cc0218e1c44cb671
24 changes: 19 additions & 5 deletions csharp/ql/lib/Linq/Helpers.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
baywet marked this conversation as resolved.
Outdated
hasNullDefault(elementType)
or
exists(DefaultValueExpr defaultValue |
defaultValue = ret.getExpr().stripCasts() and
Comment thread
baywet marked this conversation as resolved.
Outdated
(
defaultValue.getType() = elementType
or
hasNullDefault(elementType) and
hasNullDefault(defaultValue.getType())
)
)
)
Comment thread
baywet marked this conversation as resolved.
Outdated
}

/** Holds if the type's qualified name is "System.Linq.Enumerable" */
Expand Down Expand Up @@ -186,7 +200,7 @@ predicate missedFirstOrDefaultOpportunity(
not is.getCondition().getAChildExpr*() instanceof AwaitExpr and
Comment thread
baywet marked this conversation as resolved.
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 |