Compound assignment operators (and ++/--) do not create scope

[TrevorBurnham]

> coffee -bpe "incrX = -> x++"
var incrX;
incrX = function() {
  return x++;
};

Surely there ought to be a var x in there somewhere? Likewise with x += 1.

[StanAngeloff]

Why would you need a var for a non-assignment operator?

a += b
c--
d or= 1

Doing any of the above on a non-existing variable should produce an exception.

[michaelficarra]

I agree with StanAngeloff. Only assignments should declare a variable. And I hope you mean a runtime ReferenceError, not a compile-time error.

[TrevorBurnham]

I see what you're saying, but it feels weird that x = x + 1 creates scope while x++ and x += 1 don't. This has practical (though in practice rare) consequences because, as mentioned in #1121, order matters:

incrX = -> x++
x = 0
incrX()
console.log x # 1

incrX = -> x = x + 1
x = 0
incrX()
console.log x # 0

Surely we can all agree that it's undesirable for there to be different output when one replaces x = x + 1 with x++?

Perhaps consistency could be better found by preventing expressions of the form x = expr(x), where expr(x) is any expression that includes x, from creating var x declarations.

[satyr]

$ coffee -bpe 'x += y'
x += y;

$ coffee -bpe 'x = x + y'
var x;
x = x + y;

Coffee's = is pretty magical after all.

[StanAngeloff]

OK, I believe I spoke too early. Just today I had to fix an issue where or= no longer introduced new variables.

I still think it was my mistake to use or= rather than an assignment, but I the following might be a legit case:

firstChild or= someEl?.firstChild
firstChild or= otherEl?.firstChild
if boo
  if foo
    firstChild or= foo.firstChild
  firstChild or= boo.firstChild

It's crappy coding, which is not the point.

[TrevorBurnham]

Right; and I remember an issue on list comprehensions where it was said that

firstNonNull = (arr) -> result ?= x for x in arr; result

is the best way to get the first non-null value out of an array. That's great, unless there's a global called result, in which case it'll get overwritten. Worse, this means the function will only work once:

firstNonNull = (arr) -> result ?= x for x in arr; result

console.log firstNonNull [null, null, 1]    # 1
console.log firstNonNull [null, null, null] # 1

Can we tag this as a bug now?

[michaelficarra]

unless there's a global called result, in which case it'll get overwritten

Yes, in both your and StanAngeloff's code examples, the variables should be initialized first anyway. If you don't do that, you're asking to get screwed either by a global or a modification later on.

[TrevorBurnham]

Michael, I don't understand where you're coming from. It seems really, really weird to me that changing x = x or y to x or= y could introduce a bug to your code. I'm arguing that these short forms should always yield the same behavior as the long forms, especially since this is a weird exception to the rule in CoffeeScript that you should never be able to modify a global variable from a function without using window. or similar.

Are you really arguing that = creating scope and += not creating scope is desirable? Should = alone be, in satyr's words, magic?

[michaelficarra]

TrevorBurnham: It seems to me that you're suggesting I can never use closures to modify a closed-over value. Take a look at a CS variation of my proposed _.once implementation.

_.once = (fn) ->
    run_count = 0
    return ->
        return if run_count > 0
        run_count++
        fn.apply this, arguments

Should this make a var run_count declaration in the inner function? I would obviously say it should not.

edit: This may be made a little more clear by the following code:

_.once = (fn) ->
    func = ->
        return if run_count > 0
        run_count++
        fn.apply this, arguments
    run_count = 0
    func

Changing run_count++ to run_count = run_count + 1 introduces a declaration of run_count. run_count += 1 behaves like run_count++. Hm...

[TrevorBurnham]

Well, given the discussion at #1121, it looks like the consensus is that the outer declaration of run_count should be declared prior to anything that modifies it. I don't see why += or ++ should be an exception to that rule. Is readability greatly improved by moving run_count = 0 to after the function?

If so, then surely that argument would apply more generally, and scope should be made order-neutral as I suggested at #1121? Why should count = nextPositiveInteger(count) create scope while count++ doesn't?

Again, an alternative proposal would be to have x = x + y behave the same way as x += y, by not creating scope for x. That would be more consistent, at least. But I always thought of "thou shalt not modify window.x from within a function by just referring to x" as a core principle of CoffeeScript.

[michaelficarra]

"thou shalt not modify window.x from within a function by just referring to x"

Well, it's not a property of the global scope. It's just an in-scope value that needs modification and not redeclaration.

edit: I think the bug is derived from treating in-scope values as out-of-scope values when their initial assignment comes after their reference. Crap, that sounds really confusing. Here's an example, I guess:

->
  # coffee puts a declaration right here
  -> # the variable should be in scope in here, but CS doesn't think so
  variable = value

and oddly

->
  # coffee puts a declaration right here
  variable = value
  -> # the variable should be in scope in here, and CS agrees this time

So pretty much our variable declaration hoisting rules are not respected by whatever is determining if a variable is in scope.

[TrevorBurnham]

Well, it's not a property of the global scope. It's just an in-scope value that needs modification and not redeclaration.

Right, obviously += doesn't redeclare the property. However, or= and ?= potentially do:

root = window ? global
root.x = null
do -> x or= [2]
console.log root.x # [2]

Should or= and += have different scoping rules?

[michaelficarra]

I agree, there's potentially multiple issues at play here. If you haven't already, see my edit above.

[StanAngeloff]

4f4032c is the first bad commit
commit 4f4032c
Author: satyr <[email protected]>
Date: Mon Nov 1 10:42:42 2010 +0900

fixed a bug that compound assignments were declaring variables

[satyr]

4f4032c

It was a bug. Declaring with compound assignment means accessing the variable before initialization, which doesn't make sense. I made such assignments illegal later in Coco, but the change didn't get ported back.

[TrevorBurnham]

Declaring with compound assignment means accessing the variable before initialization, which doesn't make sense.

That's debatable; allowing result or= to create scope for result in a function makes perfect idiomatic sense to me. But more to the point, shouldn't the same reasoning apply to any expression of the form x = x + y, x = func(x), or anything else where x is on both the left and right sides of the =?

"Fixing" x or= y but not x = x or y doesn't seem acceptable to me; the two expressions should be interchangeable, whether it's "good style" or not.

[jashkenas]

I'm afraid that the two expressions should not be entirely interchangeable -- all compound assignment and compound operators operate under the assumption that their operands already exist in the current scope ... and should error if undeclared.

x = x or y means, assign x to a new value (which happens to be x or y).

x or= y means, take the current value of x (which should exist), and assign x = y if x is falsy.

[TrevorBurnham]

Jeremy, I urge you to reconsider this one. There are two core tenets of the "Zen of CoffeeScript" that I've heard you espouse many times that seem to be at stake here:

1. Language features should be self-documenting whenever possible. In JavaScript, and just about any other language, x = x + y and x += y are synonymous. In CoffeeScript, the relationship between the two is much harder to explain: x += y is equivalent to either x = x + y or window.x = x + y, depending on whether x is defined in an outer scope.
2. Globals must be referenced explicitly to be modified. This produces more self-documenting, human-friendly code. It also means that I can find out whether a module potentially changes the value of window.x by simply grepping it for window.x—when I'm trying to convince someone that CoffeeScript is awesome, this is one of the first things I tell them. But to be precise, I'd have to say: "Grep for window.x... then do a regex search for /x\s*([+-/*%^|&]|or|and|\|\||&&)=|x\+\+|x--/ and, in each case, check whether there's an x declaration in an outer scope."

I get what you're saying about the meaning of compound operators, but that choice comes at no small cost in language complexity.

[bhavinkamani]

+1 for reconsidering this issue.

It took me some time to resolve a nasty bug because of unintentional creation of global variable.

My code in essence looked like

->
  for num in [1..10]
    errors ?= {}
  errors

While I thought errors was locally scoped, it ended up as a global variable which created those nasty global variable bugs.
With other operators such as or, + etc. it may be fair enough to assume the existence of the left-hand side variable. But with ?, it is debatable.

It will be great to see coffeescript eliminate any unintentional global variable creations other than the official way - window.

[michaelficarra]

@bhavinkamani: See pull #1729 and issue #1627. We plan to do exactly what you request.

[bhavinkamani]

Thanks a lot @michaelficarra and everyone for great community effort on this...

[jashkenas]

@jeremyBanks' patch to add early errors for compound assignments to undeclared variables is now merged to master.

[satyr]

What about non-conditional compound assignments?

$ bin/coffee -bce 'x ||= 1'
Error: the variable "x" can't be assigned with ||= because it has not been defined.
...

$ bin/coffee -bce 'x += 1'
// Generated by CoffeeScript 1.2.1-pre

x += 1;

$ bin/coffee -bce 'x--'
// Generated by CoffeeScript 1.2.1-pre

x--;

[jashkenas]

I'd tend to think that those are acceptable because they don't introduce global variables by accident, and will fail at runtime if invalid. We could be stricter and forbid them without window. prefixing, but I'm not terribly concerned with allowing them -- unless you have a reason?

[satyr]

Sounds like this issue should be wontfix rather than duplicate.