Skip to content

Commit c97f0f4

Browse files
baywetbaywet
committed
chore: applies review suggestions
1 parent 6744d56 commit c97f0f4

6 files changed

Lines changed: 34 additions & 28 deletions

File tree

‎csharp/ql/lib/Linq/Helpers.qll‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ private predicate returnsLoopVariable(ForeachStmt fes, Stmt s, ReturnStmt ret) {
2828
private predicate hasNullDefault(Type t) { t.isRefType() or t instanceof NullableType }
2929

3030
private predicate returnsDefaultValue(ForeachStmt fes, ReturnStmt ret) {
31-
exists(BlockStmt enclosingBlock, int i, Type elementType |
32-
enclosingBlock.getStmt(i) = fes and
33-
enclosingBlock.getStmt(i + 1) = ret and
31+
exists(Type elementType |
3432
elementType = fes.getVariable().getType()
3533
|
3634
ret.getExpr().stripCasts() instanceof NullLiteral and
@@ -189,9 +187,7 @@ predicate missedWhereOpportunity(ForeachStmtGenericEnumerable fes, IfStmt is) {
189187
* That is, the loop contains a single `if` statement that accesses the loop variable,
190188
* returns the loop variable when the condition matches, and is followed by a default return.
191189
*/
192-
predicate missedFirstOrDefaultOpportunity(
193-
ForeachStmtGenericEnumerable fes, IfStmt is, ReturnStmt ret, ReturnStmt defaultRet
194-
) {
190+
predicate missedFirstOrDefaultOpportunity(ForeachStmtGenericEnumerable fes, IfStmt is) {
195191
// The loop only checks whether the current element is the first match.
196192
is = firstStmt(fes) and
197193
not exists(is.getElse()) and
@@ -201,10 +197,10 @@ predicate missedFirstOrDefaultOpportunity(
201197
va = is.getCondition().getAChildExpr*()
202198
) and
203199
not is.getCondition().getAChildExpr*() instanceof AwaitExpr and
204-
returnsLoopVariable(fes, is.getThen(), ret) and
205-
// If no element matches, the method returns the same value that FirstOrDefault would.
206-
returnsDefaultValue(fes, defaultRet) and
207-
exists(BlockStmt enclosingBlock, int i |
200+
exists(ReturnStmt ret, ReturnStmt defaultRet, BlockStmt enclosingBlock, int i |
201+
returnsLoopVariable(fes, is.getThen(), ret) and
202+
// If no element matches, the method returns the same value that FirstOrDefault would.
203+
returnsDefaultValue(fes, defaultRet) and
208204
enclosingBlock.getStmt(i) = fes and
209205
enclosingBlock.getStmt(i + 1) = defaultRet
210206
)

‎csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1+
using System;
2+
using System.Collections.Generic;
3+
14
class MissedFirstOrDefaultOpportunity
25
{
3-
public static Operation FindOperation(System.Collections.Generic.IEnumerable<Operation> operations, string operationId)
6+
public static Operation FindOperation(IEnumerable<Operation> operations, string operationId)
47
{
58
foreach (var operation in operations)
69
{
7-
if (string.Equals(operation.OperationId, operationId, System.StringComparison.Ordinal))
10+
if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
811
return operation;
912
}
1013

‎csharp/ql/src/Linq/MissedFirstOrDefaultOpportunity.ql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
import csharp
1616
import Linq.Helpers
1717

18-
from ForeachStmtGenericEnumerable fes, IfStmt is, ReturnStmt ret, ReturnStmt defaultRet
19-
where missedFirstOrDefaultOpportunity(fes, is, ret, defaultRet)
18+
from ForeachStmtGenericEnumerable fes, IfStmt is
19+
where missedFirstOrDefaultOpportunity(fes, is)
2020
select fes,
2121
"This foreach loop returns the first sequence element satisfying a $@ - consider finding the element explicitly using '.FirstOrDefault(...)'.",
2222
is.getCondition(), "predicate"

‎csharp/ql/src/Linq/MissedFirstOrDefaultOpportunityFix.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
15
class MissedFirstOrDefaultOpportunityFix
26
{
3-
public static Operation FindOperation(System.Collections.Generic.IEnumerable<Operation> operations, string operationId)
7+
public static Operation FindOperation(IEnumerable<Operation> operations, string operationId)
48
{
59
return operations.FirstOrDefault(operation =>
6-
string.Equals(operation.OperationId, operationId, System.StringComparison.Ordinal));
10+
string.Equals(operation.OperationId, operationId, StringComparison.Ordinal));
711
}
812
}

‎csharp/ql/src/Linq/MissedFirstOrDefaultOpportunityGood.cs‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,35 @@
1+
using System;
2+
using System.Collections.Generic;
3+
14
class MissedFirstOrDefaultOpportunityGood
25
{
3-
public static Operation FindOperationOrThrow(System.Collections.Generic.IEnumerable<Operation> operations, string operationId)
6+
public static Operation FindOperationOrThrow(IEnumerable<Operation> operations, string operationId)
47
{
58
foreach (var operation in operations)
69
{
7-
if (string.Equals(operation.OperationId, operationId, System.StringComparison.Ordinal))
8-
throw new System.InvalidOperationException("Unexpected operation.");
10+
if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
11+
throw new InvalidOperationException("Unexpected operation.");
912
}
1013

1114
return null;
1215
}
1316

14-
public static Operation FindReplacementOperation(System.Collections.Generic.IEnumerable<Operation> operations, string operationId)
17+
public static Operation FindReplacementOperation(IEnumerable<Operation> operations, string operationId)
1518
{
1619
foreach (var operation in operations)
1720
{
18-
if (string.Equals(operation.OperationId, operationId, System.StringComparison.Ordinal))
21+
if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
1922
return operation;
2023
}
2124

2225
return new Operation();
2326
}
2427

25-
public static string FindOperationId(System.Collections.Generic.IEnumerable<Operation> operations, string operationId)
28+
public static string FindOperationId(IEnumerable<Operation> operations, string operationId)
2629
{
2730
foreach (var operation in operations)
2831
{
29-
if (string.Equals(operation.OperationId, operationId, System.StringComparison.Ordinal))
32+
if (string.Equals(operation.OperationId, operationId, StringComparison.Ordinal))
3033
return operation.OperationId;
3134
}
3235

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
| 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 |
2-
| 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 |
3-
| 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 |
4-
| 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 |
5-
| 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 |
1+
| 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 |
2+
| 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 |
3+
| 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 |
4+
| 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 |
5+
| 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 |

0 commit comments

Comments
 (0)