Skip to content

Commit

Permalink
Warn for possible unexpected behavior when else followed by a semicolon.
Browse files Browse the repository at this point in the history
  • Loading branch information
someoneigna committed Jun 9, 2021
1 parent 1a92c01 commit 79d5928
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 0 deletions.
30 changes: 30 additions & 0 deletions docs/errors/E204.md
Original file line number Diff line number Diff line change
@@ -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");
13 changes: 13 additions & 0 deletions src/quick-lint-js/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -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'"), \
Expand Down Expand Up @@ -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; }, \
Expand Down
13 changes: 13 additions & 0 deletions src/quick-lint-js/parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()});
}
}
}
}

Expand Down
38 changes: 38 additions & 0 deletions test/test-parse-statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 79d5928

Please sign in to comment.