Closed
Bug 1339895
Opened 7 years ago
Closed 7 years ago
Javascript statement var x += 2 yields misleading error
Categories
(Core :: JavaScript Engine, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: captainhypertext, Assigned: sigmaths.scientia, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: triage-deferred)
Attachments
(1 file, 3 obsolete files)
7.20 KB,
patch
|
arai
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0 Build ID: 20170125094131 Steps to reproduce: Run the javascript statement var x += 2; Actual results: Misleading, possibly unhandled error: SyntaxError: missing ; before statement Expected results: Google Chrome returns: Uncaught SyntaxError: Unexpected token +=
Reporter | ||
Updated•7 years ago
|
Severity: normal → minor
Updated•7 years ago
|
Keywords: triage-deferred
Priority: -- → P3
Assignee | ||
Comment 1•7 years ago
|
||
I,m working on this.
Comment 2•7 years ago
|
||
Thank you!
Assignee: nobody → sigmaths.scientia
Mentor: arai.unmht
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 3•7 years ago
|
||
I changed the error message "missing ; before statement" to "expected ';', got (unexpected token)"
Attachment #8924416 -
Flags: review?(arai.unmht)
Comment 4•7 years ago
|
||
Comment on attachment 8924416 [details] [diff] [review] Improvement of error message for ASI Review of attachment 8924416 [details] [diff] [review]: ----------------------------------------------------------------- Thank you for your patch! The change looks good. Then, this patch will conflict with bug 1319416 patch, that is now in mozilla-inbound, and will be merged to mozilla-central within a day. So, you'll need to rebase your patch onto it later. (the conflict is only about surrounding lines, so the rebase won't be so hard) fscholz, can I have your opinion about the new error message? (the previous one is already documented in the following page. so the change will also be required on the document side) https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Missing_semicolon_before_statement ::: js/src/frontend/Parser.cpp @@ +2922,4 @@ > * tried to insert semicolon here > * > * Detect this situation and throw an understandable error. Otherwise > + * we'd throw a confusing "expected \';\', got TokenkindToDesk(tt)" error. please change this line to "..., got (unexpected token)" @@ +2932,4 @@ > error(JSMSG_YIELD_OUTSIDE_GENERATOR); > return false; > } > + please remove whitespaces in this line.
Attachment #8924416 -
Flags: review?(arai.unmht) → feedback?(fscholz)
Assignee | ||
Comment 5•7 years ago
|
||
I changed "expected \';\', got TokenkindToDesk(tt)" to "expected \';\', got (unexpected token)", and removed white space in the previous patch.
Attachment #8924428 -
Flags: review?(arai.unmht)
Updated•7 years ago
|
Attachment #8924416 -
Attachment is obsolete: true
Attachment #8924416 -
Flags: feedback?(fscholz)
Comment 6•7 years ago
|
||
Comment on attachment 8924428 [details] [diff] [review] Improvement of error message for ASI Review of attachment 8924428 [details] [diff] [review]: ----------------------------------------------------------------- Thanks again. Looks good. Here's try run (automation for build and test), that will take 2-3 hours. https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca4ac9d70d82c1062574e7bb5db59ba89059db91 carrying forward f? for fscholz (see comment #4)
Attachment #8924428 -
Flags: review?(arai.unmht)
Attachment #8924428 -
Flags: review+
Attachment #8924428 -
Flags: feedback?(fscholz)
Comment 7•7 years ago
|
||
now bug 1319416 get's merged to mozilla-central. please rebase your patch onto it.
Comment 8•7 years ago
|
||
Comment on attachment 8924428 [details] [diff] [review] Improvement of error message for ASI As briefly discussed in IRC, for |var x += 2;| I would not add a hint that a semicolon is missing, because that's not actually the problem in this situation. |SyntaxError: Expected ';', got identifier| is imo not much better than now, as it focuses on the semicolon still, which isn't really the problem. |SyntaxError: Unexpected token +=| is at least clearly identifying the problem. But I don't know in which other cases this error is thrown, so my comment might just apply to this one case.
Attachment #8924428 -
Flags: feedback?(fscholz)
Assignee | ||
Comment 9•7 years ago
|
||
I changed the error message to "unexpected token: (unexpected token)" and rebased
Attachment #8925436 -
Flags: review?(arai.unmht)
Updated•7 years ago
|
Attachment #8924428 -
Attachment is obsolete: true
Comment 10•7 years ago
|
||
Comment on attachment 8925436 [details] [diff] [review] Improve error message for unexpected token Review of attachment 8925436 [details] [diff] [review]: ----------------------------------------------------------------- Thanks again! We'll need to update the following documentation later (after this patch gets merged), that is linked from errordocs.js: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Missing_semicolon_before_statement here's try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e3c9bad997ad4c01961f0e7f495f0da8ac5d054
Attachment #8925436 -
Flags: review?(arai.unmht) → review+
Assignee | ||
Comment 11•7 years ago
|
||
Correction of patch
Attachment #8925436 -
Attachment is obsolete: true
Attachment #8925786 -
Flags: review?(arai.unmht)
Comment 12•7 years ago
|
||
Comment on attachment 8925786 [details] [diff] [review] Improve error message for unexpected token Review of attachment 8925786 [details] [diff] [review]: ----------------------------------------------------------------- thanks again! here's try run for tests that failed before: https://treeherder.mozilla.org/#/jobs?repo=try&revision=47176e63c40d6823f5b03782f27b46646e930149
Attachment #8925786 -
Flags: review?(arai.unmht) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 13•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/53ce0e7d27e7 Improve error message for unexpected token. r=arai
Keywords: checkin-needed
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/53ce0e7d27e7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•