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

Still generate signatures in SkipContextSensitive mode just to match on return types #25937

Merged

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Jul 25, 2018

(during inference).

Fixes #13118

I've kept it minimal - we only dive into the return type if there's a contextual return type whose contextual checking could fix a type parameter before the second, context-sensitive pass. It's worth noting that this has inherited our current slightly flawed definition of a context sensitive function return - namely that only arrow return expressions can be context sensitive (return statements cannot). That TODO is probably worth fixing at some point.

@weswigham weswigham requested review from ahejlsberg and mhegazy July 25, 2018 18:04
if (!getEffectiveReturnTypeNode(node) && hasContextSensitiveReturnExpression(node)) {
const returnType = getReturnTypeFromBody(node, checkMode);
const singleReturnSignature = createSignature(undefined, undefined, undefined, emptyArray, returnType, /*resolvedTypePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false);
return createAnonymousType(node.symbol, emptySymbols, [singleReturnSignature], emptyArray, undefined, undefined);
Copy link
Member Author

@weswigham weswigham Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth caching this type in some way? Like having an internal () => T generic that I simply instantiate here?

@weswigham
Copy link
Member Author

@typescript-bot test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 3, 2018

Heya @weswigham, I've started to run the extended test suite on this PR at e061ea1. You can monitor the build here. It should now contribute to this PR's status checks.

@weswigham
Copy link
Member Author

@typescript-bot test this now that RWC is clean

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 8, 2018

Heya @weswigham, I've started to run the extended test suite on this PR at e061ea1. You can monitor the build here. It should now contribute to this PR's status checks.

@weswigham
Copy link
Member Author

@typescript-bot test this now that RWC is clean and I've merged master 🤦‍♂️

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 8, 2018

Heya @weswigham, I've started to run the extended test suite on this PR at 8b71962. You can monitor the build here. It should now contribute to this PR's status checks.

@weswigham weswigham merged commit f6af618 into microsoft:master Aug 9, 2018
@weswigham weswigham deleted the retain-noncontextual-return-bits branch August 9, 2018 17:17
errendir added a commit to errendir/TypeScript that referenced this pull request Aug 11, 2018
* origin/master: (283 commits)
  Don't error on destructure of private property with computed property syntax (microsoft#26360)
  getDefaultExportInfo: Use `getImmediateAliasedSymbol` instead of `getAliasedSymbol` (microsoft#26364)
  review comments
  restore old algorithm
  Dont use baseURL relative absolute paths in declaration emit, use absolute paths in bundle emit (microsoft#26341)
  Update user baselines (microsoft#26358)
  Don't store @template constraint in a TypeParameterDeclaration node (microsoft#26283)
  fixAddMissingMember: Support interface and don't crash on type parameter (microsoft#25995)
  Don't include class getter in spread type (microsoft#26287)
  Don't crash on computed property in destructure (microsoft#26334)
  Check the ambientness of a symbol name before attempting to trim it (microsoft#26312)
  Still generate signatures in SkipContextSensitive mode just to match on return types (microsoft#25937)
  fix handling if there is no commonPrefix
  Actually add sorting of elaboration text to user baselines
  Ping ryan instead of mohammed for user PRs now
  handle failed lookups
  make it work for root directory
  really, really fix test(?)
  add test
  fix commonPrefix handling
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong inference from function return argument
3 participants