Merge pull request #2581 from erik-krogh/FlowUselessExpr

Approved by max-schaefer

change-notes/1.24/analysis-javascript.md

@@ -23,6 +23,7 @@
 | Clear-text logging of sensitive information (`js/clear-text-logging`) | More results | More results involving `process.env` and indirect calls to logging methods are recognized. |
 | 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. |
 
 ## Changes to libraries
 

javascript/ql/src/Expressions/ExprHasNoEffect.qll

@@ -40,7 +40,6 @@ predicate inVoidContext(Expr e) {
   exists(LogicalBinaryExpr logical | e = logical.getRightOperand() and inVoidContext(logical))
 }
 
-
 /**
  * Holds if `e` is of the form `x;` or `e.p;` and has a JSDoc comment containing a tag.
  * In that case, it is probably meant as a declaration and shouldn't be flagged by this query.
@@ -155,5 +154,7 @@ predicate hasNoEffect(Expr e) {
   not exists(FunctionExpr fe, ExprStmt es | fe = e |
     fe = es.getExpr() and
     not exists(fe.getName())
-  )	
-}
+  ) and
+  // exclude block-level flow type annotations. For example: `(name: empty)`.
+  not e.(ParExpr).getExpression().getLastToken().getNextToken().getValue() = ":"
+}

javascript/ql/test/query-tests/Expressions/ExprHasNoEffect/options

@@ -0,0 +1 @@
+semmle-extractor-options: --experimental

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

@@ -72,4 +72,6 @@ function g() {
 
 	Object.defineProperty(o, "nonTrivialGetter2", unknownGetterDef());
 	o.nonTrivialGetter2; // OK
+	
+	(o: empty); // OK.
 };