Merge pull request #2627 from asger-semmle/js-useless-expression-trycatch

Approved by esbena

change-notes/1.24/analysis-javascript.md

@@ -30,7 +30,7 @@
 | Duplicate parameter names (`js/duplicate-parameter-name`) | Fewer results | This query now recognizes additional parameters that reasonably can have duplicated names. |
 | Incomplete string escaping or encoding (`js/incomplete-sanitization`) | Fewer false positive results | This query now recognizes additional cases where a single replacement is likely to be intentional. |
 | Unbound event handler receiver (`js/unbound-event-handler-receiver`) | Fewer false positive results | This query now recognizes additional ways event handler receivers can be bound. | 
-| Expression has no effect (`js/useless-expression`) | Fewer false positive results | The query now recognizes block-level flow type annotations. |
+| Expression has no effect (`js/useless-expression`) | Fewer false positive results | The query now recognizes block-level flow type annotations and ignores the first statement of a try block. |
 | Use of call stack introspection in strict mode (`js/strict-mode-call-stack-introspection`) | Fewer false positive results | The query no longer flags expression statements. |
 
 ## Changes to libraries

javascript/ql/src/Expressions/ExprHasNoEffect.qll

@@ -156,5 +156,7 @@ predicate hasNoEffect(Expr e) {
     not exists(fe.getName())
   ) and
   // exclude block-level flow type annotations. For example: `(name: empty)`.
-  not e.(ParExpr).getExpression().getLastToken().getNextToken().getValue() = ":"
+  not e.(ParExpr).getExpression().getLastToken().getNextToken().getValue() = ":" and
+  // exclude the first statement of a try block
+  not e = any(TryStmt stmt).getBody().getStmt(0).(ExprStmt).getExpr()
 }

javascript/ql/test/query-tests/Expressions/ExprHasNoEffect/ExprHasNoEffect.expected

@@ -1,3 +1,4 @@
+| try.js:22:9:22:26 | x.ordinaryProperty | This expression has no effect. |
 | tst2.js:3:4:3:4 | 0 | This expression has no effect. |
 | tst.js:3:1:3:2 | 23 | This expression has no effect. |
 | tst.js:5:1:5:2 | 23 | This expression has no effect. |

javascript/ql/test/query-tests/Expressions/ExprHasNoEffect/try.js

@@ -0,0 +1,27 @@
+function try1(x) {
+    try {
+        x.ordinaryProperty; // OK - try/catch indicates intent to throw exception
+    } catch (e) {
+        return false;
+    }
+    return true;
+}
+
+function try2(x) {
+    try {
+        x.ordinaryProperty; // OK - try/catch indicates intent to throw exception
+        return x;
+    } catch (e) {
+        return false;
+    }
+}
+
+function try3(x) {
+    try {
+        x.ordinaryProperty()
+        x.ordinaryProperty // NOT OK
+        return x;
+    } catch (e) {
+        return false;
+    }
+}