feat(typescript): error on 'var foo: p is Type'

Report a diagnostic if a TypeScript type predicate appears where it shouldn't appear.
quick-lint · Nov 7, 2023 · 4ac79ee · 4ac79ee
1 parent aaed137
commit 4ac79ee
Show file tree

Hide file tree

Showing 11 changed files with 96 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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(