Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Disallow double-underscore-leading variables #106

Closed
satyr opened this Issue Dec 28, 2011 · 16 comments

Comments

Projects
None yet
5 participants
Owner

satyr commented Dec 28, 2011

To prevent conflict with the helper variables in the compilation.

(As in CoffeeScript, use of those variables has been tacitly forbidden but technically allowed.)

Something to consider is that this could still be circumvented via eval:

list = [1, 2, 3]
eval("__of = function() { console.log('Hijacked!'); };")
1 of list
Owner

satyr commented Dec 28, 2011

Circumvention isn't a concern. You don't even need a eval because of JS literal: __of = null

Ah, okay (I was referring to using eval if underscore-prefixed variables were disallowed). In that case, the only other concern is backward-compatibility, though I doubt prefixed variables are as common in Coco and CoffeeScript as they are in JS.

ceymard commented Dec 28, 2011

Are you sure about doing such a change ?

I do use leading _ to indicate that the variable is more of a "private" nature than another.

Owner

satyr commented Dec 29, 2011

I do use leading _ to indicate that the variable is more of a "private" nature than another.

Was anticipating that's already the case. Doing so is highly dangerous and discouraged due to the readable naming of internal temporaries.

Other options to avoid the collisions are:

  • Never use the same name as user variables for temporaries.
    • Costly due to additional scanning and matching.
    • Related: #100
  • Make the naming of temporaries more consistent (e.g. prefix __ to all) and disallow only those.
  • ?

mcfog commented Jan 5, 2012

I think name the helpers with __coco_xxxx (and maybe disallow __coco_var) is a better idea

underscore-leading variable suggest "private thing" is a python convention and it's good in javascript too.

also you can find _vars in jQuery or Dojo or many other js libs,

Owner

satyr commented Jan 5, 2012

I think name the helpers with __coco_xxxx (and maybe disallow __coco_var) is a better idea

Long prefix hurts readability. __ is about as far as we can tolerate.

ceymard commented Jan 5, 2012

Generated code is not really meant to be read directly by the programmer, and it would be nice when debugging to instantly see which variables were added by coco (instead of wondering if you did that yourself :) )

Owner

satyr commented Jan 5, 2012

Generated code is not really meant to be read directly by the programmer

Generally yes, but CoffeeScript/Coco is different in this regard. The readability of the compilation is a major goal and plays an important role for its adoption.

it would be nice when debugging to instantly see which variables were added by coco

Right. Double underscores would be good enough for that.

thejh commented Jan 8, 2012

Maybe you could just use some other prefix? Stuff that comes to my mind:

  • random prefix, different for each compiled file
  • weird unicode prefix, examples: ω, Δ, δ (yes, those are valid identifiers)

Alternatively, we could use $coco_ or so - the EcmaScript spec says:

The dollar sign is intended for use only in mechanically generated code.

Owner

satyr commented Jan 8, 2012

the EcmaScript spec says:

The dollar sign is intended for use only in mechanically generated code.

That's pre-jQuery. Nowadays it's common to see code like:

$div = $('div')

thejh commented Jan 8, 2012

hehe

thejh commented Jan 8, 2012

So, how about weird unicode?

Owner

satyr commented Jan 8, 2012

So, how about weird unicode?

Too fancy. Sticking to ASCII range avoids potential encoding issues.

Owner

satyr commented Jan 10, 2012

Title edited. (was: Disallow underscore-leading variables except _ itself)

satyr added a commit that referenced this issue Mar 6, 2012

Owner

satyr commented Apr 14, 2012

I guess those commits are enough to keep accidental conflicts for now. We'll just avoid __xxx instead of _xxx. Disallowing all __xxx isn't viable even considering Node only (__filename and __dirname).

@satyr satyr closed this Apr 14, 2012

josher19 pushed a commit to josher19/LiveScript that referenced this issue Jul 24, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment