Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion csharp/ql/consistency-queries/SsaConsistency.ql
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ query predicate localDeclWithSsaDef(LocalVariableDeclExpr d) {
exists(SsaExplicitWrite def |
d = def.getDefinition().(AssignableDefinitions::LocalVariableDefinition).getDeclaration()
|
not d = any(ForeachStmt fs).getVariableDeclExpr() and
not d = any(ForEachStmt fs).getVariableDeclExpr() and
not d = any(SpecificCatchClause scc).getVariableDeclExpr() and
not d.getVariable().getType() instanceof Struct and
not d instanceof PatternExpr and
Expand Down
30 changes: 18 additions & 12 deletions csharp/ql/lib/Linq/Helpers.qll
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ private import semmle.code.csharp.frameworks.system.collections.Generic as Gener
private import semmle.code.csharp.frameworks.system.Collections as Collections

//#################### PREDICATES ####################
private Stmt firstStmt(ForeachStmt fes) {
private Stmt firstStmt(ForEachStmt fes) {
if fes.getBody() instanceof BlockStmt
then result = fes.getBody().(BlockStmt).getStmt(0)
else result = fes.getBody()
}

private int numStmts(ForeachStmt fes) {
private int numStmts(ForEachStmt fes) {
if fes.getBody() instanceof BlockStmt
then result = count(fes.getBody().(BlockStmt).getAStmt())
else result = 1
Expand All @@ -33,12 +33,15 @@ predicate isIEnumerableType(ValueOrRefType t) {
)
}

/** DEPRECATED: Use `ForEachStmtGenericEnumerable` instead. */
deprecated class ForeachStmtGenericEnumerable = ForEachStmtGenericEnumerable;

/**
* A class of foreach statements where the iterable expression
* supports the use of the LINQ extension methods on `IEnumerable<T>`.
*/
class ForeachStmtGenericEnumerable extends ForeachStmt {
ForeachStmtGenericEnumerable() {
class ForEachStmtGenericEnumerable extends ForEachStmt {
ForEachStmtGenericEnumerable() {
exists(ValueOrRefType t | t = this.getIterableExpr().getType() |
t.getABaseType*().getUnboundDeclaration() instanceof
GenericCollections::SystemCollectionsGenericIEnumerableTInterface or
Expand All @@ -47,12 +50,15 @@ class ForeachStmtGenericEnumerable extends ForeachStmt {
}
}

/** DEPRECATED: Use `ForEachStmtEnumerable` instead. */
deprecated class ForeachStmtEnumerable = ForEachStmtEnumerable;

/**
* A class of foreach statements where the iterable expression
* supports the use of the LINQ extension methods on `IEnumerable`.
*/
class ForeachStmtEnumerable extends ForeachStmt {
ForeachStmtEnumerable() {
class ForEachStmtEnumerable extends ForEachStmt {
ForEachStmtEnumerable() {
exists(ValueOrRefType t | t = this.getIterableExpr().getType() |
t.getABaseType*() instanceof Collections::SystemCollectionsIEnumerableInterface or
t.(ArrayType).getRank() = 1
Expand All @@ -62,11 +68,11 @@ class ForeachStmtEnumerable extends ForeachStmt {

/**
* Holds if `foreach` statement `fes` could be converted to a `.All()` call.
* That is, the `ForeachStmt` contains a single `if` with a condition that
* That is, the `ForEachStmt` contains a single `if` with a condition that
* accesses the loop variable and with a body that assigns `false` to a variable
* and `break`s out of the `foreach`.
*/
predicate missedAllOpportunity(ForeachStmtGenericEnumerable fes) {
predicate missedAllOpportunity(ForEachStmtGenericEnumerable fes) {
exists(IfStmt is |
// The loop contains an if statement with no else case, and nothing else.
is = firstStmt(fes) and
Expand All @@ -90,7 +96,7 @@ predicate missedAllOpportunity(ForeachStmtGenericEnumerable fes) {
* block, the access is a cast, and the first statement is a
* local variable declaration statement `s`.
*/
predicate missedCastOpportunity(ForeachStmtEnumerable fes, LocalVariableDeclStmt s) {
predicate missedCastOpportunity(ForEachStmtEnumerable fes, LocalVariableDeclStmt s) {
s = firstStmt(fes) and
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
va = s.getAVariableDeclExpr().getAChildExpr*()
Expand All @@ -107,7 +113,7 @@ predicate missedCastOpportunity(ForeachStmtEnumerable fes, LocalVariableDeclStmt
* block, the access is a cast with the `as` operator, and the first statement
* is a local variable declaration statement `s`.
*/
predicate missedOfTypeOpportunity(ForeachStmtEnumerable fes, LocalVariableDeclStmt s) {
predicate missedOfTypeOpportunity(ForEachStmtEnumerable fes, LocalVariableDeclStmt s) {
s = firstStmt(fes) and
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
va = s.getAVariableDeclExpr().getAChildExpr*()
Expand All @@ -125,7 +131,7 @@ predicate missedOfTypeOpportunity(ForeachStmtEnumerable fes, LocalVariableDeclSt
* local variable declaration statement `s`, and the initializer does not
* contain an `await` expression (since `Select` does not support async lambdas).
*/
predicate missedSelectOpportunity(ForeachStmtGenericEnumerable fes, LocalVariableDeclStmt s) {
predicate missedSelectOpportunity(ForEachStmtGenericEnumerable fes, LocalVariableDeclStmt s) {
s = firstStmt(fes) and
forex(VariableAccess va | va = fes.getVariable().getAnAccess() |
va = s.getAVariableDeclExpr().getAChildExpr*()
Expand All @@ -140,7 +146,7 @@ predicate missedSelectOpportunity(ForeachStmtGenericEnumerable fes, LocalVariabl
* variable, and the body of the `if` is either a `continue` or there's nothing
* else in the loop than the `if`.
*/
predicate missedWhereOpportunity(ForeachStmtGenericEnumerable fes, IfStmt is) {
predicate missedWhereOpportunity(ForEachStmtGenericEnumerable fes, IfStmt is) {
// The very first thing the foreach loop does is test its iteration variable.
is = firstStmt(fes) and
exists(VariableAccess va |
Expand Down
9 changes: 6 additions & 3 deletions csharp/ql/lib/semmle/code/csharp/Stmt.qll
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ class DefaultCase extends CaseStmt, LabeledStmt {
*
* Either a `while` statement (`WhileStmt`), a `do`-`while` statement
* (`DoStmt`), a `for` statement (`ForStmt`), or a `foreach` statement
* (`ForeachStmt`).
* (`ForEachStmt`).
*/
class LoopStmt extends Stmt, @loop_stmt {
/** Gets the body of this loop statement. */
Expand Down Expand Up @@ -422,6 +422,9 @@ class ForStmt extends LoopStmt, @for_stmt {
override string getAPrimaryQlClass() { result = "ForStmt" }
}

/** DEPRECATED: Use `ForEachStmt` instead. */
deprecated class ForeachStmt = ForEachStmt;

/**
* A `foreach` loop, for example
*
Expand All @@ -431,7 +434,7 @@ class ForStmt extends LoopStmt, @for_stmt {
* }
* ```
*/
class ForeachStmt extends LoopStmt, @foreach_stmt {
class ForEachStmt extends LoopStmt, @foreach_stmt {
/**
* Gets the local variable of this `foreach` loop, if any.
*
Expand Down Expand Up @@ -564,7 +567,7 @@ class ForeachStmt extends LoopStmt, @foreach_stmt {

override string toString() { result = "foreach (... ... in ...) ..." }

override string getAPrimaryQlClass() { result = "ForeachStmt" }
override string getAPrimaryQlClass() { result = "ForEachStmt" }
Comment thread
aschackmull marked this conversation as resolved.
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@ module Ast implements AstSig<Location> {
AstNode getUpdate(int index) { result = super.getUpdate(index) }
}

final private class FinalForeachStmt = CS::ForeachStmt;
final private class FinalForEachStmt = CS::ForEachStmt;

class ForEachStmt extends FinalForeachStmt {
class ForEachStmt extends FinalForEachStmt {
Expr getVariable() {
result = this.getVariableDeclExpr() or result = this.getVariableDeclTuple()
}
Expand Down
4 changes: 2 additions & 2 deletions csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ private predicate nonNullDef(SsaExplicitWrite def) {
any(AssignableDefinitions::LocalVariableDefinition d |
d.getExpr() = any(SpecificCatchClause scc).getVariableDeclExpr()
or
d.getExpr() = any(ForeachStmt fs).getAVariableDeclExpr()
d.getExpr() = any(ForEachStmt fs).getAVariableDeclExpr()
)
)
}
Expand Down Expand Up @@ -306,7 +306,7 @@ class Dereference extends G::DereferenceableExpr {
or
this = any(LockStmt stmt).getExpr()
or
this = any(ForeachStmt stmt).getIterableExpr()
this = any(ForEachStmt stmt).getIterableExpr()
or
exists(ExtensionMethodCall emc, Parameter p |
this = emc.getArgumentForParameter(p) and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2207,7 +2207,7 @@ private predicate readContentStep(Node node1, Content c, Node node2) {
c instanceof ElementContent
or
exists(
ForeachStmt fs, SsaExplicitWrite def, AssignableDefinitions::LocalVariableDefinition defTo
ForEachStmt fs, SsaExplicitWrite def, AssignableDefinitions::LocalVariableDefinition defTo
|
node1.asExpr() = fs.getIterableExpr() and
defTo.getDeclaration() = fs.getVariableDeclExpr() and
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ class QualifiableExpr extends Expr, @qualifiable_expr {
private Expr getAnAssignOrForeachChild() {
result = any(AssignExpr e).getLeftOperand()
or
result = any(ForeachStmt fs).getVariableDeclTuple()
result = any(ForEachStmt fs).getVariableDeclTuple()
or
result = getAnAssignOrForeachChild().getAChildExpr()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ module DisposeCallOnLocalIDisposableConfig implements DataFlow::ConfigSig {
exists(UsingStmt us | us.getAnExpr() = e)
or
// Foreach calls Dispose
exists(ForeachStmt fs | fs.getIterableExpr() = e)
exists(ForEachStmt fs | fs.getIterableExpr() = e)
or
// As are disposables on which the Dispose method is called explicitly
exists(MethodCall mc |
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/src/Dead Code/DeadStoreOfLocal.ql
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class RelevantDefinition extends AssignableDefinition {
any(LocalVariableDeclExpr lvde |
lvde = any(SpecificCatchClause scc).getVariableDeclExpr()
or
lvde = any(ForeachStmt fs).getVariableDeclExpr() and
lvde = any(ForEachStmt fs).getVariableDeclExpr() and
not lvde.getName() = "_"
)
or
Expand Down
14 changes: 7 additions & 7 deletions csharp/ql/src/Language Abuse/ForeachCapture.ql
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,17 @@ predicate lambdaCaptures(AnonymousFunctionExpr lambda, Variable v) {
exists(VariableAccess va | va.getEnclosingCallable() = lambda | va.getTarget() = v)
}

predicate lambdaCapturesLoopVariable(AnonymousFunctionExpr lambda, ForeachStmt loop, Variable v) {
predicate lambdaCapturesLoopVariable(AnonymousFunctionExpr lambda, ForEachStmt loop, Variable v) {
lambdaCaptures(lambda, v) and
inForeachStmtBody(loop, lambda) and
inForEachStmtBody(loop, lambda) and
loop.getVariable() = v
}

predicate inForeachStmtBody(ForeachStmt loop, Element e) {
predicate inForEachStmtBody(ForEachStmt loop, Element e) {
e = loop.getBody()
or
exists(Element mid |
inForeachStmtBody(loop, mid) and
inForEachStmtBody(loop, mid) and
e = mid.getAChild()
)
}
Expand All @@ -53,7 +53,7 @@ module LambdaDataFlow {
exists(DataFlow::Node sink | flow(DataFlow::exprNode(lambda), sink) |
storage = getAssignmentTarget(sink.asExpr())
) and
exists(ForeachStmt loop | lambdaCapturesLoopVariable(lambda, loop, loopVar) |
exists(ForEachStmt loop | lambdaCapturesLoopVariable(lambda, loop, loopVar) |
not declaredInsideLoop(loop, storage)
)
}
Expand Down Expand Up @@ -103,9 +103,9 @@ Element getCollectionAssignmentTarget(Expr e) {
}

// Variable v is declared inside the loop body
predicate declaredInsideLoop(ForeachStmt loop, LocalVariable v) {
predicate declaredInsideLoop(ForEachStmt loop, LocalVariable v) {
exists(LocalVariableDeclStmt decl | decl.getVariableDeclExpr(_).getVariable() = v |
inForeachStmtBody(loop, decl)
inForEachStmtBody(loop, decl)
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ where
any(LocalVariableDeclAndInitExpr ass | ass.getRightOperand() instanceof ObjectCreation)
.getLeftOperand()
) and
not v = any(ForeachStmt fs).getVariable() and
not v = any(ForEachStmt fs).getVariable() and
not v = any(BindingPatternExpr vpe).getVariableDeclExpr().getVariable() and
not v = any(Attribute a).getTarget()
select v, "The contents of this container are never accessed."
2 changes: 1 addition & 1 deletion csharp/ql/src/Likely Bugs/Statements/UseBraces.ql
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class IfThenElseStmt extends IfStmt {

Stmt getTrailingBody(Stmt s) {
result = s.(ForStmt).getBody() or
result = s.(ForeachStmt).getBody() or
result = s.(ForEachStmt).getBody() or
result = s.(WhileStmt).getBody() or
result = s.(IfThenStmt).getThen() or
result = s.(IfThenElseStmt).getElse()
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/src/Linq/BadMultipleIteration.ql
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ predicate likelyNonRepeatableSequence(IEnumerableSequence seq) {

/** An access to an enumerable sequence that potentially consumes sequence elements. */
predicate potentiallyConsumingAccess(VariableAccess va) {
exists(ForeachStmt fes | va = fes.getIterableExpr())
exists(ForEachStmt fes | va = fes.getIterableExpr())
or
exists(MethodCall mc |
va = mc.getArgument(0) and
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/src/Linq/MissedAllOpportunity.ql
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import Linq.Helpers
* bool allEven = lst.All(i => i % 2 == 0);
*/

from ForeachStmtGenericEnumerable fes
from ForEachStmtGenericEnumerable fes
where missedAllOpportunity(fes)
select fes,
"This foreach loop looks as if it might be testing whether every sequence element satisfies a predicate - consider using '.All(...)'."
2 changes: 1 addition & 1 deletion csharp/ql/src/Linq/MissedCastOpportunity.ql
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import csharp
import Linq.Helpers

from ForeachStmtEnumerable fes, LocalVariableDeclStmt s
from ForEachStmtEnumerable fes, LocalVariableDeclStmt s
where missedCastOpportunity(fes, s)
select fes,
"This foreach loop immediately $@ - consider casting the sequence explicitly using '.Cast(...)'.",
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/src/Linq/MissedOfTypeOpportunity.ql
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import csharp
import Linq.Helpers

from ForeachStmtEnumerable fes, LocalVariableDeclStmt s
from ForEachStmtEnumerable fes, LocalVariableDeclStmt s
where missedOfTypeOpportunity(fes, s)
select fes,
"This foreach loop immediately uses 'as' to $@ - consider using '.OfType(...)' instead.", s,
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/src/Linq/MissedSelectOpportunity.ql
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ predicate oversized(LocalVariableDeclStmt s) {
)
}

from ForeachStmtGenericEnumerable fes, LocalVariableDeclStmt s
from ForEachStmtGenericEnumerable fes, LocalVariableDeclStmt s
where
missedSelectOpportunity(fes, s) and
not oversized(s)
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/src/Linq/MissedWhereOpportunity.ql
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import csharp
import Linq.Helpers

from ForeachStmtGenericEnumerable fes, IfStmt is
from ForEachStmtGenericEnumerable fes, IfStmt is
where
missedWhereOpportunity(fes, is) and
not missedAllOpportunity(fes)
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/test/library-tests/csharp7/ForEach.ql
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import csharp

from ForeachStmt stmt, int i
from ForEachStmt stmt, int i
select stmt, i, stmt.getVariableDeclExpr(i), stmt.getVariable(i), stmt.getIterableExpr(),
stmt.getBody()
6 changes: 3 additions & 3 deletions csharp/ql/test/library-tests/csharp7/PrintAst.expected
Original file line number Diff line number Diff line change
Expand Up @@ -864,19 +864,19 @@ CSharp7.cs:
# 283| -1: [ParameterAccess] access to parameter item
# 283| 1: [PropertyCall] access to property Value
# 283| -1: [ParameterAccess] access to parameter item
# 285| 2: [ForeachStmt] foreach (... ... in ...) ...
# 285| 2: [ForEachStmt] foreach (... ... in ...) ...
# 285| 0: [TupleExpr] (..., ...)
# 285| 0: [LocalVariableDeclExpr] Int32 a
# 285| 1: [LocalVariableDeclExpr] String b
# 285| 1: [LocalVariableAccess] access to local variable list
# 285| 2: [BlockStmt] {...}
# 287| 3: [ForeachStmt] foreach (... ... in ...) ...
# 287| 3: [ForEachStmt] foreach (... ... in ...) ...
# 287| 0: [TupleExpr] (..., ...)
# 287| 0: [LocalVariableDeclExpr] Int32 a
# 287| 1: [LocalVariableDeclExpr] String b
# 287| 1: [LocalVariableAccess] access to local variable list
# 287| 2: [BlockStmt] {...}
# 289| 4: [ForeachStmt] foreach (... ... in ...) ...
# 289| 4: [ForEachStmt] foreach (... ... in ...) ...
# 289| 0: [TupleExpr] (..., ...)
# 289| 0: [LocalVariableDeclExpr] Int32 a
# 289| 1: [LocalVariableDeclExpr] String b
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/test/library-tests/csharp8/PrintAst.expected
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ AsyncStreams.cs:
# 15| 7: [Method] F
# 15| -1: [TypeMention] Void
# 16| 4: [BlockStmt] {...}
# 17| 0: [ForeachStmt] foreach (... ... in ...) ...
# 17| 0: [ForEachStmt] foreach (... ... in ...) ...
# 17| 0: [LocalVariableDeclExpr] Int32 item
# 17| 0: [TypeMention] int
# 17| 1: [MethodCall] call to method Items
Expand Down
8 changes: 4 additions & 4 deletions csharp/ql/test/library-tests/csharp9/PrintAst.expected
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ ForeachExtension.cs:
# 23| 0: [TypeMention] Enumerable
# 23| 0: [IntLiteral] 0
# 23| 1: [IntLiteral] 10
# 24| 1: [ForeachStmt] foreach (... ... in ...) ...
# 24| 1: [ForEachStmt] foreach (... ... in ...) ...
# 24| 0: [LocalVariableDeclExpr] Int32 item
# 24| 0: [TypeMention] int
# 24| 1: [LocalVariableAccess] access to local variable enumerator1
Expand All @@ -252,17 +252,17 @@ ForeachExtension.cs:
# 28| 1: [TypeMention] int
# 28| 0: [LocalVariableAccess] access to local variable enumerator2
# 28| 1: [MethodCall] call to method GetAsyncEnumerator
# 29| 3: [ForeachStmt] foreach (... ... in ...) ...
# 29| 3: [ForEachStmt] foreach (... ... in ...) ...
# 29| 0: [LocalVariableDeclExpr] Int32 item
# 29| 0: [TypeMention] int
# 29| 1: [LocalVariableAccess] access to local variable enumerator2
# 30| 2: [BlockStmt] {...}
# 33| 4: [ForeachStmt] foreach (... ... in ...) ...
# 33| 4: [ForEachStmt] foreach (... ... in ...) ...
# 33| 0: [LocalVariableDeclExpr] Int32 item
# 33| 0: [TypeMention] int
# 33| 1: [IntLiteral] 42
# 34| 2: [BlockStmt] {...}
# 37| 5: [ForeachStmt] foreach (... ... in ...) ...
# 37| 5: [ForEachStmt] foreach (... ... in ...) ...
# 37| 0: [LocalVariableDeclExpr] Int32 i
# 37| 0: [TypeMention] int
# 37| 1: [ArrayCreation] array creation of type Int32[]
Expand Down
Loading
Loading