Elm allows invalid JavaScript identifier names; runtime failure

[jastice]

Elm allows identifier names such as x², and uses them verbatim as JS variable names. However, because JS doesn't allow this name, it fails at runtime: Uncaught SyntaxError: Unexpected token ILLEGAL

I would expect Elm to either enforce the same identifier restrictions, or transform them at compilation (as it does with JavaScript reserved words).

For reference: Post about legal characters in JS identifiers

[maxsnew]

@evancz @achudnov should this be considered an upstream (language-ecmascript) issue? If so, should we just move all identifier fixing (https://github.com/elm-lang/Elm/blob/master/compiler/Generate/JavaScript/Variable.hs) to that library?

[achudnov]

I don't know elm's code, so I can't say for sure. However, if you determine that the problem stems from language-ecmascript, please, submit an issue ticket there: https://github.com/jswebtools/language-ecmascript/issues/new

Also, I've had a quick look at the module you linked, and I fail to see anything there that is not in the library already or that is generic enough to warrant inclusion.

As for the issue itself, I have two questions:

• does elm use the predicates isValid or isValidIdentifier to check ASTs before pretty-printing? (cf http://hackage.haskell.org/package/language-ecmascript-0.16.2/docs/Language-ECMAScript3-Syntax.html#v:isValid )
• which version of language-ecmascript are you using? Anything > 0.16 should be good. (Versions < 0.16 didn't have the isValid predicate, and 0.16 itself is, unfortunately, buggy.)

[mgold]

The cabal file has the restriction >=0.15 && < 1.0 so you can try bumping it (in all three places) and going from there.

[achudnov]

@mgold, if elm compiles with language-ecmascript < 0.16, this means that it certainly doesn't use isValid. Hence, just bumping the lower bound on language-ecmascript wouldn't do anything. There's nothing the pretty-printer could do about invalid identifiers apart from failing, For backwards compatibility I have decided not to fail on invalid ASTs. You can easily have a wrapper prettyPrintIfValid :: PrettyPrint a => a -> Maybe String that outputs Just the result of prettyPrint if the AST isValid, and Nothing otherwise.

I would recommend using isValid as a sanity check on ASTs you output, particularly, in tests. However, I think you should check for valid JS identifiers in your parser; alternatively, if you want to allow invalid JS identifiers in your elm code you need to somehow translate them to valid ones when you generate JS ASTs. And make sure you only feed valid ASTs to the language-ecmascript pretty-printer by construction.

[maxsnew]

I'm fairly certain we do want to allow this so based on what you're saying, language-ecmascript does not try to do what we need here, so this is not an upstream issue. Thanks, @achudnov .

[achudnov]

language-ecmascript does not try to do what we need here

You mean fixing bad identifiers? I don't think there's a way one can do this in a semantics-preserving way apart from removing bad characters or substituting them for a know safe one, e.g. _. In fact, the misguided attempt to perform "smart" identifier fixing in 0.16 was the reason for the 0.16.1 bugfix :)

Anyway, my recommendation stands. Although I don't know your code base, so take that with a grain of salt. If you figure out something that makes the issue go away or, at least, less painful in a way that does not rely on elm-specific stuff, I'd be happy to review an issue or a pull request for it.

[jastice]

Just confirmed issue as still existing in Elm 0.16.
Specifically the x² identifier still causes a runtime error:

SyntaxError: illegal character JavaScript.elm:6681:8

Most other invalid identifiers I have tried cause a (slightly confusing) compile error instead:

I ran into something unexpected when parsing your code!

13│ x🎃 = 23
     [31m^[0m
I am looking for one of the following things:

    a pattern
    an equals sign '='
    more letters in this name
    whitespace

[evancz]

Track in #1377

[cdeszaq]

1. If Elm truly intends not to be coupled to JS as it's kernel, defining Elm's set of valid characters as "the same as JS" seems antithetical to that intention.
2. Restricting the character set may add linguistic barriers to Elm. I understand that keeping the valid character set restricted reduces the surface area for possible problems, and prevents devs from shooting themselves in the foot with "creative" names, but is that level of safety worth the restriction on those who's language does not fit into the JS box?

[mgold]

It's always possible to expand the set of allowed characters later. The more urgent concern is if we're allowing a character that we don't like, which was the case with '.