fix(typescript): allow 'extends' clause to use later parameters

quick-lint · Nov 21, 2023 · 61783bf · 61783bf
1 parent 589a7fb
commit 61783bf
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 46 deletions.
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -20,6 +20,8 @@ Semantic Versioning.
   * `export default` with a class and an interface (triggering declaration
     merging) no longer fasely reports [E0715][] ("cannot use multiple `export
     default` statements in one module").
+  * `class C<T extends U, U> {}` no longer falsely reports [E0058][] ("variable
+    used before declaration").
   * `export as namespace` statements are now parsed.
   * Assertion signatures (`function f(param): asserts param`) are now parsed.
   * `case await x:` no longer treats `:` as if it was a type annotation colon in

diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp
@@ -1644,6 +1644,22 @@ void Parser::parse_and_visit_typescript_generic_parameters(
 
   Variable_Kind parameter_kind = Variable_Kind::_generic_parameter;
 
+  // NOTE[generic-type-parameter-visit-order]: Visit order is important.
+  // Given:
+  //
+  //   <T extends U = Default>
+  //
+  // 'Default' should be visited first so references to 'T' report
+  // use-before-declaration. 'U' should be visited after 'T' so references to
+  // 'T' are legal.
+  //
+  // Also, given multiple parameters, all extends clauses should be visited
+  // after all variables are declared. For example, the following is legal:
+  //
+  //   <T extends U, U>
+  Stacked_Buffering_Visitor extends_visits =
+      this->buffering_visitor_stack_.push();
+
 next_parameter:
   std::optional<Identifier> in_keyword;
   std::optional<Identifier> out_keyword;
@@ -1723,52 +1739,39 @@ void Parser::parse_and_visit_typescript_generic_parameters(
     break;
   }
 
-  {
-    // NOTE(strager): Visit order is important. Given:
-    //
-    //   <T extends U = Default>
-    //
-    // 'Default' should be visited first so references to 'T' report
-    // use-before-declaration. 'U' should be visited after 'T' so references to
-    // 'T' are legal.
-    Stacked_Buffering_Visitor extends_visits =
-        this->buffering_visitor_stack_.push();
-    if (this->peek().type == Token_Type::kw_extends ||
-        this->peek().type == Token_Type::colon) {
+  if (this->peek().type == Token_Type::kw_extends ||
+      this->peek().type == Token_Type::colon) {
+    // <T: U>  // Invalid.
+    // <T extends U>
+    if (this->peek().type == Token_Type::colon) {
       // <T: U>  // Invalid.
-      // <T extends U>
-      if (this->peek().type == Token_Type::colon) {
-        // <T: U>  // Invalid.
-        this->diag_reporter_->report(
-            Diag_Unexpected_Colon_After_Generic_Definition{
-                .colon = this->peek().span(),
-            });
-      }
-      this->skip();
-      this->parse_and_visit_typescript_type_expression(
-          extends_visits.visitor(),
-          TypeScript_Type_Parse_Options{
-              .type_being_declared =
-                  TypeScript_Type_Parse_Options::Declaring_Type{
-                      .name = *parameter_name,
-                      .kind = parameter_kind,
-                  },
+      this->diag_reporter_->report(
+          Diag_Unexpected_Colon_After_Generic_Definition{
+              .colon = this->peek().span(),
           });
     }
+    this->skip();
+    this->parse_and_visit_typescript_type_expression(
+        extends_visits.visitor(),
+        TypeScript_Type_Parse_Options{
+            .type_being_declared =
+                TypeScript_Type_Parse_Options::Declaring_Type{
+                    .name = *parameter_name,
+                    .kind = parameter_kind,
+                },
+        });
+  }
 
-    if (this->peek().type == Token_Type::equal) {
-      // <T = Default>
-      this->skip();
-      this->parse_and_visit_typescript_type_expression(v);
-    }
-
-    QLJS_ASSERT(parameter_name.has_value());
-    v.visit_variable_declaration(*parameter_name, parameter_kind,
-                                 Variable_Declaration_Flags::none);
-
-    extends_visits.visitor().move_into(v);
+  if (this->peek().type == Token_Type::equal) {
+    // <T = Default>
+    this->skip();
+    this->parse_and_visit_typescript_type_expression(v);
   }
 
+  QLJS_ASSERT(parameter_name.has_value());
+  v.visit_variable_declaration(*parameter_name, parameter_kind,
+                               Variable_Declaration_Flags::none);
+
   switch (this->peek().type) {
   case Token_Type::greater:
     break;
@@ -1801,6 +1804,9 @@ void Parser::parse_and_visit_typescript_generic_parameters(
     goto next_parameter;
   }
   this->skip();
+
+  // See NOTE[generic-type-parameter-visit-order].
+  extends_visits.visitor().move_into(v);
 }
 
 void Parser::parse_and_visit_statement_block_no_scope(Parse_Visitor_Base &v) {

diff --git a/test/test-parse-typescript-generic.cpp b/test/test-parse-typescript-generic.cpp
@@ -373,6 +373,28 @@ TEST_F(Test_Parse_TypeScript_Generic, type_parameter_default_with_extends) {
   }
 }
 
+TEST_F(Test_Parse_TypeScript_Generic,
+       type_parameter_extends_visits_appear_after_all_default_visits) {
+  // See NOTE[generic-type-parameter-visit-order].
+
+  {
+    Spy_Visitor p = test_parse_and_visit_typescript_generic_parameters(
+        u8"<P1 extends E1 = D1, P2 extends E2 = D2>"_sv, no_diags,
+        typescript_options);
+    EXPECT_THAT(p.visits, ElementsAreArray({
+                              "visit_variable_type_use",     // D1
+                              "visit_variable_declaration",  // P1
+                              "visit_variable_type_use",     // D2
+                              "visit_variable_declaration",  // P2
+                              // All extends clauses follow:
+                              "visit_variable_type_use",  // E1
+                              "visit_variable_type_use",  // E2
+                          }));
+    EXPECT_THAT(p.variable_uses,
+                ElementsAreArray({u8"D1", u8"D2", u8"E1", u8"E2"}));
+  }
+}
+
 TEST_F(Test_Parse_TypeScript_Generic, variance_specifiers) {
   {
     Spy_Visitor p = test_parse_and_visit_typescript_generic_parameters(

diff --git a/test/test-variable-analyzer-type.cpp b/test/test-variable-analyzer-type.cpp
@@ -116,12 +116,13 @@ TEST(Test_Variable_Analyzer_Type, type_use_of_import_is_okay) {
 }
 
 TEST(Test_Variable_Analyzer_Type,
-     generic_parameter_use_before_declaration_is_an_error) {
-  test_parse_and_analyze(
-      u8"(function< T extends U, U, >() { });"_sv,
-      u8"                     ^ Diag_Variable_Used_Before_Declaration.use\n"_diag
-      u8"                        ^ .declaration"_diag,
-      typescript_analyze_options, default_globals);
+     generic_parameter_use_before_declaration_in_extends_is_allowed) {
+  test_parse_and_analyze(u8"(function< T extends U, U, >() { });"_sv, no_diags,
+                         typescript_analyze_options, default_globals);
+}
+
+TEST(Test_Variable_Analyzer_Type,
+     generic_parameter_use_before_declaration_in_default_is_not_allowed) {
   test_parse_and_analyze(
       u8"(function< T extends number = U, U, >() { });"_sv,
       u8"                                 ^ Diag_Variable_Used_Before_Declaration.declaration\n"_diag