C#: Fix false positives in cs/dereferenced-value-may-be-null

Author: hvitved

csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll

@@ -578,8 +578,12 @@ module Internal {
    */
   Expr getNullEquivParent(Expr e) {
     result = any(QualifiableExpr qe |
-      qe.getQualifier() = e and
       qe.isConditional() and
+      (
+        e = qe.getQualifier()
+        or
+        e = qe.(ExtensionMethodCall).getArgument(0)
+      ) and
       (
         // The accessed declaration must have a value type in order
         // for `only if` to hold

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

@@ -155,6 +155,9 @@ private predicate defMaybeNull(Ssa::Definition def, string msg, Element reason)
     reason = de.getANullCheck(_, true) and
     msg = "as suggested by $@ null check" and
     not def instanceof Ssa::PseudoDefinition and
+    strictcount(Location l |
+      l = any(Ssa::Definition def0 | de = def0.getARead()).getLocation()
+    ) = 1 and
     not nonNullDef(def) and
     // Don't use a check as reason if there is a `null` assignment
     not def.(Ssa::ExplicitDefinition).getADefinition().getSource() instanceof MaybeNullExpr

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

@@ -267,7 +267,7 @@ public void Ex22()
         try
         {
             o = Make();
-            o.ToString(); // GOOD (false positive)
+            o.ToString(); // GOOD
         }
         finally
         {
@@ -292,7 +292,7 @@ public void Ex24(bool b)
         string s = b ? null : "";
         if (s?.M2() == 0)
         {
-            s.ToString(); // GOOD (false positive)
+            s.ToString(); // GOOD
         }
     }
 

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

@@ -1453,6 +1453,9 @@
 | E.cs:293:13:293:13 | access to local variable s | non-null | E.cs:292:20:292:32 | ... ? ... : ... | non-null |
 | E.cs:293:13:293:13 | access to local variable s | null | E.cs:292:20:292:20 | access to parameter b | true |
 | E.cs:293:13:293:13 | access to local variable s | null | E.cs:292:20:292:32 | ... ? ... : ... | null |
+| E.cs:293:13:293:24 | ... == ... | true | E.cs:293:15:293:19 | call to method M2 | non-null |
+| E.cs:293:15:293:19 | call to method M2 | non-null | E.cs:293:13:293:13 | access to local variable s | non-null |
+| 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 |

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

@@ -199,6 +199,7 @@
 | E.cs:284:9:284:9 | access to local variable o | E.cs:284:9:284:9 | access to local variable o | null | true |
 | E.cs:293:13:293:13 | access to local variable s | E.cs:293:13:293:13 | access to local variable s | non-null | false |
 | E.cs:293:13:293:13 | access to local variable s | E.cs:293:13:293:13 | access to local variable s | null | true |
+| E.cs:293:13:293:24 | ... == ... | E.cs:293:15:293:19 | call to method M2 | true | false |
 | Forwarding.cs:9:14:9:30 | call to method IsNullOrEmpty | Forwarding.cs:9:14:9:14 | access to local variable s | false | false |
 | Forwarding.cs:14:13:14:32 | call to method IsNotNullOrEmpty | Forwarding.cs:14:13:14:13 | access to local variable s | true | false |
 | Forwarding.cs:19:14:19:23 | call to method IsNull | Forwarding.cs:19:14:19:14 | access to local variable s | false | false |

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

@@ -68,9 +68,7 @@
 | E.cs:230:9:230:9 | access to local variable x | Variable $@ may be null here because of $@ assignment. | E.cs:225:13:225:13 | x | x | E.cs:227:13:227:20 | ... = ... | this |
 | E.cs:235:16:235:16 | access to parameter i | Variable $@ may be null here because it has a nullable type. | E.cs:233:26:233:26 | i | i | E.cs:233:26:233:26 | i | this |
 | 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:270:13:270:13 | access to local variable o | Variable $@ may be null here as suggested by $@ null check. | E.cs:266:16:266:16 | o | o | E.cs:274:17:274:25 | ... != ... | this |
 | 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:295:13:295:13 | access to local variable s | Variable $@ may be null here because of $@ assignment. | E.cs:292:16:292:16 | s | s | E.cs:292:16:292:32 | String s = ... | this |
 | 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 | 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 | 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 |