diff --git a/docs/errors/E204.md b/docs/errors/E204.md new file mode 100644 index 0000000000..0444bfbc1f --- /dev/null +++ b/docs/errors/E204.md @@ -0,0 +1,30 @@ +# E204: semicolon after else may be causing unexpected behavior. + +A semicolon next to the `else` keyword may cause its body to be executed even when previous `if` statement condition is true. + + if (true) { + console.log("true"); + } else; { + console.log("else"); + } + +Example output +``` +> true +> else +``` + +To avoid this behavior, remove the semicolon between the else keyword and the opening brace of the else block. + + if (true) { + console.log("true"); + } else { + console.log("else"); + } + +Or if the else body is not required remove it completely. + + if (true) { + console.log("true"); + } + console.log("always"); \ No newline at end of file diff --git a/src/quick-lint-js/error.h b/src/quick-lint-js/error.h index f8fe1db56a..c3d3e4e860 100644 --- a/src/quick-lint-js/error.h +++ b/src/quick-lint-js/error.h @@ -209,6 +209,12 @@ QLJS_TRANSLATABLE("commas are not allowed after spread parameter"), \ comma)) \ \ + QLJS_ERROR_TYPE( \ + error_else_has_empty_body, "E205", { source_code_span where; }, \ + .warning(QLJS_TRANSLATABLE("else has empty body; consider removing the " \ + "redundant 'else'"), \ + where)) \ + \ QLJS_ERROR_TYPE( \ error_else_has_no_if, "E065", { source_code_span else_token; }, \ .error(QLJS_TRANSLATABLE("'else' has no corresponding 'if'"), \ @@ -1048,6 +1054,13 @@ QLJS_TRANSLATABLE("for-of loop expression cannot have semicolons"), \ semicolon)) \ \ + QLJS_ERROR_TYPE( \ + error_unexpected_semicolon_after_else, "E204", \ + { 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; }, \ diff --git a/src/quick-lint-js/parse.h b/src/quick-lint-js/parse.h index 3d8e093bd3..f573a2df0d 100644 --- a/src/quick-lint-js/parse.h +++ b/src/quick-lint-js/parse.h @@ -2516,7 +2516,20 @@ class parser { if (this->peek().type == token_type::kw_else) { this->skip(); + + const token token_after_else = this->peek(); parse_and_visit_body(); + + if (token_after_else.type == token_type::semicolon && + this->peek().type == token_type::left_curly) { + if (this->peek().has_leading_newline) { + this->error_reporter_->report( + error_else_has_empty_body{.where = this->peek().span()}); + } else { + this->error_reporter_->report(error_unexpected_semicolon_after_else{ + .semicolon = token_after_else.span()}); + } + } } } diff --git a/test/test-parse-statement.cpp b/test/test-parse-statement.cpp index ec07a762d4..2b60cb2ac6 100644 --- a/test/test-parse-statement.cpp +++ b/test/test-parse-statement.cpp @@ -578,6 +578,44 @@ TEST(test_parse, else_without_if) { } } +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")); // (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";")))); + } +} + +TEST(test_parse, else_has_empty_body) { + { + spy_visitor v; + padded_string code(u8"if (cond) { body; } else;\n{ }"_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")); // (if) + EXPECT_THAT( + v.errors, + ElementsAre(ERROR_TYPE_FIELD( + error_else_has_empty_body, where, + offsets_matcher(&code, strlen(u8"if (cond) { body; } else {"), + u8"}")))); + } +} + TEST(test_parse, block_statement) { { spy_visitor v = parse_and_visit_statement(u8"{ }"_sv);