From 90b6ce8d1e9ec74b4d73abc68a5748968b3e45c5 Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Wed, 18 Oct 2023 23:15:16 -0400 Subject: [PATCH 01/14] feat: add diag missing fallthrough comment to diagnostic types --- 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 +++++++ .../i18n/translation-table-generated.cpp | 4 +++- .../i18n/translation-table-generated.h | 5 +++-- .../i18n/translation-table-test-generated.h | 13 ++++++++++++- 7 files changed, 45 insertions(+), 5 deletions(-) diff --git a/po/messages.pot b/po/messages.pot index baa254be30..8332ddba1b 100644 --- a/po/messages.pot +++ b/po/messages.pot @@ -557,6 +557,10 @@ msgstr "" msgid "this case will run instead" msgstr "" +#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +msgid "missing fallthrough comment" +msgstr "" + #: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp msgid "'else' has no corresponding 'if'" msgstr "" diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp index e53d4f6276..f14e3c6ffc 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp @@ -1423,6 +1423,20 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = { }, }, + // Diag_Explicit_Fallthrough_Comment_In_Switch + { + .code = 423, + .severity = Diagnostic_Severity::warning, + .message_formats = { + QLJS_TRANSLATABLE("missing fallthrough comment"), + }, + .message_args = { + { + Diagnostic_Message_Arg_Info(offsetof(Diag_Explicit_Fallthrough_Comment_In_Switch, end_of_case), Diagnostic_Arg_Type::source_code_span), + }, + }, + }, + // Diag_Else_Has_No_If { .code = 65, diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.h b/src/quick-lint-js/diag/diagnostic-metadata-generated.h index 28d0409760..7abc1f2934 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.h +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.h @@ -103,6 +103,7 @@ namespace quick_lint_js { QLJS_DIAG_TYPE_NAME(Diag_Dot_Not_Allowed_After_Generic_Arguments_In_Type) \ QLJS_DIAG_TYPE_NAME(Diag_Dot_Dot_Is_Not_An_Operator) \ QLJS_DIAG_TYPE_NAME(Diag_Duplicated_Cases_In_Switch_Statement) \ + QLJS_DIAG_TYPE_NAME(Diag_Explicit_Fallthrough_Comment_In_Switch) \ QLJS_DIAG_TYPE_NAME(Diag_Else_Has_No_If) \ QLJS_DIAG_TYPE_NAME(Diag_Equals_Does_Not_Distribute_Over_Or) \ QLJS_DIAG_TYPE_NAME(Diag_Escaped_Character_Disallowed_In_Identifiers) \ @@ -442,7 +443,7 @@ namespace quick_lint_js { /* END */ // clang-format on -inline constexpr int Diag_Type_Count = 428; +inline constexpr int Diag_Type_Count = 429; 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 07918727b5..a94fd971c5 100644 --- a/src/quick-lint-js/diag/diagnostic-types-2.h +++ b/src/quick-lint-js/diag/diagnostic-types-2.h @@ -759,6 +759,13 @@ struct Diag_Duplicated_Cases_In_Switch_Statement { Source_Code_Span duplicated_switch_case; }; +struct Diag_Explicit_Fallthrough_Comment_In_Switch { + [[qljs::diag("E0423", Diagnostic_Severity::warning)]] // + [[qljs::message("missing fallthrough comment", + ARG(end_of_case))]] // + Source_Code_Span end_of_case; +}; + struct Diag_Else_Has_No_If { [[qljs::diag("E0065", Diagnostic_Severity::error)]] // [[qljs::message("'else' has no corresponding 'if'", ARG(else_token))]] // diff --git a/src/quick-lint-js/i18n/translation-table-generated.cpp b/src/quick-lint-js/i18n/translation-table-generated.cpp index ace2dc583b..b6a385bc7f 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.cpp +++ b/src/quick-lint-js/i18n/translation-table-generated.cpp @@ -363,7 +363,8 @@ const Translation_Table translation_data = { {0, 0, 0, 0, 0, 35}, // {36, 34, 39, 35, 40, 37}, // {0, 0, 0, 0, 0, 39}, // - {33, 7, 40, 40, 33, 58}, // + {0, 0, 0, 0, 0, 58}, // + {33, 7, 40, 40, 33, 28}, // {24, 11, 33, 22, 23, 24}, // {34, 63, 43, 41, 33, 32}, // {41, 11, 49, 43, 0, 39}, // @@ -2156,6 +2157,7 @@ const Translation_Table translation_data = { u8"missing exported name in import type\0" u8"missing expression between parentheses\0" u8"missing expression in placeholder within template literal\0" + u8"missing fallthrough comment\0" u8"missing for loop header\0" u8"missing function parameter list\0" u8"missing header and body for 'for' loop\0" diff --git a/src/quick-lint-js/i18n/translation-table-generated.h b/src/quick-lint-js/i18n/translation-table-generated.h index 16657ed1f5..3b5481e519 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 = 522; -constexpr std::size_t translation_table_string_table_size = 79941; +constexpr std::uint16_t translation_table_mapping_table_size = 523; +constexpr std::size_t translation_table_string_table_size = 79969; constexpr std::size_t translation_table_locale_table_size = 35; QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( @@ -378,6 +378,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( "missing exported name in import type"sv, "missing expression between parentheses"sv, "missing expression in placeholder within template literal"sv, + "missing fallthrough comment"sv, "missing for loop header"sv, "missing function parameter list"sv, "missing header and body for 'for' loop"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 f50238d171..d985354fdd 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[521] = { +inline const Translated_String test_translation_table[522] = { { "\"global-groups\" entries must be strings"_translatable, u8"\"global-groups\" entries must be strings", @@ -3900,6 +3900,17 @@ inline const Translated_String test_translation_table[521] = { u8"missing expression in placeholder within template literal", }, }, + { + "missing fallthrough comment"_translatable, + u8"missing fallthrough comment", + { + u8"missing fallthrough comment", + u8"missing fallthrough comment", + u8"missing fallthrough comment", + u8"missing fallthrough comment", + u8"missing fallthrough comment", + }, + }, { "missing for loop header"_translatable, u8"missing for loop header", From 9a31c296fc303e3ba1bef28a258f8a09f7368631 Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Sat, 21 Oct 2023 13:41:32 -0400 Subject: [PATCH 02/14] feat: add functionality to find comments and report to diag if not found --- src/quick-lint-js/fe/parse-statement.cpp | 42 +++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp index 8902971c06..82a37bc9ad 100644 --- a/src/quick-lint-js/fe/parse-statement.cpp +++ b/src/quick-lint-js/fe/parse-statement.cpp @@ -2766,8 +2766,36 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { bool keep_going = true; bool is_before_first_switch_case = true; + Token prev_token; Hash_Set cases; while (keep_going) { + auto is_comment_line = [&]() -> bool { + const Char8* th = this->lexer().end_of_previous_token(); + bool f = false; + int i = 0; + for(;;) { + switch(th[i]) { + case '/': + if (th[i+1] == '/') { + f = true; + } + goto exit_loop; + // skip empty lines to reach comment + case ' ': + case '\n': + case '\r': + case '\t': + case '\f': + case '\v': + break; + default: + goto exit_loop; + } + i++; + } + exit_loop:; + return f; + }; switch (this->peek().type) { case Token_Type::right_curly: this->skip(); @@ -2775,6 +2803,12 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { break; case Token_Type::kw_case: { + if (!is_before_first_switch_case && prev_token.type != Token_Type::kw_break && prev_token.type != Token_Type::kw_case && !is_comment_line()) { + this->diag_reporter_->report( + Diag_Explicit_Fallthrough_Comment_In_Switch{ + .end_of_case = prev_token.span() }); + } + prev_token = this->peek(); is_before_first_switch_case = false; Source_Code_Span case_token_span = this->peek().span(); this->skip(); @@ -2806,18 +2840,24 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { } case Token_Type::kw_default: + if (!is_before_first_switch_case && prev_token.type != Token_Type::kw_break && prev_token.type != Token_Type::kw_case && !is_comment_line()) { + this->diag_reporter_->report( + Diag_Explicit_Fallthrough_Comment_In_Switch{ + .end_of_case = prev_token.span() }); + } is_before_first_switch_case = false; this->skip(); QLJS_PARSER_UNIMPLEMENTED_IF_NOT_TOKEN(Token_Type::colon); this->skip(); break; - + default: { if (is_before_first_switch_case) { this->diag_reporter_->report(Diag_Statement_Before_First_Switch_Case{ .unexpected_statement = this->peek().span(), }); } + prev_token = this->peek(); bool parsed_statement = this->parse_and_visit_statement( v, Parse_Statement_Options{ .possibly_followed_by_another_statement = true, From a95534f6494f9174893d9a831378f4bb7d09acba Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Sat, 21 Oct 2023 13:44:58 -0400 Subject: [PATCH 03/14] fix: remove unintended diag warning from other tests cases due to newly introduced diag E0423 --- test/test-parse.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test-parse.cpp b/test/test-parse.cpp index 474815828e..62abc0fa09 100644 --- a/test/test-parse.cpp +++ b/test/test-parse.cpp @@ -263,6 +263,7 @@ TEST_F(Test_Parse, asi_between_expression_statement_and_switch_label) { switch (x) { case a: f() + //fallthrough case b: g() } @@ -278,6 +279,7 @@ TEST_F(Test_Parse, asi_between_expression_statement_and_switch_label) { switch (x) { case a: f() + //fallthrough default: g() } From 221cd6a61bcd7b6ba95ccc4cb905bc4de3f5fb77 Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Sat, 21 Oct 2023 13:46:04 -0400 Subject: [PATCH 04/14] test: add new test cases for E0423 diag warn --- test/test-parse-warning.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/test-parse-warning.cpp b/test/test-parse-warning.cpp index 518eff78cc..bd41038db0 100644 --- a/test/test-parse-warning.cpp +++ b/test/test-parse-warning.cpp @@ -436,6 +436,31 @@ TEST_F(Test_Parse_Warning, warn_on_xor_operation_used_as_exponentiation) { test_parse_and_visit_expression(u8"4 ^ 3"_sv, no_diags); test_parse_and_visit_expression(u8"(x+2)^a"_sv, no_diags); } +TEST_F(Test_Parse_Warning, Diag_Explicit_Fallthrough_Comment_In_Switch) { + test_parse_and_visit_statement( + u8"switch(cond1){case 1:\nfoo()\ncase 2:\nbar() //fallthrough\ndefault:}"_sv, // + u8" ^^^ Diag_Explicit_Fallthrough_Comment_In_Switch"_diag); + test_parse_and_visit_statement( + u8"switch(cond1){case 1:\nfoo()\ncase 2:\nlongBarFn()\ndefault:}"_sv, // + u8" ^^^^^^^^^ Diag_Explicit_Fallthrough_Comment_In_Switch"_diag, // + u8" ^^^ Diag_Explicit_Fallthrough_Comment_In_Switch"_diag); + // check for false positive + test_parse_and_visit_statement( + u8R"(switch(cond1){ + case 1: + case 2: + default: + })"_sv, no_diags); + test_parse_and_visit_statement( + u8R"(switch(cond1){ + case 1: + foo() + + //fallthrough + case 2: + bar()//fallthrough + default:})"_sv, no_diags); +} } } From b23aa3ebf7b379ee9d75b2df5e4a84f3c084960d Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Sat, 21 Oct 2023 14:23:57 -0400 Subject: [PATCH 05/14] fix: clang-format --- src/quick-lint-js/diag/diagnostic-types-2.h | 2 +- src/quick-lint-js/fe/parse-statement.cpp | 58 +++++++++++---------- test/test-parse-warning.cpp | 8 +-- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/src/quick-lint-js/diag/diagnostic-types-2.h b/src/quick-lint-js/diag/diagnostic-types-2.h index a94fd971c5..9acff27e5d 100644 --- a/src/quick-lint-js/diag/diagnostic-types-2.h +++ b/src/quick-lint-js/diag/diagnostic-types-2.h @@ -762,7 +762,7 @@ struct Diag_Duplicated_Cases_In_Switch_Statement { struct Diag_Explicit_Fallthrough_Comment_In_Switch { [[qljs::diag("E0423", Diagnostic_Severity::warning)]] // [[qljs::message("missing fallthrough comment", - ARG(end_of_case))]] // + ARG(end_of_case))]] // Source_Code_Span end_of_case; }; diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp index 82a37bc9ad..5e35e2fe3a 100644 --- a/src/quick-lint-js/fe/parse-statement.cpp +++ b/src/quick-lint-js/fe/parse-statement.cpp @@ -2770,30 +2770,30 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { Hash_Set cases; while (keep_going) { auto is_comment_line = [&]() -> bool { - const Char8* th = this->lexer().end_of_previous_token(); + const Char8 *th = this->lexer().end_of_previous_token(); bool f = false; int i = 0; - for(;;) { - switch(th[i]) { - case '/': - if (th[i+1] == '/') { - f = true; - } - goto exit_loop; - // skip empty lines to reach comment - case ' ': - case '\n': - case '\r': - case '\t': - case '\f': - case '\v': - break; - default: - goto exit_loop; + for (;;) { + switch (th[i]) { + case '/': + if (th[i + 1] == '/') { + f = true; + } + goto exit_loop; + // skip empty lines to reach comment + case ' ': + case '\n': + case '\r': + case '\t': + case '\f': + case '\v': + break; + default: + goto exit_loop; } - i++; + i++; } - exit_loop:; + exit_loop:; return f; }; switch (this->peek().type) { @@ -2803,10 +2803,12 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { break; case Token_Type::kw_case: { - if (!is_before_first_switch_case && prev_token.type != Token_Type::kw_break && prev_token.type != Token_Type::kw_case && !is_comment_line()) { + if (!is_before_first_switch_case && + prev_token.type != Token_Type::kw_break && + prev_token.type != Token_Type::kw_case && !is_comment_line()) { this->diag_reporter_->report( - Diag_Explicit_Fallthrough_Comment_In_Switch{ - .end_of_case = prev_token.span() }); + Diag_Explicit_Fallthrough_Comment_In_Switch{.end_of_case = + prev_token.span()}); } prev_token = this->peek(); is_before_first_switch_case = false; @@ -2840,17 +2842,19 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { } case Token_Type::kw_default: - if (!is_before_first_switch_case && prev_token.type != Token_Type::kw_break && prev_token.type != Token_Type::kw_case && !is_comment_line()) { + if (!is_before_first_switch_case && + prev_token.type != Token_Type::kw_break && + prev_token.type != Token_Type::kw_case && !is_comment_line()) { this->diag_reporter_->report( - Diag_Explicit_Fallthrough_Comment_In_Switch{ - .end_of_case = prev_token.span() }); + Diag_Explicit_Fallthrough_Comment_In_Switch{.end_of_case = + prev_token.span()}); } is_before_first_switch_case = false; this->skip(); QLJS_PARSER_UNIMPLEMENTED_IF_NOT_TOKEN(Token_Type::colon); this->skip(); break; - + default: { if (is_before_first_switch_case) { this->diag_reporter_->report(Diag_Statement_Before_First_Switch_Case{ diff --git a/test/test-parse-warning.cpp b/test/test-parse-warning.cpp index bd41038db0..79f0ea4165 100644 --- a/test/test-parse-warning.cpp +++ b/test/test-parse-warning.cpp @@ -442,7 +442,7 @@ TEST_F(Test_Parse_Warning, Diag_Explicit_Fallthrough_Comment_In_Switch) { u8" ^^^ Diag_Explicit_Fallthrough_Comment_In_Switch"_diag); test_parse_and_visit_statement( u8"switch(cond1){case 1:\nfoo()\ncase 2:\nlongBarFn()\ndefault:}"_sv, // - u8" ^^^^^^^^^ Diag_Explicit_Fallthrough_Comment_In_Switch"_diag, // + u8" ^^^^^^^^^ Diag_Explicit_Fallthrough_Comment_In_Switch"_diag, // u8" ^^^ Diag_Explicit_Fallthrough_Comment_In_Switch"_diag); // check for false positive test_parse_and_visit_statement( @@ -450,7 +450,8 @@ TEST_F(Test_Parse_Warning, Diag_Explicit_Fallthrough_Comment_In_Switch) { case 1: case 2: default: - })"_sv, no_diags); + })"_sv, + no_diags); test_parse_and_visit_statement( u8R"(switch(cond1){ case 1: @@ -459,7 +460,8 @@ TEST_F(Test_Parse_Warning, Diag_Explicit_Fallthrough_Comment_In_Switch) { //fallthrough case 2: bar()//fallthrough - default:})"_sv, no_diags); + default:})"_sv, + no_diags); } } } From 731cce5fb408bc46ab27a7f1edb0eabee59bed0b Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Sat, 21 Oct 2023 15:02:36 -0400 Subject: [PATCH 06/14] feat: add error docs for E0423 --- docs/errors/E0423.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 docs/errors/E0423.md diff --git a/docs/errors/E0423.md b/docs/errors/E0423.md new file mode 100644 index 0000000000..367619122d --- /dev/null +++ b/docs/errors/E0423.md @@ -0,0 +1,29 @@ +# E0423: missing fallthrough comment in switch case + +Switch Cases in javascript fallthrough to the next case if the `break` statement is not added at the end of the case. +Since there is no explicit way of communication whether the fallthrough is intentional or not, it is recommended to use a comment indicating fallthrough. + +```javascript +function test (c) { + switch (c) { + case 1: + foo(); + default: + bar(); + } +} +``` + +To fix this error, place a comment at the end of `case 1` indicating fallthrough + +```javascript +function test (c) { + switch (c) { + case 1: + foo(); + //fallthrough + default: + bar(); + } +} +``` From f8ed5facfe36f191a45b8c026caf37e1b4956d21 Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Sat, 28 Oct 2023 13:01:58 -0400 Subject: [PATCH 07/14] feat: use more explicit message for E0423 diag --- docs/errors/E0423.md | 2 +- src/quick-lint-js/diag/diagnostic-types-2.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/errors/E0423.md b/docs/errors/E0423.md index 367619122d..e842c64d11 100644 --- a/docs/errors/E0423.md +++ b/docs/errors/E0423.md @@ -1,4 +1,4 @@ -# E0423: missing fallthrough comment in switch case +# E0423: missing 'break;' or '// fallthrough' comment between statement and 'case' Switch Cases in javascript fallthrough to the next case if the `break` statement is not added at the end of the case. Since there is no explicit way of communication whether the fallthrough is intentional or not, it is recommended to use a comment indicating fallthrough. diff --git a/src/quick-lint-js/diag/diagnostic-types-2.h b/src/quick-lint-js/diag/diagnostic-types-2.h index 9acff27e5d..50c8c7e76c 100644 --- a/src/quick-lint-js/diag/diagnostic-types-2.h +++ b/src/quick-lint-js/diag/diagnostic-types-2.h @@ -759,9 +759,9 @@ struct Diag_Duplicated_Cases_In_Switch_Statement { Source_Code_Span duplicated_switch_case; }; -struct Diag_Explicit_Fallthrough_Comment_In_Switch { +struct Diag_Fallthrough_Without_Comment_In_Switch { [[qljs::diag("E0423", Diagnostic_Severity::warning)]] // - [[qljs::message("missing fallthrough comment", + [[qljs::message("missing 'break;' or '// fallthrough' comment between statement and 'case'", ARG(end_of_case))]] // Source_Code_Span end_of_case; }; From a190be555913de5e61019e05a231e3886d83f669 Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Tue, 31 Oct 2023 19:32:20 -0400 Subject: [PATCH 08/14] fix: change diagnostic name and match message with docs --- po/messages.pot | 2 +- .../diag/diagnostic-metadata-generated.cpp | 6 ++--- .../diag/diagnostic-metadata-generated.h | 2 +- src/quick-lint-js/fe/parse-statement.cpp | 4 ++-- .../i18n/translation-table-generated.cpp | 8 +++---- .../i18n/translation-table-generated.h | 4 ++-- .../i18n/translation-table-test-generated.h | 22 +++++++++---------- 7 files changed, 24 insertions(+), 24 deletions(-) diff --git a/po/messages.pot b/po/messages.pot index 8332ddba1b..d934f1ea30 100644 --- a/po/messages.pot +++ b/po/messages.pot @@ -558,7 +558,7 @@ msgid "this case will run instead" msgstr "" #: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp -msgid "missing fallthrough comment" +msgid "missing 'break;' or '// fallthrough' comment between statement and 'case'" msgstr "" #: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp index f14e3c6ffc..5e5d05775b 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp @@ -1423,16 +1423,16 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = { }, }, - // Diag_Explicit_Fallthrough_Comment_In_Switch + // Diag_Fallthrough_Without_Comment_In_Switch { .code = 423, .severity = Diagnostic_Severity::warning, .message_formats = { - QLJS_TRANSLATABLE("missing fallthrough comment"), + QLJS_TRANSLATABLE("missing 'break;' or '// fallthrough' comment between statement and 'case'"), }, .message_args = { { - Diagnostic_Message_Arg_Info(offsetof(Diag_Explicit_Fallthrough_Comment_In_Switch, end_of_case), Diagnostic_Arg_Type::source_code_span), + Diagnostic_Message_Arg_Info(offsetof(Diag_Fallthrough_Without_Comment_In_Switch, end_of_case), 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 7abc1f2934..9b767b1380 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.h +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.h @@ -103,7 +103,7 @@ namespace quick_lint_js { QLJS_DIAG_TYPE_NAME(Diag_Dot_Not_Allowed_After_Generic_Arguments_In_Type) \ QLJS_DIAG_TYPE_NAME(Diag_Dot_Dot_Is_Not_An_Operator) \ QLJS_DIAG_TYPE_NAME(Diag_Duplicated_Cases_In_Switch_Statement) \ - QLJS_DIAG_TYPE_NAME(Diag_Explicit_Fallthrough_Comment_In_Switch) \ + QLJS_DIAG_TYPE_NAME(Diag_Fallthrough_Without_Comment_In_Switch) \ QLJS_DIAG_TYPE_NAME(Diag_Else_Has_No_If) \ QLJS_DIAG_TYPE_NAME(Diag_Equals_Does_Not_Distribute_Over_Or) \ QLJS_DIAG_TYPE_NAME(Diag_Escaped_Character_Disallowed_In_Identifiers) \ diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp index 5e35e2fe3a..c46ab5d544 100644 --- a/src/quick-lint-js/fe/parse-statement.cpp +++ b/src/quick-lint-js/fe/parse-statement.cpp @@ -2807,7 +2807,7 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { prev_token.type != Token_Type::kw_break && prev_token.type != Token_Type::kw_case && !is_comment_line()) { this->diag_reporter_->report( - Diag_Explicit_Fallthrough_Comment_In_Switch{.end_of_case = + Diag_Fallthrough_Without_Comment_In_Switch{.end_of_case = prev_token.span()}); } prev_token = this->peek(); @@ -2846,7 +2846,7 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { prev_token.type != Token_Type::kw_break && prev_token.type != Token_Type::kw_case && !is_comment_line()) { this->diag_reporter_->report( - Diag_Explicit_Fallthrough_Comment_In_Switch{.end_of_case = + Diag_Fallthrough_Without_Comment_In_Switch{.end_of_case = prev_token.span()}); } is_before_first_switch_case = false; diff --git a/src/quick-lint-js/i18n/translation-table-generated.cpp b/src/quick-lint-js/i18n/translation-table-generated.cpp index b6a385bc7f..dfb0387d08 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.cpp +++ b/src/quick-lint-js/i18n/translation-table-generated.cpp @@ -333,7 +333,8 @@ const Translation_Table translation_data = { {35, 20, 49, 55, 39, 38}, // {34, 44, 0, 36, 0, 38}, // {58, 48, 46, 50, 28, 52}, // - {0, 40, 0, 28, 0, 27}, // + {0, 0, 0, 0, 0, 27}, // + {0, 40, 0, 28, 0, 74}, // {29, 22, 33, 28, 26, 26}, // {0, 0, 0, 55, 0, 51}, // {48, 27, 63, 27, 0, 24}, // @@ -363,8 +364,7 @@ const Translation_Table translation_data = { {0, 0, 0, 0, 0, 35}, // {36, 34, 39, 35, 40, 37}, // {0, 0, 0, 0, 0, 39}, // - {0, 0, 0, 0, 0, 58}, // - {33, 7, 40, 40, 33, 28}, // + {33, 7, 40, 40, 33, 58}, // {24, 11, 33, 22, 23, 24}, // {34, 63, 43, 41, 33, 32}, // {41, 11, 49, 43, 0, 39}, // @@ -2127,6 +2127,7 @@ const Translation_Table translation_data = { u8"missing ':' in conditional expression\0" u8"missing '<>' and '' to enclose multiple children\0" u8"missing '=' after variable\0" + u8"missing 'break;' or '// fallthrough' comment between statement and 'case'\0" u8"missing 'if' after 'else'\0" u8"missing 'while (condition)' for do-while statement\0" u8"missing TypeScript type\0" @@ -2157,7 +2158,6 @@ const Translation_Table translation_data = { u8"missing exported name in import type\0" u8"missing expression between parentheses\0" u8"missing expression in placeholder within template literal\0" - u8"missing fallthrough comment\0" u8"missing for loop header\0" u8"missing function parameter list\0" u8"missing header and body for 'for' loop\0" diff --git a/src/quick-lint-js/i18n/translation-table-generated.h b/src/quick-lint-js/i18n/translation-table-generated.h index 3b5481e519..75fe5bcc10 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.h +++ b/src/quick-lint-js/i18n/translation-table-generated.h @@ -19,7 +19,7 @@ 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 = 523; -constexpr std::size_t translation_table_string_table_size = 79969; +constexpr std::size_t translation_table_string_table_size = 80015; constexpr std::size_t translation_table_locale_table_size = 35; QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( @@ -348,6 +348,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( "missing ':' in conditional expression"sv, "missing '<>' and '' to enclose multiple children"sv, "missing '=' after variable"sv, + "missing 'break;' or '// fallthrough' comment between statement and 'case'"sv, "missing 'if' after 'else'"sv, "missing 'while (condition)' for do-while statement"sv, "missing TypeScript type"sv, @@ -378,7 +379,6 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( "missing exported name in import type"sv, "missing expression between parentheses"sv, "missing expression in placeholder within template literal"sv, - "missing fallthrough comment"sv, "missing for loop header"sv, "missing function parameter list"sv, "missing header and body for 'for' loop"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 d985354fdd..7c5820792b 100644 --- a/src/quick-lint-js/i18n/translation-table-test-generated.h +++ b/src/quick-lint-js/i18n/translation-table-test-generated.h @@ -3570,6 +3570,17 @@ inline const Translated_String test_translation_table[522] = { u8"saknar '=' efter variabel", }, }, + { + "missing 'break;' or '// fallthrough' comment between statement and 'case'"_translatable, + u8"missing 'break;' or '// fallthrough' comment between statement and 'case'", + { + u8"missing 'break;' or '// fallthrough' comment between statement and 'case'", + u8"missing 'break;' or '// fallthrough' comment between statement and 'case'", + u8"missing 'break;' or '// fallthrough' comment between statement and 'case'", + u8"missing 'break;' or '// fallthrough' comment between statement and 'case'", + u8"missing 'break;' or '// fallthrough' comment between statement and 'case'", + }, + }, { "missing 'if' after 'else'"_translatable, u8"missing 'if' after 'else'", @@ -3900,17 +3911,6 @@ inline const Translated_String test_translation_table[522] = { u8"missing expression in placeholder within template literal", }, }, - { - "missing fallthrough comment"_translatable, - u8"missing fallthrough comment", - { - u8"missing fallthrough comment", - u8"missing fallthrough comment", - u8"missing fallthrough comment", - u8"missing fallthrough comment", - u8"missing fallthrough comment", - }, - }, { "missing for loop header"_translatable, u8"missing for loop header", From d73c09a7e123d70bc95cdcb79d2f8ff15d3cfaa4 Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Tue, 31 Oct 2023 23:44:01 -0400 Subject: [PATCH 09/14] fix: change diag name in test --- test/test-parse-warning.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test-parse-warning.cpp b/test/test-parse-warning.cpp index 79f0ea4165..38a64bdb4b 100644 --- a/test/test-parse-warning.cpp +++ b/test/test-parse-warning.cpp @@ -436,14 +436,14 @@ TEST_F(Test_Parse_Warning, warn_on_xor_operation_used_as_exponentiation) { test_parse_and_visit_expression(u8"4 ^ 3"_sv, no_diags); test_parse_and_visit_expression(u8"(x+2)^a"_sv, no_diags); } -TEST_F(Test_Parse_Warning, Diag_Explicit_Fallthrough_Comment_In_Switch) { +TEST_F(Test_Parse_Warning, Diag_Fallthrough_Without_Comment_In_Switch) { test_parse_and_visit_statement( u8"switch(cond1){case 1:\nfoo()\ncase 2:\nbar() //fallthrough\ndefault:}"_sv, // - u8" ^^^ Diag_Explicit_Fallthrough_Comment_In_Switch"_diag); + u8" ^^^ Diag_Fallthrough_Without_Comment_In_Switch"_diag); test_parse_and_visit_statement( u8"switch(cond1){case 1:\nfoo()\ncase 2:\nlongBarFn()\ndefault:}"_sv, // - u8" ^^^^^^^^^ Diag_Explicit_Fallthrough_Comment_In_Switch"_diag, // - u8" ^^^ Diag_Explicit_Fallthrough_Comment_In_Switch"_diag); + u8" ^^^^^^^^^ Diag_Fallthrough_Without_Comment_In_Switch"_diag, // + u8" ^^^ Diag_Fallthrough_Without_Comment_In_Switch"_diag); // check for false positive test_parse_and_visit_statement( u8R"(switch(cond1){ From 2516ebc3cbd6bd1429b91c8a9ebb607e690c9c01 Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Wed, 1 Nov 2023 18:55:18 -0400 Subject: [PATCH 10/14] feat: check if token is valid end for switch case --- src/quick-lint-js/fe/lex.cpp | 7 +++- src/quick-lint-js/fe/parse-statement.cpp | 53 ++++++++++-------------- src/quick-lint-js/fe/token.h | 1 + 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/quick-lint-js/fe/lex.cpp b/src/quick-lint-js/fe/lex.cpp index 79f991c56d..e6fcaaf7e0 100644 --- a/src/quick-lint-js/fe/lex.cpp +++ b/src/quick-lint-js/fe/lex.cpp @@ -142,6 +142,7 @@ void Lexer::parse_bom_before_shebang() { [[gnu::noinline]] void Lexer::parse_current_token() { this->last_last_token_end_ = this->last_token_.end; this->last_token_.has_leading_newline = false; + this->last_token_.has_leading_comment = false; this->skip_whitespace(); while (!this->try_parse_current_token()) { @@ -906,6 +907,7 @@ Lexer::Parsed_Template_Body Lexer::parse_template_body( void Lexer::skip_in_jsx() { this->last_last_token_end_ = this->last_token_.end; this->last_token_.has_leading_newline = false; + this->last_token_.has_leading_comment = false; this->skip_whitespace(); retry: @@ -980,6 +982,7 @@ const Char8* Lexer::find_equal_greater_in_jsx_children() const { void Lexer::skip_less_less_as_less() { QLJS_ASSERT(this->peek().type == Token_Type::less_less); this->last_token_.has_leading_newline = false; + this->last_token_.has_leading_comment = false; this->last_token_.type = Token_Type::less; this->last_token_.begin += 1; this->last_last_token_end_ = this->last_token_.begin; @@ -1156,6 +1159,7 @@ void Lexer::insert_semicolon() { this->last_token_.type = Token_Type::semicolon; this->last_token_.has_leading_newline = false; + this->last_token_.has_leading_comment = false; this->last_token_.begin = this->input_; this->last_token_.end = this->input_; } @@ -1950,7 +1954,7 @@ QLJS_WARNING_POP void Lexer::skip_block_comment() { QLJS_SLOW_ASSERT(this->input_[0] == '/' && this->input_[1] == '*'); const Char8* c = this->input_ + 2; - + this->last_token_.has_leading_comment = true; #if QLJS_HAVE_X86_SSE2 using Bool_Vector = Bool_Vector_16_SSE2; using Char_Vector = Char_Vector_16_SSE2; @@ -2048,6 +2052,7 @@ void Lexer::skip_line_comment_body() { using Char_Vector = Char_Vector_1; #endif + this->last_token_.has_leading_comment = true; auto found_comment_end = [&]() { int n = newline_character_size(this->input_); diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp index c46ab5d544..b143fe3c44 100644 --- a/src/quick-lint-js/fe/parse-statement.cpp +++ b/src/quick-lint-js/fe/parse-statement.cpp @@ -2768,35 +2768,26 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { bool is_before_first_switch_case = true; Token prev_token; Hash_Set cases; + auto is_valid_end_of_case = [](Token_Type tk) { + switch (tk) { + case Token_Type::kw_return: + case Token_Type::kw_continue: + case Token_Type::kw_throw: + case Token_Type::kw_break: + case Token_Type::kw_case: + //Temporarily return false to omit diag with if statments + case Token_Type::kw_if: + case Token_Type::kw_try: + case Token_Type::kw_while: + case Token_Type::kw_for: + return true; + default: + return false; + } + }; while (keep_going) { - auto is_comment_line = [&]() -> bool { - const Char8 *th = this->lexer().end_of_previous_token(); - bool f = false; - int i = 0; - for (;;) { - switch (th[i]) { - case '/': - if (th[i + 1] == '/') { - f = true; - } - goto exit_loop; - // skip empty lines to reach comment - case ' ': - case '\n': - case '\r': - case '\t': - case '\f': - case '\v': - break; - default: - goto exit_loop; - } - i++; - } - exit_loop:; - return f; - }; switch (this->peek().type) { + case Token_Type::right_curly: this->skip(); keep_going = false; @@ -2804,8 +2795,8 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { case Token_Type::kw_case: { if (!is_before_first_switch_case && - prev_token.type != Token_Type::kw_break && - prev_token.type != Token_Type::kw_case && !is_comment_line()) { + !is_valid_end_of_case(prev_token.type) && + !this->peek().has_leading_comment) { this->diag_reporter_->report( Diag_Fallthrough_Without_Comment_In_Switch{.end_of_case = prev_token.span()}); @@ -2843,8 +2834,8 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { case Token_Type::kw_default: if (!is_before_first_switch_case && - prev_token.type != Token_Type::kw_break && - prev_token.type != Token_Type::kw_case && !is_comment_line()) { + !is_valid_end_of_case(prev_token.type) && + !this->peek().has_leading_comment) { this->diag_reporter_->report( Diag_Fallthrough_Without_Comment_In_Switch{.end_of_case = prev_token.span()}); diff --git a/src/quick-lint-js/fe/token.h b/src/quick-lint-js/fe/token.h index 7c1a61fe8a..d9eddb5319 100644 --- a/src/quick-lint-js/fe/token.h +++ b/src/quick-lint-js/fe/token.h @@ -318,6 +318,7 @@ struct Token { Token_Type type; bool has_leading_newline; + bool has_leading_comment; // Used only if this is a keyword token or an identifier token. // If the token contains no escape sequences, .normalized_identifier is From f07f0525113c29ec290f26dc2bb71c7aacc96921 Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Sat, 4 Nov 2023 11:29:21 -0400 Subject: [PATCH 11/14] fix: diag should display before case/default --- src/quick-lint-js/fe/parse-statement.cpp | 8 +++++--- test/test-parse-warning.cpp | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp index b143fe3c44..10ecdcaf16 100644 --- a/src/quick-lint-js/fe/parse-statement.cpp +++ b/src/quick-lint-js/fe/parse-statement.cpp @@ -2775,10 +2775,11 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { case Token_Type::kw_throw: case Token_Type::kw_break: case Token_Type::kw_case: - //Temporarily return false to omit diag with if statments + //Temporarily return true to omit diag with these statments case Token_Type::kw_if: case Token_Type::kw_try: case Token_Type::kw_while: + case Token_Type::kw_do: case Token_Type::kw_for: return true; default: @@ -2799,7 +2800,7 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { !this->peek().has_leading_comment) { this->diag_reporter_->report( Diag_Fallthrough_Without_Comment_In_Switch{.end_of_case = - prev_token.span()}); + Source_Code_Span::unit(this->peek().begin) }); } prev_token = this->peek(); is_before_first_switch_case = false; @@ -2836,9 +2837,10 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { if (!is_before_first_switch_case && !is_valid_end_of_case(prev_token.type) && !this->peek().has_leading_comment) { + this->diag_reporter_->report( Diag_Fallthrough_Without_Comment_In_Switch{.end_of_case = - prev_token.span()}); + Source_Code_Span::unit(this->peek().begin) }); } is_before_first_switch_case = false; this->skip(); diff --git a/test/test-parse-warning.cpp b/test/test-parse-warning.cpp index 38a64bdb4b..9dc3c258a4 100644 --- a/test/test-parse-warning.cpp +++ b/test/test-parse-warning.cpp @@ -439,11 +439,11 @@ TEST_F(Test_Parse_Warning, warn_on_xor_operation_used_as_exponentiation) { TEST_F(Test_Parse_Warning, Diag_Fallthrough_Without_Comment_In_Switch) { test_parse_and_visit_statement( u8"switch(cond1){case 1:\nfoo()\ncase 2:\nbar() //fallthrough\ndefault:}"_sv, // - u8" ^^^ Diag_Fallthrough_Without_Comment_In_Switch"_diag); + u8" ` Diag_Fallthrough_Without_Comment_In_Switch"_diag); test_parse_and_visit_statement( u8"switch(cond1){case 1:\nfoo()\ncase 2:\nlongBarFn()\ndefault:}"_sv, // - u8" ^^^^^^^^^ Diag_Fallthrough_Without_Comment_In_Switch"_diag, // - u8" ^^^ Diag_Fallthrough_Without_Comment_In_Switch"_diag); + u8" ` Diag_Fallthrough_Without_Comment_In_Switch"_diag, // + u8" ` Diag_Fallthrough_Without_Comment_In_Switch"_diag); // check for false positive test_parse_and_visit_statement( u8R"(switch(cond1){ From 1be8e485deb6aa13f76c9b63ac1d866a7291b960 Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Thu, 9 Nov 2023 23:37:03 -0500 Subject: [PATCH 12/14] fix: clang-format and use more correct variable name --- src/quick-lint-js/diag/diagnostic-types-2.h | 6 ++- src/quick-lint-js/fe/parse-statement.cpp | 50 ++++++++++----------- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/quick-lint-js/diag/diagnostic-types-2.h b/src/quick-lint-js/diag/diagnostic-types-2.h index d6306c70d6..2e05b00d01 100644 --- a/src/quick-lint-js/diag/diagnostic-types-2.h +++ b/src/quick-lint-js/diag/diagnostic-types-2.h @@ -761,8 +761,10 @@ struct Diag_Duplicated_Cases_In_Switch_Statement { struct Diag_Fallthrough_Without_Comment_In_Switch { [[qljs::diag("E0427", Diagnostic_Severity::warning)]] // - [[qljs::message("missing 'break;' or '// fallthrough' comment between statement and 'case'", - ARG(end_of_case))]] // + [ + [qljs::message("missing 'break;' or '// fallthrough' comment between " + "statement and 'case'", + ARG(end_of_case))]] // Source_Code_Span end_of_case; }; diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp index f1b6df00b7..f2170306ad 100644 --- a/src/quick-lint-js/fe/parse-statement.cpp +++ b/src/quick-lint-js/fe/parse-statement.cpp @@ -2815,29 +2815,28 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { bool keep_going = true; bool is_before_first_switch_case = true; - Token prev_token; + Token previous_statement_first_token; Hash_Set cases; auto is_valid_end_of_case = [](Token_Type tk) { switch (tk) { - case Token_Type::kw_return: - case Token_Type::kw_continue: - case Token_Type::kw_throw: - case Token_Type::kw_break: - case Token_Type::kw_case: - //Temporarily return true to omit diag with these statments - case Token_Type::kw_if: - case Token_Type::kw_try: - case Token_Type::kw_while: - case Token_Type::kw_do: - case Token_Type::kw_for: - return true; - default: - return false; + case Token_Type::kw_return: + case Token_Type::kw_continue: + case Token_Type::kw_throw: + case Token_Type::kw_break: + case Token_Type::kw_case: + // Temporarily return true to omit diag with these statments + case Token_Type::kw_if: + case Token_Type::kw_try: + case Token_Type::kw_while: + case Token_Type::kw_do: + case Token_Type::kw_for: + return true; + default: + return false; } }; while (keep_going) { switch (this->peek().type) { - case Token_Type::right_curly: this->skip(); keep_going = false; @@ -2845,13 +2844,12 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { case Token_Type::kw_case: { if (!is_before_first_switch_case && - !is_valid_end_of_case(prev_token.type) && + !is_valid_end_of_case(previous_statement_first_token.type) && !this->peek().has_leading_comment) { - this->diag_reporter_->report( - Diag_Fallthrough_Without_Comment_In_Switch{.end_of_case = - Source_Code_Span::unit(this->peek().begin) }); + this->diag_reporter_->report(Diag_Fallthrough_Without_Comment_In_Switch{ + .end_of_case = Source_Code_Span::unit(this->peek().begin)}); } - prev_token = this->peek(); + previous_statement_first_token = this->peek(); is_before_first_switch_case = false; Source_Code_Span case_token_span = this->peek().span(); this->skip(); @@ -2884,12 +2882,10 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { case Token_Type::kw_default: if (!is_before_first_switch_case && - !is_valid_end_of_case(prev_token.type) && + !is_valid_end_of_case(previous_statement_first_token.type) && !this->peek().has_leading_comment) { - - this->diag_reporter_->report( - Diag_Fallthrough_Without_Comment_In_Switch{.end_of_case = - Source_Code_Span::unit(this->peek().begin) }); + this->diag_reporter_->report(Diag_Fallthrough_Without_Comment_In_Switch{ + .end_of_case = Source_Code_Span::unit(this->peek().begin)}); } is_before_first_switch_case = false; this->skip(); @@ -2903,7 +2899,7 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { .unexpected_statement = this->peek().span(), }); } - prev_token = this->peek(); + previous_statement_first_token = this->peek(); bool parsed_statement = this->parse_and_visit_statement( v, Parse_Statement_Options{ .possibly_followed_by_another_statement = true, From 6cc35622088a163bf758beba8075fdfd0168c217 Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Sun, 19 Nov 2023 22:13:46 -0500 Subject: [PATCH 13/14] fix: remove false positive diag --- src/quick-lint-js/fe/lex.cpp | 4 ++-- src/quick-lint-js/fe/parse-statement.cpp | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/quick-lint-js/fe/lex.cpp b/src/quick-lint-js/fe/lex.cpp index 523f3e7d6b..d36779a7ab 100644 --- a/src/quick-lint-js/fe/lex.cpp +++ b/src/quick-lint-js/fe/lex.cpp @@ -1955,7 +1955,7 @@ QLJS_WARNING_POP void Lexer::skip_block_comment() { QLJS_SLOW_ASSERT(this->input_[0] == '/' && this->input_[1] == '*'); const Char8* c = this->input_ + 2; - this->last_token_.has_leading_comment = true; + this->last_token_.has_leading_comment = true; #if QLJS_HAVE_X86_SSE2 using Bool_Vector = Bool_Vector_16_SSE2; using Char_Vector = Char_Vector_16_SSE2; @@ -2053,7 +2053,7 @@ void Lexer::skip_line_comment_body() { using Char_Vector = Char_Vector_1; #endif - this->last_token_.has_leading_comment = true; + this->last_token_.has_leading_comment = true; auto found_comment_end = [&]() { int n = newline_character_size(this->input_); diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp index f2170306ad..5318429aa4 100644 --- a/src/quick-lint-js/fe/parse-statement.cpp +++ b/src/quick-lint-js/fe/parse-statement.cpp @@ -2815,7 +2815,7 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { bool keep_going = true; bool is_before_first_switch_case = true; - Token previous_statement_first_token; + std::optional previous_statement_first_token; Hash_Set cases; auto is_valid_end_of_case = [](Token_Type tk) { switch (tk) { @@ -2830,6 +2830,7 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { case Token_Type::kw_while: case Token_Type::kw_do: case Token_Type::kw_for: + case Token_Type::left_curly: return true; default: return false; @@ -2844,7 +2845,7 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { case Token_Type::kw_case: { if (!is_before_first_switch_case && - !is_valid_end_of_case(previous_statement_first_token.type) && + !is_valid_end_of_case(previous_statement_first_token->type) && !this->peek().has_leading_comment) { this->diag_reporter_->report(Diag_Fallthrough_Without_Comment_In_Switch{ .end_of_case = Source_Code_Span::unit(this->peek().begin)}); @@ -2882,7 +2883,7 @@ void Parser::parse_and_visit_switch(Parse_Visitor_Base &v) { case Token_Type::kw_default: if (!is_before_first_switch_case && - !is_valid_end_of_case(previous_statement_first_token.type) && + !is_valid_end_of_case(previous_statement_first_token->type) && !this->peek().has_leading_comment) { this->diag_reporter_->report(Diag_Fallthrough_Without_Comment_In_Switch{ .end_of_case = Source_Code_Span::unit(this->peek().begin)}); From 9f9607966813c6b89c0977ece0580bc6edc3131e Mon Sep 17 00:00:00 2001 From: Yash Masani Date: Sun, 19 Nov 2023 22:23:18 -0500 Subject: [PATCH 14/14] fix: use console in docs instead of undeclared functions --- docs/errors/E0427.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/errors/E0427.md b/docs/errors/E0427.md index 667758a5b3..e950152ef0 100644 --- a/docs/errors/E0427.md +++ b/docs/errors/E0427.md @@ -7,9 +7,9 @@ Since there is no explicit way of communication whether the fallthrough is inten function test (c) { switch (c) { case 1: - foo(); + console.log('case 1'); default: - bar(); + console.log('default'); } } ``` @@ -20,10 +20,10 @@ To fix this error, place a comment at the end of `case 1` indicating fallthrough function test (c) { switch (c) { case 1: - foo(); + console.log('case 1'); //fallthrough default: - bar(); + console.log('default'); } } ```