Contextually type parenthesized expressions #1628
Conversation
Preserve const enums in typescriptServices.js
…d template expressions.
…late expressions.
Fixed contextual type resolution and type checking for tagged template expressions.
Add internal definitions to a diffrent .d.ts files
Fix issue #1503 with modules and imports sharing a name
Switch order of switch cases in parsePrimaryExpression
…source.
Previously we would just treat each merge marker as trivia and then continue
scanning and parsing like normal. This worked well in some scenarios, but
fell down in others like:
```
class C {
public foo() {
<<<<<<< HEAD
this.bar();
}
=======
this.baz();
}
>>>>>>> Branch
public bar() { }
}
```
The problem stems from the previous approach trying to incorporate both branches of the merge into
the final tree. In a case like this, that approach breaks down entirely. The the parser ends up
seeing the close curly in both included sections, and it considers the class finished. Then, it
starts erroring when it encounters "public bar()".
The fix is to only incorporate one of these sections into the tree. Specifically, we only include
the first section. The second sectoin is treated like trivia and does not affect the parse at all.
To make the experience more pleasant we do *lexically* classify the second section. That way it
does not appear as just plain black text in the editor. Instead, it will have appropriate lexicla
classifications for keywords, literals, comments, operators, punctuation, etc. However, any syntactic
or semantic feature will not work in the second block due to this being trivia as far as any feature
is concerned.
This experience is still much better than what we had originally (where merge markers would absolutely)
destroy the parse tree. And it is better than what we checked in last week, which could easily create
a borked tree for many types of merges.
Now, almost all merges should still leave the tree in good shape. All LS features will work in the
first section, and lexical classification will work in the second.
…with a non-empty literal. Fixes #1570
…asses Allow uninstantiated modules before clodules
consider type parameters always visible
Moved EmitHost to utilities.ts so that it can be edited alone, and to fix resulting declaration file
Don't emit an empty template head literal if there's a template span with a non-empty literal.
extract map copying logic to a separate function
… case for tagged templates.
DanielRosenwasser
changed the title
|
I'm a bit confused by this. Isn't this a fairly big breaking change? The language decision was, up front, that parentheses would turn off contextual typing. With this change users may have code that now errors where it didn't before. |
|
Yes, issue #920 that this is fixing notes this is a breaking change, adding the tag for clarity. |
|
This break definitely concerns me. Specifically, because it not just a break that occurs because we're deciding to change some off-hand part of the language, but because we're changing a part of the language we explicitly spec'ed as as a means to opt out of contextual typing. i.e. we told users "if you don't want contextual typing, here's how you get out of it." and we're now taking that away. |
|
Yup, I agree that it's something that we should discuss at a design meeting. But I'll flat out admit that I put out this PR so that we could actually discuss it. The work is here, it's no longer an issue of getting around to it, just of deciding whether or not it's an appropriate change. As a side note, we do mention it exactly once in the spec. We don't mention it at all in the TypeScript handbook. It's more likely that people learn about this "feature" by shooting themselves in the foot than by our documentation. |
|
It was already discussed at the design meeting (see the comments in #920). Casting to |
|
Actually, @RyanCavanaugh is right, we discussed this several weeks ago, my memory didn't serve me correctly. Also, I think Github is having a caching issue of some sort; if I try to create a new PR from this branch to master, it will only show the last 4 commits (d5f0281 to 22174a1). I'll try updating this branch with a new commit of some sort, and if not I'll open up a different PR. |
|
Can you resend this PR with the change by itself? |
This PR addresses #920.