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

Prefer elaborating on expressions which could be called to produce a correct type by suggesting such #27016

Merged
merged 2 commits into from
Sep 11, 2018

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Sep 11, 2018

didyoumean

Fixes #25308

(Note that this adds the messages, not a potential quickfix. A quickfix should be more narrow than the suggestion, as only a 0-argument call/construct has an obvious quickfix)

@weswigham
Copy link
Member Author

cc @DanielRosenwasser

const callSignatures = getSignaturesOfType(source, SignatureKind.Call);
const constructSignatures = getSignaturesOfType(source, SignatureKind.Construct);
for (const signatures of [constructSignatures, callSignatures]) {
if (length(signatures)) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you need the length() call with some?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not.

@@ -10585,6 +10585,9 @@ namespace ts {

function elaborateError(node: Expression | undefined, source: Type, target: Type): boolean {
if (!node || isOrHasGenericConditional(target)) return false;
if (!isTypeAssignableTo(source, target) && elaborateDidYouMeanToCallOrConstruct(node, source, target)) {
Copy link
Member

Choose a reason for hiding this comment

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

This should ideally be cheap since the original call will have some of the relationships cached, right?

Also, technically I think you should be using the original relationship, not assignability, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and yes.

return !(returnType.flags & (TypeFlags.Any | TypeFlags.Never)) && isTypeAssignableTo(returnType, target);
})) {
const resultObj: { error?: Diagnostic } = {};
checkTypeAssignableTo(source, target, node, /*errorMessage*/ undefined, /*containingChain*/ undefined, resultObj);
Copy link
Member

Choose a reason for hiding this comment

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

TIL errorOutputContainer

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I added that awhile ago. Not the happiest about it, but a side channel's a side channel, since most callers just need the boolean return.

checkTypeAssignableTo(source, target, node, /*errorMessage*/ undefined, /*containingChain*/ undefined, resultObj);
const diagnostic = resultObj.error!;
addRelatedInfo(diagnostic, createDiagnosticForNode(
node,
Copy link
Member

Choose a reason for hiding this comment

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

Kind of funny that the "related span" is the same span.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's still probably the best place to put it. Wanna do something else?

@weswigham weswigham merged commit 1c13792 into microsoft:master Sep 11, 2018
"category": "Message",
"code": 6212
},
"Did you mean to use `new` with this expression?": {
Copy link
Member

Choose a reason for hiding this comment

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

new [](start = 25, length = 5)

Elsewhere, "new" is in single quotes.

@weswigham weswigham deleted the give-me-a-ring branch September 14, 2018 20:51
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.

"Did you forget to call 'X'?" for assignability errors
3 participants