Skip to content

Commit

Permalink
Fix js2-node-get-enclosing-scope on a statement function's name
Browse files Browse the repository at this point in the history
When called with a statement function's name node,
js2-node-get-enclosing-scope would errorneously return the function itself.
That's not really the scope where this name is defined (it should be the
function's parent scope).

This bug affects js2-refactor's rename variable functionality.  Example:

    (function(){
        function |foo() {
            var |foo = 5;
            console.log(|foo);
        }
        foo();
    })();

(where '|' marks possible cursor position).  Type M-x js2r-rename-var with
the cursor on one of the marked positions, and it'll offer to rename the
following occurrences (marked with underscores):

    (function(){
        function _foo_() {
            var _foo_ = 5;
            console.log(_foo_);
        }
        foo();
    })();

This is incorrect, as the name is shadowed inside the function.  It should
instead rename these two:

    (function(){
        function _foo_() {
            var foo = 5;
            console.log(foo);
        }
        _foo_();
    })();

This patch fixes this.
  • Loading branch information
mishoo committed Jun 25, 2023
1 parent dd7abb2 commit c47ea93
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
9 changes: 9 additions & 0 deletions js2-mode.el
Original file line number Diff line number Diff line change
Expand Up @@ -2344,6 +2344,15 @@ If any given node in NODES is nil, doesn't record that link."
(defun js2-node-get-enclosing-scope (node)
"Return the innermost `js2-scope' node surrounding NODE.
Returns nil if there is no enclosing scope node."
;; when node is the name of a function statement, the enclosing
;; scope is not that function itself but the surrounding scope.
(let ((parent (js2-node-parent node)))
(when (and (js2-name-node-p node)
(js2-function-node-p parent)
(eq 'FUNCTION_STATEMENT (js2-function-node-form parent))
(eq node (js2-function-node-name parent)))
(setq node parent)))
;; dig up to find the closest scope parent
(while (and (setq node (js2-node-parent node))
(not (js2-scope-p node))))
node)
Expand Down
13 changes: 11 additions & 2 deletions tests/parser.el
Original file line number Diff line number Diff line change
Expand Up @@ -1214,11 +1214,14 @@ the test."
(should (= (js2-symbol-decl-type var-entry) js2-VAR))
(should (js2-name-node-p (js2-symbol-ast-node var-entry)))))

(defun js2-test-scope-of-nth-variable-satisifies-predicate (variable nth predicate)
(defun js2-get-nth-variable-enclosing-scope (variable nth)
(goto-char (point-min))
(dotimes (_ (1+ nth)) (search-forward variable))
(forward-char -1)
(let ((scope (js2-node-get-enclosing-scope (js2-node-at-point))))
(js2-node-get-enclosing-scope (js2-node-at-point)))

(defun js2-test-scope-of-nth-variable-satisifies-predicate (variable nth predicate)
(let ((scope (js2-get-nth-variable-enclosing-scope variable nth)))
(should (funcall predicate (js2-get-defining-scope scope variable)))))

(js2-deftest for-node-is-declaration-scope "for (let i = 0; i; ++i) {};"
Expand All @@ -1239,6 +1242,12 @@ the test."
(js2-mode--and-parse)
(js2-test-scope-of-nth-variable-satisifies-predicate "x" 0 #'js2-comp-loop-node-p))

(js2-deftest function-name-enclosing-scope-is-parent-scope "(function(){ var bar; function foo(){} })()"
(js2-mode--and-parse)
(let ((scope1 (js2-get-nth-variable-enclosing-scope "bar" 0))
(scope2 (js2-get-nth-variable-enclosing-scope "foo" 0)))
(should (eq scope1 scope2))))

(js2-deftest array-comp-has-parent-scope
"var a,b=[for (i of [[1,2]]) for (j of i) j * a];"
(js2-mode--and-parse)
Expand Down

0 comments on commit c47ea93

Please sign in to comment.