Merge pull request #1648 from hvitved/csharp/unchecked-return-lambda

C#: Fix false positives in `cs/unchecked-return-value`

Author: lcartey

change-notes/1.22/analysis-csharp.md

@@ -24,6 +24,7 @@
 | Number of commits | No results | Query has been removed. |
 | Poorly documented files with many authors | No results | Query has been removed. |
 | Recent activity | No results | Query has been removed. |
+| Unchecked return value (`cs/unchecked-return-value`) | Fewer false positive results | Method calls that are expression bodies of `void` callables (for example, the call to `Foo` in `void Bar() => Foo()`) are no longer considered to use the return value. |
 
 ## Changes to code extraction
 

csharp/ql/src/API Abuse/UncheckedReturnValue.ql

@@ -89,7 +89,14 @@ predicate whitelist(Method m) {
 }
 
 class DiscardedMethodCall extends MethodCall {
-  DiscardedMethodCall() { this.getParent() instanceof ExprStmt }
+  DiscardedMethodCall() {
+    this.getParent() instanceof ExprStmt
+    or
+    exists(Callable c |
+      this = c.getExpressionBody() and
+      not c.canReturn(this)
+    )
+  }
 
   string query() {
     exists(Method m |

csharp/ql/src/semmle/code/csharp/Callable.qll

@@ -209,7 +209,8 @@ class Callable extends DotNet::Callable, Parameterizable, ExprOrStmtParent, @cal
   override predicate canReturn(DotNet::Expr e) {
     exists(ReturnStmt ret | ret.getEnclosingCallable() = this | e = ret.getExpr())
     or
-    e = getExpressionBody()
+    e = this.getExpressionBody() and
+    not this.getReturnType() instanceof VoidType
   }
 
   /** Holds if this callable can yield return the expression `e`. */

csharp/ql/test/query-tests/API Abuse/UncheckedReturnValue/UncheckedReturnValue.cs

@@ -158,3 +158,23 @@ class C5 { }
     class C6 : C5 { }
     class C7 : C6 { }
 }
+
+class C5
+{
+    int M1() => 0;
+
+    void M2()
+    {
+        // GOOD
+        M1();
+        Action a = () => M1();
+        a = () => M1();
+        a = () => M1();
+        a = () => M1();
+        a = () => M1();
+        a = () => M1();
+        a = () => M1();
+        a = () => M1();
+        a = () => M1();
+    }
+}