Skip to content

Improve binding element type inference using CheckMode (rebased) #56753

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

Merged
40 changes: 30 additions & 10 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11460,7 +11460,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// contextual type or, if the element itself is a binding pattern, with the type implied by that binding
// pattern.
const contextualType = isBindingPattern(element.name) ? getTypeFromBindingPattern(element.name, /*includePatternInType*/ true, /*reportErrors*/ false) : unknownType;
return addOptionality(widenTypeInferredFromInitializer(element, checkDeclarationInitializer(element, CheckMode.Normal, contextualType)));
return addOptionality(widenTypeInferredFromInitializer(element, checkDeclarationInitializer(element, reportErrors ? CheckMode.Normal : CheckMode.Contextual, contextualType)));
}
if (isBindingPattern(element.name)) {
return getTypeFromBindingPattern(element.name, includePatternInType, reportErrors);
Expand Down Expand Up @@ -11618,24 +11618,24 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return false;
}

function getTypeOfVariableOrParameterOrProperty(symbol: Symbol): Type {
function getTypeOfVariableOrParameterOrProperty(symbol: Symbol, checkMode?: CheckMode): Type {
const links = getSymbolLinks(symbol);
if (!links.type) {
const type = getTypeOfVariableOrParameterOrPropertyWorker(symbol);
const type = getTypeOfVariableOrParameterOrPropertyWorker(symbol, checkMode);
// For a contextually typed parameter it is possible that a type has already
// been assigned (in assignTypeToParameterAndFixTypeParameters), and we want
// to preserve this type. In fact, we need to _prefer_ that type, but it won't
// be assigned until contextual typing is complete, so we need to defer in
// cases where contextual typing may take place.
if (!links.type && !isParameterOfContextSensitiveSignature(symbol)) {
if (!links.type && !isParameterOfContextSensitiveSignature(symbol) && !checkMode) {
links.type = type;
}
return type;
}
return links.type;
}

function getTypeOfVariableOrParameterOrPropertyWorker(symbol: Symbol): Type {
function getTypeOfVariableOrParameterOrPropertyWorker(symbol: Symbol, checkMode?: CheckMode): Type {
// Handle prototype property
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that there is a non-zero chance that this PR will get picked up at some point (cc @jakebailey ). If the team decides to go with it then this should return errorType:

Suggested change
return anyType;
return errorType;

I think that there is no harm in returning that here already now (unless proven otherwise)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment. Since, it's an error (of course, not a circularity one but still), I think you're right about changing the return value from anyType to errorType.

Applied in the last commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I might have read this late or smth. I think that errorType should be returned if you encounter the circularity error but here you explicitly want to prevent the error from being reported and I think that it should extend to the returned type. So despite my initial reaction, I would recommend returning anyType here. Sorry for the confusion 🙈

if (symbol.flags & SymbolFlags.Prototype) {
return getTypeOfPrototypeProperty(symbol);
Expand Down Expand Up @@ -11678,6 +11678,16 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (symbol.flags & SymbolFlags.ValueModule && !(symbol.flags & SymbolFlags.Assignment)) {
return getTypeOfFuncClassEnumModule(symbol);
}

// When trying to get the *contextual* type of a binding element, it's possible to fall in a loop and therefore
// end up in a circularity-like situation. This is not a true circularity so we should not report such an error.
// For example, here the looping could happen when trying to get the type of `a` (binding element):
//
// const { a, b = a } = { a: 0 }
//
if (isBindingElement(declaration) && checkMode === CheckMode.Contextual) {
return errorType;
}
return reportCircularityError(symbol);
}
let type: Type;
Expand Down Expand Up @@ -11750,6 +11760,16 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (symbol.flags & SymbolFlags.ValueModule && !(symbol.flags & SymbolFlags.Assignment)) {
return getTypeOfFuncClassEnumModule(symbol);
}

// When trying to get the *contextual* type of a binding element, it's possible to fall in a loop and therefore
// end up in a circularity-like situation. This is not a true circularity so we should not report such an error.
// For example, here the looping could happen when trying to get the type of `a` (binding element):
//
// const { a, b = a } = { a: 0 }
//
if (isBindingElement(declaration) && checkMode === CheckMode.Contextual) {
return type;
}
return reportCircularityError(symbol);
}
return type;
Expand Down Expand Up @@ -12032,7 +12052,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return getTypeOfSymbol(symbol);
}

function getTypeOfSymbol(symbol: Symbol): Type {
function getTypeOfSymbol(symbol: Symbol, checkMode?: CheckMode): Type {
const checkFlags = getCheckFlags(symbol);
if (checkFlags & CheckFlags.DeferredType) {
return getTypeOfSymbolWithDeferredType(symbol);
Expand All @@ -12047,7 +12067,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return getTypeOfReverseMappedSymbol(symbol as ReverseMappedSymbol);
}
if (symbol.flags & (SymbolFlags.Variable | SymbolFlags.Property)) {
return getTypeOfVariableOrParameterOrProperty(symbol);
return getTypeOfVariableOrParameterOrProperty(symbol, checkMode);
}
if (symbol.flags & (SymbolFlags.Function | SymbolFlags.Method | SymbolFlags.Class | SymbolFlags.Enum | SymbolFlags.ValueModule)) {
return getTypeOfFuncClassEnumModule(symbol);
Expand Down Expand Up @@ -29069,8 +29089,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
}

function getNarrowedTypeOfSymbol(symbol: Symbol, location: Identifier) {
const type = getTypeOfSymbol(symbol);
function getNarrowedTypeOfSymbol(symbol: Symbol, location: Identifier, checkMode?: CheckMode) {
const type = getTypeOfSymbol(symbol, checkMode);
const declaration = symbol.valueDeclaration;
if (declaration) {
// If we have a non-rest binding element with no initializer declared as a const variable or a const-like
Expand Down Expand Up @@ -29233,7 +29253,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {

checkNestedBlockScopedBinding(node, symbol);

let type = getNarrowedTypeOfSymbol(localOrExportSymbol, node);
let type = getNarrowedTypeOfSymbol(localOrExportSymbol, node, checkMode);
const assignmentKind = getAssignmentTargetKind(node);

if (assignmentKind) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@

=== destructuringArrayBindingPatternAndAssignment3.ts ===
const [a, b = a] = [1]; // ok
>a : any
>b : any
>a : any
>a : number
>b : number
>a : number
>[1] : [number]
>1 : 1

const [c, d = c, e = e] = [1]; // error for e = e
>c : any
>d : any
>c : any
>c : number
>d : number
>c : number
>e : any
>e : any
>[1] : [number]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
//// [tests/cases/conformance/es6/destructuring/destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts] ////

//// [destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts]
// To be inferred as `number`
function f1() {
const [a1, b1 = a1] = [1];
const [a2, b2 = 1 + a2] = [1];
}

// To be inferred as `string`
function f2() {
const [a1, b1 = a1] = ['hi'];
const [a2, b2 = a2 + '!'] = ['hi'];
}

// To be inferred as `string | number`
function f3() {
const [a1, b1 = a1] = ['hi', 1];
const [a2, b2 = a2 + '!'] = ['hi', 1];
}

// Based on comment:
// - https://github.com/microsoft/TypeScript/issues/49989#issuecomment-1852694486
declare const yadda: [number, number] | undefined
function f4() {
const [ a, b = a ] = yadda ?? [];
}


//// [destructuringArrayBindingPatternAndAssignment5SiblingInitializer.js]
// To be inferred as `number`
function f1() {
var _a = [1], a1 = _a[0], _b = _a[1], b1 = _b === void 0 ? a1 : _b;
var _c = [1], a2 = _c[0], _d = _c[1], b2 = _d === void 0 ? 1 + a2 : _d;
}
// To be inferred as `string`
function f2() {
var _a = ['hi'], a1 = _a[0], _b = _a[1], b1 = _b === void 0 ? a1 : _b;
var _c = ['hi'], a2 = _c[0], _d = _c[1], b2 = _d === void 0 ? a2 + '!' : _d;
}
// To be inferred as `string | number`
function f3() {
var _a = ['hi', 1], a1 = _a[0], _b = _a[1], b1 = _b === void 0 ? a1 : _b;
var _c = ['hi', 1], a2 = _c[0], _d = _c[1], b2 = _d === void 0 ? a2 + '!' : _d;
}
function f4() {
var _a = yadda !== null && yadda !== void 0 ? yadda : [], a = _a[0], _b = _a[1], b = _b === void 0 ? a : _b;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
//// [tests/cases/conformance/es6/destructuring/destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts] ////

=== destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts ===
// To be inferred as `number`
function f1() {
>f1 : Symbol(f1, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 0, 0))

const [a1, b1 = a1] = [1];
>a1 : Symbol(a1, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 2, 11))
>b1 : Symbol(b1, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 2, 14))
>a1 : Symbol(a1, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 2, 11))

const [a2, b2 = 1 + a2] = [1];
>a2 : Symbol(a2, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 3, 11))
>b2 : Symbol(b2, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 3, 14))
>a2 : Symbol(a2, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 3, 11))
}

// To be inferred as `string`
function f2() {
>f2 : Symbol(f2, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 4, 1))

const [a1, b1 = a1] = ['hi'];
>a1 : Symbol(a1, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 8, 11))
>b1 : Symbol(b1, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 8, 14))
>a1 : Symbol(a1, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 8, 11))

const [a2, b2 = a2 + '!'] = ['hi'];
>a2 : Symbol(a2, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 9, 11))
>b2 : Symbol(b2, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 9, 14))
>a2 : Symbol(a2, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 9, 11))
}

// To be inferred as `string | number`
function f3() {
>f3 : Symbol(f3, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 10, 1))

const [a1, b1 = a1] = ['hi', 1];
>a1 : Symbol(a1, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 14, 11))
>b1 : Symbol(b1, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 14, 14))
>a1 : Symbol(a1, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 14, 11))

const [a2, b2 = a2 + '!'] = ['hi', 1];
>a2 : Symbol(a2, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 15, 11))
>b2 : Symbol(b2, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 15, 14))
>a2 : Symbol(a2, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 15, 11))
}

// Based on comment:
// - https://github.com/microsoft/TypeScript/issues/49989#issuecomment-1852694486
declare const yadda: [number, number] | undefined
>yadda : Symbol(yadda, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 20, 13))

function f4() {
>f4 : Symbol(f4, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 20, 49))

const [ a, b = a ] = yadda ?? [];
>a : Symbol(a, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 22, 11))
>b : Symbol(b, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 22, 14))
>a : Symbol(a, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 22, 11))
>yadda : Symbol(yadda, Decl(destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts, 20, 13))
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
//// [tests/cases/conformance/es6/destructuring/destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts] ////

=== destructuringArrayBindingPatternAndAssignment5SiblingInitializer.ts ===
// To be inferred as `number`
function f1() {
>f1 : () => void

const [a1, b1 = a1] = [1];
>a1 : number
>b1 : number
>a1 : number
>[1] : [number]
>1 : 1

const [a2, b2 = 1 + a2] = [1];
>a2 : number
>b2 : number
>1 + a2 : number
>1 : 1
>a2 : number
>[1] : [number]
>1 : 1
}

// To be inferred as `string`
function f2() {
>f2 : () => void

const [a1, b1 = a1] = ['hi'];
>a1 : string
>b1 : string
>a1 : string
>['hi'] : [string]
>'hi' : "hi"

const [a2, b2 = a2 + '!'] = ['hi'];
>a2 : string
>b2 : string
>a2 + '!' : string
>a2 : string
>'!' : "!"
>['hi'] : [string]
>'hi' : "hi"
}

// To be inferred as `string | number`
function f3() {
>f3 : () => void

const [a1, b1 = a1] = ['hi', 1];
>a1 : string
>b1 : string | number
>a1 : string
>['hi', 1] : [string, number]
>'hi' : "hi"
>1 : 1

const [a2, b2 = a2 + '!'] = ['hi', 1];
>a2 : string
>b2 : string | number
>a2 + '!' : string
>a2 : string
>'!' : "!"
>['hi', 1] : [string, number]
>'hi' : "hi"
>1 : 1
}

// Based on comment:
// - https://github.com/microsoft/TypeScript/issues/49989#issuecomment-1852694486
declare const yadda: [number, number] | undefined
>yadda : [number, number]

function f4() {
>f4 : () => void

const [ a, b = a ] = yadda ?? [];
>a : number
>b : number
>a : number
>yadda ?? [] : [number, number] | []
>yadda : [number, number]
>[] : []
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
//// [tests/cases/conformance/es6/destructuring/destructuringObjectBindingPatternAndAssignment9SiblingInitializer.ts] ////

//// [destructuringObjectBindingPatternAndAssignment9SiblingInitializer.ts]
// To be inferred as `number`
function f1() {
const { a1, b1 = a1 } = { a1: 1 };
const { a2, b2 = 1 + a2 } = { a2: 1 };
}

// To be inferred as `string`
function f2() {
const { a1, b1 = a1 } = { a1: 'hi' };
const { a2, b2 = a2 + '!' } = { a2: 'hi' };
}

// To be inferred as `string | number`
function f3() {
const { a1, b1 = a1 } = { a1: 'hi', b1: 1 };
const { a2, b2 = a2 + '!' } = { a2: 'hi', b2: 1 };
}

// Based on comment:
// - https://github.com/microsoft/TypeScript/issues/49989#issuecomment-1852694486
declare const yadda: { a?: number, b?: number } | undefined
function f4() {
const { a, b = a } = yadda ?? {};
}


//// [destructuringObjectBindingPatternAndAssignment9SiblingInitializer.js]
// To be inferred as `number`
function f1() {
var _a = { a1: 1 }, a1 = _a.a1, _b = _a.b1, b1 = _b === void 0 ? a1 : _b;
var _c = { a2: 1 }, a2 = _c.a2, _d = _c.b2, b2 = _d === void 0 ? 1 + a2 : _d;
}
// To be inferred as `string`
function f2() {
var _a = { a1: 'hi' }, a1 = _a.a1, _b = _a.b1, b1 = _b === void 0 ? a1 : _b;
var _c = { a2: 'hi' }, a2 = _c.a2, _d = _c.b2, b2 = _d === void 0 ? a2 + '!' : _d;
}
// To be inferred as `string | number`
function f3() {
var _a = { a1: 'hi', b1: 1 }, a1 = _a.a1, _b = _a.b1, b1 = _b === void 0 ? a1 : _b;
var _c = { a2: 'hi', b2: 1 }, a2 = _c.a2, _d = _c.b2, b2 = _d === void 0 ? a2 + '!' : _d;
}
function f4() {
var _a = yadda !== null && yadda !== void 0 ? yadda : {}, a = _a.a, _b = _a.b, b = _b === void 0 ? a : _b;
}
Loading