Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Semi-colons required? #25

Closed
jeffshaver opened this issue Dec 21, 2015 · 21 comments
Closed

Semi-colons required? #25

jeffshaver opened this issue Dec 21, 2015 · 21 comments

Comments

@jeffshaver
Copy link

Are semi-colons required per the syntax?

class C {
  static prop = 'a';
}
// or is this ok too?
class C {
  static prop = 'a'
}

The reason I ask is that up until today, I had been omitting the semi-colon. I write without semi-colons to begin with, but it seemed to make complete sense since classes don't use semi-colons or commas for its regular syntax. However, I started using Flow today with React and it keeps erroring out saying there is a syntax error. It goes away if I use the semi-colon like first part in the example above.

If the syntax requires it, than I am ok with that I guess. But it does seem out of place and I don't know why you would need the semi-colon.

@jeffmo
Copy link
Member

jeffmo commented Dec 21, 2015

Currently we require semicolons to avoid unbounded lookahead issues during parsing (a requirement TC39 has enforced for JS so far for engine performance reasons). For example:

class Foo { bar=baz [qux]() { ... } }

(Don't know whether to interpret this as bar = baz[qux] vs bar = baz; [qux]() { ... } until the lexer gets all the way past the qux expression in brackets and hits a paren).

@jeffmo jeffmo closed this as completed Dec 21, 2015
@michaelficarra
Copy link
Member

Actually, I think it would have to go all the way to the opening curly brace because the parentheses can form both the argument list of a call expression and the parameter list of a method (for most parameter lists).

@jeffmo
Copy link
Member

jeffmo commented Dec 22, 2015

Yes, that's right

@rstacruz
Copy link

rstacruz commented Jan 7, 2016

wouldn't a semicolon not be needed in this case, then?

class C {
  static prop = 'a'
  start () {
  }
}

@allenwb
Copy link
Member

allenwb commented Jan 7, 2016

Isn't this all covered by the Automatic Semicolon Insertion Rules? How is a missing semicolon here any different form any other context? If (for some reason) you didn't want ASI to happen the grammar rules for static fields would have to include an explicit [no LineTemerator here] annotation.

@michaelficarra
Copy link
Member

@allenwb Yes, semicolons should be automatically inserted for LiteralPropertyNames like the one mentioned by @rstacruz. The grammar will still contain a requirement for a semicolon because of ComputedPropertyNames, but they won't always need to be in the source text.

@mjackson
Copy link

mjackson commented Jan 8, 2016

@jeffmo Is class Foo { [qux]() { ... } } already a thing? Seems like that's the real issue here.

@michaelficarra
Copy link
Member

It is already a thing.

@jmm
Copy link

jmm commented Jan 8, 2016

@allenwb The list of statements in 11.9 is just informative, correct? In other words, if the list weren't there the remaining text would fully describe the rules?

@allenwb
Copy link
Member

allenwb commented Jan 8, 2016

Yes, that list is only informative.

@jmm
Copy link

jmm commented Jan 8, 2016

@allenwb Ok, thank you. Hmm, it does seem to me like you all are right about ASI handling this in the unambiguous cases. In the earlier example, if you don't disambiguate it with a literal semicolon, then it just winds up a syntax error, right? Perhaps @jeffmo has some trickier case in mind that I can't think of?

@jeffmo
Copy link
Member

jeffmo commented Jan 8, 2016

Yea, I think you might be right @jmm. I went in with this restriction with the mindset of creating an all-cases-work grammar, but I suppose if we don't require semicolons then only some cases don't work (and an optional semicolon becomes required only in those cases).

I will follow up here next week and plan to give an update on this at the coming TC39 meeting at the end of January.

@jmm
Copy link

jmm commented Jan 8, 2016

Thanks @jeffmo! Just to be clear, the earlier commenters were the ones pointing this out and I've just been reading their comments and the spec to try to make sure I understand it, and it seems to me that they're correct.

I went in with this restriction with the mindset of creating an all-cases-work grammar, but I suppose if we don't require semicolons then only some cases don't work (and an optional semicolon becomes required only in those cases).

Yeah I know what you mean. I'm not a big fan of ASI. Like @allenwb said, I think the only way you could impose a restriction here would be something like:

PropertyName = AssignmentExpression [no LineTerminator here] ;

(If that's even valid -- I'd have to re-read the rules some more :/)

@allenwb
Copy link
Member

allenwb commented Jan 8, 2016

@jmm
The no LineTerminator annotation would be valid, but it probably wouldn't make semicolon-first people very happy. I think the best approach is to simply require the semicolon and let ASI to its work. The cases that ASI can't fix will require an explicit semi in the code.

@jmm
Copy link

jmm commented Jan 8, 2016

Thanks @allenwb! I agree it'd be weird to do that. I only mentioned it to illustrate, in case it wasn't obvious from your earlier comment how explicit you would need to be to pre-empt ASI and make it a requirement of the grammar. E.g. per babel/babel#3225 (comment).

@jeffshaver
Copy link
Author

I'm also totally fine with only requiring them in the same ways that js already does. i.e. line beginning in [ and some others. I think that makes the most sense.

@max-mapper
Copy link

The cases that ASI can't fix will require an explicit semi in the code

Does anyone have a list of these? I'm just curious what all the new cases are being introduced are

@michaelficarra
Copy link
Member

@maxogden You will have to use an explicit parenthesis before computed property names and before generator methods.

class A {
  b; // <-- this semi is necessary
  [c](){}
  d; // <-- and so is this one
  *e(){}
}

edit: I made an erroneous last-second edit before posting. See below for correction.

@allenwb
Copy link
Member

allenwb commented Jan 10, 2016

class A {
  b; // <-- this semi is necessary
  [c](){}

Nope. A semi-colon will be automatically inserted in the above case. Consulting the enhanced ClassElement grammar we can see that b [c] is not a valid head of any ClassElement. b is a valid head, but it must be followed by one of ;, =, or ( . [ is not in the follow set so it is an “offending token” according to the ASI specification . That offending token is separated from the previous token by a LineTerminator, According to ASI rule 1, a ; will be inserted before the offending token.

  d; // <-- and so is this one
  *e(){}

Nope, again. In this case the offending token is *.

}

The only time that a explicit semicolon would be required is when you have something like:

prop1 = x ; //<-- this semi is necesary
[computedPropName]
static prop2 = y ; //<-- and so is this one
[1](){}
}

x[ and y[ are valid heads of an AssignmentExpression and hence ASI is not invoked. This is comparable to what happens in a StatementList for situations like:

x = y // <-- ASI will not put a semi here
[0]

@max-mapper
Copy link

@allenwb that makes sense, thank you!

@michaelficarra
Copy link
Member

Haha oops, I initially had those as initialisers, but then removed them to simplify and forgot it changed how it parses. Here's what I meant:

class A {
  b = b; // <-- this semi is necessary
  [c](){}
  d = d; // <-- and so is this one
  *e(){}
}

Sorry about that @maxogden.

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

No branches or pull requests

8 participants