Skip to content

Assume that type node annotations resolving to error types can be reused #60195

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
Oct 11, 2024
Merged

Assume that type node annotations resolving to error types can be reused #60195

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
0e0d6f6
Assume that type node annotations resolving to error types can be reused
Andarist Oct 10, 2024
abed89d
update baselines
Andarist Oct 11, 2024
7b187cf
Merge branch 'main' into fix/confusing-cascading-id-error
jakebailey Oct 11, 2024
7327fbf
Update baseline
jakebailey Oct 11, 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
6 changes: 6 additions & 0 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6240,6 +6240,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
}
let annotationType = getTypeFromTypeNodeWithoutContext(existing);
if (isErrorType(annotationType)) {
// allow "reusing" type nodes that resolve to error types
// those can't truly be reused but it prevents cascading errors in isolatedDeclarations
// for source with errors there is no guarantee to emit correct code anyway
return true;
}
Comment on lines +6244 to +6248
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could be handled elsewhere but this place is an easy and effective fix.

Note that in a sense this was already happening - just in different situations. Consider this:

export const foo3 = (type: Unresolved): void => {};

In here both the annotationType and type are error types, they just happen to be the same aliased error types. So they are assumed to be the same on the basis of typeFromTypeNode === type in typeNodeIsEquivalentToType.

The reported case is the same yet different. Both types are also error types there but the type is a regular non-aliased error type whereas the annotationType is an aliased error type. Based on that typeNodeIsEquivalentToType doesn't assume that they are the same and thus typeFromParameter goes into inferTypeOfDeclaration which leads to reporting the error.

Copy link
Member

Choose a reason for hiding this comment

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

Is there a condition in which this will cause us to try and print something unprintable?

Or will it not matter because it'll be an error anyway?

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 propose this fix mainly on the assumption of "will it not matter because it'll be an error anyway". It seems to me that if the error is there anyway then any emit is basically a fair game. It should reuse the annotation node - so at the very least it should just be broken in the same way.

if (requiresAddingUndefined && annotationType) {
annotationType = getOptionalType(annotationType, !isParameter(node));
}
Expand Down
24 changes: 12 additions & 12 deletions tests/baselines/reference/circularTypeofWithVarOrFunc.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,16 @@ type typeAlias2 = typeof varOfAliasedType2;
> : ^^^

function func(): typeAlias3 { return null; }
>func : () => any
> : ^^^^^^^^^
>func : () => typeAlias3
> : ^^^^^^

var varOfAliasedType3 = func();
>varOfAliasedType3 : any
> : ^^^
>func() : any
> : ^^^
>func : () => any
> : ^^^^^^^^^
>func : () => typeAlias3
> : ^^^^^^

type typeAlias3 = typeof varOfAliasedType3;
>typeAlias3 : any
Expand All @@ -54,12 +54,12 @@ interface Input {
type R = ReturnType<typeof mul>;
>R : any
> : ^^^
>mul : (input: Input) => any
> : ^ ^^ ^^^^^^^^
>mul : (input: Input) => R
> : ^ ^^ ^^^^^

function mul(input: Input): R {
>mul : (input: Input) => any
> : ^ ^^ ^^^^^^^^
>mul : (input: Input) => R
> : ^ ^^ ^^^^^
>input : Input
> : ^^^^^

Expand All @@ -85,12 +85,12 @@ function mul(input: Input): R {
type R2 = ReturnType<typeof f>;
>R2 : any
> : ^^^
>f : () => any
> : ^^^^^^^^^
>f : () => R2
> : ^^^^^^

function f(): R2 { return 0; }
>f : () => any
> : ^^^^^^^^^
>f : () => R2
> : ^^^^^^
>0 : 0
> : ^

14 changes: 14 additions & 0 deletions tests/baselines/reference/isolatedDeclarationErrorTypes1.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
isolatedDeclarationErrorTypes1.ts(3,28): error TS2307: Cannot find module 'foo' or its corresponding type declarations.


==== isolatedDeclarationErrorTypes1.ts (1 errors) ====
// https://github.com/microsoft/TypeScript/issues/60192

import { Unresolved } from "foo";
~~~~~
!!! error TS2307: Cannot find module 'foo' or its corresponding type declarations.

export const foo1 = (type?: Unresolved): void => {};
export const foo2 = (type?: Unresolved | undefined): void => {};
export const foo3 = (type: Unresolved): void => {};

30 changes: 30 additions & 0 deletions tests/baselines/reference/isolatedDeclarationErrorTypes1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//// [tests/cases/compiler/isolatedDeclarationErrorTypes1.ts] ////

//// [isolatedDeclarationErrorTypes1.ts]
// https://github.com/microsoft/TypeScript/issues/60192

import { Unresolved } from "foo";

export const foo1 = (type?: Unresolved): void => {};
export const foo2 = (type?: Unresolved | undefined): void => {};
export const foo3 = (type: Unresolved): void => {};


//// [isolatedDeclarationErrorTypes1.js]
"use strict";
// https://github.com/microsoft/TypeScript/issues/60192
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo3 = exports.foo2 = exports.foo1 = void 0;
const foo1 = (type) => { };
exports.foo1 = foo1;
const foo2 = (type) => { };
exports.foo2 = foo2;
const foo3 = (type) => { };
exports.foo3 = foo3;


//// [isolatedDeclarationErrorTypes1.d.ts]
import { Unresolved } from "foo";
export declare const foo1: (type?: Unresolved) => void;
export declare const foo2: (type?: Unresolved | undefined) => void;
export declare const foo3: (type: Unresolved) => void;
23 changes: 23 additions & 0 deletions tests/baselines/reference/isolatedDeclarationErrorTypes1.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//// [tests/cases/compiler/isolatedDeclarationErrorTypes1.ts] ////

=== isolatedDeclarationErrorTypes1.ts ===
// https://github.com/microsoft/TypeScript/issues/60192

import { Unresolved } from "foo";
>Unresolved : Symbol(Unresolved, Decl(isolatedDeclarationErrorTypes1.ts, 2, 8))

export const foo1 = (type?: Unresolved): void => {};
>foo1 : Symbol(foo1, Decl(isolatedDeclarationErrorTypes1.ts, 4, 12))
>type : Symbol(type, Decl(isolatedDeclarationErrorTypes1.ts, 4, 21))
>Unresolved : Symbol(Unresolved, Decl(isolatedDeclarationErrorTypes1.ts, 2, 8))

export const foo2 = (type?: Unresolved | undefined): void => {};
>foo2 : Symbol(foo2, Decl(isolatedDeclarationErrorTypes1.ts, 5, 12))
>type : Symbol(type, Decl(isolatedDeclarationErrorTypes1.ts, 5, 21))
>Unresolved : Symbol(Unresolved, Decl(isolatedDeclarationErrorTypes1.ts, 2, 8))

export const foo3 = (type: Unresolved): void => {};
>foo3 : Symbol(foo3, Decl(isolatedDeclarationErrorTypes1.ts, 6, 12))
>type : Symbol(type, Decl(isolatedDeclarationErrorTypes1.ts, 6, 21))
>Unresolved : Symbol(Unresolved, Decl(isolatedDeclarationErrorTypes1.ts, 2, 8))

33 changes: 33 additions & 0 deletions tests/baselines/reference/isolatedDeclarationErrorTypes1.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//// [tests/cases/compiler/isolatedDeclarationErrorTypes1.ts] ////

=== isolatedDeclarationErrorTypes1.ts ===
// https://github.com/microsoft/TypeScript/issues/60192

import { Unresolved } from "foo";
>Unresolved : any
> : ^^^

export const foo1 = (type?: Unresolved): void => {};
>foo1 : (type?: Unresolved) => void
> : ^ ^^^ ^^^^^
>(type?: Unresolved): void => {} : (type?: Unresolved) => void
> : ^ ^^^ ^^^^^
>type : any
> : ^^^

export const foo2 = (type?: Unresolved | undefined): void => {};
>foo2 : (type?: Unresolved | undefined) => void
> : ^ ^^^ ^^^^^
>(type?: Unresolved | undefined): void => {} : (type?: Unresolved | undefined) => void
> : ^ ^^^ ^^^^^
>type : any
> : ^^^

export const foo3 = (type: Unresolved): void => {};
>foo3 : (type: Unresolved) => void
> : ^ ^^ ^^^^^
>(type: Unresolved): void => {} : (type: Unresolved) => void
> : ^ ^^ ^^^^^
>type : Unresolved
> : ^^^^^^^^^^

4 changes: 2 additions & 2 deletions tests/baselines/reference/isolatedDeclarationsAddUndefined2.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ export function test2(x?: Unresolved | undefined): void {}
> : ^^^

export function test3(x?: Unresolved): void {}
>test3 : (x?: any) => void
> : ^ ^^^^^^^^^^^
>test3 : (x?: Unresolved) => void
> : ^ ^^^ ^^^^^
>x : any
> : ^^^

13 changes: 13 additions & 0 deletions tests/cases/compiler/isolatedDeclarationErrorTypes1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @isolatedDeclarations: true
// @strict: true
// @declaration: true
// @moduleResolution: nodenext
// @module: nodenext

// https://github.com/microsoft/TypeScript/issues/60192

import { Unresolved } from "foo";

export const foo1 = (type?: Unresolved): void => {};
export const foo2 = (type?: Unresolved | undefined): void => {};
export const foo3 = (type: Unresolved): void => {};