1268806 - The SyntaxTreeVisitor in Parser.jsm fails to process computed names for object properties

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Jarda Snajdr [:jsnajdr]]

Steps to reproduce:
1. Try to debug a script like this:

let o = {
  [1 + 2]: 3
};

2. Do something that involves running the SyntaxTreeVisitor. Either mouseover a variable name to trigger variable bubble view, or search in function names (@something).

Actual result:
SyntaxTreeVisitor throws an error because it doesn't have a handler for ComputedName type:
  TypeError: this[key.type] is not a function

Comment 1

[Jarda Snajdr [:jsnajdr]]

Attachment: The SyntaxTreeVisitor in Parser.jsm fails to process computed names for object properties

- added a handler for ComputedName AST type (that's the main part of the patch)
- converted methods to ES6 form, to fix ESLint errors about missing spaces between "function" and "()"
- added a mochitest

Comment 2

[Jarda Snajdr [:jsnajdr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=479c39bb9700

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8746972 [details] [diff] [review]
The SyntaxTreeVisitor in Parser.jsm fails to process computed names for object properties

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

In the future *please* split out separate changes (like `foo: function () { ... }` => `foo() { ... }`) into separate patches. It makes reviewing patches a lot easier, since it makes it clear whether a given hunk is related to the stated change in functionality or not. In this case, there are more unrelated changes than related changes. This might not seem like a big deal, but for a reviewer who only touched this file maybe one time a couple years ago, it makes a big difference. Thanks!

Comment 4

[Jarda Snajdr [:jsnajdr]]

(In reply to Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] from comment #3)
> In the future *please* split out separate changes into separate patches.

Thanks for the review Nick and sorry for making your job harder. I usually split patches into multiple ones, but this time I thought it's simple enough to be merged into one. I'll always split them in the future.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/d8fa41fcfda5

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d8fa41fcfda5