Anyone familiar with the codebase available to review this? @vendethiel? @xixixao?

This one's only two lines plus a test.

This won't catch code like class extends A then constructor: (@a) -> super((=> @a)()), but I'm not sure if that's supposedly valid. (or if super(=> @a) in general is valid).

It looks like super(=> a) is valid, though class extends A then constructor: (@a) -> super((=> @a)()) shouldn’t be valid for the same reason that class extends A then constructor: (@a) -> super(@a) shouldn’t be. Yikes this is tricky . . . I guess to do it right we need to traverse down the tree of everything passed to super as arguments, and check them all for this references. Maybe it’s easier to just do the shenanigans that @connec was thinking about, although those will surely have their own difficult cases.

It looks like super(=> a) is valid

I meant specifically using => @a. It doesn't error at compile time, but I can't seem to get the try it online to evaluate code.

I meant specifically using => @a. It doesn’t error at compile time, but I can’t seem to get the try it online to evaluate code.

That’s the issue: we’re generating valid JavaScript, that just happens not to correspond to the user’s intent. See:

class extends parent
  constructor: (@arg) ->
    super(=> @arg)

(class extends parent {
  constructor(arg) {
    super(() => {
      return this.arg;
    });
    this.arg = arg;
  }
});

Babel allows this, but it’s not the user’s intent because this.arg isn’t defined when it’s used inside the super argument.

Okay @vendethiel, I’ve found a way to search the tree of nodes that make up super‘s arguments, to find any references to this within them. This covers the case you pointed out above.

This goes a bit too deep:

coffee -bcs
class extends A then constructor: (@a) -> super(class then constructor: (@b) -> @b)

[stdin]:1:74: error: Can't call super with @params in derived class constructors
class extends A then constructor: (@a) -> super(class then constructor: (@b) -> @b)

I tried using Base::contains, which stops traversing at scope boundaries, but it failed your example. I ran into a similar issue with function parameters, in that our checks don't work too well when someone puts gobs of code in there. Ultimately I think we couldn't find a solution that was perfectly precise, that went deep enough but not too deep, and so we opted to err on the side of too deep. The thinking was that blocks of code in function parameters is an edge case, easily refactored, and it's better to throw an unnecessary error than potentially miss throwing one.

I think the same could be said for this case. Would you agree?

Base::contains (or rather Base::traverseChildren) doesn't differentiate "scope crossing" between whether it is this-aware. This might be doable, this might not be, this might not be worth it. It's just a decision we take, I'm fine with it whichever way we decide.

Thanks. I guess let's leave this open another day to see if anyone else has an opinion. Any other notes?