Skip to content

Commit 102490a

Browse files
hvitvedhvitved
committed
C#: Simplify extraction of is expressions and case statements
1 parent 9804105 commit 102490a

13 files changed

Lines changed: 80 additions & 77 deletions

File tree

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

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,19 +131,14 @@ private IsPattern(ExpressionNodeInfo info) : base(info.SetKind(ExprKind.IS))
131131

132132
private void PopulatePattern(PatternSyntax pattern, TypeSyntax optionalType, SyntaxToken varKeyword, VariableDesignationSyntax designation)
133133
{
134-
bool isVar = optionalType is null;
135-
if (!isVar)
136-
Expressions.TypeAccess.Create(cx, optionalType, this, 1);
137-
134+
var isVar = optionalType is null;
138135
if (!(designation is null) && cx.Model(pattern).GetDeclaredSymbol(designation) is ILocalSymbol symbol)
139136
{
140137
var type = Type.Create(cx, symbol.Type);
141-
142-
if (isVar)
143-
new Expression(new ExpressionInfo(cx, type, cx.Create(varKeyword.GetLocation()), ExprKind.TYPE_ACCESS, this, 1, false, null));
144-
145138
VariableDeclaration.Create(cx, symbol, type, cx.Create(pattern.GetLocation()), cx.Create(designation.GetLocation()), isVar, this, 2);
146139
}
140+
else if (!isVar)
141+
Expressions.TypeAccess.Create(cx, optionalType, this, 1);
147142
}
148143

149144
protected override void Populate()
@@ -152,7 +147,7 @@ protected override void Populate()
152147
switch (Syntax.Pattern)
153148
{
154149
case ConstantPatternSyntax constantPattern:
155-
Create(cx, constantPattern.Expression, this, 3);
150+
Create(cx, constantPattern.Expression, this, 1);
156151
return;
157152
case VarPatternSyntax varPattern:
158153
PopulatePattern(varPattern, null, varPattern.VarKeyword, varPattern.Designation);
@@ -161,7 +156,7 @@ protected override void Populate()
161156
PopulatePattern(declPattern, declPattern.Type, default(SyntaxToken), declPattern.Designation);
162157
return;
163158
case RecursivePatternSyntax recPattern:
164-
new RecursivePattern(cx, recPattern, this, 3);
159+
new RecursivePattern(cx, recPattern, this, 1);
165160
return;
166161
default:
167162
throw new InternalError(Syntax, "Is pattern not handled");

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

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,27 +67,23 @@ class CasePattern : Case<CasePatternSwitchLabelSyntax>
6767
private CasePattern(Context cx, CasePatternSwitchLabelSyntax node, Switch parent, int child)
6868
: base(cx, node, parent, child) { }
6969

70-
private void PopulatePattern(PatternSyntax pattern, TypeSyntax optionalType, SyntaxToken varKeyword, VariableDesignationSyntax designation)
70+
private void PopulatePattern(PatternSyntax pattern, TypeSyntax optionalType, VariableDesignationSyntax designation)
7171
{
7272
var isVar = optionalType is null;
73-
if (!isVar)
74-
Expressions.TypeAccess.Create(cx, optionalType, this, 1);
75-
7673
switch (designation)
7774
{
7875
case SingleVariableDesignationSyntax _:
7976
if (cx.Model(pattern).GetDeclaredSymbol(designation) is ILocalSymbol symbol)
8077
{
8178
var type = Type.Create(cx, symbol.Type);
82-
83-
if (isVar)
84-
new Expression(new ExpressionInfo(cx, type, cx.Create(varKeyword.GetLocation()), ExprKind.TYPE_ACCESS, this, 1, false, null));
85-
8679
Expressions.VariableDeclaration.Create(cx, symbol, type, cx.Create(pattern.GetLocation()), cx.Create(designation.GetLocation()), isVar, this, 0);
8780
}
8881
break;
8982
case DiscardDesignationSyntax discard:
90-
new Expressions.Discard(cx, discard, this, 0);
83+
if (!isVar)
84+
Expressions.TypeAccess.Create(cx, optionalType, this, 0);
85+
else
86+
new Expressions.Discard(cx, discard, this, 0);
9187
break;
9288
case null:
9389
break;
@@ -104,10 +100,10 @@ protected override void Populate()
104100
switch (Stmt.Pattern)
105101
{
106102
case VarPatternSyntax varPattern:
107-
PopulatePattern(varPattern, null, varPattern.VarKeyword, varPattern.Designation);
103+
PopulatePattern(varPattern, null, varPattern.Designation);
108104
break;
109105
case DeclarationPatternSyntax declarationPattern:
110-
PopulatePattern(declarationPattern, declarationPattern.Type, default(SyntaxToken), declarationPattern.Designation);
106+
PopulatePattern(declarationPattern, declarationPattern.Type, declarationPattern.Designation);
111107
break;
112108
case ConstantPatternSyntax pattern:
113109
Expression.Create(cx, pattern.Expression, this, 0);
@@ -121,7 +117,7 @@ protected override void Populate()
121117

122118
if (Stmt.WhenClause != null)
123119
{
124-
Expression.Create(cx, Stmt.WhenClause.Condition, this, 2);
120+
Expression.Create(cx, Stmt.WhenClause.Condition, this, 1);
125121
}
126122
}
127123

‎csharp/ql/src/Bad Practices/Control-Flow/ConstantCondition.ql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ class ConstantNullnessCondition extends ConstantCondition {
7272
ConstantNullnessCondition() {
7373
forex(ControlFlow::Node cfn | cfn = this.getAControlFlowNode() |
7474
exists(ControlFlow::SuccessorTypes::NullnessSuccessor t, ControlFlow::Node s |
75-
s = cfn.getASuccessorByType(t) |
75+
s = cfn.getASuccessorByType(t)
76+
|
7677
b = t.getValue() and
7778
not s.isJoin()
7879
) and

‎csharp/ql/src/semmle/code/csharp/Assignable.qll‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,9 +306,8 @@ module AssignableInternal {
306306
/** A local variable declaration at the top-level of a pattern. */
307307
class TopLevelPatternDecl extends LocalVariableDeclExpr {
308308
private PatternMatch pm;
309-
TopLevelPatternDecl() {
310-
this = pm.getPattern().(BindingPatternExpr).getVariableDeclExpr()
311-
}
309+
310+
TopLevelPatternDecl() { this = pm.getPattern().(BindingPatternExpr).getVariableDeclExpr() }
312311

313312
PatternMatch getMatch() { result = pm }
314313
}

‎csharp/ql/src/semmle/code/csharp/ExprOrStmtParent.qll‎

Lines changed: 3 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,8 @@ predicate expr_parent_top_level_adjusted(Expr child, int i, @top_level_exprorstm
1919
}
2020

2121
/**
22-
* Holds if `case` statement `cs` is a type check of the form `case int _`,
23-
* where `ta` is the type access `int`.
24-
*/
25-
private predicate discardTypeCaseStmt(CaseStmt cs, TypeAccess ta) {
26-
expr_parent(ta, 1, cs) and
27-
expr_parent(any(DiscardExpr de), 0, cs)
28-
}
29-
30-
/**
31-
* The `expr_parent()` relation adjusted for expandable assignments, `is` expressions,
32-
* and `case` statements. For example, the assignment `x += y` is extracted as
22+
* The `expr_parent()` relation adjusted for expandable assignments. For example,
23+
* the assignment `x += y` is extracted as
3324
*
3425
* ```
3526
* +=
@@ -78,35 +69,7 @@ private predicate expr_parent_adjusted(Expr child, int i, ControlFlowElement par
7869
not ao.hasExpandedAssignment() and
7970
expr_parent(child, i, parent)
8071
)
81-
else
82-
if parent instanceof IsExpr
83-
then
84-
i = 0 and
85-
expr_parent(child, i, parent)
86-
or
87-
// e.g. `x is string s` or `x is null`
88-
i = 1 and
89-
expr_parent(child, any(int j | j in [2 .. 3]), parent)
90-
or
91-
// e.g. `x is string`
92-
i = 1 and
93-
not expr_parent(_, any(int j | j in [2 .. 3]), parent) and
94-
expr_parent(child, i, parent)
95-
else
96-
if parent instanceof CaseStmt
97-
then
98-
// e.g. `case string s:` or `case 5:`
99-
i = 0 and
100-
expr_parent(child, i, parent) and
101-
not discardTypeCaseStmt(parent, _)
102-
or
103-
// e.g. `case string _`
104-
i = 0 and
105-
discardTypeCaseStmt(parent, child)
106-
or
107-
i = 1 and
108-
expr_parent(child, 2, parent)
109-
else expr_parent(child, i, parent)
72+
else expr_parent(child, i, parent)
11073
}
11174

11275
/**

‎csharp/ql/src/semmle/code/csharp/controlflow/internal/Completion.qll‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -417,9 +417,7 @@ predicate switchMatching(Switch s, Case c, PatternExpr pe) {
417417
pe = c.getPattern()
418418
}
419419

420-
private predicate mustHaveMatchingCompletion(Switch s, PatternExpr pe) {
421-
switchMatching(s, _, pe)
422-
}
420+
private predicate mustHaveMatchingCompletion(Switch s, PatternExpr pe) { switchMatching(s, _, pe) }
423421

424422
/**
425423
* Holds if a normal completion of `cfe` must be a matching completion. Thats is,

‎csharp/ql/src/semmle/code/csharp/exprs/Expr.qll‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ class ConstantPatternExpr extends PatternExpr {
313313
*/
314314
class TypePatternExpr extends PatternExpr {
315315
private Type t;
316+
316317
TypePatternExpr() {
317318
t = this.(TypeAccess).getTarget() or
318319
t = this.(LocalVariableDeclExpr).getVariable().getType()
@@ -517,7 +518,7 @@ class Case extends PatternMatch, @case {
517518
/**
518519
* Gets the `when` expression of this case, if any. For example, `s.Length < 10`
519520
* in `string s when s.Length < 10 => s`
520-
*/
521+
*/
521522
Expr getCondition() { none() }
522523

523524
/** Gets the body of this `case`. */

‎csharp/ql/test/library-tests/csharp7/TypeCase2.ql‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import csharp
22

33
from Case c, boolean isVar, VariablePatternExpr vpe
4-
where vpe = c.getPattern() and
5-
if vpe.isImplicitlyTyped() then isVar = true else isVar = false
4+
where
5+
vpe = c.getPattern() and
6+
if vpe.isImplicitlyTyped() then isVar = true else isVar = false
67
select c, vpe, vpe.getType().toString(), isVar

‎csharp/ql/test/library-tests/csharp8/patterns.ql‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,4 @@ query predicate isRecursivePatternExprWithDecl(
6666
decl = expr.getRecursivePattern().getVariableDeclExpr()
6767
}
6868

69-
query predicate labeledPatternExpr(LabeledPatternExpr e, string s) {
70-
s = e.getLabel()
71-
}
69+
query predicate labeledPatternExpr(LabeledPatternExpr e, string s) { s = e.getLabel() }

‎csharp/ql/test/library-tests/csharp8/ranges.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ Expr stripConversions(Expr expr) {
1111
if getConversion(expr, _) then getConversion(expr, result) else result = expr
1212
}
1313

14-
query predicate indexes(IndexExpr e, Expr c) { c = e.getExpr() }
14+
query predicate indexes(IndexExpr e, Expr c) { c = e.getExpr() }
1515

1616
query predicate ranges(RangeExpr e) { any() }
1717

0 commit comments

Comments
 (0)