Skip to content

Fixed error spans for SatisfiesExpression check nodes #56918

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 19, 2024
Merged

Fixed error spans for SatisfiesExpression check nodes #56918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b795c92
Fixed error span for argument checks when the argument is a `Satisfie…
Andarist Jan 1, 2024
13c2882
add extra test cases
Andarist Jan 1, 2024
e0c2268
Fixed arrow functions
Andarist Jan 1, 2024
ffc7426
fix the same thing for tuple elements
Andarist Jan 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,7 @@ import {
isRightSideOfQualifiedNameOrPropertyAccess,
isRightSideOfQualifiedNameOrPropertyAccessOrJSDocMemberName,
isSameEntityName,
isSatisfiesExpression,
isSetAccessor,
isSetAccessorDeclaration,
isShorthandAmbientModuleSymbol,
Expand Down Expand Up @@ -20542,7 +20543,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const elem = node.elements[i];
if (isOmittedExpression(elem)) continue;
const nameType = getNumberLiteralType(i);
yield { errorNode: elem, innerExpression: elem, nameType };
const checkNode = getEffectiveCheckNode(elem);
yield { errorNode: checkNode, innerExpression: checkNode, nameType };
}
}

Expand Down Expand Up @@ -33939,6 +33941,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was looking for other solutions that wouldn't explicitly target argument nodes. However, I concluded that this is quite a specific situation because it's the only situation (that I could find) when the SatisfiesExpression node is also the location on which other errors (argument checks) are meant to be reported. Usually the "final target" (like an assignment target) of SatisfiesExpression is different so there is no such overlap~ like here.

Copy link

Choose a reason for hiding this comment

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

Usually the "final target" (like an assignment target) of SatisfiesExpression is different so there is no such overlap~ like here.

Counterpoint:

((): null => ({}) satisfies unknown)();

Copy link

Choose a reason for hiding this comment

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

Food for thought: If the arrow function above is rewritten with braces, then the error span is correctly placed on the return. For braceless arrow functions the "final target" could be the => token.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Counterpoint:

Great example :P I'll fix this in a moment

For braceless arrow functions the "final target" could be the => token.

I prefer to highlight smaller spans over huge ones but this one feels just too short. OTOH, an elementwise elaboration can highlight even a single character if a property is that short. I'm not sure if it would be completely apparent to everybody that highlighted => is about the return value. It's not that it doesn't make sense when explained - it's just that in isolation it doesn't feel completely intuitive/unambiguous

function getEffectiveCheckNode(argument: Expression): Expression {
argument = skipParentheses(argument);
return isSatisfiesExpression(argument) ? skipParentheses(argument.expression) : argument;
}

function getSignatureApplicabilityError(
node: CallLikeExpression,
args: readonly Expression[],
Expand Down Expand Up @@ -33982,7 +33989,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// we obtain the regular type of any object literal arguments because we may not have inferred complete
// parameter types yet and therefore excess property checks may yield false positives (see #17041).
const checkArgType = checkMode & CheckMode.SkipContextSensitive ? getRegularTypeOfObjectLiteral(argType) : argType;
if (!checkTypeRelatedToAndOptionallyElaborate(checkArgType, paramType, relation, reportErrors ? arg : undefined, arg, headMessage, containingMessageChain, errorOutputContainer)) {
const effectiveCheckArgumentNode = getEffectiveCheckNode(arg);
if (!checkTypeRelatedToAndOptionallyElaborate(checkArgType, paramType, relation, reportErrors ? effectiveCheckArgumentNode : undefined, effectiveCheckArgumentNode, headMessage, containingMessageChain, errorOutputContainer)) {
Debug.assert(!reportErrors || !!errorOutputContainer.errors, "parameter should have errors when reporting errors");
maybeAddMissingAwaitInfo(arg, checkArgType, paramType);
return errorOutputContainer.errors || emptyArray;
Expand All @@ -33994,7 +34002,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const restArgCount = args.length - argCount;
const errorNode = !reportErrors ? undefined :
restArgCount === 0 ? node :
restArgCount === 1 ? args[argCount] :
restArgCount === 1 ? getEffectiveCheckNode(args[argCount]) :
setTextRangePosEnd(createSyntheticExpression(node, spreadType), args[argCount].pos, args[args.length - 1].end);
if (!checkTypeRelatedTo(spreadType, restType, relation, errorNode, headMessage, /*containingMessageChain*/ undefined, errorOutputContainer)) {
Debug.assert(!reportErrors || !!errorOutputContainer.errors, "rest parameter should have errors when reporting errors");
Expand Down Expand Up @@ -37285,12 +37293,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const exprType = checkExpression(node.body);
const returnOrPromisedType = returnType && unwrapReturnType(returnType, functionFlags);
if (returnOrPromisedType) {
const effectiveCheckNode = getEffectiveCheckNode(node.body);
if ((functionFlags & FunctionFlags.AsyncGenerator) === FunctionFlags.Async) { // Async function
const awaitedType = checkAwaitedType(exprType, /*withAlias*/ false, node.body, Diagnostics.The_return_type_of_an_async_function_must_either_be_a_valid_promise_or_must_not_contain_a_callable_then_member);
checkTypeAssignableToAndOptionallyElaborate(awaitedType, returnOrPromisedType, node.body, node.body);
const awaitedType = checkAwaitedType(exprType, /*withAlias*/ false, effectiveCheckNode, Diagnostics.The_return_type_of_an_async_function_must_either_be_a_valid_promise_or_must_not_contain_a_callable_then_member);
checkTypeAssignableToAndOptionallyElaborate(awaitedType, returnOrPromisedType, effectiveCheckNode, effectiveCheckNode);
}
else { // Normal function
checkTypeAssignableToAndOptionallyElaborate(exprType, returnOrPromisedType, node.body, node.body);
checkTypeAssignableToAndOptionallyElaborate(exprType, returnOrPromisedType, effectiveCheckNode, effectiveCheckNode);
}
}
}
Expand Down
146 changes: 146 additions & 0 deletions tests/baselines/reference/typeSatisfaction_errorLocations1.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
typeSatisfaction_errorLocations1.ts(4,5): error TS2345: Argument of type '{}' is not assignable to parameter of type '{ a: true; }'.
Property 'a' is missing in type '{}' but required in type '{ a: true; }'.
typeSatisfaction_errorLocations1.ts(5,7): error TS2322: Type 'number' is not assignable to type 'true'.
typeSatisfaction_errorLocations1.ts(6,5): error TS2345: Argument of type '{ a: number; }' is not assignable to parameter of type '{ a: true; }'.
Types of property 'a' are incompatible.
Type 'number' is not assignable to type 'true'.
typeSatisfaction_errorLocations1.ts(11,10): error TS2345: Argument of type '{}' is not assignable to parameter of type '{ a: true; }'.
Property 'a' is missing in type '{}' but required in type '{ a: true; }'.
typeSatisfaction_errorLocations1.ts(12,12): error TS2322: Type 'number' is not assignable to type 'true'.
typeSatisfaction_errorLocations1.ts(13,10): error TS2345: Argument of type '{ a: number; }' is not assignable to parameter of type '{ a: true; }'.
Types of property 'a' are incompatible.
Type 'number' is not assignable to type 'true'.
typeSatisfaction_errorLocations1.ts(16,5): error TS2345: Argument of type '[{ a: boolean; }]' is not assignable to parameter of type 'T'.
'T' could be instantiated with an arbitrary type which could be unrelated to '[{ a: boolean; }]'.
typeSatisfaction_errorLocations1.ts(18,5): error TS2345: Argument of type '[{ a: true; }]' is not assignable to parameter of type 'T'.
'[{ a: true; }]' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint '{ a: true; }[]'.
typeSatisfaction_errorLocations1.ts(21,43): error TS2322: Type 'number' is not assignable to type 'boolean'.
typeSatisfaction_errorLocations1.ts(23,23): error TS2322: Type 'boolean' is not assignable to type 'number'.
typeSatisfaction_errorLocations1.ts(25,20): error TS1360: Type 'number' does not satisfy the expected type 'boolean'.
typeSatisfaction_errorLocations1.ts(26,7): error TS2322: Type '1' is not assignable to type 'true'.
typeSatisfaction_errorLocations1.ts(29,18): error TS2322: Type 'string' is not assignable to type 'number'.
typeSatisfaction_errorLocations1.ts(31,20): error TS1360: Type 'readonly [10, "20"]' does not satisfy the expected type 'number[]'.
The type 'readonly [10, "20"]' is 'readonly' and cannot be assigned to the mutable type 'number[]'.
typeSatisfaction_errorLocations1.ts(34,9): error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
typeSatisfaction_errorLocations1.ts(36,9): error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
typeSatisfaction_errorLocations1.ts(39,3): error TS2322: Type 'string' is not assignable to type 'number'.
typeSatisfaction_errorLocations1.ts(43,3): error TS2322: Type 'string' is not assignable to type 'number'.
typeSatisfaction_errorLocations1.ts(43,16): error TS1360: Type 'string' does not satisfy the expected type 'number'.
typeSatisfaction_errorLocations1.ts(46,22): error TS2741: Property 'a' is missing in type '{}' but required in type '{ a: true; }'.
typeSatisfaction_errorLocations1.ts(47,24): error TS2322: Type 'number' is not assignable to type 'true'.
typeSatisfaction_errorLocations1.ts(48,21): error TS2322: Type '{ a: number; }' is not assignable to type '{ a: true; }'.
Types of property 'a' are incompatible.
Type 'number' is not assignable to type 'true'.


==== typeSatisfaction_errorLocations1.ts (22 errors) ====
const obj1 = { a: 1 };

const fn1 = (s: { a: true }) => {};
fn1({} satisfies unknown);
~~
!!! error TS2345: Argument of type '{}' is not assignable to parameter of type '{ a: true; }'.
!!! error TS2345: Property 'a' is missing in type '{}' but required in type '{ a: true; }'.
!!! related TS2728 typeSatisfaction_errorLocations1.ts:3:19: 'a' is declared here.
fn1({ a: 1 } satisfies unknown);
~
!!! error TS2322: Type 'number' is not assignable to type 'true'.
!!! related TS6500 typeSatisfaction_errorLocations1.ts:3:19: The expected type comes from property 'a' which is declared here on type '{ a: true; }'
fn1(obj1 satisfies unknown);
~~~~
!!! error TS2345: Argument of type '{ a: number; }' is not assignable to parameter of type '{ a: true; }'.
!!! error TS2345: Types of property 'a' are incompatible.
!!! error TS2345: Type 'number' is not assignable to type 'true'.

class Cls1 {
constructor(p: { a: true }) {}
}
new Cls1({} satisfies unknown);
~~
!!! error TS2345: Argument of type '{}' is not assignable to parameter of type '{ a: true; }'.
!!! error TS2345: Property 'a' is missing in type '{}' but required in type '{ a: true; }'.
!!! related TS2728 typeSatisfaction_errorLocations1.ts:9:20: 'a' is declared here.
new Cls1({ a: 1 } satisfies unknown);
~
!!! error TS2322: Type 'number' is not assignable to type 'true'.
!!! related TS6500 typeSatisfaction_errorLocations1.ts:9:20: The expected type comes from property 'a' which is declared here on type '{ a: true; }'
new Cls1(obj1 satisfies unknown);
~~~~
!!! error TS2345: Argument of type '{ a: number; }' is not assignable to parameter of type '{ a: true; }'.
!!! error TS2345: Types of property 'a' are incompatible.
!!! error TS2345: Type 'number' is not assignable to type 'true'.

function fn2<T extends { a: true }[]>(f: (...args: T) => void) {
f({ a: true } satisfies unknown);
~~~~~~~~~~~
!!! error TS2345: Argument of type '[{ a: boolean; }]' is not assignable to parameter of type 'T'.
!!! error TS2345: 'T' could be instantiated with an arbitrary type which could be unrelated to '[{ a: boolean; }]'.
const o = { a: true as const };
f(o satisfies unknown);
~
!!! error TS2345: Argument of type '[{ a: true; }]' is not assignable to parameter of type 'T'.
!!! error TS2345: '[{ a: true; }]' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint '{ a: true; }[]'.
}

const tuple1: [boolean, boolean] = [true, 100 satisfies unknown];
~~~
!!! error TS2322: Type 'number' is not assignable to type 'boolean'.

const obj2 = { a: 10, b: true } satisfies Record<string, number>;
~
!!! error TS2322: Type 'boolean' is not assignable to type 'number'.

const literal1 = 1 satisfies boolean;
~~~~~~~~~
!!! error TS1360: Type 'number' does not satisfy the expected type 'boolean'.
const literal2: true = 1 satisfies number;
~~~~~~~~
!!! error TS2322: Type '1' is not assignable to type 'true'.

declare function fn3(...args: unknown[]): void;
fn3(10, ...([10, "20"] satisfies number[]));
~~~~
!!! error TS2322: Type 'string' is not assignable to type 'number'.
const tuple2 = [10, "20"] as const;
fn3(10, ...(tuple2 satisfies number[]));
~~~~~~~~~
!!! error TS1360: Type 'readonly [10, "20"]' does not satisfy the expected type 'number[]'.
!!! error TS1360: The type 'readonly [10, "20"]' is 'readonly' and cannot be assigned to the mutable type 'number[]'.

declare function fn4(...args: number[]): void;
fn4(10, ...(["10", "20"] satisfies readonly string[]));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
const tuple3 = ["10", "20"] as const;
fn4(10, ...(tuple3 satisfies readonly string[]));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.

function fn5(): number {
return "foo" satisfies unknown;
~~~~~~
!!! error TS2322: Type 'string' is not assignable to type 'number'.
}

function fn6(): number {
return "foo" satisfies number;
~~~~~~
!!! error TS2322: Type 'string' is not assignable to type 'number'.
~~~~~~~~~
!!! error TS1360: Type 'string' does not satisfy the expected type 'number'.
}

((): { a: true } => ({}) satisfies unknown)();
~~
!!! error TS2741: Property 'a' is missing in type '{}' but required in type '{ a: true; }'.
!!! related TS2728 typeSatisfaction_errorLocations1.ts:46:8: 'a' is declared here.
((): { a: true } => ({ a: 1 }) satisfies unknown)();
~
!!! error TS2322: Type 'number' is not assignable to type 'true'.
!!! related TS6500 typeSatisfaction_errorLocations1.ts:47:8: The expected type comes from property 'a' which is declared here on type '{ a: true; }'
((): { a: true } => obj1 satisfies unknown)();
~~~~
!!! error TS2322: Type '{ a: number; }' is not assignable to type '{ a: true; }'.
!!! error TS2322: Types of property 'a' are incompatible.
!!! error TS2322: Type 'number' is not assignable to type 'true'.

131 changes: 131 additions & 0 deletions tests/baselines/reference/typeSatisfaction_errorLocations1.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
//// [tests/cases/conformance/expressions/typeSatisfaction/typeSatisfaction_errorLocations1.ts] ////

=== typeSatisfaction_errorLocations1.ts ===
const obj1 = { a: 1 };
>obj1 : Symbol(obj1, Decl(typeSatisfaction_errorLocations1.ts, 0, 5))
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 0, 14))

const fn1 = (s: { a: true }) => {};
>fn1 : Symbol(fn1, Decl(typeSatisfaction_errorLocations1.ts, 2, 5))
>s : Symbol(s, Decl(typeSatisfaction_errorLocations1.ts, 2, 13))
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 2, 17))

fn1({} satisfies unknown);
>fn1 : Symbol(fn1, Decl(typeSatisfaction_errorLocations1.ts, 2, 5))

fn1({ a: 1 } satisfies unknown);
>fn1 : Symbol(fn1, Decl(typeSatisfaction_errorLocations1.ts, 2, 5))
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 4, 5))

fn1(obj1 satisfies unknown);
>fn1 : Symbol(fn1, Decl(typeSatisfaction_errorLocations1.ts, 2, 5))
>obj1 : Symbol(obj1, Decl(typeSatisfaction_errorLocations1.ts, 0, 5))

class Cls1 {
>Cls1 : Symbol(Cls1, Decl(typeSatisfaction_errorLocations1.ts, 5, 28))

constructor(p: { a: true }) {}
>p : Symbol(p, Decl(typeSatisfaction_errorLocations1.ts, 8, 14))
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 8, 18))
}
new Cls1({} satisfies unknown);
>Cls1 : Symbol(Cls1, Decl(typeSatisfaction_errorLocations1.ts, 5, 28))

new Cls1({ a: 1 } satisfies unknown);
>Cls1 : Symbol(Cls1, Decl(typeSatisfaction_errorLocations1.ts, 5, 28))
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 11, 10))

new Cls1(obj1 satisfies unknown);
>Cls1 : Symbol(Cls1, Decl(typeSatisfaction_errorLocations1.ts, 5, 28))
>obj1 : Symbol(obj1, Decl(typeSatisfaction_errorLocations1.ts, 0, 5))

function fn2<T extends { a: true }[]>(f: (...args: T) => void) {
>fn2 : Symbol(fn2, Decl(typeSatisfaction_errorLocations1.ts, 12, 33))
>T : Symbol(T, Decl(typeSatisfaction_errorLocations1.ts, 14, 13))
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 14, 24))
>f : Symbol(f, Decl(typeSatisfaction_errorLocations1.ts, 14, 38))
>args : Symbol(args, Decl(typeSatisfaction_errorLocations1.ts, 14, 42))
>T : Symbol(T, Decl(typeSatisfaction_errorLocations1.ts, 14, 13))

f({ a: true } satisfies unknown);
>f : Symbol(f, Decl(typeSatisfaction_errorLocations1.ts, 14, 38))
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 15, 5))

const o = { a: true as const };
>o : Symbol(o, Decl(typeSatisfaction_errorLocations1.ts, 16, 7))
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 16, 13))
>const : Symbol(const)

f(o satisfies unknown);
>f : Symbol(f, Decl(typeSatisfaction_errorLocations1.ts, 14, 38))
>o : Symbol(o, Decl(typeSatisfaction_errorLocations1.ts, 16, 7))
}

const tuple1: [boolean, boolean] = [true, 100 satisfies unknown];
>tuple1 : Symbol(tuple1, Decl(typeSatisfaction_errorLocations1.ts, 20, 5))

const obj2 = { a: 10, b: true } satisfies Record<string, number>;
>obj2 : Symbol(obj2, Decl(typeSatisfaction_errorLocations1.ts, 22, 5))
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 22, 14))
>b : Symbol(b, Decl(typeSatisfaction_errorLocations1.ts, 22, 21))
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --))

const literal1 = 1 satisfies boolean;
>literal1 : Symbol(literal1, Decl(typeSatisfaction_errorLocations1.ts, 24, 5))

const literal2: true = 1 satisfies number;
>literal2 : Symbol(literal2, Decl(typeSatisfaction_errorLocations1.ts, 25, 5))

declare function fn3(...args: unknown[]): void;
>fn3 : Symbol(fn3, Decl(typeSatisfaction_errorLocations1.ts, 25, 42))
>args : Symbol(args, Decl(typeSatisfaction_errorLocations1.ts, 27, 21))

fn3(10, ...([10, "20"] satisfies number[]));
>fn3 : Symbol(fn3, Decl(typeSatisfaction_errorLocations1.ts, 25, 42))

const tuple2 = [10, "20"] as const;
>tuple2 : Symbol(tuple2, Decl(typeSatisfaction_errorLocations1.ts, 29, 5))
>const : Symbol(const)

fn3(10, ...(tuple2 satisfies number[]));
>fn3 : Symbol(fn3, Decl(typeSatisfaction_errorLocations1.ts, 25, 42))
>tuple2 : Symbol(tuple2, Decl(typeSatisfaction_errorLocations1.ts, 29, 5))

declare function fn4(...args: number[]): void;
>fn4 : Symbol(fn4, Decl(typeSatisfaction_errorLocations1.ts, 30, 40))
>args : Symbol(args, Decl(typeSatisfaction_errorLocations1.ts, 32, 21))

fn4(10, ...(["10", "20"] satisfies readonly string[]));
>fn4 : Symbol(fn4, Decl(typeSatisfaction_errorLocations1.ts, 30, 40))

const tuple3 = ["10", "20"] as const;
>tuple3 : Symbol(tuple3, Decl(typeSatisfaction_errorLocations1.ts, 34, 5))
>const : Symbol(const)

fn4(10, ...(tuple3 satisfies readonly string[]));
>fn4 : Symbol(fn4, Decl(typeSatisfaction_errorLocations1.ts, 30, 40))
>tuple3 : Symbol(tuple3, Decl(typeSatisfaction_errorLocations1.ts, 34, 5))

function fn5(): number {
>fn5 : Symbol(fn5, Decl(typeSatisfaction_errorLocations1.ts, 35, 49))

return "foo" satisfies unknown;
}

function fn6(): number {
>fn6 : Symbol(fn6, Decl(typeSatisfaction_errorLocations1.ts, 39, 1))

return "foo" satisfies number;
}

((): { a: true } => ({}) satisfies unknown)();
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 45, 6))

((): { a: true } => ({ a: 1 }) satisfies unknown)();
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 46, 6))
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 46, 22))

((): { a: true } => obj1 satisfies unknown)();
>a : Symbol(a, Decl(typeSatisfaction_errorLocations1.ts, 47, 6))
>obj1 : Symbol(obj1, Decl(typeSatisfaction_errorLocations1.ts, 0, 5))

Loading