Closed
Bug 1310045
Opened 8 years ago
Closed 8 years ago
incorrect error when accidentally using the destructuring operator
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: michiel, Unassigned, Mentored)
References
Details
Attachments
(1 file)
900 bytes,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
I was trying to use the destructuring operator but did not notice that the following ES7 syntax is not supported yet:
({a, b, ...rest} = {a:1, b:2, c:3, d:4});
However, the JS error this throws seems to be based on "what the engine is doing" rather than being based on the code the user wrote:
SyntaxError: invalid property id
There is no "id" in the code I wrote, so the expected error is more something like "SyntaxError: unexpected token '...'" or a similarly indicative notice that lets the user figure out that Firefox is tripping up over the "..." syntax.
Comment 1•8 years ago
|
||
The error message here could perhaps be better -- but also, if we have the right developer UI, it shouldn't *need* to be, because it should include location information from SpiderMonkey. This location information is certainly present, and it's definitely calculated correctly, because our JS shell displays that information quite correctly:
js> ({a, b, ...rest} = {a:1, b:2, c:3, d:4});
typein:1:9 SyntaxError: invalid property id:
typein:1:9 ({a, b, ...rest} = {a:1, b:2, c:3, d:4});
typein:1:9 .........^
What developer UI are you using? It may be worth filing a bug against something else to more clearly expose that location, in addition to crafting a better error message here.
As to this error: it looks like this is trivially fixed by modifying the |default| case in Parser.cpp's |Parser<ParseHandler>::propertyName|. The proper change is likely something along the lines of reporting an error using JSMSG_UNEXPECTED_TOKEN, with the exact code looking something similar to how the other uses of that error number look.
Mentor: jwalden+bmo
This is just in the plain Firefox dev tools "console" tab.
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64
Version: unspecified → 49 Branch
Comment 3•8 years ago
|
||
unfortunately, devtools console doesn't show location information for console input :/
at least it should show line/column
Attachment #8807935 -
Flags: review?(jwalden+bmo)
Attachment #8807935 -
Attachment is patch: true
Comment 7•8 years ago
|
||
Comment on attachment 8807935 [details] [diff] [review]
patch.txt
Review of attachment 8807935 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good. Do you need someone to land this for you?
Attachment #8807935 -
Flags: review?(jwalden+bmo) → review+
Comment 8•8 years ago
|
||
(IRC conversation says the answer is yes. I've rebased the patch on tip and queued it up for landing, the next time I'm landing stuff.)
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b7949845f17
When a non-property-name token is observed instead of a property name in an object/class literal or destructuring pattern, indicate the unexpected token the way most unexpected tokens are indicated using JSMSG_UNEXPECTED_TOKEN. r=jwalden
Comment 10•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•