From 61783bf780b74c38417e0f4c9b22c9e0c579f7e6 Mon Sep 17 00:00:00 2001 From: "Matthew \"strager\" Glazar" Date: Tue, 21 Nov 2023 18:09:03 -0500 Subject: [PATCH] fix(typescript): allow 'extends' clause to use later parameters --- docs/CHANGELOG.md | 2 + src/quick-lint-js/fe/parse-statement.cpp | 86 +++++++++++++----------- test/test-parse-typescript-generic.cpp | 22 ++++++ test/test-variable-analyzer-type.cpp | 13 ++-- 4 files changed, 77 insertions(+), 46 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index d1b9c37ba8..a857464f16 100644 --- 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 {}` 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 index bbadb0268f..a7395fa059 100644 --- 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: + // + // + // + // '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: + // + // + Stacked_Buffering_Visitor extends_visits = + this->buffering_visitor_stack_.push(); + next_parameter: std::optional in_keyword; std::optional out_keyword; @@ -1723,52 +1739,39 @@ void Parser::parse_and_visit_typescript_generic_parameters( break; } - { - // NOTE(strager): Visit order is important. Given: - // - // - // - // '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) { + // // Invalid. + // + if (this->peek().type == Token_Type::colon) { // // Invalid. - // - if (this->peek().type == Token_Type::colon) { - // // 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) { - // - 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) { + // + 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 index ba6f4ff2ea..697edb43b0 100644 --- 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""_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 index fbb49b1e1e..69201de0a5 100644 --- 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