Consistent behaviour for (constructor) method returning

[metakeule]

Please have a look at the following inconsistency. I see that it is a variant of https://github.com/jashkenas/coffee-script/issues/17 and that the way to go is to return code>@</code for constructing methods (or using class). But nothing prevents people coming from JavaScript to write a working case (like b) and then later someone changes the last line and things go wild in a way that is hard to debug.

So please consider to prevent the access to .prototype of a method that is not returning code>@</code. Prevent the production of time bombs.

a = () -> {}
a.prototype.alias = 'a not there'

b = () -> ""
b.prototype.alias = 'b is there'

c = () -> []
c.prototype.alias = 'c not there'

d = () -> 5
d.prototype.alias = 'd is there'

e = () -> new Date()
e.prototype.alias = 'e not there'

console.log(new a().alias) # => undefined
console.log(new b().alias) # => b is there
console.log(new c().alias) # => undefined
console.log(new d().alias) # => d is there
console.log(new e().alias) # => undefined

[michaelficarra]

Using the class construct is the workaround you're requesting. The function assigned to the special constructor property will have its implicit return value ignored, returning the new instance instead.

[metakeule]

As I said, I am aware of class and it is an acceptable solution. However, it should not be possible to run into something like this:

plop = (arr) -> 
  @_saved = arr.pop()
  arr.pop()
plop.prototype.saved =  -> @_saved
basket1 = new plop ["Bananas": 12, "Apples", "Oranges"]
console.log basket1.saved() # everything works fine

but someday....

basket2 = new plop ["Apples", "Oranges": 13, "Bananas"] # still works...
console.log basket2.saved() # => boom, WTF!!

A constructed example, but I think you'll get the point.

[michaelficarra]

That would require a runtime test, something we try to avoid. What's your suggested compilation?

[metakeule]

I would suggest to remove access to .prototype if the last value depends on an variable outside the scope of the method and (being defined inside the function) if it is no object. Then the examples from above would all raise immediate errors. Maybe it is even possible to prevent access to .prototype for all functions that don't have code>@</code as return value / aren't constructed via class.

I am not sure if there are side effects or use cases that would be affected. It should be discussed with a broader audience.

Simply setting .prototype to undefined and then setting it back (if last value is code>@</code / class is used) might work.

[michaelficarra]

remove access to prototype

Please define this.

[paulmillr]

Simply setting .prototype to undefined

http://es5.github.com/#x15.2.3.1

This property has the attributes {[[Writable]]: false, [[Enumerable]]: false, [[Configurable]]: false }.

It cannot be changed via standard.

[metakeule]

I just tried it in node.js and it could be overwritten. However, since coffeescript is a compiler and has an AST, it should be possible to throw compiler errors. I dunno how practical it is. Unfortunately, I don't have the time to digg through the codebase and prepare a pull request. It was just an idea that seamed practical at the first sight.

You guys will have better solutions. I just wanted to make you aware of the problem. If the answer is: Yes, but not fixable - so it be then....

But I think it should be possible to avoid certain patterns at compilation time (in the same file at least, but then we have closures that could be used, too...).

A quick test suggests that undefined has no prototype. Maybe it is possible to use that "feature".....

[metakeule]

that would at least be consistent and a little improvement:

plop = (arr) -> 
  @_saved = arr.pop()
  arr.pop()
Object.freeze(plop.prototype) # or Object.seal(plop.prototype) or Object.preventExtensions(plop.prototype)
plop.prototype.saved = -> @_saved
basket1 = new plop ["Bananas": 12, "Apples", "Oranges"]
console.log basket1.saved() # Cannot call method 'saved' of undefined

Although it would be better if it raised here:

plop.prototype.saved = -> @_saved

[michaelficarra]

@metakeule: I was hoping you would realise this yourself, in trying to explain how we prevent accesses to the prototype property:

 obj[Math.round(Math.random()) ? 'prototype' : 'a'].b = c

Non-determinism exists.

[metakeule]

I can't see any Non-determinism. I proposed four possible solutions.

plop.prototype = undefined  # ok, contradicts the standards
Object.freeze(plop.prototype) 
Object.seal(plop.prototype)
Object.preventExtensions(plop.prototype)

[michaelficarra]

@metakeule: And what input causes the compiled output to contain those statements?

[metakeule]

If we are lucky and there are no unwanted side effects: Any function that hasn't code>@</code as last (returning) value.

[metakeule]

If there are unwanted side effects, we could at least check if the last value had an explicit return and could output those statements only if there was no explicit return and no code>@</code as returning value. That could be an acceptable hack.

[michaelficarra]

So, if I understand correctly now, you want every function to have a frozen prototype upon definition? So, compile a = -> to

var a, _ref;
a = (_ref = function(){}, Object.freeze(_ref.prototype), _ref);

Is there even a cross-platform (ES3) way to freeze the prototype?

[metakeule]

Here is an overview on browser support:

http://kangax.github.com/es5-compat-table/

It would not be available on older browsers (feature detection). But this is only a proposal of a possible implementation. Think about it. There might be better solutions. I just think the case should be solved.

I have no idea why someone would need to modify the prototype of a function in other cases than initializing a new object, but there might be use cases.

It could also be a performance problem... I am not sure if it is relevant. But with the explicit return the developer could at least circumvent the freeze if he knows what he does.

[metakeule]

in ES5 strict mode it even raises early (node.js):

Can't add property b, object is not extensible

[metakeule]

Maybe obj.prototype = null; could be used for ES < 5 where Object.freeze() is not available.

[paulmillr]

meh, the problem is bullshit.

• ten(+++)-fold performance impact on all functions.
• not possible to adequately check writable status. Well, it's possible via Object.GOPD, but that's another 100-fold performance impact.

etc

Never saw coffee code that uses the behavior.

[michaelficarra]

@paulmillr: I have to agree. I was just trying to be diplomatic.

[metakeule]

@paulmillr , @michaelficarra

your class construct sets the prototype as well!!

coffeescript 1.2.0

class A

class B extends A

compiles to

# snip 
   ctor.prototype = parent.prototype; child.prototype = new ctor; child.__super__ = parent.prototype;

so I can't see why this should be more compliant to ES than

obj.prototype = null;

which should also have no big impact on performance. (expecially if it could be avoided by explicit returns)

[michaelficarra]

@metakeule: You can't define the prototype to be null:

$ node
> ctor = function(){}
[Function]
> Object.getPrototypeOf(ctor)
[Function: Empty]
> ctor.prototype = null
null
> Object.getPrototypeOf(ctor)
[Function: Empty]
> 

[paulmillr]

And classes are used not as often as functions. Consider an impact on readability after the change.

[ghost]

@michaelficarra I think you meant...

$ node
> ctor = function(){}
[Function]
> Object.getPrototypeOf(new ctor)
{}
> ctor.prototype = null
null
> Object.getPrototypeOf(new ctor)
{}
>

[metakeule]

@michaelficarra I can't understand what you are trying to tell me with the example.

p = -> "p"
p.prototype = null
console.log p() # => "p", function still works

p.prototype.a = -> "a" # Cannot set property 'a' of null

exactly what we want. Also the specs for ES3 states that null is the terminating value in the prototype chain.

[paulmillr]

Well, guys. I really can't understand your attitude. First of all I reported a bug. Would you please confirm that this is undesired behaviour? Not?

yep, it's undesired

I saw this code and there was at least another bug report https://github.com/jashkenas/coffee-script/issues/17 that had the same problem. And it resulted in a major feature namely class which did not fix the bug, but gave a workaround.

class syntax wasn't created especially for the problem. It was created because it's readable etc.

You did not confirm that this problem should be solved, you wanted a solution. If I had a solution and the time to implement it, I would have made a pull request.

which we would have rejected

But I invested some time and gave you some idea to get started. I have never said that this is the best solution, read my posts.

There is no great solution for the problem. period.

But instead of trying to solve the problem your argue just to tell me that this is no real problem? If you think so, please say so. I showed you that there are ways around. But that were only initial thoughts.

well, yes, it's not a real problem.

Readability is a real problem compared to this. I don't want to debug js that will have (_ref = function() {}, _ref.prototype = undefined) etc.

I find your treatment highly disrespectful. You pretend to diplomatic but you have a highly arrogant attitude. Why do you treat my proposals as absurd if I told you that they are just initial thoughts? We don't you investigate and try to find a solution?

Because we know that there are no great solutions.

[metakeule]

@paulmillr It is obvious that functions are more often used as classes.

Therefor the suggestion to allow the explicit return to prevent that. The real problem is for sure the implicit return in coffeescript.

The easiest (and best IMHO) solution would be to simply drop it. Are there any other reasons than syntactic sugar and compatibility breakage for the feature to persist?

After years of ruby coding I find myself always explicit returning at least to show if the return was intended or a side effect.

Anyway I don't thing coffeescript should introduce subtile bugs that javascript doesn't have (like this one)

[metakeule]

@paulmillr You reject a pull request before it's even made. Whow!

[michaelficarra]

@kitcambridge: Yep, that's what I meant. Sorry for any confusion. I am tired. It is late here on the east coast.

@paulmillr:

which we would have rejected

This is a true statement, but it'd be more helpful if you were less succinct and explained how this could be the only outcome, instead of just saying that it would be rejected, allowing @metakeule to infer that it would be without reason.

edit:

@metakeule:

It is obvious that functions are more often used as classes.

Come on, you know that's not true.

[paulmillr]

ಠ_ಠ

[metakeule]

@michaelficarra, @kitcambridge

Ok, I see now, what you meant. I guess it is because the prototype of null is defined at the end of the prototype chain and so the prototype is considered to be an object. But for our case is has no meaning, since it behaves as we want: the function works like before but nothing could be assigned to the prototype.

What makes the solution even better that Object.freeze() since a new prototype could be assigned later if needed.

Ok, if you guys think there are no other possible solutions than the proposed it burns down to the question, if the readability issue does matter that much and if it would be ok to have the developers explicitly state the return in order to get readability and full performance.

If that are no options for you, the question is, if a compatibility breaking dropping of the autoreturn feature could be an option in the future. I would love to hear any reasons for this feature besides eye-candyness and compatibility.

[michaelficarra]

dropping of the autoreturn feature could be an option in the future

I really don't think so. That feature is a major benefit of using coffeescript. We have -> so we don't have to labour over the horrible 8-character function keyword all the time. We have no instead of false because it reads better. Implicit returns just fit naturally with coffeescript's philosophies.

I would love to hear any reasons for this feature besides eye-candyness and compatibility.

It makes the code easy to read, and especially easy to write. That's an important aspect of a language that's just a syntactic veil over JS. CS allows developers to describe the behaviour of their program in an efficient, natural way. If we wanted to force them to use the return keyword in every function they write, they might as well just use JS and re-invent the list comprehension every time they need to use one.

[metakeule]

Well if you are concerned about the length of the word "return" you could easily use another sign, (for example <-). There are a lot's of benefits from using coffeescript, I can't see how adding "some kind of return sign" affects the usefulness of the other shortcuts and conveniences. It is also a visual aid and a sign of intent to mark what you want to have returned.

Still, any other reason than comfort? Any technical reason?

Syntactically nice could be to put the autoreturn into the closing bracket: =} for autoreturn and } for no autoreturn. minmal invasive.

[michaelficarra]

@metakeule: In (by far) the most common case, the last expression in a function body is also returned. By making this behaviour the default, we save keystrokes in most cases. Sometimes, there are multiple returns, so the return keyword is still useful. Users can also simulate the absence of this feature by making undefined the last expression in their function. But, really, the feature is desirable in almost all cases.

[metakeule]

ok, then it should at least be highlighted in the documentation, that classical prototyped instantiation is not supported and the use of class is recommended.

[michaelficarra]

@metakeule: It is supported, and it works just as it does in JS. You just have to understand how functions are specified in CS. The desugared semantics are exactly the same.

[metakeule]

this bug proves that it does not work like in javascript. this problem does not exist in javascript.. and this behaviour is not documented anywhere. and @paulmillr said it is undesired behaviour. also if coffeescript is just "just a syntactic veil over JS" than the syntactic differences matter. exspecially if they lead to subtile different behaviours.

frankly, I have the impression, that you don't want to handle this bug at all. you neither want to fix it, nor to document it. That is even worse than how the php community handles bugs. at least do they document them....

[michaelficarra]

@metakeule: How does it not exist in JS? Of course it exists in JS.

$ node
> ctor = function(){ ({}); }
[Function]
> new ctor instanceof ctor
true
> ctor = function(){ return {}; }
[Function]
> new ctor instanceof ctor
false
> 

We just make it easier for you to accidentally write function(){ return {}; } when you meant to write function(){ ({}); }. That is the bug, and there's no fix for that, since we really want to keep implicit returns.

I have the impression, that you don't want to handle this bug at all. you neither want to fix it, not to document it.

These false statements of yours are starting to get on my nerves. You have no reason to believe that. We've been telling you this the whole time: it's not really a "bug", and we can't see anything we could do to make it better. I've left the issue open this long, in case you were going to put forward a possible solution, but I no longer see this issue being productive beyond the unacceptable option mentioned above. Closing.

[metakeule]

Just for the record:

• you did not tell that you consider this not being a bug, you do this now for the first time. what is a bug other than an undesired behaviour (see above: yep, it's undesired)
• for almost every bug there is a workaround. so you might always say "you could have written it this way"
• you did nothing towards a solution, you always gave the impression that a solution is not possible but nevertheless asked for one (confusing at least)
• I thought coffeescript was supposed to make javascript programmer's life easier not more complicated by introducing "undesired behaviour", but apparently saving keystrokes is more important than expected behaviour (BTW coffeescript heals a lot of unexpected behaviour of javascript eg. by replacing == with ===, why not this time?)
• if you think, this behaviour is not so revelant, that's fairly ok, but than you should have said it directly and not waste my time
• sometimes the view from inside a project is different than from the outside. A programmer spending hours to find the cause of that weird behaviour might choose not to look further into coffeescript. This Bug might pop up at the end of a very long call chain and one has possibly wade through large parts of an application only to find out that it happened at a completely other end.
• while it is still javascript that exposes the bug, in javascript the bug would not happen since nobody would write a return when constructing an object

[metakeule]

@michaelficarra @paulmillr

I am no native speaker (as you might have noticed) In the sentence

It is obvious that functions are more often used as classes.

I used the wrong word. I meant to write

It is obvious that functions are more often used *than* classes.

sorry for the confusion. the german word "als" has both meanings. That confused me...

[paulmillr]

Just to be clear: it's an undesired behavior if you won't take into account coffeescript semantics. As @michaelficarra said, it works just as in js. CoffeeScript is not JS, of course it has differences.

There are things you can't resolve adequately without breaking coffee semantics. Like this one.

[csubagio]

Well that was a vigorous discussion. @metakeule, their collective point is that you should not be surprised that writing bad coffeescript is as bad as writing bad javascript. To wit:

• You would not write a return statement in a JS constructor function because you know that's a mistake. You should not be writing JS if you do not know this.
• You omit returns from Coffeescript functions because you know that Coffeescript doesn't need one to return the result of the last function; therefore you'd know that you need to control the return value when writing a constructor function. You haven't learned enough Coffeescript if you do not know this.
• Usage of Coffeescript without learning about about those two facts can result in the edge case you describe.
• Usage of either language without enough knowledge can result in any number of broken cases.
• Defense against this particular broken case is too costly to implement versus the chances of it happening.

You're also misunderstanding the Coffeescript remit to some degree: it states that it is about making javascript prettier. That is considered a virtue here, so saying that anything is 'merely' syntactic sugar isn't going to win you any arguments here :)

I for one enjoy the auto return, and am confident that I'd never write a constructor in the broken manner. In fact, I'd be very likely to rewrite any code I saw in a Coffeescript codebase that I was participating in, that attempted to write a constructor directly, into a class declaration instead. That's the real defense.

[michaelficarra]

@csubagio: Very well put. Thank you for that summary.

[metakeule]

@csubagio I would say that it is a matter of making things easy or not. CoffeeScript makes it easy to write solid javascript (that runs through jslint) without having to know all js quirks around. The code in question wasn't even mine and I clarified that I fully understood the problem at the very first paragraph. There is no need to assume weak understanding of javascript or coffeescript. There is simply no sign of it. The question was, if coffeescript could or should prevent such a trap or not. It is easy to overlook this combination of js and coffeescript effects.

By the way: Even the js2coffee converter does not recognize the issue and has "limit understanding of javascript" in this point. It does not recognize a constructor function and therefor can't handle it differently.

Maybe one should open an issue for the js2coffee project. But then again, it might be considered too much overhead for the parser.

PS Don't fall into the trap of groupthink while keeping to confirm each other.

[metakeule]

You might continue to discuss the issue here https://github.com/rstacruz/js2coffee/issues/128