Merge pull request #692 from hvitved/csharp/maybe-null-as-expression

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

Author: calumgrant

csharp/ql/src/semmle/code/csharp/dataflow/Nullness.qll

@@ -39,6 +39,8 @@ class MaybeNullExpr extends Expr {
     this.(AssignExpr).getRValue() instanceof MaybeNullExpr
     or
     this.(Cast).getExpr() instanceof MaybeNullExpr
+    or
+    this instanceof AsExpr
   }
 }
 

csharp/ql/test/query-tests/Nullness/E.cs

@@ -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 : "";
 }

csharp/ql/test/query-tests/Nullness/Implications.expected

@@ -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 |

csharp/ql/test/query-tests/Nullness/NullMaybe.expected

@@ -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 (...) ... |
@@ -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 (...) ... |
@@ -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 |