super compiles differently depending on how class method is defined

[TrevorBurnham]

super compiles differently in these two snippets:

class X
  @classMethod: ->
    super

and

class X
  @classMethod = ->
    super

The way it compiles in the second snippet seems like a bug to me. See http://stackoverflow.com/a/19865440/66226.

[michaelficarra]

Please please please can we kill super now? It's not at all intuitive what it should be doing here.

[vendethiel]

At least disable it in static methods :(.

[connec]

This seems like a compiler bug, in that I would expect the latter form to fail the same way as the following:

-> super
[stdin]:1:4: error: cannot call super outside of an instance method.
-> super
   ^^^^^

@variable = -> ... is not an instance method (neither, of course, is @variable: -> ..., but it does at least use property syntax in a class body, which implies some kind of CoffeeScript magic...).

[TrevorBurnham]

I think it makes sense that you can write

class Parent
  @classMethod: ->
    # ...

class Child extends Parent
  @classMethod: ->
    super

and calling Child.classMethod will invoke Parent.classMethod. It's just inconsistent that you can't do this with @classMethod = ->, which is otherwise equivalent.

[epidemian]

For the record, a similar bug appears when using normal functions inside the class body:

class X
  fun = ->
    super

var X;

X = (function() {
  var fun;

  function X() {}

  fun = function() {
    return X.__super__.fun.apply(this, arguments);
  };

  return X;

})();

And referencing super in the class body triggers a weird compilation error:

$ cat super.rb 
class X
  super
$ coffee super.rb 
super.rb:2:3: error: Class bodies shouldn't reference arguments
  super
  ^^^^^

[xixixao]

Wow, until today I thought the only way to create static methods was this way:

@staticMethod = ->

Then I found this #279 with

@staticMethod: ->

How did we get into this state where both are possible? That will be I guess impossible to remove from the language, in which case the behavior should match. super makes sense here as much as it makes sense in instance methods (unless OOP == Java in your mindset).

[michaelficarra]

@xixixao: Yes, super only makes sense in classical OOP (Java/C++/etc.). In prototypal OOP, we have prototype chains and nothing more. I have never once used super, and it's not because I'm trying to avoid it; it's just not idiomatic.

[xixixao]

@michaelficarra

1. How do classes and super classes fit prototypical OOP? The syntax CoffeeScript provides enables us to use a classical paradigm inside a language that is not primarily based on it. This is fine, the same way you can write C in nonmutable functional style and Haskell in imperative style with monads - and on and on.
2. What I hinted at was that some other OO languages, not Java, have proper inheritance chain between class objects themselves, and anyone who has coded a lot of Java surely encountered a situation where they wished Java did as well (of top of my head, Objective C).

[erisdev]

@xixixao CoffeeScript doesn't add class-based OOP, it just offers syntactic sugar that makes JavaScript's standard prototypal inheritance look like classes—and, regrettably, adds that super business that has caused so much trouble and confusion. Probably should've used a keyword other than class to make that clear since it seems to confuse the hell outta people.

I don't see the problem with writing Foo::method.call(self, argle, bargle) if you really need to call a method from further up the inheritance chain. That makes it explicit what's being called.

[xixixao]

@erisdiscord I didn't say that CoffeeScript adds class-based OOP and every language in the end is just a syntactic sugar over

00000101 00100110
...

I assume Jeremy didn't change his mind over the last three months, so super is probably here to stay. If you don't like classes, I don't see the problem with writing

object = {data: 4}
method = (n) -> this.data + n
alert method.call object, 3

Makes it more explicit what's being called.

[erisdev]

@xixixao alright, mr. smartypants. I think you know what I meant. CoffeeScript expands on JavaScript's semantics, but it doesn't somehow automagically change the native prototypal inheritance into classical inheritance.

And that's a cute example, but it's not really analogous, now, is it? Besides, there are actually valid cases where you'd write code (kind of) like that. 😒 Calling a known ancestor's implementation of a method you've consciously shadowed with your own implementation ain't quite the same situation as calling methods on arbitrary objects, though.

[xixixao]

@erisdiscord And again, I never said it did.

It is quite analogous.

a = new A
a.method()

What function am I calling here? Method declared in class A, or its super class, or somewhere else where someone overwrote A.prototype.method? The point is, what all this "sugar" gives you is an easier, idiomatic way of expressing programs, in certain paradigm, in this case classical OOP. And back to the topic, if we have super for instance methods and nothing is stopping us from having it work for static methods as well, it should work across the syntax variations (unfortunate) that are part of the language. It can be useful. Like a.method(). If we want to support only prototypical OOP then we should throw out super and static methods and instance methods and classes altogether.

[jashkenas]

If we want to support only prototypical OOP then we should throw out super and static methods and instance methods and classes altogether.

Baloney, my friend. Pure baloney ;)

Prototypes and classes are both ways of accomplishing inheritance with objects. If you have inheritance, then you have super — it's an implied concept. If you have inheritance, then you have instance methods (duh). If you have inheritance, and an "abstract" thing that you inherit from, then you have a class — no matter what you decide to call it.

[xixixao]

That was sarcasm.

If you have inheritance, then you have super — it's an implied concept.

I totally agree.

[jashkenas]

Glad to hear it ;) Ah, GitHub tickets, where nuance is lost, body language is invisible, and smirks go unseen...

[hpaulj]

In case it helps the discussion, this block of code illustrates the various possible combinations of variable assignment (@,=,:) and super

class XXX
   bar4 : ->
     alert 'XXX : bar4'
   bar5 : ->
     alert 'XXX : bar5'
     return 'bar5'
   bar6 : ->
     alert 'XXX : bar6'
     return 'bar6'
  @bar7 : ->
    alert 'XXX @bar7'

class Foo extends XXX
  #bar1 = alert 'test1' # run by Foo()
  bar2 : 'test2'  # bar attribute of Foo.prototype
  @bar3 = 'test3'  # @bar : 'test'
  bar4 = ()->  #
    alert 'Foo bar4'
    super  # runs XXX:bar4
  bar4() # run fn internal to Foo
  bar5 : ()->
    alert 'Foo bar5'
    super  # runs XXX:bar5
  @bar6 = ()->
    alert 'Foo = @Bar6'
    super # runs XXX:bar6
  @bar7 : ()->  # adds constructor
    alert 'Foo : @bar7'
    super # runs XXX@bar7
  class Bar extends Foo
    constructor : ->
      super

alert new Foo().bar2
alert Foo.bar3
new Foo().bar5()
Foo.bar6()
Foo.bar7()

In node.coffee superReference, a constructor is included if method.static.
In addProperties

if assign.variable.this
   method.static = yes

I haven't traced this earlier, but I'm guessing that @bar7 : sets this, but @bar6 = does not.

[hpaulj]

#1598 (comment)

looks like the same issue, revisiting 'Support super in static/class methods?' from several years ago.

[marchaefner]

@connec is quite right. The source code seems to intend exactly this behaviour, but METHOD_DEF does not do what you might think when you just glance over it. Edit: Way of the mark, we are actually missing a bit of logic in Class. And it looks like it's simpler to make the 2nd case work (instead of throwing an error).

---

@epidemian

referencing super in the class body triggers a weird compilation error

This is caused by the somewhat clunky node construction in the grammar and the unavoidable order of validity checks (The search for illegal arguments happens while constructing the class body and illegal super is detected when compiling the super call.)

Compare:

$ bin/coffee -bce 'class then super arg'
[stdin]:1:12: error: cannot call super outside of an instance method.
class then super arg
           ^^^^^^^^^

A dedicated Super node would prevent this and also make for much nicer code. (A PR for that should appear in the not too distant future...)

[Fresheyeball]

Please lets not disable super in static methods. The intuitive behavior for me is the one that compiles from @foo : -> super(). Does anyone know the original intent? http://stackoverflow.com/questions/19850865/coffeescript-static-inheritance-of-child-classes

[hpaulj]

#627 Static functions and the '=>' parameter may be relevant. The method used there to identify 'static' functions appears to work just as well for @static= as for @static:, where as the one currently used with 'super' only works with the : version.

[Fresheyeball]

I just learned another interesting thing :

class Foo 
  @bar = =>
    @thing = 'wack'

this results in a _this = this; that is never used in the compiled javascript. Might be a related issue.

[hpaulj]

The var _this = this; declaration is generated by

  else if not @static
    o.scope.parent.assign '_this', 'this'

So it looks it is sensitive to the same method.static setting.

In the development version the approach to binding for the 'fat arrow' has been changed substantially
#3143

There is no longer a difference between @: and @=. This code using @static is no longer needed.

[jashkenas]

@marchaefner I was going to start tackling this, but your "dedicated super node" sounds like a lovely approach. Should I look at other things while awaiting your PR, or take a stab at it?

[marchaefner]

@jashkenas: I already got a quick'n'dirty fix for this issue and #2949. Just need to work on a proper description since this has some side effects. (I think only positive ones.)

The Super node itself would just fix the confusing error message. It also allows for a nicer fix for #2367, though that should also be fixable by a well placed ? -- haven't tested this one thoroughly. (If you feel like writing pretty code have at it.)

[hpaulj]

Looks like marchaefner has come up with a solution. But just in case it might be useful for further discussions, here 's a fix that works for me. I implemented it in the 1.6.3 release, but it should work in the development master

In nodes.coffee class Class I add a testStatic method that is a stripped down version of addProperties, and call that in walkBody. It is designed to add value.static to a @foo= node. addProperties is never called on that type of node.

11/13 - I rewrote testStatic so it handles both the : and = forms.

  # test whether node is a static method
  # extracted from addProperties
  # handles both @foo: and @foo=
  testStatic: (node) ->
    if node instanceof Assign
      if node.variable.base.value isnt 'constructor'
        if node.variable.this
          func = node.value
          func.static = yes
          # dont set bound context here
    else if node instanceof Value and node.isObject(true)
      # handle '@foo:' case
      props = node.base.properties[..]
      while assign = props.shift()
        @testStatic(assign)

  # Walk the body of the class, looking for prototype properties to be converted.
  walkBody: (name, o) ->
    @traverseChildren false, (child) =>
      cont = true
      return false if child instanceof Class
      if child instanceof Block
        for node, i in exps = child.expressions
          @testStatic node # ADDED TEST FOR  '@foo='
          if node instanceof Value and node.isObject(true)
            cont = false
            exps[i] = @addProperties node, name, o
        child.expressions = exps = flatten exps
      cont and child not instanceof Class