Skip to content

Commit

Permalink
feat(typescript): improve diagnostic on '<<' in extends clause
Browse files Browse the repository at this point in the history
The following TypeScript code is legal:

    class C<T> {}
    new C<<T>() => null>();

    interface I<T> {}
    class D implements I< <T>() => null> {}
    interface I2 extends I< <T>() => null> {}

But the following is not:

    interface I<T> {}
    class D implements I<<T>() => null> {}
    interface I2 extends I<<T>() => null> {}

Report a nice diagnostic on the erroneous '<<' in these cases to help
the programmer write correct TypeScript code.
  • Loading branch information
strager committed Dec 12, 2023
1 parent b76c649 commit 64bfe94
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 5 deletions.
3 changes: 3 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ Semantic Versioning.
conditional expression with a function in the truthy branch.
(`cond ? (t) : param => body` continues to be parsed as a conditional
expression with a function in the falsy branch.)
* Using `<<` in an interface's `extends` clause or a class's `implements`
clause now reports [E0429][] instead of misleading diagnostics.

### Fixed

Expand Down Expand Up @@ -1318,6 +1320,7 @@ Beta release.
[E0398]: https://quick-lint-js.com/errors/E0398/
[E0426]: https://quick-lint-js.com/errors/E0426/
[E0427]: https://quick-lint-js.com/errors/E0427/
[E0429]: https://quick-lint-js.com/errors/E0429/
[E0450]: https://quick-lint-js.com/errors/E0450/
[E0451]: https://quick-lint-js.com/errors/E0451/
[E0452]: https://quick-lint-js.com/errors/E0452/
Expand Down
4 changes: 4 additions & 0 deletions po/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -1537,6 +1537,10 @@ msgstr ""
msgid "interfaces cannot contain static blocks"
msgstr ""

#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
msgid "space is required between '<' and '<' inside 'extends' clause"
msgstr ""

#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
msgid "'declare class' cannot contain static block"
msgstr ""
Expand Down
14 changes: 14 additions & 0 deletions src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4573,6 +4573,20 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = {
},
},

// Diag_TypeScript_Generic_Less_Less_Not_Split
{
.code = 429,
.severity = Diagnostic_Severity::error,
.message_formats = {
QLJS_TRANSLATABLE("space is required between '<' and '<' inside 'extends' clause"),
},
.message_args = {
{
Diagnostic_Message_Arg_Info(offsetof(Diag_TypeScript_Generic_Less_Less_Not_Split, expected_space), Diagnostic_Arg_Type::source_code_span),
},
},
},

// Diag_TypeScript_Declare_Class_Cannot_Contain_Static_Block_Statement
{
.code = 332,
Expand Down
3 changes: 2 additions & 1 deletion src/quick-lint-js/diag/diagnostic-metadata-generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ namespace quick_lint_js {
QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Inline_Type_Export_Not_Allowed_In_Type_Only_Export) \
QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Inline_Type_Import_Not_Allowed_In_Type_Only_Import) \
QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Interfaces_Cannot_Contain_Static_Blocks) \
QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Generic_Less_Less_Not_Split) \
QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Declare_Class_Cannot_Contain_Static_Block_Statement) \
QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Declare_Field_Not_Allowed_In_JavaScript) \
QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Declare_Field_Cannot_Use_Private_Identifier) \
Expand Down Expand Up @@ -450,7 +451,7 @@ namespace quick_lint_js {
/* END */
// clang-format on

inline constexpr int Diag_Type_Count = 436;
inline constexpr int Diag_Type_Count = 437;

extern const Diagnostic_Info all_diagnostic_infos[Diag_Type_Count];
}
Expand Down
10 changes: 10 additions & 0 deletions src/quick-lint-js/diag/diagnostic-types-2.h
Original file line number Diff line number Diff line change
Expand Up @@ -2367,6 +2367,16 @@ struct Diag_TypeScript_Interfaces_Cannot_Contain_Static_Blocks {
Source_Code_Span static_token;
};

struct Diag_TypeScript_Generic_Less_Less_Not_Split {
[[qljs::diag("E0429", Diagnostic_Severity::error)]] //
// clang-format off
[[qljs::message("space is required between '<' and '<' inside 'extends' "
"or 'implements' clause",
ARG(expected_space))]] //
// clang-format on
Source_Code_Span expected_space;
};

struct Diag_TypeScript_Declare_Class_Cannot_Contain_Static_Block_Statement {
[[qljs::diag("E0332", Diagnostic_Severity::error)]] //
[[qljs::message("'declare class' cannot contain static block",
Expand Down
11 changes: 10 additions & 1 deletion src/quick-lint-js/fe/parse-class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2100,8 +2100,17 @@ void Parser::parse_and_visit_typescript_interface_reference(
v.visit_variable_type_use(ident);
}

if (this->peek().type == Token_Type::less) {
if (this->peek().type == Token_Type::less ||
this->peek().type == Token_Type::less_less) {
// extends SomeType<Arg>
// extends SomeType< <T>() => ReturnType>
if (this->peek().type == Token_Type::less_less) {
// extends SomeType<<T>() => ReturnType> // Invalid.
const Char8 *second_less = this->peek().begin + 1;
this->diag_reporter_->report(Diag_TypeScript_Generic_Less_Less_Not_Split{
.expected_space = Source_Code_Span::unit(second_less),
});
}
this->parse_and_visit_typescript_generic_arguments(v);
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/quick-lint-js/i18n/translation-table-generated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ const Translation_Table translation_data = {
{0, 0, 0, 0, 0, 9}, //
{11, 9, 0, 10, 0, 42}, //
{0, 0, 0, 0, 0, 19}, //
{0, 0, 0, 0, 0, 62}, //
{0, 0, 0, 0, 0, 19}, //
{0, 0, 0, 0, 0, 41}, //
{0, 0, 0, 0, 0, 25}, //
Expand Down Expand Up @@ -2259,6 +2260,7 @@ const Translation_Table translation_data = {
u8"see here\0"
u8"semicolon is not allowed after decorators\0"
u8"something happened\0"
u8"space is required between '<' and '<' inside 'extends' clause\0"
u8"spread starts here\0"
u8"spread tuple elements cannot be optional\0"
u8"static block starts here\0"
Expand Down
5 changes: 3 additions & 2 deletions src/quick-lint-js/i18n/translation-table-generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ namespace quick_lint_js {
using namespace std::literals::string_view_literals;

constexpr std::uint32_t translation_table_locale_count = 5;
constexpr std::uint16_t translation_table_mapping_table_size = 532;
constexpr std::size_t translation_table_string_table_size = 80499;
constexpr std::uint16_t translation_table_mapping_table_size = 533;
constexpr std::size_t translation_table_string_table_size = 80561;
constexpr std::size_t translation_table_locale_table_size = 35;

QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
Expand Down Expand Up @@ -471,6 +471,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
"see here"sv,
"semicolon is not allowed after decorators"sv,
"something happened"sv,
"space is required between '<' and '<' inside 'extends' clause"sv,
"spread starts here"sv,
"spread tuple elements cannot be optional"sv,
"static block starts here"sv,
Expand Down
13 changes: 12 additions & 1 deletion src/quick-lint-js/i18n/translation-table-test-generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ struct Translated_String {
};

// clang-format off
inline const Translated_String test_translation_table[531] = {
inline const Translated_String test_translation_table[532] = {
{
"\"global-groups\" entries must be strings"_translatable,
u8"\"global-groups\" entries must be strings",
Expand Down Expand Up @@ -4923,6 +4923,17 @@ inline const Translated_String test_translation_table[531] = {
u8"something happened",
},
},
{
"space is required between '<' and '<' inside 'extends' clause"_translatable,
u8"space is required between '<' and '<' inside 'extends' clause",
{
u8"space is required between '<' and '<' inside 'extends' clause",
u8"space is required between '<' and '<' inside 'extends' clause",
u8"space is required between '<' and '<' inside 'extends' clause",
u8"space is required between '<' and '<' inside 'extends' clause",
u8"space is required between '<' and '<' inside 'extends' clause",
},
},
{
"spread starts here"_translatable,
u8"spread starts here",
Expand Down
27 changes: 27 additions & 0 deletions test/test-parse-typescript-class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1546,6 +1546,33 @@ TEST_F(Test_Parse_TypeScript_Class, implement_multiple_things) {
ElementsAreArray({u8"Apple", u8"Banana", u8"Carrot"}));
}

// TODO(#1114): Report the same diagnostic for 'extends'.
TEST_F(Test_Parse_TypeScript_Class,
implements_generic_with_arrow_type_requires_space) {
{
Spy_Visitor p = test_parse_and_visit_module(
u8"class C implements I<<T>() => ReturnType<T>> {}"_sv, //
u8" ` Diag_TypeScript_Generic_Less_Less_Not_Split"_diag,
typescript_options);
EXPECT_THAT(
p.variable_declarations,
ElementsAreArray({generic_param_decl(u8"T"), class_decl(u8"C")}));
EXPECT_THAT(p.variable_uses,
ElementsAreArray({u8"I", u8"ReturnType", u8"T"}));
}

{
Spy_Visitor p = test_parse_and_visit_module(
u8"class C implements I< <T>() => ReturnType<T>> {}"_sv, no_diags,
typescript_options);
EXPECT_THAT(
p.variable_declarations,
ElementsAreArray({generic_param_decl(u8"T"), class_decl(u8"C")}));
EXPECT_THAT(p.variable_uses,
ElementsAreArray({u8"I", u8"ReturnType", u8"T"}));
}
}

TEST_F(Test_Parse_TypeScript_Class, parameter_property_in_constructor) {
{
Spy_Visitor p = test_parse_and_visit_module(
Expand Down
26 changes: 26 additions & 0 deletions test/test-parse-typescript-interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,32 @@ TEST_F(Test_Parse_TypeScript_Interface, extends_generic) {
EXPECT_THAT(p.variable_uses, ElementsAreArray({u8"A", u8"B"}));
}

TEST_F(Test_Parse_TypeScript_Interface,
extends_generic_with_arrow_type_requires_space) {
{
Spy_Visitor p = test_parse_and_visit_module(
u8"interface I extends Base<<T>() => ReturnType<T>> {}"_sv, //
u8" ` Diag_TypeScript_Generic_Less_Less_Not_Split"_diag,
typescript_options);
EXPECT_THAT(
p.variable_declarations,
ElementsAreArray({interface_decl(u8"I"), generic_param_decl(u8"T")}));
EXPECT_THAT(p.variable_uses,
ElementsAreArray({u8"Base", u8"ReturnType", u8"T"}));
}

{
Spy_Visitor p = test_parse_and_visit_module(
u8"interface I extends Base< <T>() => ReturnType<T>> {}"_sv, no_diags,
typescript_options);
EXPECT_THAT(
p.variable_declarations,
ElementsAreArray({interface_decl(u8"I"), generic_param_decl(u8"T")}));
EXPECT_THAT(p.variable_uses,
ElementsAreArray({u8"Base", u8"ReturnType", u8"T"}));
}
}

TEST_F(Test_Parse_TypeScript_Interface, unclosed_interface_statement) {
{
Spy_Visitor p = test_parse_and_visit_module(
Expand Down

0 comments on commit 64bfe94

Please sign in to comment.