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

this-property assignments should use autoType to get control flow narrowing #37900

Closed
sandersn opened this issue Apr 10, 2020 · 3 comments · Fixed by #37920
Closed

this-property assignments should use autoType to get control flow narrowing #37900

sandersn opened this issue Apr 10, 2020 · 3 comments · Fixed by #37920
Labels
checkJs Relates to checking JavaScript using TypeScript Domain: JavaScript The issue relates to JavaScript specifically Domain: This-Typing The issue relates to providing types to this In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@sandersn
Copy link
Member

Based on discussion in #37827, we could further improve typing of this-property assignments if we used autoType whenever we can't tell what type they should have.

This has three open questions:

  1. Performance.
  2. Is there benefit in real code? This only helps if (1) we aren't getting the right type some of time where (2) the auto type would get the right type. This is likely to be when a property is used repeatedly in a long method.
  3. What changes are actually needed. Just returning autoType doesn't work, because control flow narrowing doesn't seem to track this-property assignments as references.
@sandersn
Copy link
Member Author

implementation hiccup number 1 is that a union with auto turns into any, not auto.

@ahejlsberg
Copy link
Member

I think you could do this by utilizing logic similar to that in checkPropertyInitialization where we check that properties are definitely assigned in the constructor. In particular, the control flow walk we do in isPropertyInitializedInConstructor where we start from a virtual flow node representing all constructor return points. If you were to pass in autoType for the declared type and undefinedType for the initial type, the result should be a CFA'd aggregate of the possible assigned values upon return from the constructor. You'd furthermore have to special case this.xxx property access expressions inside the constructor to similarly use autoType and undefinedType for declared and initial types (which would allow this.x = this.x + ... without circularity errors). But for references outside the constructor you would rely on the CFA computed type.

@ahejlsberg
Copy link
Member

@sandersn I've put up an experimental prototype of this in #37920.

@DanielRosenwasser DanielRosenwasser added checkJs Relates to checking JavaScript using TypeScript Domain: JavaScript The issue relates to JavaScript specifically Domain: This-Typing The issue relates to providing types to this In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Apr 13, 2020
@ahejlsberg ahejlsberg added this to the TypeScript 4.0 milestone Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
checkJs Relates to checking JavaScript using TypeScript Domain: JavaScript The issue relates to JavaScript specifically Domain: This-Typing The issue relates to providing types to this In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants