Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CoffeeScript use strict #1547

Closed
geraldalewis opened this issue Jul 26, 2011 · 24 comments
Closed

CoffeeScript use strict #1547

geraldalewis opened this issue Jul 26, 2011 · 24 comments

Comments

@geraldalewis
Copy link
Contributor

use strict

ECMAScript 5 introduces a strict mode that reduces bugs, increases security, and eliminates some difficult to use language features. The coding standards strict mode requires should be adopted by CoffeeScript.


Better Code

CoffeeScript emits lowest common denominator JavaScript. From a different angle: CoffeeScript emits JavaScript that is not implementation specific. Just as there are constraints on output imposed by IE6's nonconforming behavior (no function declarations), there too should be constraints on input imposed by better engineered environments.

User Expectations

CoffeeScript throws errors when it encounters syntax errors (e.g., one can't write -> return return). A user employing the use strict directive will expect to see syntax errors at compile time (pre-evaluation), just as they would in a JavaScript environment.

Best Practices

CoffeeScript emits JavaScript that adheres to best practices. ES5 strict codifies many best practices, and fixes some unsafe and error-prone operations. CoffeeScript already shares some constraints with strict (e.g., no with) and fixes others that strict targets (e.g., in CoffeeScript, vars are auto-declared for variable safety).

Early Errors

"Early errors" are syntax errors detected prior to evaluation (all others are runtime errors). Most strict mode errors are syntax errors (of type SyntaxError). CoffeeScript should only check for these types of errors. 16 Errors states: "An implementation shall not treat other kinds of errors as early errors even if the compiler can prove that a construct cannot execute without error under any circumstances" (emphasis mine).

Early Strict Syntax Errors

The following CoffeeScript snippets would be considered "early errors" under ES5 strict.

7.8.3 Numeric Literals

oct = 0121            # octals

B.1.2 String Literals

octEscSeq = "\121"    # octal escape sequences

11.1.5 Object Initialiser

x: 1, x: 1            # duplicate prop names in object literal

11.4.1 The delete Operator

x = 1; delete x       # delete var
delete undefined      # delete undefined 
(x)-> delete x        # delete function argument 

13 Function Definition (13.1 Strict Mode Restrictions)

(x,x)->               # duplicate formal param

7.6.1.2 Future Reserved Words

implements = 1        # assigning to future reserved keyword
interface = 1
let = 1
package = 1
private = 1
protected = 1
public = 1
static = 1
yield = 1

eval and arguments restrictions

12.14 The try Statement (12.14.1 Strict Mode Restrictions)

try e catch eval      # catch identifier eval
try e catch arguments # catch identifier arguments

13 Function Definition (13.1 Strict Mode Restrictions)

(eval)->              # identifier "eval" in formal params
(arguments)->         # identifier "arguments" in formal params
class eval            # function declaration `function eval(){}`
class arguments       # function declaration `function arguments(){}`

11.13.1 Simple Assignment

eval = 1              # assigning to eval
arguments = 1         # assigning to arguments

11.3.1 Postfix Increment Operator

eval++                # incrementing eval
arguments++           # incrementing arguments

  as well as Postfix Decrement and Prefix Increment and Decrement


What Users Would Give Up

Some coding conventions would need to be abandoned, namely:

Identical param names

  • used for parameters that are not germane to the function's body
  • useful in callbacks

e.g.,

(_,_,_,buffer) -> #read buffer

However, as @jashkenas stated "...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."

@satyr also offered a workaround: ({},{},{},buffer) -> #read buffer would still be legal

Identical prop names in object literals

  • could be useful for debugging

@satyr's example:

o =
  a: console.debug "before a"
  a: getA()
  b: console.debug "before b"
  b: getB()

I'd argue that it's likely this feature is more often the cause of bugs foo: 1, bar: 2, bar: 3 # user meant 'baz' and catching those errors would outweigh its other uses.

(and one could still write):

o =
  a: do -> console.debug "before a"; getA()
  b: do -> console.debug "before b"; getB()

Sources/More Information
Annotated ES5 Spec
ES5 Spec: Annex C "The Strict Mode of ECMAScript "
JavaScript Strict Mode Tests
ECMA-262-5 in detail. by Dmitry Soshnikov
ES5 Browser Compatibility Table by @kangax

@josh
Copy link
Contributor

josh commented Jul 26, 2011

(subscribe)

@ayosec
Copy link

ayosec commented Jul 26, 2011

+1

@michaelficarra
Copy link
Collaborator

@josh: there's now a link at the bottom of issues that allows you to {,un}subscribe without posting. It's actually been there for a few months now.

@jashkenas
Copy link
Owner

@michaelficarra: @josh works at GitHub ;)

In any case, I think this ticket makes a very strong argument for having CoffeeScript be "use strict" compliant to the extent statically possible.

The only real downsides here are the inability to use the FutureReservedWords as identifiers ... but it seems like folks don't tend to do that:

http://www.google.com/codesearch#search/&q=%5Cbvar%5Cs%2B(implements%7Cinterface%7Clet%7Cpackage%7Cprivate%7Cprotected%7Cpublic%7Cstatic%7Cyield)%5Cb&type=cs

@michaelficarra
Copy link
Collaborator

So let's clarify this proposal. Do we enforce strict mode restrictions everywhere or only in contexts that begin with a "use strict" directive? If everywhere, do we output a "use strict" directive at the top of our IIFE wrapper?

Side note: the delete undefined example from the OP exposes a bug in our current compilation, but if we're going to disallow deletion of "a direct reference to a variable, function argument, or function name", this problem will just go away.

@michaelficarra
Copy link
Collaborator

edit: never mind, @geraldalewis already got it

@geraldalewis
Copy link
Contributor Author

Good point @michaelficarra

Since strict mode affects some subtle runtime behavior, and since that runtime behavior is not cross-browser/environment, strict mode should be opt-in. For instance, in non-strict, changing the value of a property of the arguments object will change the value of the corresponding argument binding, while in strict there's no dynamic binding between arguments and the individual arguments defined by the formal params.[2]

From @kangax's ES5 Browser Compatibility Table:
strict

((x)-> x = 2; arguments[0] is 1)(1) # true

non-strict

((x)-> x = 2; arguments[0] is 1)(1) # false

That'd probably break some users' code if they're not expecting it (not that there isn't arguments weirdness in different implementations[2]).

Related to the "use strict" directive prologue itself: it might be a good idea for the compiler to communicate that "use strict" shouldn't appear outside an IIFE when compiling with --bare in case the file is concatenated with other js files. Douglas Crockford warns "If a file with a "use strict"; preamble has sloppy code appended to it, the sloppy code will be processed as strict and will probably fail. There is an easy rule: Do not mix strict and sloppy in the same file, but we have already seen some famous websites get this wrong."[3] Would a warning work? (I can't find precedents for warnings outside of compiling with the --lint option; am I missing some?). There may be a use case for "use strict" outside an IIFE I'm not thinking of, however.

Would love to hear about the bug delete undefined exposed -- tried to find it myself last night to no avail.

[1] ES5 Annotated Spec 10.6 Section 1
[2] Angus Croll, The JavaScript arguments object… and beyond
[3] Douglas Crockford, Strict Mode is Coming to Town

@jashkenas
Copy link
Owner

I think if we do this, we enforce our static strict mode restrictions everywhere, but we do not put any "use strict" directives into your code for you.

@geraldalewis
Copy link
Contributor Author

Agreed @jashkenas -- I just came back to clarify that the early strict syntax checks should be made regardless of whether or not the user has chosen strict mode.

@geraldalewis
Copy link
Contributor Author

In the interest of deepening the discussion, it was proposed in #1002 (duplicate parameter names) that instead of throwing an error, CoffeeScript could rewrite the offending code. The following CoffeeScript snippets show:

1. code that would raise a SyntaxError under `use strict`
2. conversion of that code to a safe form (expressed in CoffeeScript)
3. the sanitized JavaScript output

For the record, I think SyntaxErrors are a better way to handle strict mode restrictions.

7.8.3 Numeric Literals

oct = 0121
oct = 81                  # convert to decimal
var oct = 81              # js output

B.1.2 String Literals

octEscSeq = "\121"
octEscSeq = "\x51"        # convert to hex escape sequence
var octEscSeq = "\x51"    # js output

11.1.5 Object Initialiser

x: console.log('x'), x: 1 
x: do -> console.log('x'); 2  # each assignment wrapped into single `IIFE`
{ x: (function() {            # js output; return second assignment's value
  console.log('x'); 
  return 2; })() 
};

11.4.1 The delete Operator

x = 1; delete x           
x = 1;                    # remove `delete x`
var x = 1;                # js output

13 Function Definition (13.1 Strict Mode Restrictions)

(x,x) ->                  
(x,x2) ->                 # rename 2nd param to be unique
function(x, x2) {}        # js output

7.6.1.2 Future Reserved Words

implements = 1            
_implements = 1           # `_` prefix added
var _implements = 1;      # js output

The eval/arguments issues can be resolved with a rename

try e catch eval          
try e catch _eval         # `_` prefix added
try { e; } catch (_eval) {}# js output

(arguments)->             
(_arguments)->            # `_` prefix added
function(_arguments) {}   # js output

class eval
class _eval               # `_` prefix added
function _eval(){}        # js output

arguments = 1
_arguments = 1            # `_` prefix added
var _arguments = 1        # js output

eval++
_eval++                   # `_` prefix added
_eval++                   # js output

Obfuscation and Intent

However, these code transformations would subvert the intention behind the strict restrictions. In the case of duplicate param names and duplicate object props, this strategy would further obfuscate hard to detect bugs. A user attempting to delete a var, arg or function declaration should be warned that their action isn't having the intended effect. eval and arguments as identifiers is a bad idea.

@jashkenas
Copy link
Owner

Agreed -- much much better to SyntaxError than to change your semantics on the sly.

@michaelficarra
Copy link
Collaborator

For the record, I think SyntaxErrors are a better way to handle strict mode restrictions.

I think the conversion is better for some, but not all, strict mode restrictions.

7.8.3 Numeric Literals

oct = 0121
oct = 81                  # convert to decimal
var oct = 81              # js output

+1

B.1.2 String Literals

octEscSeq = "\121"
octEscSeq = "\x51"        # convert to hex escape sequence
var octEscSeq = "\x51"    # js output

+1

11.1.5 Object Initialiser

x: console.log('x'), x: 1
x: do -> console.log('x'); 2  # each assignment wrapped into single `IIFE`
{ x: (function() {            # js output; return second assignment's value
  console.log('x');
  return 2; })()
};

I'd prefer this conversion:

o = {};
o.x = console.log('x');
o.x = 1;

11.4.1 The delete Operator

x = 1; delete x
x = 1;                    # remove `delete x`
var x = 1;                # js output

-1 no way to preserve semantics: SyntaxError

13 Function Definition (13.1 Strict Mode Restrictions)

(x,x) ->
(x,x2) ->                 # rename 2nd param to be unique
function(x, x2) {}        # js output

(x,x) -> should be function(_unused, x){}

7.6.1.2 Future Reserved Words

implements = 1
_implements = 1           # `_` prefix added
var _implements = 1;      # js output
var implement\u0073 = 1

The eval/arguments issues can be resolved with a rename

try e catch eval
try e catch _eval         # `_` prefix added
try { e; } catch (_eval) {}# js output

-1 no way to preserve semantics: SyntaxError

(arguments)->
(_arguments)->            # `_` prefix added
function(_arguments) {}   # js output

-1 no way to preserve semantics: SyntaxError

class eval
class _eval               # `_` prefix added
function _eval(){}        # js output

-1 no way to preserve semantics: SyntaxError

arguments = 1
_arguments = 1            # `_` prefix added
var _arguments = 1        # js output

-1 no way to preserve semantics: SyntaxError

eval++
_eval++                   # `_` prefix added
_eval++                   # js output

-1 no way to preserve semantics: SyntaxError

Obfuscation and Intent

However, these code transformations would subvert the intention behind the strict restrictions. In the case of duplicate param names and duplicate object props, this strategy would further obfuscate hard to detect bugs.

A user attempting to delete a var, arg or function declaration should be warned that their action isn't having the intended effect.

No, it should definitely be a SyntaxError, no question.

eval and arguments as identifiers is a bad idea.

agreed.

@geraldalewis
Copy link
Contributor Author

JSLint throws an error if you don't include the "use strict" pragma* . Developers naïvely include it to prevent the error, but do not properly test their code with a browser that supports strict mode. Or code with the global "use strict" directive is concat'ed with non-strict code, breaking it. Bugs are then filed against browsers who correctly throw errors when "use strict" breaks some functionality on a big website.

This may lead to what @BrendanEich calls "strict quirks mode".

Raising SyntaxErrors on "early errors" will not only help devs who incorrectly enable strict mode, but will add a layer of defense for CoffeeScript programs that are concat'ed into strict mode. "use strict" is a good and valuable thing for JavaScript, and therefore it's good for CoffeeScript. We can do our part and require better, safer, and more reliable code from CoffeeScript developers.


* Unless the "sloppy" flag is set to true, something I doubt a dev who uses JSLint would like to do. By calling it "sloppy", Crockford has, frustratingly, created a moral imperative for employing "use strict" (like Knuth did [tongue-in-cheek] with Literate Programming -- "nobody wants to admit to an illiterate program.")

geraldalewis added a commit to geraldalewis/coffee-script that referenced this issue Jan 16, 2012
geraldalewis added a commit to geraldalewis/coffee-script that referenced this issue Jan 16, 2012
geraldalewis added a commit to geraldalewis/coffee-script that referenced this issue Jan 16, 2012
RegExp updated (thanks @michaelficarra)
and hex escapes for colors in Cakefile

tests updated (thanks @satyr)

error message conforms to existing Lexer SyntaxErrors
geraldalewis added a commit to geraldalewis/coffee-script that referenced this issue Jan 16, 2012
geraldalewis added a commit to geraldalewis/coffee-script that referenced this issue Jan 16, 2012
…ibited

updated error message (thanks @davidchambers)

code style fixes
geraldalewis added a commit to geraldalewis/coffee-script that referenced this issue Jan 16, 2012
geraldalewis added a commit to geraldalewis/coffee-script that referenced this issue Jan 16, 2012
geraldalewis added a commit to geraldalewis/coffee-script that referenced this issue Jan 16, 2012
geraldalewis added a commit to geraldalewis/coffee-script that referenced this issue Jan 16, 2012
… 0o777

Allows octals in the form '0o777' and '0O777'

Case insensitive

Disallows decimals prefixed with '0'
geraldalewis added a commit to geraldalewis/coffee-script that referenced this issue Jan 16, 2012
@michaelficarra
Copy link
Collaborator

I think this can be closed.

@satyr
Copy link
Collaborator

satyr commented Apr 11, 2012

$ coffee -e '"use strict"; p: 1, "p": 2'

.:6
    "p": 2
    ^^^
SyntaxError: Duplicate data property in object literal not allowed in strict mode

@satyr
Copy link
Collaborator

satyr commented Apr 11, 2012

$ node -e '"use strict"; "\0"'

$ coffee -e '"\0"'
SyntaxError: octal escape sequences "\0" are not allowed on line 1

@devongovett
Copy link

I am also wondering about the "\0" octal escape sequences being disallowed as well. If @satyr is correct these are allowed in JS even in strict mode. I have an SQL escape function that needs to replace "\0" with "\0" and now it won't compile. Any reason why, or is this a bug?

@BrendanEich
Copy link

SpiderMonkey shell:

js> "use strict"; "\0"
"\x00"
js> "use strict"; "\00"
typein:2: SyntaxError: octal literals and octal escape sequences are deprecated:
typein:2: "use strict"; "\00"
typein:2: ..............^
js> "use strict"; "\01" 
typein:3: SyntaxError: octal literals and octal escape sequences are deprecated:
typein:3: "use strict"; "\01"
typein:3: ..............^

The special case is for "\0" only, not anything else truly octal or "noctal" (not quite octal, e.g. \08, \09, etc.).

@devongovett: it's impossible to say what is going wrong without seeing that function, but it's not anything to do with strict mode as spec'ed or implemented in Firefox.

/be

@devongovett
Copy link

@BrendanEich Well here is the function:

escapeSQL = (val) ->
    val = val.replace /[\0\n\r\b\t\\\'\"\x1a]/g, (s) ->
        switch s
            when "\0" then "\\0"
            when "\n" then "\\n"
            when "\r" then "\\r"
            when "\b" then "\\b"
            when "\t" then "\\t"
            when "\x1a" then "\\Z"
            else "\\" + s

    return "'" + val + "'"

The error is thrown by the CoffeeScript compiler, not the JS side of things. CoffeeScript apparently isn't special casing "\0" like SpiderMonkey, V8 and probably others do. Luckily, this is workaroundable for the time being by using "\u0000" instead of "\0", but it still should be fixed.

@BrendanEich
Copy link

@devongovett new issue on file?

/be

@michaelficarra
Copy link
Collaborator

@devongovett: This should be fixed by 46ff770. Go ahead and try that out.

@eugenioclrc
Copy link

+1

@Almad
Copy link

Almad commented Dec 3, 2014

Related to #566 (that is open)...

My question more is: would it be possible to at least add it as an option? I'd be glad to have it feature-flagged, but we'd want to give it a try (on server).

Or is it wontfix for another reason?

@hankphung
Copy link

+1 : Cannot delete any variable.

-module.coffee:101:5: error: delete operand may not be argument or var
    delete   parser
    ^^^^^^^^^^^^^^^
    L100:     delete   parser

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

No branches or pull requests