From 8087a6c9f2f1ad8ab1076f6fa5243d38c2a3f774 Mon Sep 17 00:00:00 2001 From: Valekh Bayramov Date: Fri, 22 Sep 2023 16:21:09 -0700 Subject: [PATCH] feat: new diag for missing commas between array elements fix: wiped commented code, clang format --- docs/errors/E0712.md | 15 +++++++++++++++ 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 | 8 +++++++- src/quick-lint-js/fe/parse-expression.cpp | 12 +++++++++++- .../i18n/translation-table-generated.cpp | 4 +++- .../i18n/translation-table-generated.h | 5 +++-- .../i18n/translation-table-test-generated.h | 13 ++++++++++++- test/test-parse-expression.cpp | 10 ++++++++++ 10 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 docs/errors/E0712.md diff --git a/docs/errors/E0712.md b/docs/errors/E0712.md new file mode 100644 index 0000000000..39ec3bc815 --- /dev/null +++ b/docs/errors/E0712.md @@ -0,0 +1,15 @@ +# E0712: missing ',' between array elements + +This error occurs when there is a missing comma (',') between elements in an array +declaration or initialization. In JavaScript, commas are used to separate individual +elements within an array, and the absence of a comma will lead to a syntax error. + +```javascript +let myArray = [1 2 3]; +``` + +To fix this error, you need to add a comma between each element within the array declaration or initialization + +```javascript +let myArray = [1, 2, 3]; +``` diff --git a/po/messages.pot b/po/messages.pot index 11693a479a..fc53566b34 100644 --- a/po/messages.pot +++ b/po/messages.pot @@ -1977,6 +1977,10 @@ msgstr "" msgid "missing expression in placeholder within template literal" msgstr "" +#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +msgid "missing ',' between array elements" +msgstr "" + #: test/test-diagnostic-formatter.cpp #: test/test-vim-qflist-json-diag-reporter.cpp msgid "something happened" diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp index a14a211c16..767fd895f6 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp @@ -6092,6 +6092,20 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = { }, }, }, + + // Diag_Missing_Comma_Between_Array_Elements + { + .code = 712, + .severity = Diagnostic_Severity::error, + .message_formats = { + QLJS_TRANSLATABLE("missing ',' between array elements"), + }, + .message_args = { + { + Diagnostic_Message_Arg_Info(offsetof(Diag_Missing_Comma_Between_Array_Elements, expected_comma), Diagnostic_Arg_Type::source_code_span), + }, + }, + }, }; } diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.h b/src/quick-lint-js/diag/diagnostic-metadata-generated.h index 89140d943d..786fe2a4a7 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.h +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.h @@ -420,10 +420,11 @@ namespace quick_lint_js { QLJS_DIAG_TYPE_NAME(Diag_Variable_Assigned_To_Self_Is_Noop) \ QLJS_DIAG_TYPE_NAME(Diag_Xor_Used_As_Exponentiation) \ QLJS_DIAG_TYPE_NAME(Diag_Expected_Expression_In_Template_Literal) \ + QLJS_DIAG_TYPE_NAME(Diag_Missing_Comma_Between_Array_Elements) \ /* END */ // clang-format on -inline constexpr int Diag_Type_Count = 409; +inline constexpr int Diag_Type_Count = 410; 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 08987ea0d5..fa90b2d64f 100644 --- a/src/quick-lint-js/diag/diagnostic-types-2.h +++ b/src/quick-lint-js/diag/diagnostic-types-2.h @@ -3136,8 +3136,14 @@ struct Diag_Expected_Expression_In_Template_Literal { ARG(placeholder))]] // Source_Code_Span placeholder; }; -} +struct Diag_Missing_Comma_Between_Array_Elements { + [[qljs::diag("E0712", Diagnostic_Severity::error)]] // + [[qljs::message("missing ',' between array elements", + ARG(expected_comma))]] // + Source_Code_Span expected_comma; +}; +} QLJS_WARNING_POP // quick-lint-js finds bugs in JavaScript programs. diff --git a/src/quick-lint-js/fe/parse-expression.cpp b/src/quick-lint-js/fe/parse-expression.cpp index 3960976886..1aa816fd4f 100644 --- a/src/quick-lint-js/fe/parse-expression.cpp +++ b/src/quick-lint-js/fe/parse-expression.cpp @@ -578,6 +578,7 @@ Expression* Parser::parse_primary_expression(Parse_Visitor_Base& v, case Token_Type::left_square: { const Char8* left_square_begin = this->peek().begin; const Char8* right_square_end; + bool met_expression = false; this->skip(); Expression_Arena::Vector children( @@ -588,11 +589,14 @@ Expression* Parser::parse_primary_expression(Parse_Visitor_Base& v, this->skip(); break; } - // TODO(strager): Require commas between expressions. + if (this->peek().type == Token_Type::comma) { + met_expression = false; this->skip(); continue; } + const Char8* previous_expession_end = + this->lexer_.end_of_previous_token(); const Char8* child_begin = this->peek().begin; Expression* child = this->parse_expression(v, Precedence{.commas = false}); @@ -612,6 +616,12 @@ Expression* Parser::parse_primary_expression(Parse_Visitor_Base& v, right_square_end = expected_right_square; break; } + if (met_expression) { + this->diag_reporter_->report(Diag_Missing_Comma_Between_Array_Elements{ + .expected_comma = Source_Code_Span::unit(previous_expession_end), + }); + } + met_expression = true; children.emplace_back(child); } Expression* ast = this->make_expression( diff --git a/src/quick-lint-js/i18n/translation-table-generated.cpp b/src/quick-lint-js/i18n/translation-table-generated.cpp index 5ce529d2be..ed4d9bcf69 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.cpp +++ b/src/quick-lint-js/i18n/translation-table-generated.cpp @@ -307,7 +307,8 @@ const Translation_Table translation_data = { {0, 0, 0, 0, 0, 46}, // {0, 0, 0, 0, 0, 56}, // {68, 21, 0, 52, 0, 40}, // - {59, 43, 61, 49, 50, 39}, // + {0, 0, 0, 0, 0, 39}, // + {59, 43, 61, 49, 50, 35}, // {0, 0, 0, 44, 0, 42}, // {44, 2, 0, 64, 0, 57}, // {35, 20, 49, 55, 39, 38}, // @@ -2074,6 +2075,7 @@ const Translation_Table translation_data = { u8"misleading use of ',' operator in conditional statement\0" u8"misleading use of ',' operator in index\0" u8"mismatched JSX tags; expected ''\0" + u8"missing ',' between array elements\0" u8"missing ',' between variable declarations\0" u8"missing ',', ';', or newline between object type entries\0" u8"missing '...' in JSX attribute spread\0" diff --git a/src/quick-lint-js/i18n/translation-table-generated.h b/src/quick-lint-js/i18n/translation-table-generated.h index 2e469b2028..7836a022ba 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 = 496; -constexpr std::size_t translation_table_string_table_size = 78710; +constexpr std::uint16_t translation_table_mapping_table_size = 497; +constexpr std::size_t translation_table_string_table_size = 78745; constexpr std::size_t translation_table_locale_table_size = 35; QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( @@ -322,6 +322,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( "misleading use of ',' operator in conditional statement"sv, "misleading use of ',' operator in index"sv, "mismatched JSX tags; expected ''"sv, + "missing ',' between array elements"sv, "missing ',' between variable declarations"sv, "missing ',', ';', or newline between object type entries"sv, "missing '...' in JSX attribute spread"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 0f41154af3..6456475abc 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[495] = { +inline const Translated_String test_translation_table[496] = { { "\"global-groups\" entries must be strings"_translatable, u8"\"global-groups\" entries must be strings", @@ -3284,6 +3284,17 @@ inline const Translated_String test_translation_table[495] = { u8"mismatched JSX tags; expected ''", }, }, + { + "missing ',' between array elements"_translatable, + u8"missing ',' between array elements", + { + u8"missing ',' between array elements", + u8"missing ',' between array elements", + u8"missing ',' between array elements", + u8"missing ',' between array elements", + u8"missing ',' between array elements", + }, + }, { "missing ',' between variable declarations"_translatable, u8"missing ',' between variable declarations", diff --git a/test/test-parse-expression.cpp b/test/test-parse-expression.cpp index e4410b5e26..2b31aa5e7d 100644 --- a/test/test-parse-expression.cpp +++ b/test/test-parse-expression.cpp @@ -1843,6 +1843,16 @@ TEST_F(Test_Parse_Expression, malformed_array_literal) { }); EXPECT_EQ(summarize(ast), "array()"); } + + { + Test_Parser p(u8"[a b]"_sv, capture_diags); + Expression* ast = p.parse_expression(); + assert_diagnostics(p.code, p.errors, + { + u8"Diag_Missing_Comma_Between_Array_Elements"_diag, + }); + EXPECT_EQ(summarize(ast), "array(var a, var b)"); + } } TEST_F(Test_Parse_Expression, object_literal) {