[elpa] master cdbdca4 39/49: Rewrite js2--classify-variables, focusing only on name nodes

[Dmitry Gutov]

branch: master
commit cdbdca46393dd687eccf9ef18d833212a4a3853b
Author: Lele Gaifax <address@hidden>
Commit: Lele Gaifax <address@hidden>

    Rewrite js2--classify-variables, focusing only on name nodes
    
    This streamlines the function, avoiding multiple paths that obfuscated the 
logic:
    it should be now easier to reason about its behavior and integrate other 
corner
    cases, if/when needed.
---
 js2-mode.el     |  152 ++++++++++++++++++++++++++++++-------------------------
 tests/parser.el |   22 ++++----
 2 files changed, 93 insertions(+), 81 deletions(-)

diff --git a/js2-mode.el b/js2-mode.el
index e0ad317..6c60522 100644
--- a/js2-mode.el
+++ b/js2-mode.el
@@ -7136,14 +7136,6 @@ in the cdr of the entry.
             (setq used nil))
           (puthash sym (cons inition (if used (list symbol))) vars))))))
 
-(defun js2--add-or-update-symbols (targets inition used vars)
-  "Determine the state of each symbol in TARGETS.
-TARGETS may be either a single js2-name-node, a js2-array-node or a 
js2-object-node.
-In the first case simply call `js2--add-or-update-symbol' forwarding the same 
arguments.
-The latter two cases happen in destructuring assignments: recursively update 
the symbols."
-  (dolist (elt (js2--collect-declared-symbols targets))
-    (js2--add-or-update-symbol elt inition used vars)))
-
 (defun js2--collect-declared-symbols (node strict)
   "Collect the `js-name-node' symbols declared in NODE and return a list of 
them.
 NODE is either `js2-array-node', `js2-object-node', or `js2-name-node'.
@@ -7202,67 +7194,91 @@ are ignored."
     (js2-visit-ast
      js2-mode-ast
      (lambda (node end-p)
-       (when (null end-p)
-         (cond
-          ((js2-var-init-node-p node)
-           ;; take note about possibly initialized declarations
-           (let ((target (js2-var-init-node-target node))
-                 (initializer (js2-var-init-node-initializer node)))
-             (when target
-               (let* ((parent (js2-node-parent node))
-                      (grandparent (if parent (js2-node-parent parent)))
-                      (inited (not (null initializer))))
-                 (unless inited
-                   (setq inited
-                         (and grandparent
-                              (js2-for-in-node-p grandparent)
-                              (memq target
-                                    (mapcar #'js2-var-init-node-target
-                                            (js2-var-decl-node-kids
-                                             (js2-for-in-node-iterator 
grandparent)))))))
-                 (js2--add-or-update-symbols target inited nil vars)))))
-
-          ((js2-assign-node-p node)
-           ;; take note about assignments
-           (js2--add-or-update-symbols (js2-assign-node-left node) t nil vars))
-
-          ((js2-prop-get-node-p node)
-           ;; handle x.y.z nodes, considering only x
-           (let ((left (js2-prop-get-node-left node)))
-             (when (js2-name-node-p left)
-               (js2--add-or-update-symbols left nil t vars))))
-
-          ((js2-name-node-p node)
-           ;; take note about used variables
-           (let ((parent (js2-node-parent node)))
-             (when parent
-               (unless (or (and (js2-var-init-node-p parent) ; handled above
-                                (memq node (js2--collect-declared-symbols
-                                            (js2-var-init-node-target 
parent))))
-                           (and (js2-assign-node-p parent)
-                                (memq node (js2--collect-declared-symbols
-                                            (js2-assign-node-left parent))))
-                           (js2-prop-get-node-p parent))
-                 (let ((used t) inited)
+       (when (and (null end-p) (js2-name-node-p node))
+         (let* ((parent (js2-node-parent node))
+                (function-param (and (js2-function-node-p parent)
+                                     (memq node (js2-function-node-params 
parent)))))
+           (when parent
+             (if (js2-prop-get-node-p parent)
+                 ;; If we are within a prop-get, e.g. the "bar" in "foo.bar",
+                 ;; just mark "foo" as used
+                 (let ((left (js2-prop-get-node-left parent)))
+                   (when (js2-name-node-p left)
+                     (js2--add-or-update-symbol left nil t vars)))
+               (let ((granparent parent)
+                     var-init-node
+                     assign-node
+                     object-key         ; is name actually an object prop key?
+                     declared           ; is it declared in narrowest scope?
+                     assigned
+                     (used (null function-param)))
+                 ;; Determine the closest var-init-node and assign-node: this
+                 ;; is needed because the name may be within a "destructured"
+                 ;; declaration/assignment, so we cannot just take its parent
+                 (while (and granparent (not (js2-scope-p granparent)))
                    (cond
-                    ((and (js2-function-node-p parent)
-                          (js2-wrapper-function-p parent))
-                     (setq inited (if (memq node (js2-function-node-params 
parent)) ?P t)))
-
-                    ((js2-for-in-node-p parent)
-                     (if (eq node (js2-for-in-node-iterator parent))
-                         (setq inited t used nil)))
-
-                    ((js2-function-node-p parent)
-                     (setq inited (if (memq node (js2-function-node-params 
parent)) ?P t)
-                           used nil)))
-
-                   (unless used
-                     (let ((grandparent (js2-node-parent parent)))
-                       (when grandparent
-                         (setq used (js2-return-node-p grandparent)))))
-
-                   (js2--add-or-update-symbols node inited used vars))))))))
+                    ((js2-var-init-node-p granparent)
+                     (when (null var-init-node)
+                       (setq var-init-node granparent)))
+                    ((js2-assign-node-p granparent)
+                     (when (null assign-node)
+                       (setq assign-node granparent))))
+                   (setq granparent (js2-node-parent granparent)))
+
+                 ;; If we are within a var-init-node, determine if the name is
+                 ;; declared and initialized
+                 (when var-init-node
+                   (let ((target (js2-var-init-node-target var-init-node)))
+                     (setq declared (memq node (js2--collect-declared-symbols 
target nil)))
+                     ;; Is there an initializer for the declared variable?
+                     (when (js2-var-init-node-initializer var-init-node)
+                       (setq assigned declared)
+                       ;; Determine if the name is actually a literal object 
key
+                       ;; that we shall ignore later
+                       (when (and (not declared)
+                                  (js2-object-prop-node-p parent)
+                                  (eq node (js2-object-prop-node-left parent)))
+                         (setq object-key t)))
+                     ;; Maybe this is a for loop and the variable is one of 
its iterators?
+                     (unless assigned
+                       (let* ((gp (js2-node-parent parent))
+                              (ggp (if gp (js2-node-parent gp))))
+                         (when (and ggp (js2-for-in-node-p ggp))
+                           (setq assigned
+                                 (memq node
+                                       (cl-loop for kid in 
(js2-var-decl-node-kids
+                                                            
(js2-for-in-node-iterator ggp))
+                                                with syms = '()
+                                                do (setq syms
+                                                         (append syms
+                                                                 
(js2--collect-declared-symbols
+                                                                  
(js2-var-init-node-target kid)
+                                                                  nil)))
+                                                finally return syms))))))))
+
+                 ;; Ignore literal object keys, which are not really variables
+                 (unless object-key
+                   (when function-param
+                     (setq assigned ?P))
+
+                   (when (null assigned)
+                     (cond
+                      ((js2-for-in-node-p parent)
+                       (setq assigned (eq node (js2-for-in-node-iterator 
parent))
+                             used (not assigned)))
+                      ((js2-function-node-p parent)
+                       (setq assigned t
+                             used (js2-wrapper-function-p parent)))
+                      (assign-node
+                       (setq assigned (memq node (js2--collect-declared-symbols
+                                                  (js2-assign-node-left 
assign-node)
+                                                  nil))
+                             used (not assigned)))))
+
+                   (when declared
+                     (setq used nil))
+
+                   (js2--add-or-update-symbol node assigned used vars)))))))
        t))
     vars))
 
diff --git a/tests/parser.el b/tests/parser.el
index ac44d99..de97343 100644
--- a/tests/parser.el
+++ b/tests/parser.el
@@ -1157,7 +1157,7 @@ the test."
 
 (js2-deftest-classify-variables prop-get-function-assignment
   "(function(w) { w.f = function() { var a=42, m; return a; }; })(window);"
-  '("address@hidden:P" 11 16 "address@hidden:I" 55 "address@hidden:U"))
+  '("address@hidden:P" 16 "address@hidden:I" 55 "address@hidden:U"))
 
 (js2-deftest-classify-variables let-declaration
   "function foo () { let x,y=1; return x; }"
@@ -1184,8 +1184,8 @@ the test."
   '("address@hidden:U" "address@hidden:I" 64))
 
 (js2-deftest-classify-variables prop-get-assignment
-  "function foo () { var x={y:{z:{}}}; x.y.z=42; }"
-  '("address@hidden:U" "address@hidden:I" 37))
+  "function foo () { var y,x={y:{z:{}}}; x.y.z=42; }"
+  '("address@hidden:U" "address@hidden:U" "address@hidden:I" 39))
 
 (js2-deftest-classify-variables unused-function-argument
   "function foo (a) { return 42; }"
@@ -1212,8 +1212,8 @@ the test."
   '("address@hidden:U" "address@hidden:N" 30 "address@hidden:U" 
"address@hidden:I" 28))
 
 (js2-deftest-classify-variables return-named-function
-  "function foo() { var a=42; return function bar() { return a; } }"
-  '("address@hidden:U" "address@hidden:I" 59 "address@hidden:I" 44))
+  "function foo() { var a=42; return function bar() { return a; }; }"
+  '("address@hidden:U" "address@hidden:I" 59 "address@hidden:U"))
 
 (js2-deftest-classify-variables named-wrapper-function
   "function foo() { var a; (function bar() { a=42; })(); return a; }"
@@ -1221,20 +1221,16 @@ the test."
 
 (js2-deftest-classify-variables destructure-array
   "function foo(x,y) { let [u,v] = [x,y]; }"
-  '("address@hidden:U" "address@hidden:P" 34 "address@hidden:P" 36 
"address@hidden:I" 26 "address@hidden:I" 28))
+  '("address@hidden:U" "address@hidden:P" 34 "address@hidden:P" 36 
"address@hidden:U" "address@hidden:U"))
 
 (js2-deftest-classify-variables destructure-object
   "function foo(x,y) { var {p: [, w], q: z} = {p: [x, 2, 3], q: y}; }"
-  '("address@hidden:U" "address@hidden:P" 49 "address@hidden:P" 62 
"address@hidden:I" 32 "address@hidden:I" 39))
+  '("address@hidden:U" "address@hidden:P" 49 "address@hidden:P" 62 
"address@hidden:U" "address@hidden:U"))
 
 (js2-deftest-classify-variables destructure-object-shorthand
   "function foo(x,y) { var {p, q} = {p: x, q: y}; }"
-  ;; FIXME: this is imprecise although harmless: it should really be
-  ;; ("address@hidden:U" "address@hidden:P" 38 "address@hidden:P" 44 
"address@hidden:I" 35 "address@hidden:I" 41)
-  '("address@hidden:U" "address@hidden:P" 38 "address@hidden:P" 44 
"address@hidden:I" 26 35 "address@hidden:I" 29 41))
+  '("address@hidden:U" "address@hidden:P" 38 "address@hidden:P" 44 
"address@hidden:U" "address@hidden:U"))
 
 (js2-deftest-classify-variables destructure-object-mixed
   "function foo() { let {a, b, c = 3} = {a: 1, b: 2}; }"
-  ;; FIXME: this is imprecise although harmless: it should really be
-  ;; ("address@hidden:U" "address@hidden:I" 23 39 "address@hidden:I" 45 
"address@hidden:I"))
-  '("address@hidden:U" "address@hidden:I" 23 39 "address@hidden:I" 26 45 
"address@hidden:I" 29))
+  '("address@hidden:U" "address@hidden:U" "address@hidden:U" 
"address@hidden:U"))