Skip to content

Commit

Permalink
Revert "Allow (non-assert) type predicates to narrow by discriminant"… (
Browse files Browse the repository at this point in the history
  • Loading branch information
gabritto committed Mar 15, 2024
1 parent 7f11456 commit 485c7c5
Show file tree
Hide file tree
Showing 8 changed files with 434 additions and 61 deletions.
98 changes: 47 additions & 51 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26741,11 +26741,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
function hasMatchingArgument(expression: CallExpression | NewExpression, reference: Node) {
if (expression.arguments) {
for (const argument of expression.arguments) {
if (
isOrContainsMatchingReference(reference, argument)
|| optionalChainContainsReference(argument, reference)
|| getCandidateDiscriminantPropertyAccess(argument, reference)
) {
if (isOrContainsMatchingReference(reference, argument) || optionalChainContainsReference(argument, reference)) {
return true;
}
}
Expand All @@ -26759,51 +26755,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return false;
}

function getCandidateDiscriminantPropertyAccess(expr: Expression, reference: Node) {
if (isBindingPattern(reference) || isFunctionExpressionOrArrowFunction(reference) || isObjectLiteralMethod(reference)) {
// When the reference is a binding pattern or function or arrow expression, we are narrowing a pesudo-reference in
// getNarrowedTypeOfSymbol. An identifier for a destructuring variable declared in the same binding pattern or
// parameter declared in the same parameter list is a candidate.
if (isIdentifier(expr)) {
const symbol = getResolvedSymbol(expr);
const declaration = symbol.valueDeclaration;
if (declaration && (isBindingElement(declaration) || isParameter(declaration)) && reference === declaration.parent && !declaration.initializer && !declaration.dotDotDotToken) {
return declaration;
}
}
}
else if (isAccessExpression(expr)) {
// An access expression is a candidate if the reference matches the left hand expression.
if (isMatchingReference(reference, expr.expression)) {
return expr;
}
}
else if (isIdentifier(expr)) {
const symbol = getResolvedSymbol(expr);
if (isConstantVariable(symbol)) {
const declaration = symbol.valueDeclaration!;
// Given 'const x = obj.kind', allow 'x' as an alias for 'obj.kind'
if (
isVariableDeclaration(declaration) && !declaration.type && declaration.initializer && isAccessExpression(declaration.initializer) &&
isMatchingReference(reference, declaration.initializer.expression)
) {
return declaration.initializer;
}
// Given 'const { kind: x } = obj', allow 'x' as an alias for 'obj.kind'
if (isBindingElement(declaration) && !declaration.initializer) {
const parent = declaration.parent.parent;
if (
isVariableDeclaration(parent) && !parent.type && parent.initializer && (isIdentifier(parent.initializer) || isAccessExpression(parent.initializer)) &&
isMatchingReference(reference, parent.initializer)
) {
return declaration;
}
}
}
}
return undefined;
}

function getFlowNodeId(flow: FlowNode): number {
if (!flow.id || flow.id < 0) {
flow.id = nextFlowId;
Expand Down Expand Up @@ -28160,12 +28111,57 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return result;
}

function getCandidateDiscriminantPropertyAccess(expr: Expression) {
if (isBindingPattern(reference) || isFunctionExpressionOrArrowFunction(reference) || isObjectLiteralMethod(reference)) {
// When the reference is a binding pattern or function or arrow expression, we are narrowing a pesudo-reference in
// getNarrowedTypeOfSymbol. An identifier for a destructuring variable declared in the same binding pattern or
// parameter declared in the same parameter list is a candidate.
if (isIdentifier(expr)) {
const symbol = getResolvedSymbol(expr);
const declaration = symbol.valueDeclaration;
if (declaration && (isBindingElement(declaration) || isParameter(declaration)) && reference === declaration.parent && !declaration.initializer && !declaration.dotDotDotToken) {
return declaration;
}
}
}
else if (isAccessExpression(expr)) {
// An access expression is a candidate if the reference matches the left hand expression.
if (isMatchingReference(reference, expr.expression)) {
return expr;
}
}
else if (isIdentifier(expr)) {
const symbol = getResolvedSymbol(expr);
if (isConstantVariable(symbol)) {
const declaration = symbol.valueDeclaration!;
// Given 'const x = obj.kind', allow 'x' as an alias for 'obj.kind'
if (
isVariableDeclaration(declaration) && !declaration.type && declaration.initializer && isAccessExpression(declaration.initializer) &&
isMatchingReference(reference, declaration.initializer.expression)
) {
return declaration.initializer;
}
// Given 'const { kind: x } = obj', allow 'x' as an alias for 'obj.kind'
if (isBindingElement(declaration) && !declaration.initializer) {
const parent = declaration.parent.parent;
if (
isVariableDeclaration(parent) && !parent.type && parent.initializer && (isIdentifier(parent.initializer) || isAccessExpression(parent.initializer)) &&
isMatchingReference(reference, parent.initializer)
) {
return declaration;
}
}
}
}
return undefined;
}

function getDiscriminantPropertyAccess(expr: Expression, computedType: Type) {
// As long as the computed type is a subset of the declared type, we use the full declared type to detect
// a discriminant property. In cases where the computed type isn't a subset, e.g because of a preceding type
// predicate narrowing, we use the actual computed type.
if (declaredType.flags & TypeFlags.Union || computedType.flags & TypeFlags.Union) {
const access = getCandidateDiscriminantPropertyAccess(expr, reference);
const access = getCandidateDiscriminantPropertyAccess(expr);
if (access) {
const name = getAccessedPropertyName(access);
if (name) {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3505,6 +3505,7 @@ export function convertJsonOption(
convertJsonOption(opt.element, value, basePath, errors, propertyAssignment, valueExpression, sourceFile);
}
else if (!isString(opt.type)) {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
return convertJsonOptionOfCustomType(opt as CommandLineOptionOfCustomType, value as string, errors, valueExpression, sourceFile);
}
const validatedValue = validateJsonOptionValue(opt, value, errors, valueExpression, sourceFile);
Expand Down
69 changes: 69 additions & 0 deletions tests/baselines/reference/discriminantNarrowingCouldBeCircular.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//// [tests/cases/compiler/discriminantNarrowingCouldBeCircular.ts] ////

//// [discriminantNarrowingCouldBeCircular.ts]
// #57705, 57690
declare function is<T>(v: T): v is T;
const o: Record<string, string> | undefined = {};
if (o) {
for (const key in o) {
const value = o[key];
if (is<string>(value)) {
}
}
}

type SomeRecord = { a: string };
declare const kPresentationInheritanceParents: { [tagName: string]: string[] };
declare function parentElementOrShadowHost(element: SomeRecord): SomeRecord | undefined;

function getImplicitAriaRole(element: SomeRecord) {
let ancestor: SomeRecord | null = element;
while (ancestor) {
const parent = parentElementOrShadowHost(ancestor);
const parents = kPresentationInheritanceParents[ancestor.a];
if (!parents || !parent || !parents.includes(parent.a))
break;
ancestor = parent;
}
}

declare function isPlainObject2<T>(
data: unknown,
): data is Record<PropertyKey, unknown>;

declare const myObj2: unknown;
if (isPlainObject2(myObj2)) {
for (const key of ["a", "b", "c"]) {
const deeper = myObj2[key];
const deeperKeys = isPlainObject2(deeper) ? Object.keys(deeper) : [];
}
}


//// [discriminantNarrowingCouldBeCircular.js]
"use strict";
var o = {};
if (o) {
for (var key in o) {
var value = o[key];
if (is(value)) {
}
}
}
function getImplicitAriaRole(element) {
var ancestor = element;
while (ancestor) {
var parent = parentElementOrShadowHost(ancestor);
var parents = kPresentationInheritanceParents[ancestor.a];
if (!parents || !parent || !parents.includes(parent.a))
break;
ancestor = parent;
}
}
if (isPlainObject2(myObj2)) {
for (var _i = 0, _a = ["a", "b", "c"]; _i < _a.length; _i++) {
var key = _a[_i];
var deeper = myObj2[key];
var deeperKeys = isPlainObject2(deeper) ? Object.keys(deeper) : [];
}
}
129 changes: 129 additions & 0 deletions tests/baselines/reference/discriminantNarrowingCouldBeCircular.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
//// [tests/cases/compiler/discriminantNarrowingCouldBeCircular.ts] ////

=== discriminantNarrowingCouldBeCircular.ts ===
// #57705, 57690
declare function is<T>(v: T): v is T;
>is : Symbol(is, Decl(discriminantNarrowingCouldBeCircular.ts, 0, 0))
>T : Symbol(T, Decl(discriminantNarrowingCouldBeCircular.ts, 1, 20))
>v : Symbol(v, Decl(discriminantNarrowingCouldBeCircular.ts, 1, 23))
>T : Symbol(T, Decl(discriminantNarrowingCouldBeCircular.ts, 1, 20))
>v : Symbol(v, Decl(discriminantNarrowingCouldBeCircular.ts, 1, 23))
>T : Symbol(T, Decl(discriminantNarrowingCouldBeCircular.ts, 1, 20))

const o: Record<string, string> | undefined = {};
>o : Symbol(o, Decl(discriminantNarrowingCouldBeCircular.ts, 2, 5))
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --))

if (o) {
>o : Symbol(o, Decl(discriminantNarrowingCouldBeCircular.ts, 2, 5))

for (const key in o) {
>key : Symbol(key, Decl(discriminantNarrowingCouldBeCircular.ts, 4, 12))
>o : Symbol(o, Decl(discriminantNarrowingCouldBeCircular.ts, 2, 5))

const value = o[key];
>value : Symbol(value, Decl(discriminantNarrowingCouldBeCircular.ts, 5, 9))
>o : Symbol(o, Decl(discriminantNarrowingCouldBeCircular.ts, 2, 5))
>key : Symbol(key, Decl(discriminantNarrowingCouldBeCircular.ts, 4, 12))

if (is<string>(value)) {
>is : Symbol(is, Decl(discriminantNarrowingCouldBeCircular.ts, 0, 0))
>value : Symbol(value, Decl(discriminantNarrowingCouldBeCircular.ts, 5, 9))
}
}
}

type SomeRecord = { a: string };
>SomeRecord : Symbol(SomeRecord, Decl(discriminantNarrowingCouldBeCircular.ts, 9, 1))
>a : Symbol(a, Decl(discriminantNarrowingCouldBeCircular.ts, 11, 19))

declare const kPresentationInheritanceParents: { [tagName: string]: string[] };
>kPresentationInheritanceParents : Symbol(kPresentationInheritanceParents, Decl(discriminantNarrowingCouldBeCircular.ts, 12, 13))
>tagName : Symbol(tagName, Decl(discriminantNarrowingCouldBeCircular.ts, 12, 50))

declare function parentElementOrShadowHost(element: SomeRecord): SomeRecord | undefined;
>parentElementOrShadowHost : Symbol(parentElementOrShadowHost, Decl(discriminantNarrowingCouldBeCircular.ts, 12, 79))
>element : Symbol(element, Decl(discriminantNarrowingCouldBeCircular.ts, 13, 43))
>SomeRecord : Symbol(SomeRecord, Decl(discriminantNarrowingCouldBeCircular.ts, 9, 1))
>SomeRecord : Symbol(SomeRecord, Decl(discriminantNarrowingCouldBeCircular.ts, 9, 1))

function getImplicitAriaRole(element: SomeRecord) {
>getImplicitAriaRole : Symbol(getImplicitAriaRole, Decl(discriminantNarrowingCouldBeCircular.ts, 13, 88))
>element : Symbol(element, Decl(discriminantNarrowingCouldBeCircular.ts, 15, 29))
>SomeRecord : Symbol(SomeRecord, Decl(discriminantNarrowingCouldBeCircular.ts, 9, 1))

let ancestor: SomeRecord | null = element;
>ancestor : Symbol(ancestor, Decl(discriminantNarrowingCouldBeCircular.ts, 16, 5))
>SomeRecord : Symbol(SomeRecord, Decl(discriminantNarrowingCouldBeCircular.ts, 9, 1))
>element : Symbol(element, Decl(discriminantNarrowingCouldBeCircular.ts, 15, 29))

while (ancestor) {
>ancestor : Symbol(ancestor, Decl(discriminantNarrowingCouldBeCircular.ts, 16, 5))

const parent = parentElementOrShadowHost(ancestor);
>parent : Symbol(parent, Decl(discriminantNarrowingCouldBeCircular.ts, 18, 9))
>parentElementOrShadowHost : Symbol(parentElementOrShadowHost, Decl(discriminantNarrowingCouldBeCircular.ts, 12, 79))
>ancestor : Symbol(ancestor, Decl(discriminantNarrowingCouldBeCircular.ts, 16, 5))

const parents = kPresentationInheritanceParents[ancestor.a];
>parents : Symbol(parents, Decl(discriminantNarrowingCouldBeCircular.ts, 19, 9))
>kPresentationInheritanceParents : Symbol(kPresentationInheritanceParents, Decl(discriminantNarrowingCouldBeCircular.ts, 12, 13))
>ancestor.a : Symbol(a, Decl(discriminantNarrowingCouldBeCircular.ts, 11, 19))
>ancestor : Symbol(ancestor, Decl(discriminantNarrowingCouldBeCircular.ts, 16, 5))
>a : Symbol(a, Decl(discriminantNarrowingCouldBeCircular.ts, 11, 19))

if (!parents || !parent || !parents.includes(parent.a))
>parents : Symbol(parents, Decl(discriminantNarrowingCouldBeCircular.ts, 19, 9))
>parent : Symbol(parent, Decl(discriminantNarrowingCouldBeCircular.ts, 18, 9))
>parents.includes : Symbol(Array.includes, Decl(lib.es2016.array.include.d.ts, --, --))
>parents : Symbol(parents, Decl(discriminantNarrowingCouldBeCircular.ts, 19, 9))
>includes : Symbol(Array.includes, Decl(lib.es2016.array.include.d.ts, --, --))
>parent.a : Symbol(a, Decl(discriminantNarrowingCouldBeCircular.ts, 11, 19))
>parent : Symbol(parent, Decl(discriminantNarrowingCouldBeCircular.ts, 18, 9))
>a : Symbol(a, Decl(discriminantNarrowingCouldBeCircular.ts, 11, 19))

break;
ancestor = parent;
>ancestor : Symbol(ancestor, Decl(discriminantNarrowingCouldBeCircular.ts, 16, 5))
>parent : Symbol(parent, Decl(discriminantNarrowingCouldBeCircular.ts, 18, 9))
}
}

declare function isPlainObject2<T>(
>isPlainObject2 : Symbol(isPlainObject2, Decl(discriminantNarrowingCouldBeCircular.ts, 24, 1))
>T : Symbol(T, Decl(discriminantNarrowingCouldBeCircular.ts, 26, 32))

data: unknown,
>data : Symbol(data, Decl(discriminantNarrowingCouldBeCircular.ts, 26, 35))

): data is Record<PropertyKey, unknown>;
>data : Symbol(data, Decl(discriminantNarrowingCouldBeCircular.ts, 26, 35))
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --))
>PropertyKey : Symbol(PropertyKey, Decl(lib.es5.d.ts, --, --))

declare const myObj2: unknown;
>myObj2 : Symbol(myObj2, Decl(discriminantNarrowingCouldBeCircular.ts, 30, 15))

if (isPlainObject2(myObj2)) {
>isPlainObject2 : Symbol(isPlainObject2, Decl(discriminantNarrowingCouldBeCircular.ts, 24, 1))
>myObj2 : Symbol(myObj2, Decl(discriminantNarrowingCouldBeCircular.ts, 30, 15))

for (const key of ["a", "b", "c"]) {
>key : Symbol(key, Decl(discriminantNarrowingCouldBeCircular.ts, 32, 16))

const deeper = myObj2[key];
>deeper : Symbol(deeper, Decl(discriminantNarrowingCouldBeCircular.ts, 33, 13))
>myObj2 : Symbol(myObj2, Decl(discriminantNarrowingCouldBeCircular.ts, 30, 15))
>key : Symbol(key, Decl(discriminantNarrowingCouldBeCircular.ts, 32, 16))

const deeperKeys = isPlainObject2(deeper) ? Object.keys(deeper) : [];
>deeperKeys : Symbol(deeperKeys, Decl(discriminantNarrowingCouldBeCircular.ts, 34, 13))
>isPlainObject2 : Symbol(isPlainObject2, Decl(discriminantNarrowingCouldBeCircular.ts, 24, 1))
>deeper : Symbol(deeper, Decl(discriminantNarrowingCouldBeCircular.ts, 33, 13))
>Object.keys : Symbol(ObjectConstructor.keys, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --))
>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>keys : Symbol(ObjectConstructor.keys, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --))
>deeper : Symbol(deeper, Decl(discriminantNarrowingCouldBeCircular.ts, 33, 13))
}
}

0 comments on commit 485c7c5

Please sign in to comment.