Skip to content

C#: Switch expression guards #1375

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.

Sign up for GitHub

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

Merged
calumgrant merged 1 commit into from
Jun 3, 2019
Merged

C#: Switch expression guards #1375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 12 additions & 8 deletions csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ module AbstractValues {
}

/** An integer value. */
class IntergerValue extends AbstractValue, TIntegerValue {
class IntegerValue extends AbstractValue, TIntegerValue {
/** Gets the underlying integer value. */
int getValue() { this = TIntegerValue(result) }

override predicate branch(ControlFlowElement cfe, ConditionalSuccessor s, Expr e) { none() }

override BooleanValue getDualValue() { none() }
override IntegerValue getDualValue() { none() }

override Expr getAnExpr() {
result.getValue().toInt() = this.getValue() and
Expand Down Expand Up @@ -199,7 +199,9 @@ class DereferenceableExpr extends Expr {
// incorrectly `int`, while it should have been `int?`. We apply
// `getNullEquivParent()` as a workaround
this = getNullEquivParent*(e) and
t = e.getType()
t = e.getType() and
not this instanceof SwitchCaseExpr and
not this instanceof PatternExpr
|
t instanceof NullableType and
isNullableType = true
Expand Down Expand Up @@ -734,6 +736,8 @@ module Internal {
Guard() {
this.getType() instanceof BoolType and
not this instanceof BoolLiteral and
not this instanceof SwitchCaseExpr and
not this instanceof PatternExpr and
val = TBooleanValue(_)
or
this instanceof DereferenceableExpr and
Expand Down Expand Up @@ -1037,9 +1041,9 @@ module Internal {
ck.isInequality() and branch = false
)
or
result = any(PatternMatch pm |
pm.getExpr() = e1 and
e2 = pm.getPattern().(ConstantPatternExpr) and
result = any(IsExpr ie |
ie.getExpr() = e1 and
e2 = ie.getPattern().(ConstantPatternExpr) and
branch = true
)
)
Expand Down Expand Up @@ -1068,11 +1072,11 @@ module Internal {
* then `o` is guaranteed to be equal to `""`.
*/
private Expr getAMatchingEqualityCheck(Expr e1, MatchValue v, Expr e2) {
exists(Switch s, ConstCase case | case = v.getCase() |
exists(Switch s, Case case | case = v.getCase() |
e1 = s.getExpr() and
result = e1 and
case = s.getACase() and
e2 = case.getPattern() and
e2 = case.getPattern().(ConstantPatternExpr) and
v.isMatch()
)
}
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/test/library-tests/controlflow/guards/ExtractorOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
// semmle-extractor-options: --cil
// semmle-extractor-options: --cil /langversion:8.0
16 changes: 16 additions & 0 deletions csharp/ql/test/library-tests/controlflow/guards/GuardedControlFlowNode.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,22 @@
| Guards.cs:269:11:269:12 | access to parameter o1 | Guards.cs:268:13:268:41 | call to operator == | Guards.cs:268:13:268:14 | access to parameter o1 | true |
| Guards.cs:269:11:269:12 | access to parameter o1 | Guards.cs:268:16:268:25 | call to method GetType | Guards.cs:268:13:268:14 | access to parameter o1 | non-null |
| Guards.cs:271:11:271:12 | access to parameter o1 | Guards.cs:270:13:270:42 | call to operator == | Guards.cs:270:13:270:14 | access to parameter o1 | true |
| Guards.cs:279:17:279:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match Action<Object> a |
| Guards.cs:279:17:279:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match "" |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<Object> a |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<String> a |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match null |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match "" |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<Object> a |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<String> a |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | null |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match "" |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<Object> a |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<String> a |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match null |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Splitting.cs:13:17:13:17 | [b (line 9): true] access to parameter o | Splitting.cs:12:17:12:17 | access to parameter o | Splitting.cs:12:17:12:17 | access to parameter o | non-null |
| Splitting.cs:13:17:13:17 | [b (line 9): true] access to parameter o | Splitting.cs:12:17:12:25 | ... != ... | Splitting.cs:12:17:12:17 | access to parameter o | true |
| Splitting.cs:14:13:14:13 | [b (line 9): false] access to parameter b | Splitting.cs:11:13:11:13 | access to parameter b | Splitting.cs:11:13:11:13 | access to parameter b | false |
Expand Down
16 changes: 16 additions & 0 deletions csharp/ql/test/library-tests/controlflow/guards/GuardedExpr.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,22 @@
| Guards.cs:269:11:269:12 | access to parameter o1 | Guards.cs:268:13:268:41 | call to operator == | Guards.cs:268:13:268:14 | access to parameter o1 | true |
| Guards.cs:269:11:269:12 | access to parameter o1 | Guards.cs:268:16:268:25 | call to method GetType | Guards.cs:268:13:268:14 | access to parameter o1 | non-null |
| Guards.cs:271:11:271:12 | access to parameter o1 | Guards.cs:270:13:270:42 | call to operator == | Guards.cs:270:13:270:14 | access to parameter o1 | true |
| Guards.cs:279:17:279:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match Action<Object> a |
| Guards.cs:279:17:279:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match "" |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<Object> a |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<String> a |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | match null |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match "" |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<Object> a |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<String> a |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | null |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match "" |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<Object> a |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match Action<String> a |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-match null |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Splitting.cs:13:17:13:17 | access to parameter o | Splitting.cs:12:17:12:17 | access to parameter o | Splitting.cs:12:17:12:17 | access to parameter o | non-null |
| Splitting.cs:13:17:13:17 | access to parameter o | Splitting.cs:12:17:12:25 | ... != ... | Splitting.cs:12:17:12:17 | access to parameter o | true |
| Splitting.cs:23:24:23:24 | access to parameter o | Splitting.cs:22:17:22:17 | access to parameter o | Splitting.cs:22:17:22:17 | access to parameter o | non-null |
Expand Down
65 changes: 65 additions & 0 deletions csharp/ql/test/library-tests/controlflow/guards/Guards.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,4 +270,69 @@ void M22(object o1, object o2)
if (o1?.GetType() == o2?.GetType())
o1.ToString(); // not null guarded
}

string M23(object o)
{
return o switch
{
Action<object> a =>
o.ToString(), // null guarded
Action<string> a =>
a.ToString(), // not null (but not a guard)
"" =>
o.ToString(), // null guarded
null =>
o.ToString(), // not null guarded
_ =>
o.ToString() // null guarded
};
}

int M24(bool b1)
{
var b2 = true;
if (b1)
b2 = false;
return b2 switch
{
true => 0,
_ => 1
};
}

int M25(bool b1)
{
var b2 = false;
if (b1)
b2 = true;
return b2 switch
{
true => 0,
_ => 1
};
}

int M26(bool b)
{
var i = 0;
if (b)
i = 1;
return i switch
{
1 => 0,
_ => 1
};
}

int M27(bool b)
{
var e = E.A;
if (b)
e = E.B;
return e switch
{
E.B => 0,
_ => 1
};
}
}
26 changes: 26 additions & 0 deletions csharp/ql/test/library-tests/controlflow/guards/Implications.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,32 @@
| Guards.cs:268:16:268:25 | call to method GetType | non-null | Guards.cs:268:13:268:14 | access to parameter o1 | non-null |
| Guards.cs:270:16:270:25 | call to method GetType | non-null | Guards.cs:270:13:270:14 | access to parameter o1 | non-null |
| Guards.cs:270:33:270:42 | call to method GetType | non-null | Guards.cs:270:30:270:31 | access to parameter o2 | non-null |
| Guards.cs:276:16:276:16 | access to parameter o | match "" | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Guards.cs:276:16:276:16 | access to parameter o | match Action<Object> a | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Guards.cs:276:16:276:16 | access to parameter o | match Action<String> a | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Guards.cs:276:16:276:16 | access to parameter o | match null | Guards.cs:276:16:276:16 | access to parameter o | null |
| Guards.cs:276:16:276:16 | access to parameter o | non-match null | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Guards.cs:281:17:281:17 | access to local variable a | non-null | Guards.cs:276:16:276:16 | access to parameter o | non-null |
| Guards.cs:281:17:281:17 | access to local variable a | null | Guards.cs:276:16:276:16 | access to parameter o | null |
| Guards.cs:293:13:293:21 | Boolean b2 = ... | false | Guards.cs:293:13:293:14 | access to local variable b2 | false |
| Guards.cs:293:13:293:21 | Boolean b2 = ... | true | Guards.cs:293:13:293:14 | access to local variable b2 | true |
| Guards.cs:295:13:295:22 | ... = ... | false | Guards.cs:295:13:295:14 | access to local variable b2 | false |
| Guards.cs:295:13:295:22 | ... = ... | true | Guards.cs:295:13:295:14 | access to local variable b2 | true |
| Guards.cs:296:16:296:17 | access to local variable b2 | match true | Guards.cs:294:13:294:14 | access to parameter b1 | false |
| Guards.cs:296:16:296:17 | access to local variable b2 | match true | Guards.cs:296:16:296:17 | access to local variable b2 | true |
| Guards.cs:305:13:305:22 | Boolean b2 = ... | false | Guards.cs:305:13:305:14 | access to local variable b2 | false |
| Guards.cs:305:13:305:22 | Boolean b2 = ... | true | Guards.cs:305:13:305:14 | access to local variable b2 | true |
| Guards.cs:307:13:307:21 | ... = ... | false | Guards.cs:307:13:307:14 | access to local variable b2 | false |
| Guards.cs:307:13:307:21 | ... = ... | true | Guards.cs:307:13:307:14 | access to local variable b2 | true |
| Guards.cs:308:16:308:17 | access to local variable b2 | match true | Guards.cs:306:13:306:14 | access to parameter b1 | true |
| Guards.cs:308:16:308:17 | access to local variable b2 | match true | Guards.cs:308:16:308:17 | access to local variable b2 | true |
| Guards.cs:308:16:308:17 | access to local variable b2 | non-match true | Guards.cs:306:13:306:14 | access to parameter b1 | false |
| Guards.cs:320:16:320:16 | access to local variable i | match 1 | Guards.cs:318:13:318:13 | access to parameter b | true |
| Guards.cs:320:16:320:16 | access to local variable i | match 1 | Guards.cs:320:16:320:16 | access to local variable i | 1 |
| Guards.cs:320:16:320:16 | access to local variable i | non-match 1 | Guards.cs:318:13:318:13 | access to parameter b | false |
| Guards.cs:332:16:332:16 | access to local variable e | match access to constant B | Guards.cs:330:13:330:13 | access to parameter b | true |
| Guards.cs:332:16:332:16 | access to local variable e | match access to constant B | Guards.cs:332:16:332:16 | access to local variable e | 1 |
| Guards.cs:332:16:332:16 | access to local variable e | non-match access to constant B | Guards.cs:330:13:330:13 | access to parameter b | false |
| Splitting.cs:12:17:12:25 | ... != ... | false | Splitting.cs:12:17:12:17 | access to parameter o | null |
| Splitting.cs:12:17:12:25 | ... != ... | true | Splitting.cs:12:17:12:17 | access to parameter o | non-null |
| Splitting.cs:22:17:22:25 | ... != ... | false | Splitting.cs:22:17:22:17 | access to parameter o | null |
Expand Down
12 changes: 12 additions & 0 deletions csharp/ql/test/library-tests/controlflow/guards/MatchingGuardedExpr.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,15 @@
| Guards.cs:162:24:162:24 | access to parameter o | Guards.cs:151:17:151:17 | access to parameter o | Guards.cs:155:13:155:34 | case ...: | non-match Action<String> a | false |
| Guards.cs:162:24:162:24 | access to parameter o | Guards.cs:151:17:151:17 | access to parameter o | Guards.cs:157:13:157:20 | case ...: | non-match "" | false |
| Guards.cs:162:24:162:24 | access to parameter o | Guards.cs:151:17:151:17 | access to parameter o | Guards.cs:159:13:159:22 | case ...: | non-match null | false |
| Guards.cs:279:17:279:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:278:13:279:28 | ... => ... | match Action<Object> a | true |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:278:13:279:28 | ... => ... | non-match Action<Object> a | false |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:280:13:281:28 | ... => ... | non-match Action<String> a | false |
| Guards.cs:283:17:283:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:282:13:283:28 | ... => ... | match "" | true |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:278:13:279:28 | ... => ... | non-match Action<Object> a | false |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:280:13:281:28 | ... => ... | non-match Action<String> a | false |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:282:13:283:28 | ... => ... | non-match "" | false |
| Guards.cs:285:17:285:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:284:13:285:28 | ... => ... | match null | true |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:278:13:279:28 | ... => ... | non-match Action<Object> a | false |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:280:13:281:28 | ... => ... | non-match Action<String> a | false |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:282:13:283:28 | ... => ... | non-match "" | false |
| Guards.cs:287:17:287:17 | access to parameter o | Guards.cs:276:16:276:16 | access to parameter o | Guards.cs:284:13:285:28 | ... => ... | non-match null | false |
3 changes: 3 additions & 0 deletions csharp/ql/test/library-tests/controlflow/guards/NullGuardedExpr.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@
| Guards.cs:205:13:205:13 | access to parameter o |
| Guards.cs:208:17:208:17 | access to parameter o |
| Guards.cs:269:11:269:12 | access to parameter o1 |
| Guards.cs:279:17:279:17 | access to parameter o |
| Guards.cs:283:17:283:17 | access to parameter o |
| Guards.cs:287:17:287:17 | access to parameter o |
| Splitting.cs:13:17:13:17 | access to parameter o |
| Splitting.cs:23:24:23:24 | access to parameter o |
| Splitting.cs:35:13:35:13 | access to parameter o |
Expand Down