Fix parsing of x=>y=>{x;y;}

quick-lint-js parses the following code incorrectly [1]:

    (x => y => { x; y; });

This confuses the linter, leading to false positives:

    hello.js:1:14: warning: use of undeclared variable: x [E0057]

Why does this false positive happen? The inner function is sent to the
linter immediately, then the outer function's parameter list is sent to
the linter:

    visit_enter_function_scope
      visit_variable_declaration      // y
      visit_enter_function_scope_body
      visit_variable_use              // x
      visit_variable_use              // y
    visit_exit_function_scope
    visit_enter_function_scope
      visit_variable_declaration      // x
      visit_enter_function_scope_body
    visit_exit_function_scope

This didn't happen in the past. Commit
8087d21 changed the behavior, causing
arrow functions with statements to be visited immediately, but deferring
visitation for arrow functions with expressions.

Fix the parser to visit arrow functions with expressions immediately
too:

    visit_enter_function_scope
      visit_variable_declaration      // x
      visit_enter_function_scope_body
      visit_enter_function_scope
        visit_variable_declaration      // y
        visit_enter_function_scope_body
        visit_variable_use              // x
        visit_variable_use              // y
      visit_exit_function_scope
    visit_exit_function_scope

This fixes the false positive.

Cleanup will happen in future commits.

[1] #636

Author: strager

docs/CHANGELOG.md

@@ -6,6 +6,13 @@ based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 quick-lint-js' version numbers are arbitrary. quick-lint-js does *not* adhere to
 Semantic Versioning.
 
+## Unreleased
+
+### Fixed
+
+* Curried arrow functions like `a => b => { a; b; }` no longer falsely reports
+  [E0057][]. (Thanks to [Christian Mund][] for reporting.)
+
 ## 2.0.0 (2022-02-08)
 
 [Downloads](https://c.quick-lint-js.com/releases/2.0.0/)
@@ -372,6 +379,7 @@ Beta release.
 
 [AidenThing]: https://github.com/AidenThing
 [Amir]: https://github.com/ahmafi
+[Christian Mund]: https://github.com/kkkrist
 [Daniel La Rocque]: https://github.com/dlarocque
 [David Vasileff]: https://github.com/dav000
 [Erlliam Mejia]: https://github.com/erlliam

src/quick-lint-js/parse-expression-inl.h

@@ -38,17 +38,11 @@ void parser::visit_expression(expression* ast, Visitor& v,
       this->visit_expression(child, v, context);
     }
   };
-  auto visit_parameters = [&]() {
-    for (expression* parameter : ast->children()) {
-      this->visit_binding_element(parameter, v, variable_kind::_parameter,
-                                  /*declaring_token=*/std::nullopt,
-                                  /*init_kind=*/variable_init_kind::normal);
-    }
-  };
   switch (ast->kind()) {
   case expression_kind::_class:
   case expression_kind::_invalid:
   case expression_kind::_missing:
+  case expression_kind::arrow_function_with_expression:
   case expression_kind::arrow_function_with_statements:
   case expression_kind::function:
   case expression_kind::import:
@@ -77,15 +71,6 @@ void parser::visit_expression(expression* ast, Visitor& v,
     visit_children();
     break;
   }
-  case expression_kind::arrow_function_with_expression: {
-    auto* arrow = static_cast<expression::arrow_function_with_expression*>(ast);
-    v.visit_enter_function_scope();
-    visit_parameters();
-    v.visit_enter_function_scope_body();
-    this->visit_expression(arrow->body_, v, variable_context::rhs);
-    v.visit_exit_function_scope();
-    break;
-  }
   case expression_kind::assignment: {
     expression* lhs = ast->child_0();
     expression* rhs = ast->child_1();
@@ -1839,11 +1824,31 @@ expression* parser::parse_arrow_function_body_impl(
     return this->make_expression<expression::arrow_function_with_statements>(
         attributes, ast->children(), parameter_list_begin, span_end);
   } else {
+    v.visit_enter_function_scope();
+
+    // TODO(strager): Avoid creating a temporary expression just to iterate over
+    // the parameters.
+    expression* ast =
+        this->make_expression<expression::arrow_function_with_expression>(
+            attributes, std::forward<Args>(args)...,
+            this->make_expression<expression::_invalid>(this->peek().span()),
+            parameter_list_begin);
+    for (expression* parameter : ast->children()) {
+      this->visit_binding_element(parameter, v, variable_kind::_parameter,
+                                  /*declaring_token=*/std::nullopt,
+                                  /*init_kind=*/variable_init_kind::normal);
+    }
+
+    v.visit_enter_function_scope_body();
+    // TODO(strager): Use parse_and_visit_expression instead.
     expression* body =
         this->parse_expression(v, precedence{
                                       .commas = false,
                                       .in_operator = allow_in_operator,
                                   });
+    this->visit_expression(body, v, variable_context::rhs);
+    v.visit_exit_function_scope();
+
     return this->make_expression<expression::arrow_function_with_expression>(
         attributes, std::forward<Args>(args)..., body, parameter_list_begin);
   }

test/test-parse-function.cpp

@@ -513,6 +513,51 @@ TEST(test_parse, arrow_function_expression_with_statements) {
   }
 }
 
+TEST(test_parse, nested_arrow_function) {
+  for (string8_view code : {
+           u8"(x => y => (x, y));"_sv,
+           u8"(x => { (y => (x, y)); });"_sv,
+           u8"(x => y => { x; y; });"_sv,
+           u8"(x => { (y => { x; y; }); });"_sv,
+       }) {
+    spy_visitor v = parse_and_visit_statement(code);
+    SCOPED_TRACE(out_string8(code));
+    EXPECT_THAT(v.visits, ElementsAre("visit_enter_function_scope",       //
+                                      "visit_variable_declaration",       // x
+                                      "visit_enter_function_scope_body",  //
+                                      "visit_enter_function_scope",       //
+                                      "visit_variable_declaration",       // y
+                                      "visit_enter_function_scope_body",  //
+                                      "visit_variable_use",               // x
+                                      "visit_variable_use",               // y
+                                      "visit_exit_function_scope",        //
+                                      "visit_exit_function_scope"));
+    EXPECT_THAT(
+        v.variable_declarations,
+        ElementsAre(
+            spy_visitor::visited_variable_declaration{
+                u8"x", variable_kind::_parameter, variable_init_kind::normal},
+            spy_visitor::visited_variable_declaration{
+                u8"y", variable_kind::_parameter, variable_init_kind::normal}));
+    EXPECT_THAT(v.variable_uses,
+                ElementsAre(spy_visitor::visited_variable_use{u8"x"},
+                            spy_visitor::visited_variable_use{u8"y"}));
+  }
+
+  {
+    spy_visitor v = parse_and_visit_statement(u8"(a => ((b = a) => {}));"_sv);
+    EXPECT_THAT(v.visits, ElementsAre("visit_enter_function_scope",       //
+                                      "visit_variable_declaration",       // a
+                                      "visit_enter_function_scope_body",  //
+                                      "visit_enter_function_scope",       //
+                                      "visit_variable_use",               // a
+                                      "visit_variable_declaration",       // b
+                                      "visit_enter_function_scope_body",  //
+                                      "visit_exit_function_scope",        //
+                                      "visit_exit_function_scope"));
+  }
+}
+
 TEST(test_parse, function_statements_allow_trailing_commas_in_parameter_list) {
   {
     spy_visitor v = parse_and_visit_statement(u8"function f(x,) { y; });"_sv);