Skip to content

C#: Consider as expressions as maybe-null in cs/dereferenced-value-may-be-null #692

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 2 commits into from
Dec 20, 2018
Merged

C#: Consider as expressions as maybe-null in cs/dereferenced-value-may-be-null #692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ccda1c8
C#: Add nullness test using an `as` expression
hvitved Dec 14, 2018
33fcbc9
C#: Consider `as` expressions as maybe-`null` in `cs/dereferenced-val…
hvitved Dec 14, 2018
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: 2 additions & 0 deletions csharp/ql/src/semmle/code/csharp/dataflow/Nullness.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class MaybeNullExpr extends Expr {
this.(AssignExpr).getRValue() instanceof MaybeNullExpr
or
this.(Cast).getExpr() instanceof MaybeNullExpr
or
this instanceof AsExpr
}
}

Expand Down
6 changes: 6 additions & 0 deletions csharp/ql/test/query-tests/Nullness/E.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,12 @@ public void Ex24(bool b)
}
}

public void Ex25(object o)
{
var s = o as string;
s.ToString(); // BAD (maybe)
}

public bool Field;
string Make() => Field ? null : "";
}
Expand Down
12 changes: 8 additions & 4 deletions csharp/ql/test/query-tests/Nullness/Implications.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1468,10 +1468,14 @@
| E.cs:293:15:293:19 | call to method M2 | null | E.cs:293:13:293:13 | access to local variable s | null |
| E.cs:295:13:295:13 | access to local variable s | non-null | E.cs:292:20:292:32 | ... ? ... : ... | non-null |
| E.cs:295:13:295:13 | access to local variable s | null | E.cs:292:20:292:32 | ... ? ... : ... | null |
| E.cs:300:22:300:38 | ... ? ... : ... | non-null | E.cs:300:22:300:26 | access to field Field | false |
| E.cs:300:22:300:38 | ... ? ... : ... | non-null | E.cs:300:37:300:38 | "" | non-null |
| E.cs:300:22:300:38 | ... ? ... : ... | null | E.cs:300:22:300:26 | access to field Field | true |
| E.cs:300:22:300:38 | ... ? ... : ... | null | E.cs:300:30:300:33 | null | null |
| E.cs:301:13:301:27 | String s = ... | non-null | E.cs:301:13:301:13 | access to local variable s | non-null |
| E.cs:301:13:301:27 | String s = ... | null | E.cs:301:13:301:13 | access to local variable s | null |
| E.cs:302:9:302:9 | access to local variable s | non-null | E.cs:301:17:301:27 | ... as ... | non-null |
| E.cs:302:9:302:9 | access to local variable s | null | E.cs:301:17:301:27 | ... as ... | null |
| E.cs:306:22:306:38 | ... ? ... : ... | non-null | E.cs:306:22:306:26 | access to field Field | false |
| E.cs:306:22:306:38 | ... ? ... : ... | non-null | E.cs:306:37:306:38 | "" | non-null |
| E.cs:306:22:306:38 | ... ? ... : ... | null | E.cs:306:22:306:26 | access to field Field | true |
| E.cs:306:22:306:38 | ... ? ... : ... | null | E.cs:306:30:306:33 | null | null |
| Forwarding.cs:7:16:7:23 | String s = ... | non-null | Forwarding.cs:7:16:7:16 | access to local variable s | non-null |
| Forwarding.cs:7:16:7:23 | String s = ... | null | Forwarding.cs:7:16:7:16 | access to local variable s | null |
| Forwarding.cs:9:13:9:30 | !... | false | Forwarding.cs:9:14:9:30 | call to method IsNullOrEmpty | true |
Expand Down
4 changes: 4 additions & 0 deletions csharp/ql/test/query-tests/Nullness/NullMaybe.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,8 @@ nodes
| E.cs:283:13:283:22 | [b (line 279): true] SSA def(o) |
| E.cs:285:9:285:9 | access to local variable o |
| E.cs:285:9:285:9 | access to local variable o |
| E.cs:301:13:301:27 | SSA def(s) |
| E.cs:302:9:302:9 | access to local variable s |
| Forwarding.cs:7:16:7:23 | SSA def(s) |
| Forwarding.cs:14:9:17:9 | if (...) ... |
| Forwarding.cs:19:9:22:9 | if (...) ... |
Expand Down Expand Up @@ -675,6 +677,7 @@ edges
| E.cs:238:26:238:26 | SSA param(i) | E.cs:240:21:240:21 | access to parameter i |
| E.cs:283:13:283:22 | [b (line 279): false] SSA def(o) | E.cs:285:9:285:9 | access to local variable o |
| E.cs:283:13:283:22 | [b (line 279): true] SSA def(o) | E.cs:285:9:285:9 | access to local variable o |
| E.cs:301:13:301:27 | SSA def(s) | E.cs:302:9:302:9 | access to local variable s |
| Forwarding.cs:7:16:7:23 | SSA def(s) | Forwarding.cs:14:9:17:9 | if (...) ... |
| Forwarding.cs:14:9:17:9 | if (...) ... | Forwarding.cs:19:9:22:9 | if (...) ... |
| Forwarding.cs:19:9:22:9 | if (...) ... | Forwarding.cs:24:9:27:9 | if (...) ... |
Expand Down Expand Up @@ -774,6 +777,7 @@ edges
| E.cs:240:21:240:21 | access to parameter i | E.cs:238:26:238:26 | SSA param(i) | E.cs:240:21:240:21 | access to parameter i | Variable $@ may be null here because it has a nullable type. | E.cs:238:26:238:26 | i | i | E.cs:238:26:238:26 | i | this |
| E.cs:285:9:285:9 | access to local variable o | E.cs:283:13:283:22 | [b (line 279): false] SSA def(o) | E.cs:285:9:285:9 | access to local variable o | Variable $@ may be null here as suggested by $@ null check. | E.cs:283:13:283:13 | o | o | E.cs:284:9:284:9 | access to local variable o | this |
| E.cs:285:9:285:9 | access to local variable o | E.cs:283:13:283:22 | [b (line 279): true] SSA def(o) | E.cs:285:9:285:9 | access to local variable o | Variable $@ may be null here as suggested by $@ null check. | E.cs:283:13:283:13 | o | o | E.cs:284:9:284:9 | access to local variable o | this |
| E.cs:302:9:302:9 | access to local variable s | E.cs:301:13:301:27 | SSA def(s) | E.cs:302:9:302:9 | access to local variable s | Variable $@ may be null here because of $@ assignment. | E.cs:301:13:301:13 | s | s | E.cs:301:13:301:27 | String s = ... | this |
| GuardedString.cs:35:31:35:31 | access to local variable s | GuardedString.cs:7:16:7:32 | SSA def(s) | GuardedString.cs:35:31:35:31 | access to local variable s | Variable $@ may be null here because of $@ assignment. | GuardedString.cs:7:16:7:16 | s | s | GuardedString.cs:7:16:7:32 | String s = ... | this |
| NullMaybeBad.cs:7:27:7:27 | access to parameter o | NullMaybeBad.cs:13:17:13:20 | null | NullMaybeBad.cs:7:27:7:27 | access to parameter o | Variable $@ may be null here because of $@ null argument. | NullMaybeBad.cs:5:25:5:25 | o | o | NullMaybeBad.cs:13:17:13:20 | null | this |
| StringConcatenation.cs:16:17:16:17 | access to local variable s | StringConcatenation.cs:14:16:14:23 | SSA def(s) | StringConcatenation.cs:16:17:16:17 | access to local variable s | Variable $@ may be null here because of $@ assignment. | StringConcatenation.cs:14:16:14:16 | s | s | StringConcatenation.cs:14:16:14:23 | String s = ... | this |