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

Prohibiting whitespace after @ #430

Open
js-choi opened this issue Aug 31, 2021 · 16 comments
Open

Prohibiting whitespace after @ #430

js-choi opened this issue Aug 31, 2021 · 16 comments

Comments

@js-choi
Copy link

js-choi commented Aug 31, 2021

Spinning this off from a discussion on Matrix TC39 General (cc @jmdyck, @ljharb, @tabatkins):

The proposal’s specification has the following productions:

  • Decorator : @ DecoratorMemberExpression
  • Decorator : @ DecoratorCallExpression

These currently seem to allow whitespace after @, so that the following are allowed:

@      foo class C {}
@/* comment */class C {}
@ 


foo class C {}

This is probably undesirable. The @ is “clearly” part of the identifier. If any developer wants the decorator expression on a new line, they could use @().

So what we could do is define a new syntactic annotation “[contiguous]” in the Syntactic Grammar clause:

If the phrase “[contiguous]” appears in the right-hand side of a production of the syntactic grammar, it indicates that the production may not be used if there are any discarded tokens (i.e., simple white space, single-line comments, and any |MultiLineComment| that contains no line terminator) at the indicated position. In other words, a production with “[contiguous]” may be used only if the production immediately preceding the indicated position is, in the original source text, contiguous with the production immediately following the indicated position.

…and redefine those productions as:

  • Decorator : @ [contiguous] DecoratorMemberExpression
  • Decorator : @ [contiguous] DecoratorCallExpression

The Hack-pipes proposal will probably use such a “[contiguous]” annotation too for its own purposes (see tc39/proposal-hack-pipes#13 (comment)).

@jmdyck
Copy link

jmdyck commented Sep 4, 2021

If the phrase “[contiguous]” appears in the right-hand side of a production of the syntactic grammar, it indicates that the production may not be used if there are any discarded tokens (i.e., simple white space, single-line comments, and any |MultiLineComment| that contains no line terminator) at the indicated position.

What you refer to as "discarded tokens" aren't tokens, though they are input elements.

In other words, a production with “[contiguous]” may be used only if the production immediately preceding the indicated position is, in the original source text, contiguous with the production immediately following the indicated position.

I believe the latter 2 uses of "production" are mis-uses. E.g., for
Decorator : `@` [contiguous] DecoratorMemberExpression
do you intend that

  • "the production immediately preceding the indicated position" is @, and
  • "the production immediately following the indicated position" is DecoratorMemberExpression?

Because neither is a production, they are grammar symbols.

@js-choi
Copy link
Author

js-choi commented Sep 4, 2021

@jmdyck: You’re right regarding both “tokens” versus “input elements”, as well as “productions” versus “grammar symbols”. (And I meant for the symbol “immediately preceding the indicated position” to be @.)

This might be a moot point anyway. @bakkot left a comment in tc39/ecmarkup#356 (comment) about an alternate approach for decorators (as well as for ^= vs. ^== in Hack pipes):

That said, I'm not convinced this is the right approach to solve the problems you've highlighted.

For decorators, we ought to be able to just define a Decorator token along the lines of @IdentifierName. This would be a single token, like PrivateName, and would therefore not allow whitespace.

@nicolo-ribaudo
Copy link
Member

I'm not convinced that @ is clearly part of the identifier. If I have a decorator like @foo class A {}, I'm invoking the foo decorator function (declared with function foo() {}) and not a function named @foo.

They are different from private names, where you can never see the name without # (unless it refers to a different thing, obviously).

@js-choi
Copy link
Author

js-choi commented Sep 4, 2021

@nicolo-ribaudo: Does that mean you think that there should be whitespace allowed between @ and the identifier, or that you think that @ and the identifier should not be treated as one token?

@nicolo-ribaudo
Copy link
Member

Oh I forgot to reply.

I think we should permit a space between @ and the identifier. @ and the identifier are two separate things:

  • The identifier is a reference to the decorator function. The function is not named @foo but just foo.
  • The @ is an "operator" which applies the foo function to a class (or a class element) in a specific way.

It's even more clear that @ and foo are two separate things when we consider member expressions:

@foo.bar
class A {}

First you evaluate foo, then you get foo.bar and then (not right after foo) you apply the @.

I think the whitespace here is just a stylistic choice and in practice almost no one will write it, similarly to how almost no one writes +foo when using the + unary operator. However, if we really want to disallow the whitespace after @, we should also disallow it in DecoratorMemberExpression: being able to write @foo . bar but not @ foo.bar makes it feel like @ has an higher precedence than ..

@ljharb
Copy link
Member

ljharb commented Sep 22, 2021

To me, it makes it feel like it has higher importance, which it does :-)

In practice, everything where there's a stylistic choice either ends up in one of three places:

  1. everyone agrees on one idiom, after many expensive years of linting and styleguides and PR debates
  2. everyone agrees on one of two or three idioms, after many expensive years of linting and styleguides and PR debates, but these debates continue because of the different idioms
  3. nobody agrees, and we have an eternity of expensive years of linting and styleguides and PR debates.

The fourth option is "the language selects one, nobody has any choice, and nobody has to spend time or effort deciding on one". JS has traditionally not selected this option, but that doesn't mean it wouldn't have been, and isn't still, the better choice.

I hope we break with tradition and make the better choice for decorators, avoiding a whole bucket of timelines full of style debates.

@bakkot
Copy link

bakkot commented Sep 22, 2021

In practice, everything where there's a stylistic choice either ends up in one of three places:

No, there's also "everyone agrees on one idiom immediately". For example, it is possible to put two spaces between the function and the f in function f() {}, but approximately no one does that. Having the language restrict FunctionDeclaration to require exactly one space in that position would be to no one's benefit.

Conversely, having the language impose such arbitrary restrictions has real cost for tooling, and can limit flexibility in annoying ways.

The language is not the right place to impose stylistic preferences.

@ljharb
Copy link
Member

ljharb commented Sep 22, 2021

What cost for tooling? parser complexity?

@bakkot
Copy link

bakkot commented Sep 22, 2021

Parser complexity is a small part, yes, but it also means that, for example, there are more incorrect-formatted programs which Prettier will reject outright instead of being able to clean up (unless they implement and use a parser with error recovery, which is hard to do right and requires even more work), and code generators need to know they can't insert a space there (which is annoying if, e.g., your code generator tries to avoid outputting strings which look like email addresses), and so on.

@hax
Copy link
Member

hax commented Mar 29, 2022

Just found this issue :)

I asked the question in the discussion of hack-style topic token one hour ago, and I strongly prefer prohibit whitespaces after @ which would help to differentiate the topic token and decorator usage.

@pzuraq
Copy link
Collaborator

pzuraq commented Mar 31, 2022

We discussed this at plenary, and decided that it did not make sense to fully restrict whitespace after the @ here. However, there is still ongoing discussion on whether or not we should allow line terminators specifically.

@tabatkins
Copy link

Allowing whitespace between the @ and the name would complicate other usages of @ in the language, as seen in tc39/proposal-pipeline-operator#91 (comment).

@js-choi
Copy link
Author

js-choi commented Mar 31, 2022

More specifically, allowing whitespace after @ would interfere with future infix operators that are also identifiers (like as).

I don’t think it would be fatal if @ decorator class {} was allowed—just slightly annoying to specify in the future.

We would just keep parsing @ as class {} or @ as function () {} as decorated expressions, not as as infix operations on the topic, with some lookahead. (And developers should not be putting decorated expressions in pipe bodies anyway!)

See tc39/proposal-pipeline-operator#91 (comment).

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Mar 31, 2022

More specifically, allowing whitespace after @ would interfere with future infix operators that are also identifiers (like as).

@js-choi How does the space make a difference? If as was a valid infix operator and @ as b worked, I would expect @as b to work too (similarly to how both @ instanceof b and @instanceof b work).

@js-choi
Copy link
Author

js-choi commented Mar 31, 2022

@nicolo-ribaudo: The idea would be that @as x would be generally invalid that the topic reference must be separated from text words. @in x would probably be invalid too.

Having said that, I personally think both v |> @as x (as well as v |> @in x) is fine as valid syntax (though they ought to be linted).

In any case, though, I don’t think it’s a big problem for @ as topic reference if @ decorator class {} is valid.

@hax
Copy link
Member

hax commented May 24, 2022

From UX perspective, if we have both topic ref and decorators, we should make the syntax easy to differentiate. Even parsers can differ them with special rules (still very hard), it's too hard for average programmers to understand the rules and eye parse the code.

Note, in current @deco usage, most decorator users always treat @ as sigil (just like #priv) instead of prefix unary operators. Restricting the syntax will not harm to UX of decorators. But adding complex rules for topic syntax (or change @ to some other tokens) significantly harm to UX of topic ref.

Similarly I suggest we also change the escape hatch syntax to @{expr}, so @(expr) would always parse as calling topic ref.

Disallowing whitespace after @, plus changing escape hatch syntax to @{expr} solve all the (current and future possible) conflicts with decorators and topic ref.

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

No branches or pull requests

8 participants