Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C#: Basic property pattern flow. #18781

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft
52 changes: 42 additions & 10 deletions csharp/ql/lib/semmle/code/csharp/Assignable.qll
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,23 @@ module AssignableInternal {
def = TPatternDefinition(result)
}

/** A local variable declaration at the top-level of a pattern. */
class TopLevelPatternDecl extends LocalVariableDeclExpr {
/** A pattern containing a local variable declaration. */
class LocalVariablePatternDecl extends LocalVariableDeclExpr {
private PatternMatch pm;

TopLevelPatternDecl() { this = pm.getPattern().(BindingPatternExpr).getVariableDeclExpr() }
LocalVariablePatternDecl() {
exists(BindingPatternExpr bpe |
this = bpe.getVariableDeclExpr() and pm = bpe.getPatternMatch()
)
}

/** Holds if the local variable definition is at the top level of the pattern. */
predicate isTopLevel() { this = pm.getPattern().(BindingPatternExpr).getVariableDeclExpr() }

/** Holds of this local variable definition is a part of a tuple pattern. */
predicate isInTuple() { this.getParent() instanceof TuplePatternExpr }

/** Gets the pattern match that this local variable declaration (pattern) belongs to. */
PatternMatch getMatch() { result = pm }
}

Expand All @@ -297,7 +308,7 @@ module AssignableInternal {
TLocalVariableDefinition(LocalVariableDeclExpr lvde) {
not lvde.hasInitializer() and
not exists(getTupleSource(TTupleAssignmentDefinition(_, lvde))) and
not lvde instanceof TopLevelPatternDecl and
not lvde instanceof LocalVariablePatternDecl and
not lvde.isOutArgument()
} or
TImplicitParameterDefinition(Parameter p) {
Expand All @@ -309,7 +320,7 @@ module AssignableInternal {
)
} or
TAddressOfDefinition(AddressOfExpr aoe) or
TPatternDefinition(TopLevelPatternDecl tlpd)
TPatternDefinition(LocalVariablePatternDecl lvpd)

/**
* Gets the source expression assigned in tuple definition `def`, if any.
Expand Down Expand Up @@ -699,24 +710,45 @@ module AssignableDefinitions {
}

/**
* A local variable definition in a pattern, for example `x is int i`.
* A local variable definition in a pattern, for example `int i` in `x is int i`.
*/
class PatternDefinition extends AssignableDefinition, TPatternDefinition {
TopLevelPatternDecl tlpd;
LocalVariablePatternDecl lvpd;

PatternDefinition() { this = TPatternDefinition(tlpd) }
PatternDefinition() { this = TPatternDefinition(lvpd) }

/** Gets the element matches against this pattern. */
PatternMatch getMatch() { result = tlpd.getMatch() }
PatternMatch getMatch() { result = lvpd.getMatch() }

/** Gets the underlying local variable declaration. */
LocalVariableDeclExpr getDeclaration() { result = tlpd }
LocalVariableDeclExpr getDeclaration() { result = lvpd }

override Expr getSource() { result = this.getMatch().getExpr() }

override string toString() { result = this.getDeclaration().toString() }
}

/**
* A local variable definition at the top level of a pattern.
*/
class TopLevelPatternDefinition extends PatternDefinition {
TopLevelPatternDefinition() { lvpd.isTopLevel() }
}

/**
* A local variable definition in a tuple pattern.
*/
class TuplePatternDefinition extends PatternDefinition {
TuplePatternDefinition() { lvpd.isInTuple() }
}

/**
* A local variable definition in a property pattern.
*/
class PropertyPatternDefinition extends PatternDefinition {
PropertyPatternDefinition() { lvpd.getParent() instanceof PropertyPatternExpr }
}

/**
* An initializer definition for a field or a property, for example
* line 2 in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,13 @@ module ControlFlow {
result = this.getASuccessorByType(any(BooleanSuccessor t | t.getValue() = false))
}

/**
* Gets an immediate `match` predecessor, if any.
*/
Node getAMatchPredecessor() {
result = this.getAPredecessorByType(any(MatchingSuccessor t | t.isMatch()))
}

/** Gets the enclosing callable of this control flow node. */
final Callable getEnclosingCallable() { result = Impl::getNodeCfgScope(this) }
}
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ private predicate nonNullDef(Ssa::ExplicitDefinition def) {
def.getADefinition().getSource() instanceof NonNullExpr
or
exists(AssignableDefinition ad | ad = def.getADefinition() |
ad instanceof AssignableDefinitions::PatternDefinition
ad instanceof AssignableDefinitions::TopLevelPatternDefinition
or
ad =
any(AssignableDefinitions::LocalVariableDefinition d |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,16 @@

/** Provides predicates related to local data flow. */
module LocalFlow {
/**
* Holds if the pattern `e` is a top level variable pattern expression or
* if the pattern doesn't contain any variable pattern expressions.
*/
private predicate basicPattern(PatternExpr e) {
e instanceof VariablePatternExpr
or
not exists(VariablePatternExpr vpe | e.getAChild*() = vpe)

Check warning

Code scanning / CodeQL

Expression can be replaced with a cast Warning

The assignment to
vpe
in the exists(..) can replaced with an instanceof expression.
}

class LocalExprStepConfiguration extends ControlFlowReachabilityConfiguration {
LocalExprStepConfiguration() { this = "LocalExprStepConfiguration" }

Expand Down Expand Up @@ -605,11 +615,31 @@
isSuccessor = false
or
isSuccessor = true and
exists(ControlFlowElement cfe | cfe = e2.(TupleExpr).(PatternExpr).getPatternMatch() |
exists(ControlFlowElement cfe | cfe = e2.(TuplePatternExpr).getPatternMatch() |
cfe.(IsExpr).getExpr() = e1 and scope = cfe
or
exists(Switch sw | sw.getACase() = cfe and sw.getExpr() = e1 and scope = sw)
)
or
isSuccessor = true and
scope =
any(IsExpr ie |
e1 = ie.getExpr() and
e2 = ie.getPattern() and
not basicPattern(e2)
)
or
isSuccessor = true and
scope =
any(Switch e |
e1 = e.getExpr() and
e2 = e.getACase().getPattern() and
not basicPattern(e2)
)
or
isSuccessor = false and
e2 = e1.(RecursivePatternExpr).getPropertyPatterns() and
scope = e1
)
}

Expand All @@ -622,20 +652,30 @@
def.getSource() = e and
(
scope = def.getExpr() and
isSuccessor = true
isSuccessor = true and
(
not def instanceof AssignableDefinitions::PatternDefinition or
def instanceof AssignableDefinitions::TopLevelPatternDefinition
)
or
scope = def.(AssignableDefinitions::PatternDefinition).getMatch().(IsExpr) and
scope = def.(AssignableDefinitions::TopLevelPatternDefinition).getMatch().(IsExpr) and
isSuccessor = false
or
exists(Switch s |
s.getACase() = def.(AssignableDefinitions::PatternDefinition).getMatch() and
s.getACase() = def.(AssignableDefinitions::TopLevelPatternDefinition).getMatch() and
isSuccessor = true
|
scope = s.getExpr()
or
scope = s.getACase()
)
)
or
// Needed for read steps for pattern matching involving properties.
scope = def.getExpr() and
exactScope = false and
isSuccessor = false and
e = def.(AssignableDefinitions::PropertyPatternDefinition).getDeclaration()
}
}

Expand Down Expand Up @@ -896,6 +936,31 @@
)
}

/**
* TODO: Should we consider to override getType on pattern expressions?
*/
private Type getPatternType(PatternExpr pe) {
result = pe.(RecursivePatternExpr).getTypeAccess().getType()
or
not pe instanceof LabeledPatternExpr and
result = getPatternType(pe.getParent())
or
exists(Property p |
result = p.getType() and
p.getDeclaringType() = getPatternType(pe.getParent()) and
p.getName() = pe.(LabeledPatternExpr).getLabel()
)
}

private predicate patternPropertyRead(PropertyPatternExpr pe, ContentSet c, LabeledPatternExpr e) {
exists(Property prop |
e = pe.getPattern(_) and
prop.getDeclaringType() = getPatternType(pe) and
prop.getName() = e.getLabel() and
c.isProperty(prop)
)
}

/**
* Holds if `e2` is an expression that reads field or property `c` from
* expression `e1`.
Expand Down Expand Up @@ -1124,6 +1189,8 @@
or
dynamicPropertyRead(e, _, read)
or
patternPropertyRead(e, _, read)
or
arrayRead(e, read)
)
)
Expand Down Expand Up @@ -2415,6 +2482,11 @@
e2 = e1.(TupleExpr).getAnArgument() and
scope = e1 and
isSuccessor = false
or
exactScope = false and
isSuccessor = false and
patternPropertyRead(e1, _, e2) and
scope = e1
}

override predicate candidateDef(
Expand Down Expand Up @@ -2446,8 +2518,8 @@
)
or
scope =
any(TupleExpr te |
te.getAnArgument() = defTo.(AssignableDefinitions::LocalVariableDefinition).getDeclaration() and
any(TuplePatternExpr te |
te.getAnArgument() = defTo.(AssignableDefinitions::TuplePatternDefinition).getDeclaration() and
e = te and
exactScope = false and
isSuccessor = false
Expand Down Expand Up @@ -2496,8 +2568,8 @@
)
or
// item = variable in node1 = (..., variable, ...) in a case/is var (..., ...)
te = any(PatternExpr pe).getAChildExpr*() and
exists(AssignableDefinitions::LocalVariableDefinition lvd |
te = any(TuplePatternExpr pe).getAChildExpr*() and
exists(AssignableDefinitions::TuplePatternDefinition lvd |
node2.(AssignableDefinitionNode).getDefinition() = lvd and
lvd.getDeclaration() = item and
hasNodePath(x, node1, node2)
Expand Down Expand Up @@ -2541,6 +2613,15 @@
c = getResultContent()
)
or
exists(ReadStepConfiguration x, ControlFlow::Node cfn1, ControlFlow::Node cfn2 |
cfn1 = node1.getControlFlowNode() and
cfn2 = node2.getControlFlowNode() and
cfn1.getAMatchPredecessor() = cfn2 and
x.hasExprPath(_, cfn1, _, cfn2)
|
patternPropertyRead(node1.asExpr(), c, node2.asExpr())
)
or
FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
node2.(FlowSummaryNode).getSummaryNode())
or
Expand Down Expand Up @@ -2928,7 +3009,7 @@
this.asExpr() instanceof Cast
or
this.(AssignableDefinitionNode).getDefinition() instanceof
AssignableDefinitions::PatternDefinition
AssignableDefinitions::TopLevelPatternDefinition
}
}

Expand Down
7 changes: 7 additions & 0 deletions csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,13 @@ class LabeledPatternExpr extends PatternExpr {
override string getAPrimaryQlClass() { result = "LabeledPatternExpr" }
}

/**
* A tuple pattern. For example, `var (x, y)`.
*/
class TuplePatternExpr extends TupleExpr, PatternExpr {
override string getAPrimaryQlClass() { result = "TuplePatternExpr" }
}

/** A positional pattern. For example, `(int x, int y)`. */
class PositionalPatternExpr extends PatternExpr, @positional_pattern_expr {
override string toString() { result = "( ... )" }
Expand Down
4 changes: 2 additions & 2 deletions csharp/ql/test/library-tests/csharp8/PrintAst.expected
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ patterns.cs:
# 57| 1: [ConstantPatternExpr,IntLiteral] 2
# 58| 10: [BreakStmt] break;
# 59| 11: [CaseStmt] case ...:
# 59| 0: [TupleExpr] (..., ...)
# 59| 0: [TuplePatternExpr] (..., ...)
# 59| 0: [VariablePatternExpr] Int32 x
# 59| 1: [VariablePatternExpr] Int32 y
# 60| 12: [BreakStmt] break;
Expand Down Expand Up @@ -1154,7 +1154,7 @@ patterns.cs:
# 130| 1: [ConstantPatternExpr,IntLiteral] 2
# 130| 2: [IntLiteral] 2
# 131| 3: [SwitchCaseExpr] ... => ...
# 131| 0: [TupleExpr] (..., ...)
# 131| 0: [TuplePatternExpr] (..., ...)
# 131| 0: [VariablePatternExpr] Int32 x
# 131| 1: [DiscardPatternExpr] _
# 131| 2: [IntLiteral] 3
Expand Down
Loading