-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Instantiation expression can be followed by line break or binary operator #49353
Conversation
src/compiler/parser.ts
Outdated
case SyntaxKind.InterfaceKeyword: | ||
case SyntaxKind.LetKeyword: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to cover more than just these cases and have tests for them - for example
class C {
static specialFoo = f<string>
static bar = 123;
}
class D {
public specialFoo = f<string>
public bar = 123;
}
class E {
private specialFoo = f<string>
private bar = 123;
}
class F {
protected specialFoo = f<string>
protected bar = 123;
}
CC @nicolo-ribaudo @evanw @kdy1 the idea of this PR and issue is that an instantiation expression cannot be followed by a newline and some set of strict-mode reserved words. |
Other than strict-mode keywords, you might also want to special-case |
We just discussed the nuances of |
I hate any new "valid JavaScript has different semantics when parsed as TypeScript" rule, but it's so uncommon to see Maybe OT Since you are fine-tuning how instantiation expressions are parsed: it's weird that |
😬 Opened #49362 |
let
or interface
on new line
@typescript-bot test top100 |
Ran out of memory trying to compile angular ☠️ @typescript-bot test top100 |
Let's try again: |
Heya @sandersn, I've started to run the diff-based user code test suite on this PR at 816553a. You can monitor the build here. Update: The results are in! |
@typescript-bot test this |
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 816553a. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at 816553a. You can monitor the build here. Update: The results are in! |
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at 816553a. You can monitor the build here. Update: The results are in! |
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 816553a. You can monitor the build here. |
@ahejlsberg Here they are:Comparison Report - main..49353
System
Hosts
Scenarios
Developer Information: |
Heya @ahejlsberg, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
@ahejlsberg |
All test runs are clean and performance is unaffected. |
Does this PR also change the behavior of new expressions? e.g. new A<B>
C does ASI affect this code? (or as an alternative, it there a playground where I can test it myself? 😅) EDIT: Yes, ASI inserts a semicolon there: https://www.typescriptlang.org/play?ts=4.8.0-dev.20220611#code%2FHYUw7gBAggPAQgPgFAGEg= |
Hi, will this PR be released in the near future? |
This PR adjusts our parsing of instantiation expressions. When a potential type argument list is followed by
(
token,<
or>
that isn't the start of an expression,we consider that construct to be a type argument list. Otherwise we consider the construct to be a
<
relational expression followed by a '>` relational expression.Fixes #49348.
Fixes #49362.