-
Notifications
You must be signed in to change notification settings - Fork 157
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
Object syntax work #958
Merged
Merged
Object syntax work #958
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit fixes, in a fairly myopic way, the most obviously incorrect issues with object slices. In particular: * `o{k: expr}`, if `expr` is not a valid key, will now produce an error instead of compiling to `{k: o[expr]}`. (This does not interfere with the special property logic syntax `o{k: a ? b}`, which still becomes `{k: o.a ? b}`.) Anyone who really wants the other interpretation can explicitly write `o{k: (expr)}`. * Speaking of property logic, attempting to use `xor` as a property operator now also produces an error, as it was clearly excluded from the special list of operators that the AST code knew to handle for this feature despite being treated as such by the parser. * `o{(a.b)}` now correctly accesses `o[a.b]` instead of `o.ref$` (yeah, that was happening!)
This commit is a refactoring that doesn't touch the language grammar (it does touch the interface between the grammar and AST). It prepares for the grammar refactoring that comes next. No language semantics are changed. The big idea here is to have Objs hold only Props (well, and comments), and to pull out any and/or/? logic onto the Prop instead of having it wrap the Prop or the value inside the Prop. Splats in Objs are now Prop(Splat!, contents), and keyless properties are simply Props with a null key. The refactoring also pulls out slice expanding and property shorthand expanding as a distinct pass from the rest of the compilation; this makes them easier to reason about and cleans up the rest of the code a little.
This commit simplifies the parse grammar by permitting arbitrary expressions to sit inside objects and letting the AST sort out the shorthand. As a side effect, this makes `{a.b.c}` no longer a parse error, and so all expressions of the form `{expr.k}` now legally expand to `{k: expr.k}`
The destructuring label syntax `[]:k` only has an effect in a pattern position, like the LHS of an assignment. Previously, such labels were simply ignored outside of patterns; now, throw an explicit syntax error if a value with a label is compiled.
Change the semantics of splats in an object destructuring pattern to mean taking all the keys from the destructuree that have not yet been extracted into this object, instead of importing all subkeys from one specific key on the destructuree. Resolves gkz#941
I'm a little uncomfortable just merging this myself with zero feedback on it, so I'll wait two weeks for comments and, if there are no objections, merge it on May 15. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Okay, here's a big chunk of work all aimed at rationalizing the tangled web of features surrounding object property shorthand, object slices, object destructuring, and the undocumented ‘property logic’ feature which permits
{key: a ? 1} = b
to meana = b.key ? 1
, and similarly forand
,or
,&&
, or||
in place of?
.This PR does contain breaking changes, most of which are defensive (a thing that used to compile but with likely unexpected behavior is now an error). The exception is the behavior requested in #941, which is implemented here.
The individual commits have tests corresponding to each incremental unit of work, but here's a summary of all the language changes:
Were broken and now work
o{(a.b)}
now correctly accesseso[a.b]
instead of... wait for it...o.ref$
o{k: foo ? bar}
now is equivalent to{k: o.foo ? bar}
, just likeo{foo ? bar}
is equivalent to{foo: o.foo ? bar}
.o{k: foo ? bar}
used to mean{k: o[foo ? bar]}
.Were weird and now error
o{k: a.complex.expression}
used to mean{k: o[a.complex.expression]}
, buto{k: foo}
doesn't mean{k: o[foo]}
; it means{k: o.foo}
, so this is inconsistent. Nowo{k: a.complex.expression}
is an error. If you really want the old behavior, you can always writeo{k: (a.complex.expression)}
.{a xor b}
used to parse but complain abouta
andb
not existing. Now it throws a clearer ‘invalid property shorthand’ error.foo.bar = [a, b]:c
, as opposed to[a, b]:c = foo.bar
) never did anything and is now a syntax error.{...x ? y} = z
is now a syntax error. It used to compile tox = ({} <<< z.x) ? y
but the default value never applied, because the LHS of?
was always an object and thus never nully. If Use of splats in object destructuring #941 weren't being implemented, maaaybe it would make sense to change this to meanx = {} <<< (z.x ? y)
, but given that Use of splats in object destructuring #941 is part of this work, even that implementation doesn't make sense, so it's an error.Didn't parse and now do
{a.b!c?key}
is now valid everywhere{a.key}
used to be (key
is the key in both forms).Outright changes to working features
{a, b, ...c} = a: 1 b: 2 c: 3 x: 4
now means roughlya = 1; b = 2; c = c: 3 x: 4
instead ofa = 1; b = 2; c = {} <<< 3
; see Use of splats in object destructuring #941. I hope we can all agree this is a much more obvious behavior.I made an implementation decision here that may be controversial:
{a, b, ...c} = o
doesn't mean the same thing as{a, ...c, b} = o
. In the latter, ifo.b
exists,c.b
will also (whereas it is excluded along witha
in the former). Basically, the splat only excludes keys that precede it. This is symmetric with the behavior of{a, ...c, b}
in value position, wherec.a
can clobbera
, butb
will clobberc.b
. This also means that it's still valid to have multiple splats in an object destructuring: in{a, ...b, c, ...d} = o
,b.c
may exist, butd.c
won't. I don't know if this behavior is useful to anyone, but it's strictly more powerful than the alternative, and unlikely to surprise people who aren't looking for it, since the natural thing to do is to put the splat at the end of the object.