diff --git a/docs/errors/E220.md b/docs/errors/E220.md new file mode 100644 index 0000000000..32869cf9d9 --- /dev/null +++ b/docs/errors/E220.md @@ -0,0 +1,13 @@ +# E220: Semicolon after else may cause unexpected execution of its body. + +A semicolon next to the `else` keyword can cause its body to be executed even when previous `if` statement condition is true. + + if (true) { + console.log("true"); + } else; { + console.log("& false"); + } + > true + > & false + +To avoid this behavior, remove the semicolon between the else keyword and the opening brace of the else block. \ No newline at end of file diff --git a/src/quick-lint-js/error.h b/src/quick-lint-js/error.h index 1a65355e2c..07a9e81e6e 100644 --- a/src/quick-lint-js/error.h +++ b/src/quick-lint-js/error.h @@ -1028,6 +1028,13 @@ QLJS_TRANSLATABLE("for-of loop expression cannot have semicolons"), \ semicolon)) \ \ + QLJS_ERROR_TYPE( \ + error_unexpected_semicolon_after_else, "E202", \ + { source_code_span semicolon; }, \ + .warning(QLJS_TRANSLATABLE("semicolon after else may be causing " \ + "unexpected behavior"), \ + semicolon)) \ + \ QLJS_ERROR_TYPE( \ error_no_digits_in_binary_number, "E049", \ { source_code_span characters; }, \ @@ -1139,12 +1146,6 @@ { source_code_span continue_statement; }, \ .error(QLJS_TRANSLATABLE("continue can only be used inside of a loop"), \ continue_statement)) \ - \ - QLJS_ERROR_TYPE( \ - error_redundant_semicolon_after_else, "E202", \ - { source_code_span semicolon; }, \ - .warning(QLJS_TRANSLATABLE("redundant semicolon after else"), \ - semicolon)) \ /* END */ namespace quick_lint_js { diff --git a/src/quick-lint-js/parse.h b/src/quick-lint-js/parse.h index 595e0a9846..a91a78b55e 100644 --- a/src/quick-lint-js/parse.h +++ b/src/quick-lint-js/parse.h @@ -2516,14 +2516,18 @@ class parser { if (this->peek().type == token_type::kw_else) { this->skip(); - if (this->peek().type == token_type::semicolon) { - source_code_span semicolon = this->peek().span(); - this->error_reporter_->report(error_redundant_semicolon_after_else{ - .semicolon = source_code_span( - semicolon.begin(), semicolon.end()), + + bool semicolon_after_else = this->peek().type == token_type::semicolon; + const char8 *token_after_else_begin = this->peek().begin; + + parse_and_visit_body(); + if (semicolon_after_else && + this->peek().type == token_type::left_curly && + !this->peek().has_leading_newline) { + this->error_reporter_->report(error_unexpected_semicolon_after_else{ + .semicolon = source_code_span(token_after_else_begin, token_after_else_begin+1) }); } - parse_and_visit_body(); } } diff --git a/test/test-parse-statement.cpp b/test/test-parse-statement.cpp index 5db2d927a3..2d40ec5ffd 100644 --- a/test/test-parse-statement.cpp +++ b/test/test-parse-statement.cpp @@ -578,19 +578,20 @@ TEST(test_parse, else_without_if) { } } -TEST(test_parse, else_redundant_semicolon) { +TEST(test_parse, else_unexpected_semicolon) { { spy_visitor v; padded_string code(u8"if (cond) { body; } else; { body; }"_sv); parser p(&code, &v); EXPECT_TRUE(p.parse_and_visit_statement(v)); - EXPECT_THAT(v.visits, ElementsAre("visit_variable_use", // cond - "visit_enter_block_scope", // (if) - "visit_variable_use", // body - "visit_exit_block_scope")); // (else) - EXPECT_THAT(v.errors, ElementsAre(ERROR_TYPE_FIELD( - error_redundant_semicolon_after_else, semicolon, - offsets_matcher(&code, strlen(u8"if (cond) { body; } else"), u8";")))); + EXPECT_THAT(v.visits, ElementsAre("visit_variable_use", // cond + "visit_enter_block_scope", // (if) + "visit_variable_use", // body + "visit_exit_block_scope")); // (if) + EXPECT_THAT(v.errors, + ElementsAre(ERROR_TYPE_FIELD( + error_unexpected_semicolon_after_else, semicolon, + offsets_matcher(&code, strlen(u8"if (cond) { body; } else"), u8";")))); } }