From 4942fd2b84aa5989dfb78c1778fa15724a4708e7 Mon Sep 17 00:00:00 2001 From: Eli Barzilay Date: Thu, 19 Dec 2019 18:51:55 -0500 Subject: [PATCH] Make `checkPropertyNotUsedBeforeDeclaration` ignore properties of properties Use `isAccessExpression` to cover both `PropertyAccess` and `ElementAccess`. Also use it in a few other places that used both explicitly. (And also fix a random weird formatting.) Fixes #32721. --- src/compiler/checker.ts | 39 ++++++------ .../reference/recursiveFieldSetting.js | 42 +++++++++++++ .../reference/recursiveFieldSetting.symbols | 57 ++++++++++++++++++ .../reference/recursiveFieldSetting.types | 59 +++++++++++++++++++ tests/cases/compiler/recursiveFieldSetting.ts | 17 ++++++ 5 files changed, 195 insertions(+), 19 deletions(-) create mode 100644 tests/baselines/reference/recursiveFieldSetting.js create mode 100644 tests/baselines/reference/recursiveFieldSetting.symbols create mode 100644 tests/baselines/reference/recursiveFieldSetting.types create mode 100644 tests/cases/compiler/recursiveFieldSetting.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 14945a1796e..137ba91f23a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7619,8 +7619,8 @@ namespace ts { return anyType; } else if (declaration.kind === SyntaxKind.BinaryExpression || - (declaration.kind === SyntaxKind.PropertyAccessExpression || declaration.kind === SyntaxKind.ElementAccessExpression) && - declaration.parent.kind === SyntaxKind.BinaryExpression) { + isAccessExpression(declaration) && + declaration.parent.kind === SyntaxKind.BinaryExpression) { return getWidenedTypeForAssignmentDeclaration(symbol); } else if (symbol.flags & SymbolFlags.ValueModule && declaration && isSourceFile(declaration) && declaration.commonJsModuleIndicator) { @@ -21015,8 +21015,8 @@ namespace ts { const parent = func.parent; if (parent.kind === SyntaxKind.BinaryExpression && (parent).operatorToken.kind === SyntaxKind.EqualsToken) { const target = (parent).left; - if (target.kind === SyntaxKind.PropertyAccessExpression || target.kind === SyntaxKind.ElementAccessExpression) { - const { expression } = target as AccessExpression; + if (isAccessExpression(target)) { + const { expression } = target; // Don't contextually type `this` as `exports` in `exports.Point = function(x, y) { this.x = x; this.y = y; }` if (inJs && isIdentifier(expression)) { const sourceFile = getSourceFileOfNode(parent); @@ -23208,7 +23208,7 @@ namespace ts { // assignment target, and the referenced property was declared as a variable, property, // accessor, or optional method. const assignmentKind = getAssignmentTargetKind(node); - if (node.kind !== SyntaxKind.ElementAccessExpression && node.kind !== SyntaxKind.PropertyAccessExpression || + if (!isAccessExpression(node) || assignmentKind === AssignmentKind.Definite || prop && !(prop.flags & (SymbolFlags.Variable | SymbolFlags.Property | SymbolFlags.Accessor)) && !(prop.flags & SymbolFlags.Method && propType.flags & TypeFlags.Union)) { return propType; @@ -23250,8 +23250,9 @@ namespace ts { let diagnosticMessage; const declarationName = idText(right); - if (isInPropertyInitializer(node) && - !isBlockScopedNameDeclaredBeforeUse(valueDeclaration, right) + if (isInPropertyInitializer(node) + && !(isAccessExpression(node) && isAccessExpression(node.expression)) + && !isBlockScopedNameDeclaredBeforeUse(valueDeclaration, right) && !isPropertyDeclaredInAncestorClass(prop)) { diagnosticMessage = error(right, Diagnostics.Property_0_is_used_before_its_initialization, declarationName); } @@ -24144,8 +24145,8 @@ namespace ts { function getThisArgumentOfCall(node: CallLikeExpression): LeftHandSideExpression | undefined { if (node.kind === SyntaxKind.CallExpression) { const callee = skipOuterExpressions(node.expression); - if (callee.kind === SyntaxKind.PropertyAccessExpression || callee.kind === SyntaxKind.ElementAccessExpression) { - return (callee as AccessExpression).expression; + if (isAccessExpression(callee)) { + return callee.expression; } } } @@ -26499,8 +26500,8 @@ namespace ts { if (isReadonlySymbol(symbol)) { // Allow assignments to readonly properties within constructors of the same class declaration. if (symbol.flags & SymbolFlags.Property && - (expr.kind === SyntaxKind.PropertyAccessExpression || expr.kind === SyntaxKind.ElementAccessExpression) && - (expr as AccessExpression).expression.kind === SyntaxKind.ThisKeyword) { + isAccessExpression(expr) && + expr.expression.kind === SyntaxKind.ThisKeyword) { // Look for if this is the constructor for the class that `symbol` is a property of. const ctor = getContainingFunction(expr); if (!(ctor && ctor.kind === SyntaxKind.Constructor)) { @@ -26522,9 +26523,9 @@ namespace ts { } return true; } - if (expr.kind === SyntaxKind.PropertyAccessExpression || expr.kind === SyntaxKind.ElementAccessExpression) { + if (isAccessExpression(expr)) { // references through namespace import should be readonly - const node = skipParentheses((expr as AccessExpression).expression); + const node = skipParentheses(expr.expression); if (node.kind === SyntaxKind.Identifier) { const symbol = getNodeLinks(node).resolvedSymbol!; if (symbol.flags & SymbolFlags.Alias) { @@ -26539,7 +26540,7 @@ namespace ts { function checkReferenceExpression(expr: Expression, invalidReferenceMessage: DiagnosticMessage, invalidOptionalChainMessage: DiagnosticMessage): boolean { // References are combinations of identifiers, parentheses, and property accesses. const node = skipOuterExpressions(expr, OuterExpressionKinds.Assertions | OuterExpressionKinds.Parentheses); - if (node.kind !== SyntaxKind.Identifier && node.kind !== SyntaxKind.PropertyAccessExpression && node.kind !== SyntaxKind.ElementAccessExpression) { + if (node.kind !== SyntaxKind.Identifier && !isAccessExpression(node)) { error(expr, invalidReferenceMessage); return false; } @@ -26553,7 +26554,7 @@ namespace ts { function checkDeleteExpression(node: DeleteExpression): Type { checkExpression(node.expression); const expr = skipParentheses(node.expression); - if (expr.kind !== SyntaxKind.PropertyAccessExpression && expr.kind !== SyntaxKind.ElementAccessExpression) { + if (!isAccessExpression(expr)) { error(expr, Diagnostics.The_operand_of_a_delete_operator_must_be_a_property_reference); return booleanType; } @@ -36174,10 +36175,10 @@ namespace ts { } function isSimpleLiteralEnumReference(expr: Expression) { - if ( - (isPropertyAccessExpression(expr) || (isElementAccessExpression(expr) && isStringOrNumberLiteralExpression(expr.argumentExpression))) && - isEntityNameExpression(expr.expression) - ) return !!(checkExpressionCached(expr).flags & TypeFlags.EnumLiteral); + if ((isPropertyAccessExpression(expr) || (isElementAccessExpression(expr) && isStringOrNumberLiteralExpression(expr.argumentExpression))) && + isEntityNameExpression(expr.expression)) { + return !!(checkExpressionCached(expr).flags & TypeFlags.EnumLiteral); + } } function checkAmbientInitializer(node: VariableDeclaration | PropertyDeclaration | PropertySignature) { diff --git a/tests/baselines/reference/recursiveFieldSetting.js b/tests/baselines/reference/recursiveFieldSetting.js new file mode 100644 index 00000000000..afa91b433e0 --- /dev/null +++ b/tests/baselines/reference/recursiveFieldSetting.js @@ -0,0 +1,42 @@ +//// [recursiveFieldSetting.ts] +// #32721 + +class Recursive1 { + constructor(private readonly parent?: Recursive1) {} + private depth: number = this.parent ? this.parent.depth + 1 : 0; +} + +class Recursive2 { + parent!: Recursive2; + depth: number = this.parent.depth; +} + +class Recursive3 { + parent!: Recursive3; + depth: number = this.parent.alpha; + alpha = 0; +} + + +//// [recursiveFieldSetting.js] +// #32721 +var Recursive1 = /** @class */ (function () { + function Recursive1(parent) { + this.parent = parent; + this.depth = this.parent ? this.parent.depth + 1 : 0; + } + return Recursive1; +}()); +var Recursive2 = /** @class */ (function () { + function Recursive2() { + this.depth = this.parent.depth; + } + return Recursive2; +}()); +var Recursive3 = /** @class */ (function () { + function Recursive3() { + this.depth = this.parent.alpha; + this.alpha = 0; + } + return Recursive3; +}()); diff --git a/tests/baselines/reference/recursiveFieldSetting.symbols b/tests/baselines/reference/recursiveFieldSetting.symbols new file mode 100644 index 00000000000..6e59f8d790c --- /dev/null +++ b/tests/baselines/reference/recursiveFieldSetting.symbols @@ -0,0 +1,57 @@ +=== tests/cases/compiler/recursiveFieldSetting.ts === +// #32721 + +class Recursive1 { +>Recursive1 : Symbol(Recursive1, Decl(recursiveFieldSetting.ts, 0, 0)) + + constructor(private readonly parent?: Recursive1) {} +>parent : Symbol(Recursive1.parent, Decl(recursiveFieldSetting.ts, 3, 16)) +>Recursive1 : Symbol(Recursive1, Decl(recursiveFieldSetting.ts, 0, 0)) + + private depth: number = this.parent ? this.parent.depth + 1 : 0; +>depth : Symbol(Recursive1.depth, Decl(recursiveFieldSetting.ts, 3, 56)) +>this.parent : Symbol(Recursive1.parent, Decl(recursiveFieldSetting.ts, 3, 16)) +>this : Symbol(Recursive1, Decl(recursiveFieldSetting.ts, 0, 0)) +>parent : Symbol(Recursive1.parent, Decl(recursiveFieldSetting.ts, 3, 16)) +>this.parent.depth : Symbol(Recursive1.depth, Decl(recursiveFieldSetting.ts, 3, 56)) +>this.parent : Symbol(Recursive1.parent, Decl(recursiveFieldSetting.ts, 3, 16)) +>this : Symbol(Recursive1, Decl(recursiveFieldSetting.ts, 0, 0)) +>parent : Symbol(Recursive1.parent, Decl(recursiveFieldSetting.ts, 3, 16)) +>depth : Symbol(Recursive1.depth, Decl(recursiveFieldSetting.ts, 3, 56)) +} + +class Recursive2 { +>Recursive2 : Symbol(Recursive2, Decl(recursiveFieldSetting.ts, 5, 1)) + + parent!: Recursive2; +>parent : Symbol(Recursive2.parent, Decl(recursiveFieldSetting.ts, 7, 18)) +>Recursive2 : Symbol(Recursive2, Decl(recursiveFieldSetting.ts, 5, 1)) + + depth: number = this.parent.depth; +>depth : Symbol(Recursive2.depth, Decl(recursiveFieldSetting.ts, 8, 24)) +>this.parent.depth : Symbol(Recursive2.depth, Decl(recursiveFieldSetting.ts, 8, 24)) +>this.parent : Symbol(Recursive2.parent, Decl(recursiveFieldSetting.ts, 7, 18)) +>this : Symbol(Recursive2, Decl(recursiveFieldSetting.ts, 5, 1)) +>parent : Symbol(Recursive2.parent, Decl(recursiveFieldSetting.ts, 7, 18)) +>depth : Symbol(Recursive2.depth, Decl(recursiveFieldSetting.ts, 8, 24)) +} + +class Recursive3 { +>Recursive3 : Symbol(Recursive3, Decl(recursiveFieldSetting.ts, 10, 1)) + + parent!: Recursive3; +>parent : Symbol(Recursive3.parent, Decl(recursiveFieldSetting.ts, 12, 18)) +>Recursive3 : Symbol(Recursive3, Decl(recursiveFieldSetting.ts, 10, 1)) + + depth: number = this.parent.alpha; +>depth : Symbol(Recursive3.depth, Decl(recursiveFieldSetting.ts, 13, 24)) +>this.parent.alpha : Symbol(Recursive3.alpha, Decl(recursiveFieldSetting.ts, 14, 38)) +>this.parent : Symbol(Recursive3.parent, Decl(recursiveFieldSetting.ts, 12, 18)) +>this : Symbol(Recursive3, Decl(recursiveFieldSetting.ts, 10, 1)) +>parent : Symbol(Recursive3.parent, Decl(recursiveFieldSetting.ts, 12, 18)) +>alpha : Symbol(Recursive3.alpha, Decl(recursiveFieldSetting.ts, 14, 38)) + + alpha = 0; +>alpha : Symbol(Recursive3.alpha, Decl(recursiveFieldSetting.ts, 14, 38)) +} + diff --git a/tests/baselines/reference/recursiveFieldSetting.types b/tests/baselines/reference/recursiveFieldSetting.types new file mode 100644 index 00000000000..b5fcbb49028 --- /dev/null +++ b/tests/baselines/reference/recursiveFieldSetting.types @@ -0,0 +1,59 @@ +=== tests/cases/compiler/recursiveFieldSetting.ts === +// #32721 + +class Recursive1 { +>Recursive1 : Recursive1 + + constructor(private readonly parent?: Recursive1) {} +>parent : Recursive1 + + private depth: number = this.parent ? this.parent.depth + 1 : 0; +>depth : number +>this.parent ? this.parent.depth + 1 : 0 : number +>this.parent : Recursive1 +>this : this +>parent : Recursive1 +>this.parent.depth + 1 : number +>this.parent.depth : number +>this.parent : Recursive1 +>this : this +>parent : Recursive1 +>depth : number +>1 : 1 +>0 : 0 +} + +class Recursive2 { +>Recursive2 : Recursive2 + + parent!: Recursive2; +>parent : Recursive2 + + depth: number = this.parent.depth; +>depth : number +>this.parent.depth : number +>this.parent : Recursive2 +>this : this +>parent : Recursive2 +>depth : number +} + +class Recursive3 { +>Recursive3 : Recursive3 + + parent!: Recursive3; +>parent : Recursive3 + + depth: number = this.parent.alpha; +>depth : number +>this.parent.alpha : number +>this.parent : Recursive3 +>this : this +>parent : Recursive3 +>alpha : number + + alpha = 0; +>alpha : number +>0 : 0 +} + diff --git a/tests/cases/compiler/recursiveFieldSetting.ts b/tests/cases/compiler/recursiveFieldSetting.ts new file mode 100644 index 00000000000..f28bda47802 --- /dev/null +++ b/tests/cases/compiler/recursiveFieldSetting.ts @@ -0,0 +1,17 @@ +// #32721 + +class Recursive1 { + constructor(private readonly parent?: Recursive1) {} + private depth: number = this.parent ? this.parent.depth + 1 : 0; +} + +class Recursive2 { + parent!: Recursive2; + depth: number = this.parent.depth; +} + +class Recursive3 { + parent!: Recursive3; + depth: number = this.parent.alpha; + alpha = 0; +}