Disallow identifiers that start with a number#154
Disallow identifiers that start with a number#154biochimia wants to merge 1 commit intoschibsted:masterfrom
Conversation
First of all, this is a breaking change. JSLT code that uses identifiers
starting with a number will fail to parse. The only redeeming feature of
this change is that this is a common restriction in many programming
languages, which should somewhat limit the risk of mayhem.
The motivation for doing this change, and one which is reflected in the
tests, is how array slicing is picky about whitespace: `.data[1:2]`
would result in a cryptic error message, as the subscript was being
interpreted as an imported module's identifier:
com.schibsted.spt.data.jslt.JsltException: Parse error: Encountered " "]" "] "" at line 1, column 10.
Was expecting:
"(" ...
at <inline>:1:7
(...stacktrace follows...)
With this change, all identifiers (and references to them) are required
to start with a letter or underscore. Dashes are similarly disallowed,
to avoid conflicts with the use of a minus operator now or in the
future.
|
That's well spotted! Thank you. And thank you for doing the implementation as well. Your proposed fix may be the right one, but a possible alternative that's less invasive is to change the syntax of prefixes, so that they cannot start with digits. We'd need to change the definition of Also, we need tests showing that the now-illegal identifers really are flagged as errors in the parser. |
|
A third possibility is to make two breaking changes to the syntax at the same time. The one proposed here, and the rather annoying syntactic ambiguity where the end of a function or As an example, this is OK: But this is not: |
|
Picking up the conversation here again, I happened upon a couple more interesting inputs where allowing identifiers to start with numbers leads to unexpected results, namely due to the way JavaCC disambiguates tokens first by length, and then by order/priority. The following will currently all be interpreted as identifiers in JSLT:
I can look at adding more tests covering cases that should fail. Earlier I was trying to get out a YAML-based JSLT test library we have built internally but it won't necessarily help here, as it is scala-based and might not be appropriate for testing the core language because of the added dependencies. |
First of all, this is a breaking change. JSLT code that uses identifiers
starting with a number will fail to parse. The only redeeming feature of
this change is that this is a common restriction in many programming
languages, which should somewhat limit the risk of mayhem.
The motivation for doing this change, and one which is reflected in the
tests, is how array slicing is picky about whitespace:
.data[1:2]would result in a cryptic error message, as the subscript was being
interpreted as an imported module's identifier:
com.schibsted.spt.data.jslt.JsltException: Parse error: Encountered " "]" "] "" at line 1, column 10.
Was expecting:
"(" ...
at :1:7
(...stacktrace follows...)
With this change, all identifiers (and references to them) are required
to start with a letter or underscore. Dashes are similarly disallowed,
to avoid conflicts with the use of a minus operator now or in the
future.