Issue #2060 Disallow uppercase radix prefixes and exponential notation

[geraldalewis]

Disallows uppercase radix prefixes and exponential notation.

I also tweaked the error code for octal literals.

---

Uppercase radix prefixes.

$ bin/coffee -bpe '0B0'
SyntaxError: radix prefixes must be lowercase '0B0' on line 1

$ bin/coffee -bpe '0O0'
SyntaxError: radix prefixes must be lowercase '0O0' on line 1

$ bin/coffee -bpe '0X0'
SyntaxError: radix prefixes must be lowercase '0X0' on line 1

Uppercase exponential notation.

$ bin/coffee -bpe '0E0'
SyntaxError: exponential notation must be indicated with a lowercase 'e' on line 1

Deprecated octal literal notation.

$ bin/coffee -bpe '00'
SyntaxError: octal literals '00' must be prefixed with '0o' on line 1

0-prefixed decimals.

$ bin/coffee -bpe '08'
SyntaxError: decimal literals '08' must not be prefixed with '0' on line 1

[michaelficarra]

/E/.test

[geraldalewis]

Thanks. Also noticed the exponent error was the only one that didn't reference the source of the error. I'll patch and push a new version.

[michaelficarra]

LGTM

[jashkenas]

This patch is adding a lot of explicit special-case error strings ... is the behavior (error message) poor if we simply fail to match the token as a number, and proceed with the lex?

[geraldalewis]

is the behavior (error message) poor if we simply fail to match the token as a number

It would have been much easier to make the NUMBER RegExp in the lexer case-sensitive, but the error message would be baffling:

bin/coffee -bpe '0X0'
Error: Parse error on line 1: Unexpected 'IDENTIFIER'

If I were new to CoffeeScript, I'd assume hex literals were disallowed. Or, I'd file a bug report against it.

numberToken() would be more elegant without the error messages, but the process of coding in CoffeeScript would not be. I like the idea of optimizing the experience of writing CoffeeScript, while keeping in mind that it should be a good read, too.

[geraldalewis]

This patch is adding a lot of explicit special-case error strings

I keep thinking about this. I know I'm framing this poorly, but Is there a good reason for not being explicit?

[michaelficarra]

No, the descriptive errors are necessary.

[geraldalewis]

Thanks for merging! @jashkenas, I was holding off on pushing a change @michaelficarra suggested until I heard back on the error messages:

-    if /[E]/.test number
-      @error "exponential notation must be indicated with a lowercase 'e'"
+    if /E/.test number
+      @error "exponential notation '#{number}' must be indicated with a lowercase 'e'"

• removes the brackets around "E"
• indicates the error: '#{number}'

Seems like overkill to open a new pull request -- can that change be applied within GitHub?

[michaelficarra]

@geraldalewis: just push another commit to that branch and I'll merge it in.

[geraldalewis]

Sounds good -- will push shortly.

Edit: Hmm... Pushed, but it's not updating here.

[kalbasit]

Not updating because pull request has already been merged, just submit a new pull request, don't forget to mention #2061 in the new pull request so they'll be linked...