From 4ac79ee093592303a0959f275560a1205a9e7fd8 Mon Sep 17 00:00:00 2001 From: "Matthew \"strager\" Glazar" Date: Mon, 6 Nov 2023 22:59:26 -0500 Subject: [PATCH] feat(typescript): error on 'var foo: p is Type' Report a diagnostic if a TypeScript type predicate appears where it shouldn't appear. --- docs/CHANGELOG.md | 3 ++ po/messages.pot | 4 +++ .../diag/diagnostic-metadata-generated.cpp | 14 ++++++++ .../diag/diagnostic-metadata-generated.h | 3 +- src/quick-lint-js/diag/diagnostic-types-2.h | 7 ++++ src/quick-lint-js/fe/parse-expression.cpp | 3 +- src/quick-lint-js/fe/parse-type.cpp | 13 ++++++- .../i18n/translation-table-generated.cpp | 2 ++ .../i18n/translation-table-generated.h | 5 +-- .../i18n/translation-table-test-generated.h | 13 ++++++- test/test-parse-typescript-function.cpp | 35 +++++++++++++++++++ 11 files changed, 96 insertions(+), 6 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 309382d02f..f542153f19 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -14,6 +14,8 @@ Semantic Versioning. * `export as namespace` statements are now parsed. * `case await x:` no longer treats `:` as if it was a type annotation colon in an arrow function parameter list. + * If a type predicate appears outside a return type, quick-lint-js now reports + [E0426][] ("type predicates are only allowed as function return types"). ### Fixed @@ -1293,6 +1295,7 @@ Beta release. [E0383]: https://quick-lint-js.com/errors/E0383/ [E0384]: https://quick-lint-js.com/errors/E0384/ [E0398]: https://quick-lint-js.com/errors/E0398/ +[E0426]: https://quick-lint-js.com/errors/E0426/ [E0450]: https://quick-lint-js.com/errors/E0450/ [E0451]: https://quick-lint-js.com/errors/E0451/ [E0452]: https://quick-lint-js.com/errors/E0452/ diff --git a/po/messages.pot b/po/messages.pot index 13d2f7dcc9..ea3e7c348f 100644 --- a/po/messages.pot +++ b/po/messages.pot @@ -1509,6 +1509,10 @@ msgstr "" msgid "TypeScript type exports are not allowed in JavaScript" msgstr "" +#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +msgid "type predicates are only allowed as function return types" +msgstr "" + #: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp msgid "'type' cannot be used twice in export" msgstr "" diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp index a5043b249b..510a6bb47a 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp @@ -4481,6 +4481,20 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = { }, }, + // Diag_TypeScript_Type_Predicate_Only_Allowed_As_Return_Type + { + .code = 426, + .severity = Diagnostic_Severity::error, + .message_formats = { + QLJS_TRANSLATABLE("type predicates are only allowed as function return types"), + }, + .message_args = { + { + Diagnostic_Message_Arg_Info(offsetof(Diag_TypeScript_Type_Predicate_Only_Allowed_As_Return_Type, is_keyword), Diagnostic_Arg_Type::source_code_span), + }, + }, + }, + // Diag_TypeScript_Inline_Type_Export_Not_Allowed_In_Type_Only_Export { .code = 280, diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.h b/src/quick-lint-js/diag/diagnostic-metadata-generated.h index 4057886cc0..9f688f67c8 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.h +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.h @@ -309,6 +309,7 @@ namespace quick_lint_js { QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Global_Block_Not_Allowed_In_JavaScript) \ QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Global_Block_Not_Allowed_In_Namespace) \ QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Type_Export_Not_Allowed_In_JavaScript) \ + QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Type_Predicate_Only_Allowed_As_Return_Type) \ 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) \ @@ -446,7 +447,7 @@ namespace quick_lint_js { /* END */ // clang-format on -inline constexpr int Diag_Type_Count = 432; +inline constexpr int Diag_Type_Count = 433; extern const Diagnostic_Info all_diagnostic_infos[Diag_Type_Count]; } diff --git a/src/quick-lint-js/diag/diagnostic-types-2.h b/src/quick-lint-js/diag/diagnostic-types-2.h index 306439507d..bb40450e40 100644 --- a/src/quick-lint-js/diag/diagnostic-types-2.h +++ b/src/quick-lint-js/diag/diagnostic-types-2.h @@ -2318,6 +2318,13 @@ struct Diag_TypeScript_Type_Export_Not_Allowed_In_JavaScript { Source_Code_Span type_keyword; }; +struct Diag_TypeScript_Type_Predicate_Only_Allowed_As_Return_Type { + [[qljs::diag("E0426", Diagnostic_Severity::error)]] // + [[qljs::message("type predicates are only allowed as function return types", + ARG(is_keyword))]] // + Source_Code_Span is_keyword; +}; + struct Diag_TypeScript_Inline_Type_Export_Not_Allowed_In_Type_Only_Export { [[qljs::diag("E0280", Diagnostic_Severity::error)]] // [[qljs::message("'type' cannot be used twice in export", diff --git a/src/quick-lint-js/fe/parse-expression.cpp b/src/quick-lint-js/fe/parse-expression.cpp index ef803e5bd7..e106b5d7b2 100644 --- a/src/quick-lint-js/fe/parse-expression.cpp +++ b/src/quick-lint-js/fe/parse-expression.cpp @@ -2177,7 +2177,8 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v, TypeScript_Type_Parse_Options{ .allow_parenthesized_type = !is_possibly_arrow_function_return_type_annotation, - .allow_type_predicate = true, + .allow_type_predicate = + is_possibly_arrow_function_return_type_annotation, }); const Char8* type_end = this->lexer_.end_of_previous_token(); binary_builder.replace_last( diff --git a/src/quick-lint-js/fe/parse-type.cpp b/src/quick-lint-js/fe/parse-type.cpp index 27102bb05d..7fabbb8502 100644 --- a/src/quick-lint-js/fe/parse-type.cpp +++ b/src/quick-lint-js/fe/parse-type.cpp @@ -219,10 +219,21 @@ void Parser::parse_and_visit_typescript_type_expression( if (this->peek().type == Token_Type::kw_is) { // param is Type // this is Type + Source_Code_Span is_keyword = this->peek().span(); this->skip(); if (name_type != Token_Type::kw_this) { // this is Type - v.visit_variable_type_predicate_use(name); + if (parse_options.allow_type_predicate) { + v.visit_variable_type_predicate_use(name); + } else { + v.visit_variable_use(name); + } + } + if (!parse_options.allow_type_predicate) { + this->diag_reporter_->report( + Diag_TypeScript_Type_Predicate_Only_Allowed_As_Return_Type{ + .is_keyword = is_keyword, + }); } this->parse_and_visit_typescript_type_expression(v); return; diff --git a/src/quick-lint-js/i18n/translation-table-generated.cpp b/src/quick-lint-js/i18n/translation-table-generated.cpp index 18aebffa7f..1f573c60d6 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.cpp +++ b/src/quick-lint-js/i18n/translation-table-generated.cpp @@ -469,6 +469,7 @@ const Translation_Table translation_data = { {50, 25, 0, 70, 0, 78}, // {33, 21, 74, 25, 44, 21}, // {0, 0, 0, 0, 0, 26}, // + {0, 0, 0, 0, 0, 58}, // {27, 19, 30, 29, 22, 31}, // {25, 50, 0, 36, 0, 23}, // {66, 43, 31, 36, 30, 44}, // @@ -2268,6 +2269,7 @@ const Translation_Table translation_data = { u8"this tuple type is a named tuple type because at least one element has a name\0" u8"this {0} looks fishy\0" u8"try statement starts here\0" + u8"type predicates are only allowed as function return types\0" u8"type {1} is being defined here\0" u8"unclosed block comment\0" u8"unclosed class; expected '}' by end of file\0" diff --git a/src/quick-lint-js/i18n/translation-table-generated.h b/src/quick-lint-js/i18n/translation-table-generated.h index 28be58721d..6e775216c6 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.h +++ b/src/quick-lint-js/i18n/translation-table-generated.h @@ -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 = 529; -constexpr std::size_t translation_table_string_table_size = 80304; +constexpr std::uint16_t translation_table_mapping_table_size = 530; +constexpr std::size_t translation_table_string_table_size = 80362; constexpr std::size_t translation_table_locale_table_size = 35; QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( @@ -483,6 +483,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( "this tuple type is a named tuple type because at least one element has a name"sv, "this {0} looks fishy"sv, "try statement starts here"sv, + "type predicates are only allowed as function return types"sv, "type {1} is being defined here"sv, "unclosed block comment"sv, "unclosed class; expected '}' by end of file"sv, diff --git a/src/quick-lint-js/i18n/translation-table-test-generated.h b/src/quick-lint-js/i18n/translation-table-test-generated.h index b283c2ade5..28145cd15d 100644 --- a/src/quick-lint-js/i18n/translation-table-test-generated.h +++ b/src/quick-lint-js/i18n/translation-table-test-generated.h @@ -27,7 +27,7 @@ struct Translated_String { }; // clang-format off -inline const Translated_String test_translation_table[528] = { +inline const Translated_String test_translation_table[529] = { { "\"global-groups\" entries must be strings"_translatable, u8"\"global-groups\" entries must be strings", @@ -5055,6 +5055,17 @@ inline const Translated_String test_translation_table[528] = { u8"try sats startar h\u00e4r", }, }, + { + "type predicates are only allowed as function return types"_translatable, + u8"type predicates are only allowed as function return types", + { + u8"type predicates are only allowed as function return types", + u8"type predicates are only allowed as function return types", + u8"type predicates are only allowed as function return types", + u8"type predicates are only allowed as function return types", + u8"type predicates are only allowed as function return types", + }, + }, { "type {1} is being defined here"_translatable, u8"type {1} is being defined here", diff --git a/test/test-parse-typescript-function.cpp b/test/test-parse-typescript-function.cpp index 46a1e846b6..04e0c184b6 100644 --- a/test/test-parse-typescript-function.cpp +++ b/test/test-parse-typescript-function.cpp @@ -991,6 +991,41 @@ TEST_F(Test_Parse_TypeScript_Function, type_predicate_on_generator_function) { } } +TEST_F(Test_Parse_TypeScript_Function, + type_predicate_is_only_allowed_as_return_type) { + { + Spy_Visitor p = test_parse_and_visit_statement( + u8"function f(p, q: p is SomeType) {}"_sv, // + u8" ^^ Diag_TypeScript_Type_Predicate_Only_Allowed_As_Return_Type"_diag, + typescript_options); + EXPECT_THAT(p.visits, ElementsAreArray({ + "visit_enter_function_scope", // + "visit_variable_declaration", // p + "visit_variable_use", // p + "visit_variable_type_use", // SomeType + "visit_variable_declaration", // q + "visit_enter_function_scope_body", // { + "visit_exit_function_scope", // } + "visit_variable_declaration", // f + })); + EXPECT_THAT(p.variable_uses, ElementsAreArray({u8"p", u8"SomeType"})); + } + + { + test_parse_and_visit_statement( + u8"var x: p is T;"_sv, // + u8" ^^ Diag_TypeScript_Type_Predicate_Only_Allowed_As_Return_Type"_diag, + typescript_options); + } + + { + test_parse_and_visit_statement( + u8"function f(p): p is p is T {}"_sv, // + u8" ^^ Diag_TypeScript_Type_Predicate_Only_Allowed_As_Return_Type"_diag, + typescript_options); + } +} + TEST_F(Test_Parse_TypeScript_Function, type_predicate_in_type) { { Spy_Visitor p = test_parse_and_visit_typescript_type_expression(