Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bound methods with non-identifier names cause a crash #858

Closed
alangpierce opened this issue Feb 20, 2017 · 0 comments
Closed

Bound methods with non-identifier names cause a crash #858

alangpierce opened this issue Feb 20, 2017 · 0 comments
Assignees
Labels

Comments

@alangpierce
Copy link
Member

@alangpierce alangpierce commented Feb 20, 2017

decaffeinate is crashing on my CoffeeScript input:

class A
  "#{b}": => c

(repl)

I get this error:

AddVariableDeclarationsStage failed to parse: Unexpected token (3:9)
  1 | class A {
  2 |   constructor() {
> 3 |     this."#{b}" = this."#{b}".bind(this);
    |          ^
  4 |   }
  5 | 
@alangpierce alangpierce added the crash label Feb 20, 2017
@alangpierce alangpierce self-assigned this Feb 20, 2017
alangpierce added a commit to alangpierce/add-variable-declarations that referenced this issue Feb 21, 2017
Progress toward decaffeinate/decaffeinate#858

The babylon AST has ObjectMethod and ClassMethod keys that are within the method
node, which means that we were viewing any assignment within that key as being
in the method block, so the variable would get declared in the wrong place. To
work around this, we can temporarily set the scope as one higher while we
traverse the key part of the method.
alangpierce added a commit to alangpierce/decaffeinate that referenced this issue Feb 25, 2017
Fixes decaffeinate#858

The basic idea is that we make sure all bound method keys are repeatable and
that method bindings are patched after the bound method keys are, then use that
information if necessary to generate a method binding line using square bracket
notation.

There were some nuances here:
* We now need to change the patching order so constructors are patched last, so
  that we have the patched values for method names to bind. This means that we
  need to explicitly do so in BlockPatcher, and in the case where we introduce
  the constructor, we want to introduce it at the top of the class but after the
  class contents are patched. Since normally patching needs to be done in order,
  I added a way to use the `prependLeft` method from magic-string for special
  cases where we know that patching needs to be done out of order.
* Since there's a special case that a `"#{a}"` key turns into `[a]`, we need to
  properly handle the case where the string expression is repeatable. This
  requires extending the repeatable protocol a little.
alangpierce added a commit that referenced this issue Feb 26, 2017
Fixes #858

The basic idea is that we make sure all bound method keys are repeatable and
that method bindings are patched after the bound method keys are, then use that
information if necessary to generate a method binding line using square bracket
notation.

There were some nuances here:
* We now need to change the patching order so constructors are patched last, so
  that we have the patched values for method names to bind. This means that we
  need to explicitly do so in BlockPatcher, and in the case where we introduce
  the constructor, we want to introduce it at the top of the class but after the
  class contents are patched. Since normally patching needs to be done in order,
  I added a way to use the `prependLeft` method from magic-string for special
  cases where we know that patching needs to be done out of order.
* Since there's a special case that a `"#{a}"` key turns into `[a]`, we need to
  properly handle the case where the string expression is repeatable. This
  requires extending the repeatable protocol a little.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.