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

Mention "return type" in the error message when return type of a function is not matching the declared type #51449

Open
5 tasks done
mohsen1 opened this issue Nov 8, 2022 · 3 comments
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@mohsen1
Copy link
Contributor

mohsen1 commented Nov 8, 2022

Suggestion

πŸ” Search Terms

return type, error message

βœ… Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Mention "return type" when return type of a function is not matching the declared type

When a parameter type is not matching the declared type, the error message has the words "parameter X" in it which makes it super easy to spot the error. For return types it only says type X is not compatible with type Y but is not clear that it is the return type

πŸ“ƒ Motivating Example

Playground

const func: (a: string, very: number, confusing: string, error: boolean, message: string) => string = 
    (a: string, very: number, confusing: string, error: boolean, message: string) => {
        return 1
    }

Error:

Type '(a: string, very: number, confusing: string, error: boolean, message: string) => number' is not assignable to type '(a: string, very: number, confusing: string, error: boolean, message: string) => string'.
  Type 'number' is not assignable to type 'string'.(2322)

πŸ’» Use Cases

This can get tricky in very large and complex types. Just mentioning the word "return" can save a ton of time debugging issues

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Help Wanted You can do this Experience Enhancement Noncontroversial enhancements labels Nov 8, 2022
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Nov 8, 2022
@RyanCavanaugh
Copy link
Member

If someone wants a good project, I think any time we have

X is not assignable to Y
  A is not assignable to B

there's a "missing" elaboration:

X is not assignable to Y
  (Some explanatory text of why we're talking about A and B)
    A is not assignable to B

@bakkot
Copy link
Contributor

bakkot commented Nov 9, 2022

Drive-by comment since I was just looking at this code: there actually is already a diagnostic for this, which you can trigger with this example I've reduced from a test.

It's just getting suppressed, or somehow not getting triggered, in other cases. Possibly it has to do with the elidedInCompatabilityPyramid attribute which is true only for those specific return-type diagnostic messages (and no others), or with the logic in reportIncompatibleStack which switches on those diagnostics explicitly? I don't have any insight there; I just wanted to mention the existence of those diagnostics.

@gmanninglive
Copy link
Contributor

This is interesting, a little investigation and the Diagnostics @bakkot mentions were suppressed in #33473

Removing the "elidedInCompatabilityPyramid": true attribute gives a more readable message and doesn't cause the long pyramid on the example in #33361

I feel like these 4 messages could be flattened into one simpler message

"Call signature return types '{0}' and '{1}' are incompatible.": {
        "category": "Error",
        "code": 2202,
        "elidedInCompatabilityPyramid": true
    },
    "Construct signature return types '{0}' and '{1}' are incompatible.": {
        "category": "Error",
        "code": 2203,
        "elidedInCompatabilityPyramid": true
    },
    "Call signatures with no arguments have incompatible return types '{0}' and '{1}'.": {
        "category": "Error",
        "code": 2204,
        "elidedInCompatabilityPyramid": true
    },
    "Construct signatures with no arguments have incompatible return types '{0}' and '{1}'.": {
        "category": "Error",
        "code": 2205,
        "elidedInCompatabilityPyramid": true
    },

especially as the with no arguments variants are already merged in the checker...

  if (msg.code === Diagnostics.Call_signatures_with_no_arguments_have_incompatible_return_types_0_and_1.code) {
                                mappedMsg = Diagnostics.Call_signature_return_types_0_and_1_are_incompatible;
                            }
                            else if (msg.code === Diagnostics.Construct_signatures_with_no_arguments_have_incompatible_return_types_0_and_1.code) {
                                mappedMsg = Diagnostics.Construct_signature_return_types_0_and_1_are_incompatible;
                            }

Also there is another related diagnostic

    "The expected type comes from the return type of this signature.": {
        "category": "Message",
        "code": 6502
    },

That seems to trigger with an implicit return, but not with explicit return or if redefining the type on the args

Playground example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants