Improve error position/range for arrow functions with expression body

[OliverJAsh]

🔍 Search Terms

• arrow function
• return type
• type error position/range

✅ Viability Checklist

• 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 isn't a request to add a new utility type: https://github.com/microsoft/TypeScript/wiki/No-New-Utility-Types
• This feature would agree with the rest of our Design Goals: https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

⭐ Suggestion

For arrow functions with an expression body, the position of the return type error could be adjusted so it doesn't cover the entire expression. This would make it easier to identify more meaningful type errors that exist within the expression.

📃 Motivating Example

Consider the following example where we're using the pipe function inside arrow functions and the add function is missing.

declare const pipe: <A, B, C>(a: A, ab: (a: A) => B, bc: (b: B) => C) => C;

// declare const add: (x: number) => (n: number) => number;
declare const identity: <T>(x: T) => T;

// Arrow function with block body
const f = (): number => {
  return pipe(1, add(1), identity);
};

// Arrow function with expression body
const g = (): number => pipe(1, add(1), identity);

Both of these arrow functions have two type errors:

• For the return value: Type 'unknown' is not assignable to type 'number'.
• Cannot find name 'add'.

The first type error can be fixed by addressing the second type error.

When the arrow function has a block body, these two error messages are highlighted separately:

However, when the arrow function has an expression body, the range of the return type error covers the entire expression:

This makes it very difficult to spot the inner type error for Cannot find name 'add', especially in more advanced examples.

To help with this I was wondering if we could move the position of the return type so it doesn't cover the entire expression. Perhaps it could be positioned on the => that appears immediately before the expression? This would be closer to the behaviour of arrow functions with body blocks.

Note this issue does not occur when we're not using the pipe function. I believe this is because TypeScript treats the return type of add(1) as any, whereas with pipe the type argument B will be inferred as unknown (which is desired in other cases).

const g2 = (): number => identity(add(1)(1));

💻 Use Cases

See above.

[RyanCavanaugh]

Seems reasonable. If a return type annotation exists, we can put it there

// Arrow function with expression body
// Today
const g1: () => number = () => pipe(1, add(1), identity);
//                             ~~~~~~~~~~~~~~~~~~~~~~~~~
// Proposed
const g2: () => number = () => pipe(1, add(1), identity);
//                          ~~
// Or (?)
const g3 = (): number => pipe(1, add(1), identity);
//             ~~~~~~

Aside: Try to guess where the error span is in this code! 🫠

const p: () => string = (): number => 32;

[DanielRosenwasser]

I kind of like moving it to the return type itself, but either way I feel like we should adjust the error message in these cases if possible. Something like

This function returns the type 'unknown' which is not assignable to its declared type 'number'.

[fatcerberus]

I recall that I once suggested putting the error span on the => for this exact reason, by analogy to the return in the block function. IIRC @Andarist objected.

[Andarist]

The mentioned discussion happened here. I don't mind this change if the team likes it. My main problem with it is that it's a pretty short error span but OTOH you can end up with even single-character error spans anyway so that concern can be ignored.

It should be OK - especially with the adjusted error message like the one proposed by @DanielRosenwasser .

I'm pretty sure I'll this change, as I've also mentioned in that same comment 😉 :

I prefer to highlight smaller spans over huge ones

[OliverJAsh]

I would like to raise a PR for this change because I think it will significantly improve DX. Can anyone guide me to where in the code I need to make the change? Hopefully I can figure it out from there.

[Andarist]

checkFunctionExpressionOrObjectLiteralMethodDeferred passed down effectiveCheckNode to checkTypeAssignableToAndOptionallyElaborate both as the checked expression and the error node. The error node reaches getErrorSpanForNode and that's where the error span gets computed.

One way to fix this would be to pass node (aka the whole arrow function) as errorNode to checkTypeAssignableToAndOptionallyElaborate and then tweak the logic in getErrorSpanForArrowFunction. As it is, this wouldn't fix the issue because getErrorSpanForArrowFunction highlights the first line... but in your case that's the whole thing ;p

[OliverJAsh]

Thanks @Andarist but I think there may be a misunderstanding. If I'm not mistaken, getErrorSpanForArrowFunction describes error spans for arrow functions as a whole, whereas what I'm interested in here is just the span of the return expression.

I just found this PR (for #52934) of yours which fixed the issue for arrow functions with block bodies. All I want to do here is make the same change for arrow functions with expression (non-block) bodies.

I've got a WIP PR here, but I might need some more guidance.