fix(fe): don't report missing else for bad if expression

[arieldon]

This one line change (potentially) closes #1008. However, test Test_Parse.utter_garbage fails -- despite my edits. I don't understand the reason it fails either. To me, it appears like the conditions the test asserts hold true.

The issue provides this snippet of code.

function DataNotLoadedError() {}
const e = new DataNotLoadedError();

if (e isntanceof DataNotLoadedError) {
} else {
  throw e;
}

For convenience, the CLI version of quick-lint-js behaves like so before this change for the snippet of code provided in the issue.

$ ~/code/quick-lint-js/build/quick-lint-js x.js 
x.js:4:6: error: if statement is missing ')' around condition [E0018]
x.js:4:17: error: missing semicolon after statement [E0027]
x.js:4:36: error: unmatched parenthesis [E0056]
x.js:7:1: error: 'else' has no corresponding 'if' [E0065]
x.js:4:7: warning: use of undeclared variable: isntanceof [E0057]

With the change, it behaves like this.

$ ~/code/quick-lint-js-fork/build/quick-lint-js x.js 
x.js:4:7: error: unexpected identifier in expression; missing operator before [E0147]
x.js:4:18: error: unexpected identifier in expression; missing operator before [E0147]
x.js:4:7: warning: use of undeclared variable: isntanceof [E0057]

For test Test_Parse.utter_garbage that it currently fails, the program produces this output.

Value of: diagnostics
Expected: has 3 elements and there exists some permutation of elements such that:
 - element #0 has type Diag_Unexpected_Token, and
 - element #1 has type Diag_Unexpected_Identifier_In_Expression, and
 - element #2 has type Diag_Expected_Parentheses_Around_If_Condition
  Actual: { Diag_Unexpected_Token, Diag_Unexpected_Identifier_In_Expression, Diag_Expected_Parentheses_Around_If_Condition }, where the following matchers don't match any elements:
matcher #1: has type Diag_Unexpected_Identifier_In_Expression,
matcher #2: has type Diag_Expected_Parentheses_Around_If_Condition
and where the following elements don't match any matchers:
element #1: Diag_Unexpected_Identifier_In_Expression,
element #2: Diag_Expected_Parentheses_Around_If_Condition

But it appears like all the expected diagnoses appear in the actual results, no? A element corresponds to each matcher.

[arieldon]

I fixed my original mistake in test utter_garbage -- I didn't account for the position and length of spans at first. I also added a test that explicitly confirms E0065 is not included in the set of diagnoses given a bad if condition.

[strager]

Looks great! Let me know if you want to make changes or if I should merge it now.

[strager]

This test case is inside Test_Parse_Statement.else_without_if, but this test case is an else with and if. I think you should make a separate TEST_F. Perhaps Test_Parse_Statement.if_else_with_malformed_condition? (Naming is hard.)

[arieldon]

Thanks for catching that. I moved the new test into a separate TEST_F with the name you suggest.

[strager]

Not your patch: I probably should have documented why I introduced this test in the first place. 😅 It's probably just testing a crash, so changing the diagnostics sounds fine to me.

[arieldon]

It's related to issue #88 based on the Git log, and it is testing a crash.

[arieldon]

Looks great! Let me know if you want to make changes or if I should merge it now.

If you're pleased with the most recent changes then you can merge.

[strager]

Landed as Git commit 9565209.