Skip to content

Commit a1e58ce

Browse files
hvitvedcalumgrant
hvitved
authored and
calumgrant
committed
C#: Refactor recursive patterns implementation
- Extract names of properties in a propery match, using the `exprorstmt_name` relation. - Simplify extraction of properties by not distinguishing between top-level patterns and nested patterns. - Introduce `PatternExpr` to capture patterns in `is` expressions, `case` statements, and `switch` expression arms. - Generalize `IsTypeExpr`, `IsPatternExpr`, `IsRecursivePatternExpr`, and `IsConstantExpr` to just `IsExpr` with a member predicate `PatternExpr getPattern()`. - Generalize `TypeCase`, `RecursivePatternCase`, and `ConstCase` to just `CaseStmt` with a member predicate `PatternExpr getPattern()`. - Introduce classes `Switch` and `Case` as base classes of switch statements/expressions and case statements/switch expression arms, respectively. - Simplify CFG logic using the generalized classes. - Generalize guards library to cover `switch` expressions tests. - Generalize data flow library to cover `switch` expression assignments.
1 parent b28ad90 commit a1e58ce

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

60 files changed

+1198
-1307
lines changed

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Discard.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using Semmle.Extraction.Kinds;
33
using Semmle.Extraction.Entities;
44
using Microsoft.CodeAnalysis;
5+
using Microsoft.CodeAnalysis.CSharp;
56

67
namespace Semmle.Extraction.CSharp.Entities.Expressions
78
{
@@ -11,13 +12,16 @@ public Discard(ExpressionNodeInfo info) : base(info.SetKind(ExprKind.DISCARD))
1112
{
1213
}
1314

14-
public Discard(Context cx, DiscardDesignationSyntax discard, IExpressionParentEntity parent, int child) :
15-
base(new ExpressionInfo(cx, Type.Create(cx, cx.Model(discard).GetTypeInfo(discard).Type), cx.Create(discard.GetLocation()), ExprKind.DISCARD, parent, child, false, null))
15+
Discard(Context cx, CSharpSyntaxNode syntax, IExpressionParentEntity parent, int child) :
16+
base(new ExpressionInfo(cx, Type.Create(cx, cx.Model(syntax).GetTypeInfo(syntax).Type), cx.Create(syntax.GetLocation()), ExprKind.DISCARD, parent, child, false, null))
1617
{
1718
}
1819

19-
public Discard(Context cx, DiscardPatternSyntax pattern, IExpressionParentEntity parent, int child) :
20-
base(new ExpressionInfo(cx, Type.Create(cx, cx.Model(pattern).GetTypeInfo(pattern).Type), cx.Create(pattern.GetLocation()), ExprKind.DISCARD, parent, child, false, null))
20+
public Discard(Context cx, DiscardDesignationSyntax discard, IExpressionParentEntity parent, int child) : this(cx, (CSharpSyntaxNode)discard, parent, child)
21+
{
22+
}
23+
24+
public Discard(Context cx, DiscardPatternSyntax pattern, IExpressionParentEntity parent, int child) : this(cx, (CSharpSyntaxNode)pattern, parent, child)
2125
{
2226
}
2327
}

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/IsPattern.cs‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,19 @@ public static Expression CreatePattern(this Context cx, PatternSyntax syntax, IE
2828
}
2929

3030
case RecursivePatternSyntax recPattern:
31-
return new RecursivePattern(cx, recPattern, parent, child, false);
31+
return new RecursivePattern(cx, recPattern, parent, child);
3232

3333
case VarPatternSyntax varPattern:
34-
switch(varPattern.Designation)
34+
switch (varPattern.Designation)
3535
{
3636
case ParenthesizedVariableDesignationSyntax parDesignation:
3737
return VariableDeclaration.CreateParenthesized(cx, varPattern, parDesignation, parent, child);
3838
case SingleVariableDesignationSyntax varDesignation:
39-
if (cx.Model(syntax).GetDeclaredSymbol(varDesignation) is ILocalSymbol symbol2)
39+
if (cx.Model(syntax).GetDeclaredSymbol(varDesignation) is ILocalSymbol symbol)
4040
{
41-
var type = Type.Create(cx, symbol2.Type);
41+
var type = Type.Create(cx, symbol.Type);
4242

43-
return VariableDeclaration.Create(cx, symbol2, type, cx.Create(syntax.GetLocation()), cx.Create(varDesignation.GetLocation()), false, parent, child);
43+
return VariableDeclaration.Create(cx, symbol, type, cx.Create(syntax.GetLocation()), cx.Create(varDesignation.GetLocation()), false, parent, child);
4444
}
4545
else
4646
{
@@ -67,7 +67,8 @@ internal PropertyPattern(Context cx, PropertyPatternClauseSyntax pp, IExpression
6767
child = 0;
6868
foreach (var sub in pp.Subpatterns)
6969
{
70-
cx.CreatePattern(sub.Pattern, this, child++);
70+
var p = cx.CreatePattern(sub.Pattern, this, child++);
71+
cx.Emit(Tuples.exprorstmt_name(p, sub.NameColon.Name.ToString()));
7172
}
7273
}
7374
}
@@ -95,22 +96,19 @@ class RecursivePattern : Expression
9596
/// <param name="parent">The parent pattern/expression.</param>
9697
/// <param name="child">The child index of this pattern.</param>
9798
/// <param name="isTopLevel">If this pattern is in the top level of a case/is. In that case, the variable and type access are populated elsewhere.</param>
98-
public RecursivePattern(Context cx, RecursivePatternSyntax syntax, IExpressionParentEntity parent, int child, bool isTopLevel) :
99+
public RecursivePattern(Context cx, RecursivePatternSyntax syntax, IExpressionParentEntity parent, int child) :
99100
base(new ExpressionInfo(cx, Type.Create(cx, null), cx.Create(syntax.GetLocation()), ExprKind.RECURSIVE_PATTERN, parent, child, false, null))
100101
{
101-
if(!isTopLevel)
102-
{
103-
// Extract the type access
104-
if(syntax.Type is TypeSyntax t)
105-
Expressions.TypeAccess.Create(cx, t, this, 1);
102+
// Extract the type access
103+
if (syntax.Type is TypeSyntax t)
104+
Expressions.TypeAccess.Create(cx, t, this, 1);
106105

107-
// Extract the local variable declaration
108-
if (syntax.Designation is VariableDesignationSyntax designation && cx.Model(syntax).GetDeclaredSymbol(designation) is ILocalSymbol symbol)
109-
{
110-
var type = Type.Create(cx, symbol.Type);
106+
// Extract the local variable declaration
107+
if (syntax.Designation is VariableDesignationSyntax designation && cx.Model(syntax).GetDeclaredSymbol(designation) is ILocalSymbol symbol)
108+
{
109+
var type = Type.Create(cx, symbol.Type);
111110

112-
VariableDeclaration.Create(cx, symbol, type, cx.Create(syntax.GetLocation()), cx.Create(designation.GetLocation()), false, this, 0);
113-
}
111+
VariableDeclaration.Create(cx, symbol, type, cx.Create(syntax.GetLocation()), cx.Create(designation.GetLocation()), false, this, 0);
114112
}
115113

116114
if (syntax.PositionalPatternClause is PositionalPatternClauseSyntax posPc)
@@ -163,8 +161,7 @@ protected override void Populate()
163161
PopulatePattern(declPattern, declPattern.Type, default(SyntaxToken), declPattern.Designation);
164162
return;
165163
case RecursivePatternSyntax recPattern:
166-
PopulatePattern(recPattern, recPattern.Type, default(SyntaxToken), recPattern.Designation);
167-
new RecursivePattern(cx, recPattern, this, 4, true);
164+
new RecursivePattern(cx, recPattern, this, 3);
168165
return;
169166
default:
170167
throw new InternalError(Syntax, "Is pattern not handled");

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Statements/Case.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ private CasePattern(Context cx, CasePatternSwitchLabelSyntax node, Switch parent
7070
private void PopulatePattern(PatternSyntax pattern, TypeSyntax optionalType, SyntaxToken varKeyword, VariableDesignationSyntax designation)
7171
{
7272
var isVar = optionalType is null;
73-
if(!isVar)
73+
if (!isVar)
7474
Expressions.TypeAccess.Create(cx, optionalType, this, 1);
7575

7676
switch (designation)
@@ -101,7 +101,7 @@ private void PopulatePattern(PatternSyntax pattern, TypeSyntax optionalType, Syn
101101

102102
protected override void Populate()
103103
{
104-
switch(Stmt.Pattern)
104+
switch (Stmt.Pattern)
105105
{
106106
case VarPatternSyntax varPattern:
107107
PopulatePattern(varPattern, null, varPattern.VarKeyword, varPattern.Designation);
@@ -113,8 +113,7 @@ protected override void Populate()
113113
Expression.Create(cx, pattern.Expression, this, 0);
114114
break;
115115
case RecursivePatternSyntax recPattern:
116-
PopulatePattern(recPattern, recPattern.Type, default(SyntaxToken), recPattern.Designation);
117-
new Expressions.RecursivePattern(cx, recPattern, this, 4, true);
116+
new Expressions.RecursivePattern(cx, recPattern, this, 0);
118117
break;
119118
default:
120119
throw new InternalError(Stmt, "Case pattern not handled");

‎csharp/ql/src/Dead Code/DeadStoreOfLocal.ql‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,7 @@ class RelevantDefinition extends AssignableDefinition {
8787
lvde = any(ForeachStmt fs).getVariableDeclExpr()
8888
)
8989
or
90-
this instanceof AssignableDefinitions::IsPatternDefinition
91-
or
92-
this instanceof AssignableDefinitions::TypeCasePatternDefinition
90+
this instanceof AssignableDefinitions::PatternDefinition
9391
}
9492

9593
/** Holds if this assignment may be live. */

‎csharp/ql/src/Language Abuse/ChainedIs.ql‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@
1313

1414
import csharp
1515

16+
TypePatternExpr getTypeCondition(IfStmt is) { result = is.getCondition().(IsExpr).getPattern() }
17+
1618
int isCountForIfChain(IfStmt is) {
1719
exists(int rest |
1820
(if is.getElse() instanceof IfStmt then rest = isCountForIfChain(is.getElse()) else rest = 0) and
1921
(
20-
if is.getCondition().(IsTypeExpr).getCheckedType().getSourceDeclaration().fromSource()
22+
if getTypeCondition(is).getCheckedType().getSourceDeclaration().fromSource()
2123
then result = 1 + rest
2224
else result = rest
2325
)

‎csharp/ql/src/Language Abuse/DubiousTypeTestOfThis.ql‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@
1313
import csharp
1414
import semmle.code.csharp.commons.Assertions
1515

16-
from IsTypeExpr ise, ValueOrRefType t, ValueOrRefType ct
16+
from IsExpr ie, ValueOrRefType t, ValueOrRefType ct
1717
where
18-
ise.getExpr() instanceof ThisAccess and
19-
t = ise.getExpr().getType() and
20-
ct = ise.getCheckedType() and
18+
ie.getExpr() instanceof ThisAccess and
19+
t = ie.getExpr().getType() and
20+
ct = ie.getPattern().(TypePatternExpr).getCheckedType() and
2121
ct.getABaseType*() = t and
22-
not isExprInAssertion(ise)
23-
select ise,
22+
not isExprInAssertion(ie)
23+
select ie,
2424
"Testing whether 'this' is an instance of $@ in $@ introduces a dependency cycle between the two types.",
2525
ct, ct.getName(), t, t.getName()

‎csharp/ql/src/Language Abuse/UselessIsBeforeAs.ql‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@ class StructuralComparisonConfig extends StructuralComparisonConfiguration {
1717
StructuralComparisonConfig() { this = "UselessIsBeforeAs" }
1818

1919
override predicate candidate(ControlFlowElement x, ControlFlowElement y) {
20-
exists(IfStmt is, AsExpr ae, IsTypeExpr ie |
20+
exists(IfStmt is, AsExpr ae, IsExpr ie, TypeAccessPatternExpr tape |
2121
ie = is.getCondition().getAChild*() and
22-
ae.getTargetType() = ie.getCheckedType() and
22+
tape = ie.getPattern() and
23+
ae.getTargetType() = tape.getTarget() and
2324
x = ie.getExpr() and
2425
y = ae.getExpr()
2526
|

‎csharp/ql/src/Language Abuse/UselessTypeTest.ql‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@
1212

1313
import csharp
1414

15-
from IsTypeExpr ise, ValueOrRefType t, ValueOrRefType ct
15+
from IsExpr ie, ValueOrRefType t, ValueOrRefType ct
1616
where
17-
t = ise.getExpr().getType() and
18-
ct = ise.getCheckedType() and
17+
t = ie.getExpr().getType() and
18+
ct = ie.getPattern().(TypePatternExpr).getCheckedType() and
1919
ct = t.getABaseType+()
20-
select ise,
20+
select ie,
2121
"There is no need to test whether an instance of $@ is also an instance of $@ - it always is.", t,
2222
t.getName(), ct, ct.getName()

‎csharp/ql/src/Likely Bugs/Collections/WriteOnlyContainer.ql‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ where
2525
a = any(Assignment ass | ass.getRValue() instanceof ObjectCreation).getLValue()
2626
) and
2727
not v = any(ForeachStmt fs).getVariable() and
28-
not v = any(IsPatternExpr ipe).getVariableDeclExpr().getVariable() and
29-
not v = any(TypeCase tc).getVariableDeclExpr().getVariable() and
28+
not v = any(BindingPatternExpr vpe).getVariableDeclExpr().getVariable() and
3029
not v = any(Attribute a).getTarget()
3130
select v, "The contents of this container are never accessed."

‎csharp/ql/src/Likely Bugs/EqualsUsesIs.ql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@
1212
import csharp
1313
import semmle.code.csharp.frameworks.System
1414

15-
from EqualsMethod m, IsTypeExpr e, Class isType
15+
from EqualsMethod m, IsExpr e, Class isType
1616
where
1717
m.fromSource() and
1818
e.getEnclosingCallable() = m and
1919
e.getExpr().(VariableAccess).getTarget() = m.getParameter(0) and
20-
isType = e.getCheckedType() and
20+
isType = e.getPattern().(TypePatternExpr).getCheckedType() and
2121
not isType.isSealed() and
2222
not exists(MethodCall c |
2323
c.getEnclosingCallable() = m and

0 commit comments

Comments
 (0)