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

Better error message for unparenthesized function/constructor type notation in union/intersection types #39570

Merged
merged 10 commits into from
Jul 15, 2020
16 changes: 16 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1164,6 +1164,22 @@
"category": "Error",
"code": 1384
},
"Function type notation must be parenthesized when used in a union type.": {
"category": "Error",
"code": 1385
},
"Constructor type notation must be parenthesized when used in a union type.": {
"category": "Error",
"code": 1386
},
"Function type notation must be parenthesized when used in an intersection type.": {
"category": "Error",
"code": 1387
},
"Constructor type notation must be parenthesized when used in an intersection type.": {
"category": "Error",
"code": 1388
},

"The types of '{0}' are incompatible between these types.": {
"category": "Error",
Expand Down
40 changes: 35 additions & 5 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3569,30 +3569,60 @@ namespace ts {
return parsePostfixTypeOrHigher();
}

function parseFunctionOrConstructorTypeToError(
functionTypeDiagnostic: DiagnosticMessage,
constructorTypeDiagnostic: DiagnosticMessage
Copy link
Member

Choose a reason for hiding this comment

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

Instead of passing along the diagnostics, I might just pass along the information of whether or not this is a union type or an intersection type.

Copy link
Contributor Author

@uhyo uhyo Jul 14, 2020

Choose a reason for hiding this comment

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

@DanielRosenwasser Thank you for your reviews!
I updated this function so it now receives a single parameter isInUnionType: boolean.

): TypeNode | undefined {
// function type notations or constructor type notations are not allowed in this context,
// but we try parsing them for graceful error message
uhyo marked this conversation as resolved.
Show resolved Hide resolved
if (isStartOfFunctionType() || token() === SyntaxKind.NewKeyword) {
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 an already-defined isStartOfConstructorType? I'll go check...

Copy link
Member

Choose a reason for hiding this comment

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

nope, but the only other call also is followed by || token() === SntaxKind.NewKeyword. Can you move that clause into the function?

const type = parseFunctionOrConstructorType();
parseErrorAtRange(type, isFunctionTypeNode(type) ? functionTypeDiagnostic : constructorTypeDiagnostic);
return type;
}
return undefined;
}

function parseUnionOrIntersectionType(
operator: SyntaxKind.BarToken | SyntaxKind.AmpersandToken,
parseConstituentType: () => TypeNode,
createTypeNode: (types: NodeArray<TypeNode>) => UnionOrIntersectionTypeNode
createTypeNode: (types: NodeArray<TypeNode>) => UnionOrIntersectionTypeNode,
functionTypeDiagnostic: DiagnosticMessage,
constructorTypeDiagnostic: DiagnosticMessage
): TypeNode {
const pos = getNodePos();
const hasLeadingOperator = parseOptional(operator);
let type = parseConstituentType();
let type = hasLeadingOperator ?
parseFunctionOrConstructorTypeToError(functionTypeDiagnostic, constructorTypeDiagnostic) || parseConstituentType() :
parseConstituentType();
if (token() === operator || hasLeadingOperator) {
const types = [type];
while (parseOptional(operator)) {
types.push(parseConstituentType());
types.push(parseFunctionOrConstructorTypeToError(functionTypeDiagnostic, constructorTypeDiagnostic) || parseConstituentType());
}
type = finishNode(createTypeNode(createNodeArray(types, pos)), pos);
}
return type;
}

function parseIntersectionTypeOrHigher(): TypeNode {
return parseUnionOrIntersectionType(SyntaxKind.AmpersandToken, parseTypeOperatorOrHigher, factory.createIntersectionTypeNode);
return parseUnionOrIntersectionType(
SyntaxKind.AmpersandToken,
parseTypeOperatorOrHigher,
factory.createIntersectionTypeNode,
Diagnostics.Function_type_notation_must_be_parenthesized_when_used_in_an_intersection_type,
Diagnostics.Constructor_type_notation_must_be_parenthesized_when_used_in_an_intersection_type
);
}

function parseUnionTypeOrHigher(): TypeNode {
return parseUnionOrIntersectionType(SyntaxKind.BarToken, parseIntersectionTypeOrHigher, factory.createUnionTypeNode);
return parseUnionOrIntersectionType(
SyntaxKind.BarToken,
parseIntersectionTypeOrHigher,
factory.createUnionTypeNode,
Diagnostics.Function_type_notation_must_be_parenthesized_when_used_in_a_union_type,
Diagnostics.Constructor_type_notation_must_be_parenthesized_when_used_in_a_union_type
);
}

function isStartOfFunctionType(): boolean {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(1,19): error TS1386: Constructor type notation must be parenthesized when used in a union type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(2,19): error TS1386: Constructor type notation must be parenthesized when used in a union type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(3,12): error TS1386: Constructor type notation must be parenthesized when used in a union type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(4,12): error TS1386: Constructor type notation must be parenthesized when used in a union type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(5,19): error TS1386: Constructor type notation must be parenthesized when used in a union type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(8,4): error TS1386: Constructor type notation must be parenthesized when used in a union type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(11,19): error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(12,19): error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(13,12): error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(14,12): error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(15,19): error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(18,4): error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(20,37): error TS1386: Constructor type notation must be parenthesized when used in a union type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(21,31): error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(22,16): error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts(22,41): error TS1386: Constructor type notation must be parenthesized when used in a union type.


==== tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts (16 errors) ====
type U1 = string | new () => void;
~~~~~~~~~~~~~~~
!!! error TS1386: Constructor type notation must be parenthesized when used in a union type.
type U2 = string | new (foo: number) => void
~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1386: Constructor type notation must be parenthesized when used in a union type.
type U3 = | new () => number
~~~~~~~~~~~~~~~~~
!!! error TS1386: Constructor type notation must be parenthesized when used in a union type.
type U4 = | new (foo?: number) => void;
~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1386: Constructor type notation must be parenthesized when used in a union type.
type U5 = string | new (number: number, foo?: string) => void | number;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1386: Constructor type notation must be parenthesized when used in a union type.
type U6 =
| string
| new (...args: any[]) => void
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| number;
~~~~~~~~~~
!!! error TS1386: Constructor type notation must be parenthesized when used in a union type.

type I1 = string & new () => void;
~~~~~~~~~~~~~~~
!!! error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
type I2 = string & new (...foo: number[]) => void;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
type I3 = & new () => boolean
~~~~~~~~~~~~~~~~~~
!!! error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
type I4 = & new () => boolean & null;
~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
type I5 = string & new (any: any, any2: any) => any & any;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
type I6 =
& string
& new (foo: any) => void;
~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1388: Constructor type notation must be parenthesized when used in an intersection type.

type M1 = string | number & string | new () => number;
~~~~~~~~~~~~~~~~~
!!! error TS1386: Constructor type notation must be parenthesized when used in a union type.
type M2 = any & string | any & new () => void;
~~~~~~~~~~~~~~~
!!! error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
type M3 = any & new (foo: any) => void | new () => void & any;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1388: Constructor type notation must be parenthesized when used in an intersection type.
~~~~~~~~~~~~~~~~~~~~~
!!! error TS1386: Constructor type notation must be parenthesized when used in a union type.

type OK1 = string | (new ()=> void);
type OK2 = string | (new ()=> string | number);

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [unparenthesizedConstructorTypeInUnionOrIntersection.ts]
type U1 = string | new () => void;
type U2 = string | new (foo: number) => void
type U3 = | new () => number
type U4 = | new (foo?: number) => void;
type U5 = string | new (number: number, foo?: string) => void | number;
type U6 =
| string
| new (...args: any[]) => void
| number;

type I1 = string & new () => void;
type I2 = string & new (...foo: number[]) => void;
type I3 = & new () => boolean
type I4 = & new () => boolean & null;
type I5 = string & new (any: any, any2: any) => any & any;
type I6 =
& string
& new (foo: any) => void;

type M1 = string | number & string | new () => number;
type M2 = any & string | any & new () => void;
type M3 = any & new (foo: any) => void | new () => void & any;

type OK1 = string | (new ()=> void);
type OK2 = string | (new ()=> string | number);


//// [unparenthesizedConstructorTypeInUnionOrIntersection.js]
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
=== tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts ===
type U1 = string | new () => void;
>U1 : Symbol(U1, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 0, 0))

type U2 = string | new (foo: number) => void
>U2 : Symbol(U2, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 0, 34))
>foo : Symbol(foo, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 1, 24))

type U3 = | new () => number
>U3 : Symbol(U3, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 1, 44))

type U4 = | new (foo?: number) => void;
>U4 : Symbol(U4, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 2, 28))
>foo : Symbol(foo, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 3, 17))

type U5 = string | new (number: number, foo?: string) => void | number;
>U5 : Symbol(U5, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 3, 39))
>number : Symbol(number, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 4, 24))
>foo : Symbol(foo, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 4, 39))

type U6 =
>U6 : Symbol(U6, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 4, 71))

| string
| new (...args: any[]) => void
>args : Symbol(args, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 7, 9))

| number;

type I1 = string & new () => void;
>I1 : Symbol(I1, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 8, 11))

type I2 = string & new (...foo: number[]) => void;
>I2 : Symbol(I2, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 10, 34))
>foo : Symbol(foo, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 11, 24))

type I3 = & new () => boolean
>I3 : Symbol(I3, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 11, 50))

type I4 = & new () => boolean & null;
>I4 : Symbol(I4, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 12, 29))

type I5 = string & new (any: any, any2: any) => any & any;
>I5 : Symbol(I5, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 13, 37))
>any : Symbol(any, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 14, 24))
>any2 : Symbol(any2, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 14, 33))

type I6 =
>I6 : Symbol(I6, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 14, 58))

& string
& new (foo: any) => void;
>foo : Symbol(foo, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 17, 9))

type M1 = string | number & string | new () => number;
>M1 : Symbol(M1, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 17, 27))

type M2 = any & string | any & new () => void;
>M2 : Symbol(M2, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 19, 54))

type M3 = any & new (foo: any) => void | new () => void & any;
>M3 : Symbol(M3, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 20, 46))
>foo : Symbol(foo, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 21, 21))

type OK1 = string | (new ()=> void);
>OK1 : Symbol(OK1, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 21, 62))

type OK2 = string | (new ()=> string | number);
>OK2 : Symbol(OK2, Decl(unparenthesizedConstructorTypeInUnionOrIntersection.ts, 23, 36))

Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
=== tests/cases/compiler/unparenthesizedConstructorTypeInUnionOrIntersection.ts ===
type U1 = string | new () => void;
>U1 : U1

type U2 = string | new (foo: number) => void
>U2 : U2
>foo : number

type U3 = | new () => number
>U3 : new () => number

type U4 = | new (foo?: number) => void;
>U4 : new (foo?: number) => void
>foo : number

type U5 = string | new (number: number, foo?: string) => void | number;
>U5 : U5
>number : number
>foo : string

type U6 =
>U6 : U6

| string
| new (...args: any[]) => void
>args : any[]

| number;

type I1 = string & new () => void;
>I1 : I1

type I2 = string & new (...foo: number[]) => void;
>I2 : I2
>foo : number[]

type I3 = & new () => boolean
>I3 : new () => boolean

type I4 = & new () => boolean & null;
>I4 : new () => boolean & null
>null : null

type I5 = string & new (any: any, any2: any) => any & any;
>I5 : I5
>any : any
>any2 : any

type I6 =
>I6 : I6

& string
& new (foo: any) => void;
>foo : any

type M1 = string | number & string | new () => number;
>M1 : M1

type M2 = any & string | any & new () => void;
>M2 : any

type M3 = any & new (foo: any) => void | new () => void & any;
>M3 : any
>foo : any

type OK1 = string | (new ()=> void);
>OK1 : OK1

type OK2 = string | (new ()=> string | number);
>OK2 : OK2

Loading