From e508931cba4c64cd3385e0ea523ddee7255eb9e4 Mon Sep 17 00:00:00 2001
From: Thomas Levy <tlevy@evernote.com>
Date: Thu, 9 Nov 2017 08:56:50 -0800
Subject: [PATCH] Extract all bindings in variable declarations

Fixes #828 and also some other false negatives from ReassignedParameter
---
 .../test/resources/checks/ReassignedParameter.js   | 14 +++++++-------
 .../checks/variableDeclarationWithoutVar.js        |  5 +++++
 .../tree/symbols/HoistedSymbolVisitor.java         | 11 +++++------
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/javascript-checks/src/test/resources/checks/ReassignedParameter.js b/javascript-checks/src/test/resources/checks/ReassignedParameter.js
index fac0c128fc..16d752d5fe 100644
--- a/javascript-checks/src/test/resources/checks/ReassignedParameter.js
+++ b/javascript-checks/src/test/resources/checks/ReassignedParameter.js
@@ -72,11 +72,11 @@ for (var x in obj) {
 }
 
 for (var [a, b] in obj) {
-  a = foo(); // FN (SYMBOL TABLE should be improved)
+  a = foo(); // Noncompliant
 }
 
 for (let {prop1, prop2} in obj) {
-  prop1 = foo(); // FN (SYMBOL TABLE should be improved)
+  prop1 = foo(); // Noncompliant
 }
 
 for (let x of obj) {
@@ -95,16 +95,16 @@ for (z in obj) {
 }
 
 for ([a, [b]] in obj) {
-  a = foo(); // FN
-  b = foo(); // FN
+  a = foo(); // Noncompliant
+  b = foo(); // Noncompliant
 }
 
 for ({a, b} in obj) {
-  a = foo(); // FN
-  b = foo(); // FN
+  a = foo(); // Noncompliant
+  b = foo(); // Noncompliant
 }
 
 // illegal code
 for (a[1] in obj) {
-  a = foo();
+  a = foo(); // Noncompliant, FP but only because of illegal code
 }
diff --git a/javascript-checks/src/test/resources/checks/variableDeclarationWithoutVar.js b/javascript-checks/src/test/resources/checks/variableDeclarationWithoutVar.js
index fae88bb63f..1cbe7736fd 100644
--- a/javascript-checks/src/test/resources/checks/variableDeclarationWithoutVar.js
+++ b/javascript-checks/src/test/resources/checks/variableDeclarationWithoutVar.js
@@ -18,6 +18,11 @@ function fun(){
   fun(arguments) // OK
 }
 
+let unflowed;
+unflowed = false;
+
+let flowed: boolean;
+flowed = true;
 
 // Node.js-related exclusions
 var foo = exports = module.exports = {} // OK
diff --git a/javascript-frontend/src/main/java/org/sonar/javascript/tree/symbols/HoistedSymbolVisitor.java b/javascript-frontend/src/main/java/org/sonar/javascript/tree/symbols/HoistedSymbolVisitor.java
index 1e604826f7..93dfe9d69d 100644
--- a/javascript-frontend/src/main/java/org/sonar/javascript/tree/symbols/HoistedSymbolVisitor.java
+++ b/javascript-frontend/src/main/java/org/sonar/javascript/tree/symbols/HoistedSymbolVisitor.java
@@ -284,7 +284,6 @@ private void addUsages(VariableDeclarationTree tree) {
       scope = getFunctionScope();
     }
 
-    // todo Consider other BindingElementTree types
     for (BindingElementTree bindingElement : tree.variables()) {
       Symbol.Kind variableKind = getVariableKind(tree);
 
@@ -293,11 +292,11 @@ private void addUsages(VariableDeclarationTree tree) {
           symbolModel.declareSymbol(identifier.name(), variableKind, scope)
             .addUsage(identifier, Usage.Kind.DECLARATION_WRITE);
         }
-      }
-      if (bindingElement.is(Kind.BINDING_IDENTIFIER)) {
-        IdentifierTree identifierTree = (IdentifierTree) bindingElement;
-        symbolModel.declareSymbol(identifierTree.name(), variableKind, scope)
-          .addUsage(identifierTree, insideForLoopVariable ? Usage.Kind.DECLARATION_WRITE : Usage.Kind.DECLARATION);
+      } else {
+        for (IdentifierTree identifier : bindingElement.bindingIdentifiers()) {
+          symbolModel.declareSymbol(identifier.name(), variableKind, scope)
+            .addUsage(identifier, insideForLoopVariable ? Usage.Kind.DECLARATION_WRITE : Usage.Kind.DECLARATION);
+        }
       }
     }
   }