Skip to content

Fix inference for contextually typed parameters with initializers #29576

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 8 commits into from
Jan 30, 2019
Merged

Fix inference for contextually typed parameters with initializers #29576

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8f5ef10
Don't infer from initializers of parameters of contextually typed fun…
ahejlsberg Jan 24, 2019
318678a
Accept new baselines
ahejlsberg Jan 25, 2019
1bc4389
Add tests
ahejlsberg Jan 25, 2019
58e39ea
Accept new baselines
ahejlsberg Jan 25, 2019
22d46ac
Don't report circularities when parameter has itself as contextual type
ahejlsberg Jan 25, 2019
c6ccc5b
Address CR feedback
ahejlsberg Jan 25, 2019
69d1048
Accept new baselines
ahejlsberg Jan 25, 2019
25ac1ed
Merge branch 'master' into fixContextuallyTypedParameters
ahejlsberg Jan 26, 2019
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
31 changes: 13 additions & 18 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4862,17 +4862,8 @@ namespace ts {
function getTypeForBindingElement(declaration: BindingElement): Type | undefined {
const pattern = declaration.parent;
let parentType = getTypeForBindingElementParent(pattern.parent);
// If parent has the unknown (error) type, then so does this binding element
if (parentType === errorType) {
return errorType;
}
// If no type was specified or inferred for parent,
// infer from the initializer of the binding element if one is present.
// Otherwise, go with the undefined type of the parent.
if (!parentType) {
return declaration.initializer ? checkDeclarationInitializer(declaration) : parentType;
}
if (isTypeAny(parentType)) {
// If no type or an any type was inferred for parent, infer that for the binding element
if (!parentType || isTypeAny(parentType)) {
return parentType;
}
// Relax null check on ambient destructuring parameters, since the parameters have no implementation and are just documentation
Expand Down Expand Up @@ -5041,8 +5032,11 @@ namespace ts {
}
}

// Use the type of the initializer expression if one is present
if (declaration.initializer) {
const isParameterOfContextuallyTypedFunction = declaration.kind === SyntaxKind.Parameter && getContextualType(<Expression>declaration.parent);

// Use the type of the initializer expression if one is present and the declaration is
// not a parameter of a contextually typed function
if (declaration.initializer && !isParameterOfContextuallyTypedFunction) {
Copy link
Member

@weswigham weswigham Jan 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Wouldn't we rather write declaration.initializer && !(declaration.kind === SyntaxKind.Parameter && getContextualType(<Expression>declaration.parent)) so we only call getContextualType if we absolutely need to?

Copy link
Member

Choose a reason for hiding this comment

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

Or, since it's used below, write

const needsShapeAndIsNotParameterOfContextuallyTypedFunction = (declaration.initializer || isBindingPattern(declaration.name)) && !(declaration.kind === SyntaxKind.Parameter && getContextualType(<Expression>declaration.parent));

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, possibly. The value is precomputed because I use it in two locations. But I supposed I could turn it into a function that we only call when appropriate.

const type = checkDeclarationInitializer(declaration);
return addOptionality(type, isOptional);
}
Expand All @@ -5053,8 +5047,9 @@ namespace ts {
return trueType;
}

// If the declaration specifies a binding pattern, use the type implied by the binding pattern
if (isBindingPattern(declaration.name)) {
// If the declaration specifies a binding pattern and is not a parameter of a contextually
// typed function, use the type implied by the binding pattern
if (isBindingPattern(declaration.name) && !isParameterOfContextuallyTypedFunction) {
return getTypeFromBindingPattern(declaration.name, /*includePatternInType*/ false, /*reportErrors*/ true);
}

Expand Down Expand Up @@ -25638,13 +25633,13 @@ namespace ts {
const parent = node.parent.parent;
const parentType = getTypeForBindingElementParent(parent);
const name = node.propertyName || node.name;
if (!isBindingPattern(name)) {
if (parentType && !isBindingPattern(name)) {
const nameText = getTextOfPropertyName(name);
if (nameText) {
const property = getPropertyOfType(parentType!, nameText); // TODO: GH#18217
const property = getPropertyOfType(parentType, nameText);
if (property) {
markPropertyAsReferenced(property, /*nodeForCheckWriteOnly*/ undefined, /*isThisAccess*/ false); // A destructuring is never a write-only reference.
checkPropertyAccessibility(parent, !!parent.initializer && parent.initializer.kind === SyntaxKind.SuperKeyword, parentType!, property);
checkPropertyAccessibility(parent, !!parent.initializer && parent.initializer.kind === SyntaxKind.SuperKeyword, parentType, property);
}
}
}
Expand Down
48 changes: 48 additions & 0 deletions tests/baselines/reference/contextuallyTypedParametersWithInitializers.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
tests/cases/compiler/contextuallyTypedParametersWithInitializers.ts(8,27): error TS7022: '{ foo = 42 }' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.
tests/cases/compiler/contextuallyTypedParametersWithInitializers.ts(8,29): error TS7031: Binding element 'foo' implicitly has an 'any' type.
tests/cases/compiler/contextuallyTypedParametersWithInitializers.ts(14,27): error TS7006: Parameter 'foo' implicitly has an 'any' type.
tests/cases/compiler/contextuallyTypedParametersWithInitializers.ts(27,40): error TS7022: '{ foo = 42 }' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.
tests/cases/compiler/contextuallyTypedParametersWithInitializers.ts(27,42): error TS7031: Binding element 'foo' implicitly has an 'any' type.


==== tests/cases/compiler/contextuallyTypedParametersWithInitializers.ts (5 errors) ====
declare function id1<T>(input: T): T;
declare function id2<T extends (x: any) => any>(input: T): T;
declare function id3<T extends (x: { foo: any }) => any>(input: T): T;
declare function id4<T extends (x: { foo?: number }) => any>(input: T): T;
declare function id5<T extends (x?: number) => any>(input: T): T;

const f10 = function ({ foo = 42 }) { return foo };
const f11 = id1(function ({ foo = 42 }) { return foo }); // Implicit any error
~~~~~~~~~~~~
!!! error TS7022: '{ foo = 42 }' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.
Copy link
Member

Choose a reason for hiding this comment

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

How does this binding pattern circularly reference itself? o.O

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, strange. This error is caused by declaration file emit, it only appears with --declaration. Have no idea why.

Copy link
Member

Choose a reason for hiding this comment

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

getEmitDeclarations causes us to conditionally collectLinkedAliases on exported names - but this is a binding pattern in an anonymous function expression, not an exported name... o.O

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we generate the error while looking up the type of the expression for emitting declare const f11: something? We marked foo as implicitly any within the checker, however did we propagate that up to the binding pattern as a whole?

Copy link
Member Author

Choose a reason for hiding this comment

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

The circularity error is caused by the parameter having itself as its contextual type. It's actually unrelated to this PR. For example, this causes the same error on master:

declare function id<T>(input: T): T;
const f1 = id(function ({ foo }) { return foo });

~~~
!!! error TS7031: Binding element 'foo' implicitly has an 'any' type.
const f12 = id2(function ({ foo = 42 }) { return foo });
const f13 = id3(function ({ foo = 42 }) { return foo });
const f14 = id4(function ({ foo = 42 }) { return foo });

const f20 = function (foo = 42) { return foo };
const f21 = id1(function (foo = 42) { return foo }); // Implicit any error
~~~~~~~~
!!! error TS7006: Parameter 'foo' implicitly has an 'any' type.
const f22 = id2(function (foo = 42) { return foo });
const f25 = id5(function (foo = 42) { return foo });

// Repro from #28816

function id<T>(input: T): T { return input }

function getFoo ({ foo = 42 }) {
return foo;
}

const newGetFoo = id(getFoo);
const newGetFoo2 = id(function getFoo ({ foo = 42 }) {
~~~~~~~~~~~~
!!! error TS7022: '{ foo = 42 }' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.
~~~
!!! error TS7031: Binding element 'foo' implicitly has an 'any' type.
return foo;
});

114 changes: 114 additions & 0 deletions tests/baselines/reference/contextuallyTypedParametersWithInitializers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
//// [contextuallyTypedParametersWithInitializers.ts]
declare function id1<T>(input: T): T;
declare function id2<T extends (x: any) => any>(input: T): T;
declare function id3<T extends (x: { foo: any }) => any>(input: T): T;
declare function id4<T extends (x: { foo?: number }) => any>(input: T): T;
declare function id5<T extends (x?: number) => any>(input: T): T;

const f10 = function ({ foo = 42 }) { return foo };
const f11 = id1(function ({ foo = 42 }) { return foo }); // Implicit any error
const f12 = id2(function ({ foo = 42 }) { return foo });
const f13 = id3(function ({ foo = 42 }) { return foo });
const f14 = id4(function ({ foo = 42 }) { return foo });

const f20 = function (foo = 42) { return foo };
const f21 = id1(function (foo = 42) { return foo }); // Implicit any error
const f22 = id2(function (foo = 42) { return foo });
const f25 = id5(function (foo = 42) { return foo });

// Repro from #28816

function id<T>(input: T): T { return input }

function getFoo ({ foo = 42 }) {
return foo;
}

const newGetFoo = id(getFoo);
const newGetFoo2 = id(function getFoo ({ foo = 42 }) {
return foo;
});


//// [contextuallyTypedParametersWithInitializers.js]
"use strict";
var f10 = function (_a) {
var _b = _a.foo, foo = _b === void 0 ? 42 : _b;
return foo;
};
var f11 = id1(function (_a) {
var _b = _a.foo, foo = _b === void 0 ? 42 : _b;
return foo;
}); // Implicit any error
var f12 = id2(function (_a) {
var _b = _a.foo, foo = _b === void 0 ? 42 : _b;
return foo;
});
var f13 = id3(function (_a) {
var _b = _a.foo, foo = _b === void 0 ? 42 : _b;
return foo;
});
var f14 = id4(function (_a) {
var _b = _a.foo, foo = _b === void 0 ? 42 : _b;
return foo;
});
var f20 = function (foo) {
if (foo === void 0) { foo = 42; }
return foo;
};
var f21 = id1(function (foo) {
if (foo === void 0) { foo = 42; }
return foo;
}); // Implicit any error
var f22 = id2(function (foo) {
if (foo === void 0) { foo = 42; }
return foo;
});
var f25 = id5(function (foo) {
if (foo === void 0) { foo = 42; }
return foo;
});
// Repro from #28816
function id(input) { return input; }
function getFoo(_a) {
var _b = _a.foo, foo = _b === void 0 ? 42 : _b;
return foo;
}
var newGetFoo = id(getFoo);
var newGetFoo2 = id(function getFoo(_a) {
var _b = _a.foo, foo = _b === void 0 ? 42 : _b;
return foo;
});


//// [contextuallyTypedParametersWithInitializers.d.ts]
declare function id1<T>(input: T): T;
declare function id2<T extends (x: any) => any>(input: T): T;
declare function id3<T extends (x: {
foo: any;
}) => any>(input: T): T;
declare function id4<T extends (x: {
foo?: number;
}) => any>(input: T): T;
declare function id5<T extends (x?: number) => any>(input: T): T;
declare const f10: ({ foo }: {
foo?: number | undefined;
}) => number;
declare const f11: ({ foo }: any) => any;
declare const f12: ({ foo }: any) => any;
declare const f13: ({ foo }: {
foo: any;
}) => any;
declare const f14: ({ foo }: {
foo?: number | undefined;
}) => number;
declare const f20: (foo?: number) => number;
declare const f21: (foo?: any) => any;
declare const f22: (foo?: any) => any;
declare const f25: (foo?: number | undefined) => number;
declare function id<T>(input: T): T;
declare function getFoo({ foo }: {
foo?: number | undefined;
}): number;
declare const newGetFoo: typeof getFoo;
declare const newGetFoo2: ({ foo }: any) => any;
128 changes: 128 additions & 0 deletions tests/baselines/reference/contextuallyTypedParametersWithInitializers.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
=== tests/cases/compiler/contextuallyTypedParametersWithInitializers.ts ===
declare function id1<T>(input: T): T;
>id1 : Symbol(id1, Decl(contextuallyTypedParametersWithInitializers.ts, 0, 0))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 0, 21))
>input : Symbol(input, Decl(contextuallyTypedParametersWithInitializers.ts, 0, 24))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 0, 21))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 0, 21))

declare function id2<T extends (x: any) => any>(input: T): T;
>id2 : Symbol(id2, Decl(contextuallyTypedParametersWithInitializers.ts, 0, 37))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 1, 21))
>x : Symbol(x, Decl(contextuallyTypedParametersWithInitializers.ts, 1, 32))
>input : Symbol(input, Decl(contextuallyTypedParametersWithInitializers.ts, 1, 48))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 1, 21))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 1, 21))

declare function id3<T extends (x: { foo: any }) => any>(input: T): T;
>id3 : Symbol(id3, Decl(contextuallyTypedParametersWithInitializers.ts, 1, 61))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 2, 21))
>x : Symbol(x, Decl(contextuallyTypedParametersWithInitializers.ts, 2, 32))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 2, 36))
>input : Symbol(input, Decl(contextuallyTypedParametersWithInitializers.ts, 2, 57))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 2, 21))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 2, 21))

declare function id4<T extends (x: { foo?: number }) => any>(input: T): T;
>id4 : Symbol(id4, Decl(contextuallyTypedParametersWithInitializers.ts, 2, 70))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 3, 21))
>x : Symbol(x, Decl(contextuallyTypedParametersWithInitializers.ts, 3, 32))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 3, 36))
>input : Symbol(input, Decl(contextuallyTypedParametersWithInitializers.ts, 3, 61))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 3, 21))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 3, 21))

declare function id5<T extends (x?: number) => any>(input: T): T;
>id5 : Symbol(id5, Decl(contextuallyTypedParametersWithInitializers.ts, 3, 74))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 4, 21))
>x : Symbol(x, Decl(contextuallyTypedParametersWithInitializers.ts, 4, 32))
>input : Symbol(input, Decl(contextuallyTypedParametersWithInitializers.ts, 4, 52))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 4, 21))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 4, 21))

const f10 = function ({ foo = 42 }) { return foo };
>f10 : Symbol(f10, Decl(contextuallyTypedParametersWithInitializers.ts, 6, 5))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 6, 23))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 6, 23))

const f11 = id1(function ({ foo = 42 }) { return foo }); // Implicit any error
>f11 : Symbol(f11, Decl(contextuallyTypedParametersWithInitializers.ts, 7, 5))
>id1 : Symbol(id1, Decl(contextuallyTypedParametersWithInitializers.ts, 0, 0))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 7, 27))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 7, 27))

const f12 = id2(function ({ foo = 42 }) { return foo });
>f12 : Symbol(f12, Decl(contextuallyTypedParametersWithInitializers.ts, 8, 5))
>id2 : Symbol(id2, Decl(contextuallyTypedParametersWithInitializers.ts, 0, 37))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 8, 27))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 8, 27))

const f13 = id3(function ({ foo = 42 }) { return foo });
>f13 : Symbol(f13, Decl(contextuallyTypedParametersWithInitializers.ts, 9, 5))
>id3 : Symbol(id3, Decl(contextuallyTypedParametersWithInitializers.ts, 1, 61))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 9, 27))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 9, 27))

const f14 = id4(function ({ foo = 42 }) { return foo });
>f14 : Symbol(f14, Decl(contextuallyTypedParametersWithInitializers.ts, 10, 5))
>id4 : Symbol(id4, Decl(contextuallyTypedParametersWithInitializers.ts, 2, 70))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 10, 27))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 10, 27))

const f20 = function (foo = 42) { return foo };
>f20 : Symbol(f20, Decl(contextuallyTypedParametersWithInitializers.ts, 12, 5))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 12, 22))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 12, 22))

const f21 = id1(function (foo = 42) { return foo }); // Implicit any error
>f21 : Symbol(f21, Decl(contextuallyTypedParametersWithInitializers.ts, 13, 5))
>id1 : Symbol(id1, Decl(contextuallyTypedParametersWithInitializers.ts, 0, 0))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 13, 26))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 13, 26))

const f22 = id2(function (foo = 42) { return foo });
>f22 : Symbol(f22, Decl(contextuallyTypedParametersWithInitializers.ts, 14, 5))
>id2 : Symbol(id2, Decl(contextuallyTypedParametersWithInitializers.ts, 0, 37))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 14, 26))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 14, 26))

const f25 = id5(function (foo = 42) { return foo });
>f25 : Symbol(f25, Decl(contextuallyTypedParametersWithInitializers.ts, 15, 5))
>id5 : Symbol(id5, Decl(contextuallyTypedParametersWithInitializers.ts, 3, 74))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 15, 26))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 15, 26))

// Repro from #28816

function id<T>(input: T): T { return input }
>id : Symbol(id, Decl(contextuallyTypedParametersWithInitializers.ts, 15, 52))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 19, 12))
>input : Symbol(input, Decl(contextuallyTypedParametersWithInitializers.ts, 19, 15))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 19, 12))
>T : Symbol(T, Decl(contextuallyTypedParametersWithInitializers.ts, 19, 12))
>input : Symbol(input, Decl(contextuallyTypedParametersWithInitializers.ts, 19, 15))

function getFoo ({ foo = 42 }) {
>getFoo : Symbol(getFoo, Decl(contextuallyTypedParametersWithInitializers.ts, 19, 44))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 21, 18))

return foo;
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 21, 18))
}

const newGetFoo = id(getFoo);
>newGetFoo : Symbol(newGetFoo, Decl(contextuallyTypedParametersWithInitializers.ts, 25, 5))
>id : Symbol(id, Decl(contextuallyTypedParametersWithInitializers.ts, 15, 52))
>getFoo : Symbol(getFoo, Decl(contextuallyTypedParametersWithInitializers.ts, 19, 44))

const newGetFoo2 = id(function getFoo ({ foo = 42 }) {
>newGetFoo2 : Symbol(newGetFoo2, Decl(contextuallyTypedParametersWithInitializers.ts, 26, 5))
>id : Symbol(id, Decl(contextuallyTypedParametersWithInitializers.ts, 15, 52))
>getFoo : Symbol(getFoo, Decl(contextuallyTypedParametersWithInitializers.ts, 26, 22))
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 26, 40))

return foo;
>foo : Symbol(foo, Decl(contextuallyTypedParametersWithInitializers.ts, 26, 40))

});

Loading