Disallow arguments with identical names

[TrevorBurnham]

It's currently legal to have two function arguments with the same name:

foo = (a, a) -> console.log a
foo 1, 2

Apparently the output is valid JavaScript; the result (at least under Node) is 2. But I'd rather see an error at compile-time; code like this is almost certainly a typo. (For instance, an Express app written in canonical style will have several functions with the argument list (req, res)—it's pretty easy to type (req, req) or (res, res) instead, with disastrous consequences at runtime.)

[michaelficarra]

I completely disagree. This is perfectly okay in JS and should be allowed in CS as well. For a use case, how would I cleanly skip arguments like this?

callback = (a, b, _, _, _, c) -> fn a, b, c

Even though it's not the most useful of features, I would be surprised if I was not allowed to do this.

[erisdev]

I'm not sure what's so clean about declaring a function with (a, b, _, _, _, c) as its signature. It's much cleaner in my opinion to always give your arguments meaningful names. You never know who might be changing your callback in the future or what they might want it to do, and _ is about as self-documenting a name as arg or foo. Giving multiple arguments the same, meaningless name is even more confusing and pretty much immediately reads as an error.

In fact, I can't find documentation of this "feature" of JavaScript anywhere online, so I can't be sure it's consistent with the standard, nor if the precedence is the same across platforms.

If you want to put obfuscated garbage arguments in your function's signature, though, it would look cleaner if CoffeeScript translated null as an argument name to some automagical name like __ignored. Not that I think that's a good solution.

[satyr]

This is perfectly okay in JS

Unless strict.

[TrevorBurnham]

So there you go.

Michael, how about if we implement a syntax analogous to the one I've proposed at issue 870 so that you could write

callback = (a, b, ?, ?, ?, c) -> fn a, b, c

and the generated argument list would be something like (a, b, __1, __2, __3, c), where __1 etc. are unused identifiers?

That way, you could more clearly indicate that the arguments are ignored, your generated JavaScript would be more valid/readable, and folks like me would still be able to get an error message at compile time when we accidentally type (req, req) ->?

[michaelficarra]

Yep, after satyr pointed out that this was not valid in strict mode, I'd (unfortunately) have to agree that this should throw an error. I want to keep as close to strict mode compliance with JS output as we can.

An alternative solution would be to allow this and only name the last parameter with that name properly (renaming the others to some nonsense like __unused0) to preserve non-strict behaviour. But I have a feeling that that solution wouldn't be too popular.

Also, I'd like to reiterate that I am not a fan of the single-value-skipping ? syntax for either this purpose or the one proposed in #870.

[michaelficarra]

It appears we throw an error on all strict mode requirements listed at satyr's link except this one and Attempts in strict mode code to define an ObjectLiteral that has multiple data property assignments with the same name. Let's fix them both in one go.

[TrevorBurnham]

By the way, Michael, there is a (perhaps overly clever) way to write your example using pattern-matching:

callback = ([a, b]..., c) -> fn a, b, c

Of course, that assumes that exactly 6 arguments are given. Once some kind of single-value-skipping syntax is implemented (issue 870), you'll be able to use that. Though ([a, b, null, null, null, c]) -> looks a bit verbose.

[satyr]

([a, b, null, null, null, c]...) ->

({0: a, 1: b, 5: c}...) ->

[jamierumbelow]

Has anyone fixed this yet?

[michaelficarra]

@jamierumbelow: nope, that's why it's still open. Feel free to send a pull request.

[geraldalewis]

@michaelficarra -- Working on this (based on @jamierumbelow 's patch) :)

[jashkenas]

Be very careful using a hash to check for the existence of names:

!!({})['toString'] => true

... either use hasOwnProperty, or an array.

[geraldalewis]

I'm looking to build a consensus on this issue before working on my patch (#1531) again. We have three options for solving the issue of duplicate parameter names. (All involve ensuring the output JS has no functions with duplicate parameters.)

Option #1: Throw an error.

> coffee -bpe '(_,_)->'
SyntaxError: parameter names must be unique; a parameter '_' already exists.

Option #2: Generate a new, unique name.

> coffee -bpe '(_,_)->'
(function(_, _2) {});

Option #3: Throw an error, but create syntax for placeholder params.

> coffee -bpe '(-,-)->'
(function(_arg, _arg2) {});

Philosophically
Every language has some points of flexibility and some of strictness.
Are duplicated param names (those params serving no logical purpose within the body of a function) strict enough for CoffeeScript?
Are SyntaxErrors during compile time flexible enough for CoffeeScript?

Usability
Is using duplicate param names a popular practice? If so, will compiler errors be a pain point?
Would generating a unique name further occlude programmer errors from mistyping/misspellings? (foo,bar,bar)->
Does the ability to create duplicate param names warrant its own syntax? Whatever this syntax looks like, is it discoverable, yet still as useable as tersely named temp params usually are?

Consistency
ES5's strict mode calls for object literals to have unique property names as well, meaning one can't write code like:

var o = {
  prop: 0
  prop: 1
}

If CoffeeScript also disallows that kind of code, which option from above would work best? For defining props in object literals, I can't see any reason why one would want placeholder/duplicate props -- is there one? If not, generating a unique name might not be a good solution if consistency is important.

Personally, I like option #3 :)

[TrevorBurnham]

My preferred option is #1. I think strictness is the only policy here that's consistent with CoffeeScript's philosophy of being as simple and self-explanatory as possible.

#2 strikes me as untenable. Why would we accept (_, _) -> _ * _ as a valid way of writing (x, y) -> x * x?

#3 would be acceptable if there were a good syntax, but something like (a, ?, c) -> is hardly self-explanatory in a language with splats, and such a syntax would very rarely be clearer than (a, b, c) -> at any rate.

[jashkenas]

+1 for option #1

[geraldalewis]

@TrevorBurnham There is a valid reason for #2 (I believe you mention it in one of your posts above):

#node.js
fs.write( fd, buffer, offset, length, position, (_,_,buffer) -> buffer )

Callbacks with 2+ params that are not germane to the function's body can be more elegantly declared with placeholder params.

[TrevorBurnham]

@geraldalewis Yes, that code does look nice and readable. But more often, I'd think that people would give two arguments the same name by accident. In such cases, a compile-time error is very helpful. See my original post for this issue...

This is the sort of problem that's best addressed by a programming convention, not special syntactic rules. I'm guessing that Jeremy's preferred approach to your code would be to say what the first two arguments to that callback are, even though they're unused. That's a good approach. Another would be to take those argument names and shorten them down to 1-2 characters. That'll keep your code succinct but still help you figure out what those arguments are in case you want to use them later.

So style 1 (my attempt at emulating jashkenas) would give you

writeCallback = (err, written, buffer) -> buffer
fs.write fd, buffer, offset, length, position, writeCallback

and style 2 would give you

fs.write fd, buffer, offset, length, position, (e, w, buffer) -> buffer

I think either is preferable to adding yet another burden to poor _'s shoulders.

[geraldalewis]

@TrevorBurnham I agree, especially that constraining options through a strict interpretation can be really beneficial to usability. (Though I'm partially in favor of heaping more abuse upon _ by making it both a valid identifier and syntax to indicate a placeholder param ;). I'll wait for @michaelficarra to weigh in before proceeding with a refined patch for option #1.

[satyr]

You have Option 3 working already if this syntax is tolerable:

$ coffee -bpe '([], [], buffer) ->'
(function(_arg, _arg2, buffer) {
  _arg;
  _arg2;
});

The compilation needs to improve, of course.

[michaelficarra]

@geraldalewis: I've been purposely withholding my opinion because I don't feel that any argument I can make in support of any of the options is strong enough. So, for now, I'll forfeit my vote. I only recommend that _ is not treated any differently than other identifiers. That's a bad road to start going down.

[jashkenas]

@TrevorBurnham's done a fine job of "guessing Jeremy's preferred approach". The more we debate it, the more it seems like

(_, _, _, buffer) ->
  ...

is a serious antipattern. For the reader who comes across your function, intending to modify it ... not only are they unable to use the first three parameters -- they also have not the slightest clue what the parameters might be.

For a language that explicitly tries to discourage shadowing in favor of nicely named local variables, this seems like the ultimate in parameters shadowing, in a way. Every unused parameter in your program has the same name, and none of them can be used without changing it. Ugh.

[jashkenas]

Moving this party over to #1547, for general use strictness.

[rlidwka]

You can allow duplicate names, but only if the name isn't used in the function, i.e.:

# perfectly fine
(_, _, _, buffer) -> buffer 

# syntax error
(_, _, _, buffer) -> _

[vendethiel]

No, not even. A reader coming later would want to know what are these params in the first place if he needs to change the function.