Skip to content

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.


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
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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