Skip to content

Commit e8440b7

Browse files
committed
C#: Re-factor.
1 parent e2ebb0d commit e8440b7

1 file changed

Lines changed: 110 additions & 59 deletions

File tree

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

Lines changed: 110 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -116,28 +116,67 @@ class ForEachStmtEnumerable extends ForEachStmt {
116116
}
117117
}
118118

119+
private signature predicate linqCandidateSig(Stmt s, Expr e);
120+
121+
private module LinqFilterOpportunity<linqCandidateSig/2 linqCandidate> {
122+
predicate missed(ForEachStmtGenericEnumerable fes, Stmt s) {
123+
s = firstStmt(fes) and
124+
// The linq candidate expression accesses the loop variable, and the
125+
// candidate doesn't access an in, out, or ref parameter.
126+
exists(Expr candidate | linqCandidate(s, candidate) |
127+
fes.getVariable().getAnAccess() = candidate.getAChildExpr*()
128+
)
129+
}
130+
}
131+
132+
private module LinqMapOpportunity<linqCandidateSig/2 linqCandidate> {
133+
predicate missed(ForEachStmt fes, Stmt s) {
134+
s = firstStmt(fes) and
135+
// The linq candidate (and only the candidate) expression accesses the loop variable and the
136+
// candidate doesn't access an in, out, or ref parameter.
137+
exists(Expr candidate | linqCandidate(s, candidate) |
138+
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
139+
va = candidate.getAChildExpr*()
140+
)
141+
)
142+
}
143+
}
144+
145+
private predicate linqAllCandidate(Stmt s, Expr e) {
146+
s =
147+
any(IfStmt is |
148+
e = is.getCondition() and
149+
not exists(is.getElse()) and // The then case of the if assigns false to something and breaks out of the loop.
150+
exists(Assignment a, BoolLiteral bl |
151+
a = is.getThen().getAChild*() and
152+
bl = a.getRightOperand() and
153+
bl.toString() = "false"
154+
) and
155+
is.getThen().getAChild*() instanceof BreakStmt
156+
)
157+
}
158+
119159
/**
120160
* Holds if `foreach` statement `fes` could be converted to a `.All()` call.
121161
* That is, the `ForEachStmt` contains a single `if` with a condition that
122162
* accesses the loop variable and with a body that assigns `false` to a variable
123163
* and `break`s out of the `foreach`.
124164
*/
125165
predicate missedAllOpportunity(ForEachStmtGenericEnumerable fes) {
126-
exists(IfStmt is |
127-
// The loop contains an if statement with no else case, and nothing else.
128-
is = firstStmt(fes) and
129-
numStmts(fes) = 1 and
130-
not exists(is.getElse()) and
131-
// The if statement accesses the loop variable.
132-
is.getCondition().getAChildExpr*() = fes.getVariable().getAnAccess() and
133-
// The then case of the if assigns false to something and breaks out of the loop.
134-
exists(Assignment a, BoolLiteral bl |
135-
a = is.getThen().getAChild*() and
136-
bl = a.getRightOperand() and
137-
bl.toString() = "false"
138-
) and
139-
is.getThen().getAChild*() instanceof BreakStmt
140-
)
166+
// The loop contains an if statement with no else case, and nothing else.
167+
LinqFilterOpportunity<linqAllCandidate/2>::missed(fes, _) and
168+
numStmts(fes) = 1
169+
}
170+
171+
private predicate linqCastCandidate(Stmt s, Expr e) {
172+
s =
173+
any(LocalVariableDeclStmt lvds |
174+
exists(CastExpr ce |
175+
ce = lvds.getAVariableDeclExpr().getInitializer() and
176+
e = ce.getExpr() and
177+
e instanceof LocalVariableAccess
178+
)
179+
)
141180
}
142181

143182
/**
@@ -147,14 +186,18 @@ predicate missedAllOpportunity(ForEachStmtGenericEnumerable fes) {
147186
* local variable declaration statement `s`.
148187
*/
149188
predicate missedCastOpportunity(ForEachStmtEnumerable fes, LocalVariableDeclStmt s) {
150-
s = firstStmt(fes) and
151-
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
152-
va = s.getAVariableDeclExpr().getAChildExpr*()
153-
) and
154-
exists(CastExpr ce |
155-
ce = s.getAVariableDeclExpr().getInitializer() and
156-
ce.getExpr() = fes.getVariable().getAnAccess()
157-
)
189+
LinqMapOpportunity<linqCastCandidate/2>::missed(fes, s)
190+
}
191+
192+
private predicate linqOfTypeCandidate(Stmt s, Expr e) {
193+
s =
194+
any(LocalVariableDeclStmt lvds |
195+
exists(AsExpr ae |
196+
ae = lvds.getAVariableDeclExpr().getInitializer() and
197+
e = ae.getExpr() and
198+
e instanceof LocalVariableAccess
199+
)
200+
)
158201
}
159202

160203
/**
@@ -164,14 +207,16 @@ predicate missedCastOpportunity(ForEachStmtEnumerable fes, LocalVariableDeclStmt
164207
* is a local variable declaration statement `s`.
165208
*/
166209
predicate missedOfTypeOpportunity(ForEachStmtEnumerable fes, LocalVariableDeclStmt s) {
167-
s = firstStmt(fes) and
168-
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
169-
va = s.getAVariableDeclExpr().getAChildExpr*()
170-
) and
171-
exists(AsExpr ae |
172-
ae = s.getAVariableDeclExpr().getInitializer() and
173-
ae.getExpr() = fes.getVariable().getAnAccess()
174-
)
210+
LinqMapOpportunity<linqOfTypeCandidate/2>::missed(fes, s)
211+
}
212+
213+
private predicate linqSelectCandidate(Stmt s, Expr e) {
214+
s =
215+
any(LocalVariableDeclStmt lvds |
216+
e = lvds.getAVariableDeclExpr().getInitializer() and
217+
not e instanceof Cast and
218+
not e.getAChildExpr*() instanceof AwaitExpr
219+
)
175220
}
176221

177222
/**
@@ -182,12 +227,24 @@ predicate missedOfTypeOpportunity(ForEachStmtEnumerable fes, LocalVariableDeclSt
182227
* contain an `await` expression (since `Select` does not support async lambdas).
183228
*/
184229
predicate missedSelectOpportunity(ForEachStmtGenericEnumerable fes, LocalVariableDeclStmt s) {
185-
s = firstStmt(fes) and
186-
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
187-
va = s.getAVariableDeclExpr().getAChildExpr*()
188-
) and
189-
not s.getAVariableDeclExpr().getInitializer() instanceof Cast and
190-
not s.getAVariableDeclExpr().getInitializer().getAChildExpr*() instanceof AwaitExpr
230+
LinqMapOpportunity<linqSelectCandidate/2>::missed(fes, s)
231+
}
232+
233+
private predicate linqWhereCandidateCase1(Stmt s, Expr e) {
234+
s =
235+
any(IfStmt is |
236+
e = is.getCondition() and
237+
is.getThen() instanceof ContinueStmt
238+
)
239+
}
240+
241+
private predicate linqWhereCandidateCase2(Stmt s, Expr e) {
242+
s =
243+
any(IfStmt is |
244+
e = is.getCondition() and
245+
not exists(is.getElse()) and
246+
not terminatesCallable(is.getThen())
247+
)
191248
}
192249

193250
/**
@@ -197,20 +254,21 @@ predicate missedSelectOpportunity(ForEachStmtGenericEnumerable fes, LocalVariabl
197254
* else in the loop than the `if`.
198255
*/
199256
predicate missedWhereOpportunity(ForEachStmtGenericEnumerable fes, IfStmt is) {
200-
// The very first thing the foreach loop does is test its iteration variable.
201-
is = firstStmt(fes) and
202-
exists(VariableAccess va |
203-
va.getTarget() = fes.getVariable() and
204-
va = is.getCondition().getAChildExpr*()
205-
) and
206-
// It then either (a) continues, or (b) performs the entire body of the loop within the condition.
207-
(
208-
is.getThen() instanceof ContinueStmt
209-
or
210-
not exists(is.getElse()) and
211-
numStmts(fes) = 1 and
212-
not terminatesCallable(is.getThen())
213-
)
257+
// The body of the `if` is a continue.
258+
LinqFilterOpportunity<linqWhereCandidateCase1/2>::missed(fes, is)
259+
or
260+
// There's nothing else in the loop than the `if`.
261+
LinqFilterOpportunity<linqWhereCandidateCase2/2>::missed(fes, is) and
262+
numStmts(fes) = 1
263+
}
264+
265+
private predicate linqFirstOrDefaultCandidate(Stmt s, Expr e) {
266+
s =
267+
any(IfStmt is |
268+
e = is.getCondition() and
269+
not exists(is.getElse()) and
270+
not e.getAChildExpr*() instanceof AwaitExpr
271+
)
214272
}
215273

216274
/**
@@ -220,15 +278,8 @@ predicate missedWhereOpportunity(ForEachStmtGenericEnumerable fes, IfStmt is) {
220278
*/
221279
predicate missedFirstOrDefaultOpportunity(ForEachStmtGenericEnumerable fes, IfStmt is) {
222280
// The loop only checks whether the current element is the first match.
223-
is = firstStmt(fes) and
224-
not exists(is.getElse()) and
281+
LinqFilterOpportunity<linqFirstOrDefaultCandidate/2>::missed(fes, is) and
225282
numStmts(fes) = 1 and
226-
// Condition relies on loop variable.
227-
exists(VariableAccess va |
228-
va.getTarget() = fes.getVariable() and
229-
va = is.getCondition().getAChildExpr*()
230-
) and
231-
not is.getCondition().getAChildExpr*() instanceof AwaitExpr and
232283
not fes.isAsync() and
233284
not fes.getVariable().isCaptured() and
234285
returnsLoopVariable(fes, is.getThen()) and

0 commit comments

Comments
 (0)