[elpa] master 254c78c 07/33: Highlight unused and/or uninitialized variables

[Dmitry Gutov]

branch: master
commit 254c78c1b3ccc3af05ff3298bee669e6fa8a5c70
Author: Lele Gaifax <address@hidden>
Commit: Dmitry Gutov <address@hidden>

    Highlight unused and/or uninitialized variables
    
    Adding new js2-highlight-unused-variables-mode.
    
    Closes #212, closes #233
---
 js2-mode.el     |  171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 tests/parser.el |  145 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 314 insertions(+), 2 deletions(-)

diff --git a/js2-mode.el b/js2-mode.el
index d775d65..c1aaa21 100644
--- a/js2-mode.el
+++ b/js2-mode.el
@@ -1155,6 +1155,11 @@ another file, or you've got a potential bug."
   :type 'boolean
   :group 'js2-mode)
 
+(defcustom js2-warn-about-unused-function-arguments nil
+  "Non-nil to treat function arguments like declared-but-unused variables."
+  :type 'booleanp
+  :group 'js2-mode)
+
 (defcustom js2-include-jslint-globals t
   "Non-nil to include the identifiers from JSLint global
 declaration (see http://www.jslint.com/lint.html#global) in the
@@ -1793,6 +1798,12 @@ the correct number of ARGS must be provided."
 (js2-msg "msg.undeclared.variable"  ; added by js2-mode
          "Undeclared variable or function '%s'")
 
+(js2-msg "msg.unused.variable"  ; added by js2-mode
+         "Unused variable or function '%s'")
+
+(js2-msg "msg.uninitialized.variable"  ; added by js2-mode
+         "Variable '%s' referenced but never initialized")
+
 (js2-msg "msg.ref.undefined.prop"
          "Reference to undefined property '%s'")
 
@@ -7081,6 +7092,160 @@ it is considered declared."
           (js2-report-warning "msg.undeclared.variable" name pos (- end pos)
                               'js2-external-variable))))))
 
+(defun js2--add-or-update-symbol (symbol inition used vars)
+  "Add or update SYMBOL entry in VARS, an hash table.
+SYMBOL is a js2-name-node, INITION either nil, t, or ?P,
+respectively meaning that SYMBOL is a mere declaration, an
+assignment or a function parameter; when USED is t, the symbol
+node is assumed to be an usage and thus added to the list stored
+in the cdr of the entry.
+"
+  (let* ((nm (js2-name-node-name symbol))
+         (es (js2-node-get-enclosing-scope symbol))
+         (ds (js2-get-defining-scope es nm)))
+    (when (and ds (not (equal nm "arguments")))
+      (let* ((sym (js2-scope-get-symbol ds nm))
+             (var (gethash sym vars))
+             (err-var-p (js2-catch-node-p ds)))
+        (unless inition
+          (setq inition err-var-p))
+        (if var
+            (progn
+              (when (and inition (not (equal (car var) ?P)))
+                (setcar var inition))
+              (when used
+                (push symbol (cdr var))))
+          ;; do not consider the declaration of catch parameter as an usage
+          (when (and err-var-p used)
+            (setq used nil))
+          (puthash sym (cons inition (if used (list symbol))) vars))))))
+
+(defun js2--classify-variables ()
+  "Collect and classify variables declared or used within js2-mode-ast.
+Traverse the whole ast tree returning a summary of the variables
+usage as an hash-table, keyed by their corresponding symbol table
+entry.
+Each variable is described by a tuple where the car is a flag
+indicating whether the variable has been initialized and the cdr
+is a possibly empty list of name nodes where it is used. External
+symbols, i.e. those not present in the whole scopes hierarchy,
+are ignored."
+  (let ((vars (make-hash-table :test #'eq :size 100)))
+    (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-symbol target inited nil vars)))))
+
+          ((js2-assign-node-p node)
+           ;; take note about assignments
+           (let ((left (js2-assign-node-left node)))
+             (when (js2-name-node-p left)
+               (js2--add-or-update-symbol left 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-symbol 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
+                                (eq node (js2-var-init-node-target parent)))
+                           (and (js2-assign-node-p parent)
+                                (eq node (js2-assign-node-left parent)))
+                           (js2-prop-get-node-p parent))
+                 (let ((used t) inited)
+                   (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-symbol node inited used vars))))))))
+       t))
+    vars))
+
+(defun js2--get-name-node (node)
+  (cond
+   ((js2-name-node-p node) node)
+   ((js2-function-node-p node)
+    (js2-function-node-name node))
+   ((js2-class-node-p node)
+    (js2-class-node-name node))
+   ((js2-comp-loop-node-p node)
+    (js2-comp-loop-node-iterator node))
+   (t node)))
+
+(defun js2--highlight-unused-variable (symbol info)
+  (let ((name (js2-symbol-name symbol))
+        (inited (car info))
+        (refs (cdr info))
+        pos len)
+    (unless (and inited refs)
+      (if refs
+          (dolist (ref refs)
+            (setq pos (js2-node-abs-pos ref))
+            (setq len (js2-name-node-len ref))
+            (js2-report-warning "msg.uninitialized.variable" name pos len
+                                'js2-warning))
+        (when (or js2-warn-about-unused-function-arguments
+                  (not (eq inited ?P)))
+          (let* ((symn (js2-symbol-ast-node symbol))
+                 (namen (js2--get-name-node symn)))
+            (unless (js2-node-top-level-decl-p namen)
+              (setq pos (js2-node-abs-pos namen))
+              (setq len (js2-name-node-len namen))
+              (js2-report-warning "msg.unused.variable" name pos len
+                                  'js2-warning))))))))
+
+(defun js2-highlight-unused-variables ()
+  "Highlight unused variables."
+  (let ((vars (js2--classify-variables)))
+    (maphash #'js2--highlight-unused-variable vars)))
+
+;;;###autoload
+(define-minor-mode js2-highlight-unused-variables-mode
+  "Toggle highlight of unused variables."
+  :lighter ""
+  (if js2-highlight-unused-variables-mode
+      (add-hook 'js2-post-parse-callbacks
+                #'js2-highlight-unused-variables nil t)
+    (remove-hook 'js2-post-parse-callbacks
+                 #'js2-highlight-unused-variables t)))
+
 (defun js2-set-default-externs ()
   "Set the value of `js2-default-externs' based on the various
 `js2-include-?-externs' variables."
@@ -10224,7 +10389,7 @@ We should have just parsed the 'for' keyword before 
calling this function."
     pn))
 
 (defun js2-parse-comprehension (pos form)
-  (let (loops filters expr result)
+  (let (loops filters expr result last)
     (unwind-protect
         (progn
           (js2-unget-token)
@@ -10235,7 +10400,8 @@ We should have just parsed the 'for' keyword before 
calling this function."
               (js2-parse-comp-loop loop)))
           (while (js2-match-token js2-IF)
             (push (car (js2-parse-condition)) filters))
-          (setq expr (js2-parse-assign-expr)))
+          (setq expr (js2-parse-assign-expr))
+          (setq last (car loops)))
       (dolist (_ loops)
         (js2-pop-scope)))
     (setq result (make-js2-comp-node :pos pos
@@ -10246,6 +10412,7 @@ We should have just parsed the 'for' keyword before 
calling this function."
                                      :form form))
     (apply #'js2-node-add-children result (js2-comp-node-loops result))
     (apply #'js2-node-add-children result expr (js2-comp-node-filters result))
+    (setf (js2-scope-parent-scope result) last)
     result))
 
 (defun js2-parse-comp-loop (pn &optional only-of-p)
diff --git a/tests/parser.el b/tests/parser.el
index 6050a8f..1a93f36 100644
--- a/tests/parser.el
+++ b/tests/parser.el
@@ -735,6 +735,15 @@ the test."
   (let ((scope (js2-node-get-enclosing-scope (js2-node-at-point))))
     (should (js2-comp-loop-node-p (js2-get-defining-scope scope "x")))))
 
+(js2-deftest array-comp-has-parent-scope
+             "var a,b=[for (i of [[1,2]]) for (j of i) j * a];"
+  (js2-mode)
+  (search-forward "for")
+  (forward-char -3)
+  (let ((node (js2-node-at-point)))
+    (should (js2-scope-parent-scope node))
+    (should (js2-get-defining-scope node "j"))))
+
 ;;; Tokenizer
 
 (js2-deftest get-token "(1+1)"
@@ -817,3 +826,139 @@ the test."
   (js2-mode)
   (let ((node (js2-node-at-point (point-min))))
     (should (= (js2-node-len node) 3))))
+
+;;; Variables classification
+
+(defun js2--variables-summary (vars)
+  (let (r)
+    (setq vars (let (aslist)
+                 (maphash (lambda (k v) (push (cons k v) aslist)) vars)
+                 aslist))
+    (dolist (v (sort vars (lambda (a b) (< (js2-node-abs-pos 
(js2-symbol-ast-node (car a)))
+                                      (js2-node-abs-pos (js2-symbol-ast-node 
(car b)))))))
+      (let* ((symbol (car v))
+             (inition (cadr v))
+             (uses (cddr v))
+             (symn (js2-symbol-ast-node symbol))
+             (namen (js2--get-name-node symn)))
+        (push (format "address@hidden:%s"
+                      (js2-symbol-name symbol)
+                      (js2-node-abs-pos namen)
+                      (if (eq inition ?P)
+                          "P"
+                        (if uses
+                            (if inition "I" "N")
+                          "U"))) r)
+        (dolist (u (sort (cddr v) (lambda (a b) (< (js2-node-abs-pos a)
+                                              (js2-node-abs-pos b)))))
+          (push (js2-node-abs-pos u) r))))
+    (reverse r)))
+
+(defmacro js2-deftest-classify-variables (name buffer-contents summary)
+ (declare (indent defun))
+  `(ert-deftest ,(intern (format "js2-classify-variables-%s" name)) ()
+     (with-temp-buffer
+       (save-excursion
+         (insert ,buffer-contents))
+       (unwind-protect
+           (progn
+             (js2-mode)
+             (should (equal ,summary (js2--variables-summary
+                                      (js2--classify-variables)))))
+         (fundamental-mode)))))
+
+(js2-deftest-classify-variables incomplete-var-statement
+  "var"
+  '())
+
+(js2-deftest-classify-variables unused-variable
+  "function foo () { var x; return 42; }"
+  '("address@hidden:U" "address@hidden:U"))
+
+(js2-deftest-classify-variables unused-variable-declared-twice
+  "function foo (a) { var x; function bar () { var x; x=42; }; return a;}"
+  '("address@hidden:U" "address@hidden:P" 68 "address@hidden:U" 
"address@hidden:U" "address@hidden:U"))
+
+(js2-deftest-classify-variables assigned-variable
+  "function foo () { var x; x=42; return x; }"
+  '("address@hidden:U" "address@hidden:I" 39))
+
+(js2-deftest-classify-variables assignment-in-nested-function
+  "function foo () { var x; function bar () { x=42; }; }"
+  '("address@hidden:U" "address@hidden:U" "address@hidden:U"))
+
+(js2-deftest-classify-variables unused-nested-function
+  "function foo() { var i, j=1; function bar() { var x, y=42, z=i; return y; } 
return i; }"
+  '("address@hidden:U" "address@hidden:N" 62 84 "address@hidden:U" 
"address@hidden:U" "address@hidden:U" "address@hidden:I" 72 "address@hidden:U"))
+
+(js2-deftest-classify-variables prop-get-initialized
+  "function foo () { var x, y={}; y.a=x; }"
+  '("address@hidden:U" "address@hidden:N" 36 "address@hidden:I" 32))
+
+(js2-deftest-classify-variables prop-get-uninitialized
+  "function foo () { var x; if(x.foo) alert('boom'); }"
+  '("address@hidden:U" "address@hidden:N" 29))
+
+(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"))
+
+(js2-deftest-classify-variables let-declaration
+  "function foo () { let x,y=1; return x; }"
+  '("address@hidden:U" "address@hidden:N" 37 "address@hidden:U"))
+
+(js2-deftest-classify-variables external-function-call
+  "function foo (m) { console.log(m, arguments); }"
+  '("address@hidden:U" "address@hidden:P" 32))
+
+(js2-deftest-classify-variables global-function-call
+  "function bar () { return 42; } function foo (a) { return bar(); }"
+  '("address@hidden:I" 58 "address@hidden:U" "address@hidden:P"))
+
+(js2-deftest-classify-variables let-declaration-for-scope
+  "function foo () { for(let x=1,y; x<y; y++) {} }"
+  '("address@hidden:U" "address@hidden:I" 34 "address@hidden:N" 36 39))
+
+(js2-deftest-classify-variables arguments-implicit-var
+  "function foo () { var p; for(p in arguments) { return p; } }"
+  '("address@hidden:U" "address@hidden:I" 55))
+
+(js2-deftest-classify-variables catch-error-variable
+  "function foo () { try { throw 'Foo'; } catch (e) { console.log(e); }"
+  '("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))
+
+(js2-deftest-classify-variables unused-function-argument
+  "function foo (a) { return 42; }"
+  '("address@hidden:U" "address@hidden:P"))
+
+(js2-deftest-classify-variables used-function-argument
+  "function foo (a) { a=42; return a; }"
+  '("address@hidden:U" "address@hidden:P" 33))
+
+(js2-deftest-classify-variables prop-get
+  "function foo (a) { a=navigator.x||navigator.y; return a; }"
+  '("address@hidden:U" "address@hidden:P" 55))
+
+(js2-deftest-classify-variables for-in-loop
+  "function foo () { var d={}; for(var k in d) {var v=d[k]; } }"
+  '("address@hidden:U" "address@hidden:I" 42 52 "address@hidden:I" 54 
"address@hidden:U"))
+
+(js2-deftest-classify-variables array-comprehension-legacy
+  "function foo() { var j,a=[for (i of [1,2,3]) i*j]; }"
+  '("address@hidden:U" "address@hidden:N" 48 "address@hidden:U" 
"address@hidden:I" 46))
+
+(js2-deftest-classify-variables array-comprehension
+  "function foo() { var j,a=[[i,j] for (i of [1,2,3])]; }"
+  '("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))
+
+(js2-deftest-classify-variables named-wrapper-function
+  "function foo() { var a; (function bar() { a=42; })(); return a; }"
+  '("address@hidden:U" "address@hidden:I" 62 "address@hidden:I" 35))