Explicitly typed special assignments are context sensitive #25619
Conversation
This file contains hidden or 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
Explicitly typed special assignments should be context sensitive if they have an explicit type tag. Previously no special assignments were context sensitive because they are declarations, and in the common, untyped, case we inspect the right side of the assignment to get the type of the left side, and inspect the right side of the assignment to get the type of the left side, etc etc. Note that some special assignments still return `any` from checkExpression, so still don't get the right type. Fixes #25571
ghost
approved these changes
Jul 12, 2018
|
|
||
| // this-property assignment | ||
| class Thing { | ||
| constructor() { |
There was a problem hiding this comment.
oops, cut-and-paste from the issue.
sandersn
deleted the
js/explicitly-typed-special-assignments-are-context-sensitive
branch
sandersn
added a commit
that referenced
this pull request
Jul 16, 2018
Follow up to #25619: Add the necessary code to type `prototype` correctly in prototype assignments so that code like `F.prototype = { ... }` properly makes the object literal context sensitive.
sandersn
added a commit
that referenced
this pull request
Jul 16, 2018
* Explicitly typed prototype assignments:ctx sensitive Follow up to #25619: Add the necessary code to type `prototype` correctly in prototype assignments so that code like `F.prototype = { ... }` properly makes the object literal context sensitive. * Fix lint
sandersn
added a commit
that referenced
this pull request
Jul 20, 2018
* Revert "Revert "Explicitly typed special assignments are context sensitive (#25619)"" This reverts commit 16676f2. * Revert "Revert "Explicitly typed prototype assignments are context sensitive (#25688)"" This reverts commit ff8c30d. * Initial, wasteful, solution It burns a check flags. Probably necessary, but perhaps not. I haven't accepted baselines, but they are a bit questionable. I'm not sure the synthetic type is right, because I expected to see { "exports": typeof import("x") } but instead see { "x": typeof import("x") }. * Update some baselines * module.exports= always uses lhs type Conflicts between exports property assignments and exports assignments should get a union type instead of an error. * Fix lint and accept good user baselines * Add tests based on user tests. These currently fail. * Fix all but 1 of user test bugs found by typing module.exports Still quite messy and full of notes * Follow merged symbols+allow any object type This allows exports like `module.exports = new EE` to have properties added to them. Still messy, but I'm going to run user tests and look for regressions. * Update improved user baselines * Fix infinite recursion when checking module.exports * Fix bogus use-before-def error getExportSymbolOfValueSymbolIfExported should always merge its returned symbol, whether it's symbol.exportSymbol or just symbol. * Update user test baselines * Cleanup * More small cleanup * Bind `module` of `module.exports` as a special symbol Previously it was also special, but created during name resolution in the checker. It made sense when there was only one special symbol for all files, but now there is one `module` symbol per file.
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.
Explicitly typed special assignments should be context sensitive if they have an explicit type tag. Previously no special assignments were context sensitive because they are declarations, and in the common, untyped, case we inspect the right side of the assignment to get the type of the left side, and inspect the right side of the assignment to get the type of the left side, etc etc.
Note that some special assignments still return
anyfrom checkExpression, so still don't get the right type. Those test cases have a noImplicitAny error in the new test I added. I will look at improving type checking ofmodule.exportsandX.prototypenext.Fixes #25571