Closed Bug 420857 Opened 12 years ago Closed 3 years ago

Syntax error for missing brace should mention the line number of the opening brace

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: hubert.kauker, Assigned: arai)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12

When there is a JavaScript error such that a closing brace is missing,
error console only reports the location of the line number where the
closing brace is expected.
Usually this is the very last line of source code, which is quite useless.

Better: Report the line number where that improper block was STARTED, or both.

This would particularly be quite helpful in long code, where the end of file
is far away from the location where the actual error occurs.

Reproducible: Always

Steps to Reproduce:
function test1() {
        // This function is ok.
}

// This function is not ok.
function test2() {
    if(true) {
        // bla bla
        // 100 lines of some complex source code
                
    // forget to close the opening }
// Apparently close the function body
}

function test3() {
        // This function is ok.
}

Actual Results:  
Missing } after function body. Line: 18.

Expected Results:  
Improper use of { ... }. Lines: 6, 18.

or:

Opening { is not closed, missing } after function body. Lines: 6,18.
Assignee: nobody → general
Severity: normal → enhancement
Component: Error Console → JavaScript Engine
Product: Firefox → Core
QA Contact: error.console → general
Summary: Error console should report unbalanced braces better → Syntax error for missing brace should mention the line number of the opening brace
Assignee: general → nobody
Blocks: jserror
Depends on: 1283712
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Hardware: x86 → All
just like bug 104442, added a note for the place the "{" is opened, for missing "}" error.
Attachment #8843549 - Flags: review?(andrebargull)
added the note also for block statement.

this time the check is done by MUST_MATCH_TOKEN_MOD.

added MUST_MATCH_TOKEN_MOD_WITH_REPORT, that receives expression that reports error if the token doesn't match.
Attachment #8843550 - Flags: review?(andrebargull)
also for object literal.
Attachment #8843552 - Flags: review?(andrebargull)
Comment on attachment 8843549 [details] [diff] [review]
Part 1: Report the position of opening brace for missing brace error in function body.

Review of attachment 8843549 [details] [diff] [review]:
-----------------------------------------------------------------

Cool!

::: js/src/frontend/Parser.cpp
@@ +1014,5 @@
> +    SprintfLiteral(columnNumber, "%" PRIu32, column);
> +    char lineNumber[MaxWidth];
> +    SprintfLiteral(lineNumber, "%" PRIu32, line);
> +
> +    if (!notes->addNoteASCII(pc->sc()->context,

Parser<ParseHandler>::reportRedeclaration(...) uses addNoteLatin1() for the same kind of parameters, shouldn't they match (so either both addNoteLatin1 or both addNoteASCII)?

@@ +3660,5 @@
>      }
>  
>      if (bodyType == StatementListBody) {
>          bool matched;
>          if (!tokenStream.matchToken(&matched, TOK_RC, TokenStream::Operand))

Pre-existing: When at EOF, matchToken() will call ungetToken() and push back the last token before EOF. This results in a different error location for missing "}" in functions when compared to blocks. But I'm not sure this is worth fixing.
Attachment #8843549 - Flags: review?(andrebargull) → review+
Attachment #8843550 - Flags: review?(andrebargull) → review+
Attachment #8843552 - Flags: review?(andrebargull) → review+
Attachment #8843553 - Flags: review?(andrebargull) → review+
Thank you for reviewing :)

(In reply to André Bargull from comment #6)
> ::: js/src/frontend/Parser.cpp
> @@ +1014,5 @@
> > +    SprintfLiteral(columnNumber, "%" PRIu32, column);
> > +    char lineNumber[MaxWidth];
> > +    SprintfLiteral(lineNumber, "%" PRIu32, line);
> > +
> > +    if (!notes->addNoteASCII(pc->sc()->context,
> 
> Parser<ParseHandler>::reportRedeclaration(...) uses addNoteLatin1() for the
> same kind of parameters, shouldn't they match (so either both addNoteLatin1
> or both addNoteASCII)?

thanks, the redeclaration's one should use ASCII.
I was using Latin1 because it once contained variable name that can be latin1,
and now it only contains numbers, that should fit into ASCII.


> @@ +3660,5 @@
> >      }
> >  
> >      if (bodyType == StatementListBody) {
> >          bool matched;
> >          if (!tokenStream.matchToken(&matched, TOK_RC, TokenStream::Operand))
> 
> Pre-existing: When at EOF, matchToken() will call ungetToken() and push back
> the last token before EOF. This results in a different error location for
> missing "}" in functions when compared to blocks. But I'm not sure this is
> worth fixing.

yeah, it looks like we just need to use MUST_MATCH_TOKEN_MOD_WITH_REPORT here too.
https://hg.mozilla.org/integration/mozilla-inbound/rev/ecd217c511e1ea49d924f6eb8d255b15781ec940
Bug 420857 - Part 1: Report the position of opening brace for missing brace error in function body. r=anba

https://hg.mozilla.org/integration/mozilla-inbound/rev/fb042254ca987be5af5c81fcd99ad5ca087b41b7
Bug 420857 - Part 2: Report the position of opening brace for missing brace error in block. r=anba

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b077fcd66f03dc2370b3eb1ea8a282ad1b1a278
Bug 420857 - Part 3: Report the position of opening brace for missing brace error in object literal. r=anba

https://hg.mozilla.org/integration/mozilla-inbound/rev/81fd3c22387cac585f2242e28edc1dac9e131d0f
Bug 420857 - Part 4: Report the position of opening bracket for missing bracket error in array literal. r=anba
You need to log in before you can comment on or make changes to this bug.