This repository has been archived by the owner. It is now read-only.

Better error message for using super when not using an object method #212

Closed
hzoo opened this Issue Nov 1, 2016 · 7 comments

Comments

Projects
None yet
6 participants
@hzoo
Member

hzoo commented Nov 1, 2016

Object.create({}, {
  foo: {
    get: function(){
      return super.foo;
    }
  }
})

errors with

repl: 'super' outside of function or class (4:13)
  2 |   foo: {
  3 |     get: function(){
> 4 |       return super.foo;
    |              ^
  5 |     }
  6 |   }
  7 | })

since it needs to be a get() {}

We should make a better error message for this?

cc @jasonkarns @Kovensky

@Kovensky

This comment has been minimized.

Show comment
Hide comment
@Kovensky

Kovensky Nov 1, 2016

Member

Would be better to say "method or class constructor", as only those have a [[HomeObject]] (and thus access to super).

Right now it is inside a function, it's just a "free-standing" function expression that happens to be assigned as the value of an object's key.

Worse, if it did try to just use the object where the expression appears, its [[HomeObject]] would be the property descriptor itself 😄

Member

Kovensky commented Nov 1, 2016

Would be better to say "method or class constructor", as only those have a [[HomeObject]] (and thus access to super).

Right now it is inside a function, it's just a "free-standing" function expression that happens to be assigned as the value of an object's key.

Worse, if it did try to just use the object where the expression appears, its [[HomeObject]] would be the property descriptor itself 😄

@hzoo

This comment has been minimized.

Show comment
Hide comment
@hzoo

hzoo Nov 1, 2016

Member

Yeah "method or class constructor" sounds good to me?

case tt._super:
if (!this.state.inMethod && !this.options.allowSuperOutsideMethod) {
this.raise(this.state.start, "'super' outside of function or class");
}
node = this.startNode();
this.next();
if (!this.match(tt.parenL) && !this.match(tt.bracketL) && !this.match(tt.dot)) {
this.unexpected();
}
if (this.match(tt.parenL) && this.state.inMethod !== "constructor" && !this.options.allowSuperOutsideMethod) {
this.raise(node.start, "super() outside of class constructor");
}
return this.finishNode(node, "Super");

Also interesting is allowSuperOutsideMethod - we should update the readme since it says TODO https://github.com/babel/babylon/blob/master/README.md#options - not sure why you would want that

Member

hzoo commented Nov 1, 2016

Yeah "method or class constructor" sounds good to me?

case tt._super:
if (!this.state.inMethod && !this.options.allowSuperOutsideMethod) {
this.raise(this.state.start, "'super' outside of function or class");
}
node = this.startNode();
this.next();
if (!this.match(tt.parenL) && !this.match(tt.bracketL) && !this.match(tt.dot)) {
this.unexpected();
}
if (this.match(tt.parenL) && this.state.inMethod !== "constructor" && !this.options.allowSuperOutsideMethod) {
this.raise(node.start, "super() outside of class constructor");
}
return this.finishNode(node, "Super");

Also interesting is allowSuperOutsideMethod - we should update the readme since it says TODO https://github.com/babel/babylon/blob/master/README.md#options - not sure why you would want that

@Kovensky

This comment has been minimized.

Show comment
Hide comment
@Kovensky

Kovensky Nov 1, 2016

Member

allowSuperOutsideMethod would probably only make sense if super was dynamic and worked like Object.getPrototypeOf(Object.getPrototypeOf(this)).

Member

Kovensky commented Nov 1, 2016

allowSuperOutsideMethod would probably only make sense if super was dynamic and worked like Object.getPrototypeOf(Object.getPrototypeOf(this)).

@jasonkarns

This comment has been minimized.

Show comment
Hide comment
@jasonkarns

jasonkarns Nov 1, 2016

No suggestions on the error message, but I'll say that from my perspective, I'm not sure that method or class constructor would have been any more illuminating than the current message. It's absolutely precise, but also not all that helpful to the majority of users. (if they're like me)

It would probably take more work (since the error message would need to be contextual) but if going for the most helpful message, one could print exactly what needs changed to make it valid. (At least for function expressions used within property descriptors.)

jasonkarns commented Nov 1, 2016

No suggestions on the error message, but I'll say that from my perspective, I'm not sure that method or class constructor would have been any more illuminating than the current message. It's absolutely precise, but also not all that helpful to the majority of users. (if they're like me)

It would probably take more work (since the error message would need to be contextual) but if going for the most helpful message, one could print exactly what needs changed to make it valid. (At least for function expressions used within property descriptors.)

@rajasekarm

This comment has been minimized.

Show comment
Hide comment
@rajasekarm

rajasekarm Oct 27, 2017

I can work on this issue. Seems to be good for first contribution.

rajasekarm commented Oct 27, 2017

I can work on this issue. Seems to be good for first contribution.

@nicolo-ribaudo

This comment has been minimized.

Show comment
Hide comment
@nicolo-ribaudo
Member

nicolo-ribaudo commented Oct 27, 2017

@rajzshkr Go ahead!

@babel-bot

This comment has been minimized.

Show comment
Hide comment
@babel-bot

babel-bot Nov 1, 2017

This issue has been moved to babel/babel#6716.

babel-bot commented Nov 1, 2017

This issue has been moved to babel/babel#6716.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.