feat(typescript): improve diagnostic on duplicate 'in' or 'out' modif…

…iers
quick-lint · Dec 16, 2023 · c5fdcd0 · c5fdcd0
1 parent 6111898
commit c5fdcd0
Show file tree

Hide file tree

Showing 10 changed files with 208 additions and 32 deletions.
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -31,6 +31,8 @@ Semantic Versioning.
     clause, or in a `typeof` type now reports [E0429][] instead of misleading
     diagnostics.
   * Properties in object literal types can now be named with number literals.
+  * Repeating `in` or `out` generic parameter modifiers now reports [E0432][]
+    ("'in' or 'out' variance specifier cannot be listed twice").
 
 ### Fixed
 

diff --git a/po/messages.pot b/po/messages.pot
@@ -1773,6 +1773,14 @@ msgstr ""
 msgid "'out in' is not allowed; write 'in out' instead"
 msgstr ""
 
+#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
+msgid "'{0}' variance specifier cannot be listed twice"
+msgstr ""
+
+#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
+msgid "'{0}' already written here"
+msgstr ""
+
 #: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
 msgid "unclosed block comment"
 msgstr ""

diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
@@ -5243,6 +5243,24 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = {
       },
     },
 
+    // Diag_TypeScript_Variance_Keyword_Repeated
+    {
+      .code = 432,
+      .severity = Diagnostic_Severity::error,
+      .message_formats = {
+        QLJS_TRANSLATABLE("'{0}' variance specifier cannot be listed twice"),
+        QLJS_TRANSLATABLE("'{0}' already written here"),
+      },
+      .message_args = {
+        {
+          Diagnostic_Message_Arg_Info(offsetof(Diag_TypeScript_Variance_Keyword_Repeated, second_keyword), Diagnostic_Arg_Type::source_code_span),
+        },
+        {
+          Diagnostic_Message_Arg_Info(offsetof(Diag_TypeScript_Variance_Keyword_Repeated, first_keyword), Diagnostic_Arg_Type::source_code_span),
+        },
+      },
+    },
+
     // Diag_Unclosed_Block_Comment
     {
       .code = 37,

diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.h b/src/quick-lint-js/diag/diagnostic-metadata-generated.h
@@ -359,6 +359,7 @@ namespace quick_lint_js {
   QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Spread_Element_Cannot_Be_Optional) \
   QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Style_Const_Field) \
   QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Variance_Keywords_In_Wrong_Order) \
+  QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Variance_Keyword_Repeated) \
   QLJS_DIAG_TYPE_NAME(Diag_Unclosed_Block_Comment) \
   QLJS_DIAG_TYPE_NAME(Diag_Unclosed_Class_Block) \
   QLJS_DIAG_TYPE_NAME(Diag_Unclosed_Code_Block) \
@@ -453,7 +454,7 @@ namespace quick_lint_js {
   /* END */
 // clang-format on
 
-inline constexpr int Diag_Type_Count = 439;
+inline constexpr int Diag_Type_Count = 440;
 
 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
@@ -2731,6 +2731,16 @@ struct Diag_TypeScript_Variance_Keywords_In_Wrong_Order {
   Source_Code_Span out_keyword;
 };
 
+struct Diag_TypeScript_Variance_Keyword_Repeated {
+  [[qljs::diag("E0432", Diagnostic_Severity::error)]]  //
+  [[qljs::message("'{0}' variance specifier cannot be listed twice",
+                  ARG(second_keyword))]]  //
+  [[qljs::message("'{0}' already written here",
+                  ARG(first_keyword))]]  //
+  Source_Code_Span first_keyword;
+  Source_Code_Span second_keyword;
+};
+
 struct Diag_Unclosed_Block_Comment {
   [[qljs::diag("E0037", Diagnostic_Severity::error)]]             //
   [[qljs::message("unclosed block comment", ARG(comment_open))]]  //

diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp
@@ -1662,33 +1662,32 @@ void Parser::parse_and_visit_typescript_generic_parameters(
       this->buffering_visitor_stack_.push();
 
 next_parameter:
-  std::optional<Identifier> in_keyword;
-  std::optional<Identifier> out_keyword;
-
-  if (this->peek().type == Token_Type::kw_in) {
+  // 'in' and 'out' keywords.
+  struct Modifier {
+    Identifier identifier;
+    Token_Type token_type;
+  };
+  Vector<Modifier> modifiers(
+      "parse_and_visit_typescript_generic_parameters modifiers",
+      &this->temporary_memory_);
+  for (;;) {
+    switch (this->peek().type) {
     // <in T>
     // <in out T>
-    in_keyword = this->peek().identifier_name();
-    this->skip();
-  }
-
-  if (this->peek().type == Token_Type::kw_out) {
-    // <out T>
-    // <out>
-    out_keyword = this->peek().identifier_name();
-    this->skip();
-
-    if (!in_keyword.has_value() && this->peek().type == Token_Type::kw_in) {
-      // <out in T>  // Invalid.
-      in_keyword = this->peek().identifier_name();
-      this->diag_reporter_->report(
-          Diag_TypeScript_Variance_Keywords_In_Wrong_Order{
-              .in_keyword = in_keyword->span(),
-              .out_keyword = out_keyword->span(),
-          });
+    case Token_Type::kw_in:
+    case Token_Type::kw_out:
+      modifiers.push_back(Modifier{
+          .identifier = this->peek().identifier_name(),
+          .token_type = this->peek().type,
+      });
       this->skip();
+      break;
+
+    default:
+      goto done_parsing_modifiers;
     }
   }
+done_parsing_modifiers:
 
   std::optional<Identifier> parameter_name;
   switch (this->peek().type) {
@@ -1712,7 +1711,6 @@ void Parser::parse_and_visit_typescript_generic_parameters(
   case Token_Type::kw_module:
   case Token_Type::kw_namespace:
   case Token_Type::kw_of:
-  case Token_Type::kw_out:
   case Token_Type::kw_override:
   case Token_Type::kw_readonly:
   case Token_Type::kw_require:
@@ -1726,20 +1724,64 @@ void Parser::parse_and_visit_typescript_generic_parameters(
     break;
 
   case Token_Type::kw_in:
-    QLJS_PARSER_UNIMPLEMENTED();
+  case Token_Type::kw_out:
+    QLJS_UNREACHABLE();
     break;
 
   default:
-    if (out_keyword.has_value()) {
+    if (!modifiers.empty() &&
+        modifiers.back().token_type == Token_Type::kw_out) {
       // <out>
-      parameter_name = out_keyword;
-      out_keyword = std::nullopt;
+      parameter_name = modifiers.back().identifier;
+      modifiers.pop_back();
     } else {
       QLJS_PARSER_UNIMPLEMENTED();
     }
     break;
   }
 
+  // Report bad modifiers.
+  {
+    std::optional<Source_Code_Span> first_in_keyword;
+    std::optional<Source_Code_Span> first_out_keyword;
+    bool reported_out_in_misorder = false;
+    for (const Modifier &modifier : modifiers) {
+      if (modifier.token_type == Token_Type::kw_in) {
+        if (first_in_keyword.has_value()) {
+          this->diag_reporter_->report(
+              Diag_TypeScript_Variance_Keyword_Repeated{
+                  .first_keyword = *first_in_keyword,
+                  .second_keyword = modifier.identifier.span(),
+              });
+        } else {
+          first_in_keyword = modifier.identifier.span();
+        }
+      }
+      if (modifier.token_type == Token_Type::kw_out) {
+        if (first_out_keyword.has_value()) {
+          this->diag_reporter_->report(
+              Diag_TypeScript_Variance_Keyword_Repeated{
+                  .first_keyword = *first_out_keyword,
+                  .second_keyword = modifier.identifier.span(),
+              });
+        } else {
+          first_out_keyword = modifier.identifier.span();
+        }
+      }
+      if (!reported_out_in_misorder &&
+          modifier.token_type == Token_Type::kw_in &&
+          first_out_keyword.has_value()) {
+        // <out in T>  // Invalid.
+        this->diag_reporter_->report(
+            Diag_TypeScript_Variance_Keywords_In_Wrong_Order{
+                .in_keyword = modifier.identifier.span(),
+                .out_keyword = *first_out_keyword,
+            });
+        reported_out_in_misorder = true;
+      }
+    }
+  }
+
   if (this->peek().type == Token_Type::kw_extends ||
       this->peek().type == Token_Type::colon) {
     // <T: U>  // Invalid.

diff --git a/src/quick-lint-js/i18n/translation-table-generated.cpp b/src/quick-lint-js/i18n/translation-table-generated.cpp
@@ -92,7 +92,8 @@ const Translation_Table translation_data = {
         {15, 14, 0, 13, 0, 13},              //
         {0, 0, 0, 19, 0, 17},                //
         {0, 0, 0, 0, 0, 25},                 //
-        {15, 38, 0, 25, 0, 42},              //
+        {0, 0, 0, 0, 0, 42},                 //
+        {15, 38, 0, 25, 0, 27},              //
         {0, 0, 0, 0, 0, 17},                 //
         {0, 0, 0, 0, 0, 11},                 //
         {15, 17, 0, 22, 0, 38},              //
@@ -102,6 +103,7 @@ const Translation_Table translation_data = {
         {0, 0, 0, 0, 0, 32},                 //
         {69, 48, 0, 36, 0, 25},              //
         {0, 0, 0, 0, 0, 62},                 //
+        {0, 0, 0, 0, 0, 48},                 //
         {0, 0, 0, 0, 0, 39},                 //
         {68, 25, 0, 65, 0, 28},              //
         {0, 0, 0, 0, 0, 62},                 //
@@ -1904,6 +1906,7 @@ const Translation_Table translation_data = {
         u8"'with' statement\0"
         u8"'{0} []' is always '{1}'\0"
         u8"'{0}' access specifier must precede '{1}'\0"
+        u8"'{0}' already written here\0"
         u8"'{0}' found here\0"
         u8"'{0}' here\0"
         u8"'{0}' is missing on overloaded method\0"
@@ -1913,6 +1916,7 @@ const Translation_Table translation_data = {
         u8"'{0}' is not allowed with '{1}'\0"
         u8"'{0}' must precede '{1}'\0"
         u8"'{0}' operator cannot be used before '**' without parentheses\0"
+        u8"'{0}' variance specifier cannot be listed twice\0"
         u8"'{1}' is missing on overload signature\0"
         u8"'{1}' statement starts here\0"
         u8"'}' is not allowed directly in JSX text; write {{'}'} instead\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 = 541;
-constexpr std::size_t translation_table_string_table_size = 80778;
+constexpr std::uint16_t translation_table_mapping_table_size = 543;
+constexpr std::size_t translation_table_string_table_size = 80853;
 constexpr std::size_t translation_table_locale_table_size = 35;
 
 QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
@@ -107,6 +107,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
           "'with' statement"sv,
           "'{0} []' is always '{1}'"sv,
           "'{0}' access specifier must precede '{1}'"sv,
+          "'{0}' already written here"sv,
           "'{0}' found here"sv,
           "'{0}' here"sv,
           "'{0}' is missing on overloaded method"sv,
@@ -116,6 +117,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
           "'{0}' is not allowed with '{1}'"sv,
           "'{0}' must precede '{1}'"sv,
           "'{0}' operator cannot be used before '**' without parentheses"sv,
+          "'{0}' variance specifier cannot be listed twice"sv,
           "'{1}' is missing on overload signature"sv,
           "'{1}' statement starts here"sv,
           "'}' is not allowed directly in JSX text; write {{'}'} instead"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[540] = {
+inline const Translated_String test_translation_table[542] = {
     {
         "\"global-groups\" entries must be strings"_translatable,
         u8"\"global-groups\" entries must be strings",
@@ -919,6 +919,17 @@ inline const Translated_String test_translation_table[540] = {
             u8"'{0}' access specifier must precede '{1}'",
         },
     },
+    {
+        "'{0}' already written here"_translatable,
+        u8"'{0}' already written here",
+        {
+            u8"'{0}' already written here",
+            u8"'{0}' already written here",
+            u8"'{0}' already written here",
+            u8"'{0}' already written here",
+            u8"'{0}' already written here",
+        },
+    },
     {
         "'{0}' found here"_translatable,
         u8"'{0}' found here",
@@ -1018,6 +1029,17 @@ inline const Translated_String test_translation_table[540] = {
             u8"'{0}' operator cannot be used before '**' without parentheses",
         },
     },
+    {
+        "'{0}' variance specifier cannot be listed twice"_translatable,
+        u8"'{0}' variance specifier cannot be listed twice",
+        {
+            u8"'{0}' variance specifier cannot be listed twice",
+            u8"'{0}' variance specifier cannot be listed twice",
+            u8"'{0}' variance specifier cannot be listed twice",
+            u8"'{0}' variance specifier cannot be listed twice",
+            u8"'{0}' variance specifier cannot be listed twice",
+        },
+    },
     {
         "'{1}' is missing on overload signature"_translatable,
         u8"'{1}' is missing on overload signature",