Error on accessing private property through destructuring assignment, and mark property used (#26381)

* Error on accessing private property through destructuring assignment, and mark property used

* Factor out getTypeOfObjectLiteralDestructuringProperty

Author: Andy

src/compiler/checker.ts

@@ -17736,25 +17736,23 @@ namespace ts {
          * Check whether the requested property access is valid.
          * Returns true if node is a valid property access, and false otherwise.
          * @param node The node to be checked.
-         * @param left The left hand side of the property access (e.g.: the super in `super.foo`).
-         * @param type The type of left.
-         * @param prop The symbol for the right hand side of the property access.
+         * @param isSuper True if the access is from `super.`.
+         * @param type The type of the object whose property is being accessed. (Not the type of the property.)
+         * @param prop The symbol for the property being accessed.
          */
-        function checkPropertyAccessibility(node: PropertyAccessExpression | QualifiedName | VariableLikeDeclaration | ImportTypeNode, left: Expression | QualifiedName | ImportTypeNode, type: Type, prop: Symbol): boolean {
+        function checkPropertyAccessibility(
+            node: PropertyAccessExpression | QualifiedName | PropertyAccessExpression | VariableDeclaration | ParameterDeclaration | ImportTypeNode | PropertyAssignment | ShorthandPropertyAssignment | BindingElement,
+            isSuper: boolean, type: Type, prop: Symbol): boolean {
             const flags = getDeclarationModifierFlagsFromSymbol(prop);
-            const errorNode = node.kind === SyntaxKind.PropertyAccessExpression || node.kind === SyntaxKind.VariableDeclaration ?
-                node.name :
-                node.kind === SyntaxKind.ImportType ?
-                    node :
-                    (<QualifiedName>node).right;
+            const errorNode = node.kind === SyntaxKind.QualifiedName ? node.right : node.kind === SyntaxKind.ImportType ? node : node.name;
 
             if (getCheckFlags(prop) & CheckFlags.ContainsPrivate) {
                 // Synthetic property with private constituent property
                 error(errorNode, Diagnostics.Property_0_has_conflicting_declarations_and_is_inaccessible_in_type_1, symbolToString(prop), typeToString(type));
                 return false;
             }
 
-            if (left.kind === SyntaxKind.SuperKeyword) {
+            if (isSuper) {
                 // TS 1.0 spec (April 2014): 4.8.2
                 // - In a constructor, instance member function, instance member accessor, or
                 //   instance member variable initializer where this references a derived class instance,
@@ -17807,7 +17805,7 @@ namespace ts {
             // Property is known to be protected at this point
 
             // All protected properties of a supertype are accessible in a super access
-            if (left.kind === SyntaxKind.SuperKeyword) {
+            if (isSuper) {
                 return true;
             }
 
@@ -17940,7 +17938,7 @@ namespace ts {
                 checkPropertyNotUsedBeforeDeclaration(prop, node, right);
                 markPropertyAsReferenced(prop, node, left.kind === SyntaxKind.ThisKeyword);
                 getNodeLinks(node).resolvedSymbol = prop;
-                checkPropertyAccessibility(node, left, apparentType, prop);
+                checkPropertyAccessibility(node, left.kind === SyntaxKind.SuperKeyword, apparentType, prop);
                 if (assignmentKind) {
                     if (isReferenceToReadonlyEntity(<Expression>node, prop) || isReferenceThroughNamespaceImport(<Expression>node)) {
                         error(right, Diagnostics.Cannot_assign_to_0_because_it_is_a_constant_or_a_read_only_property, idText(right));
@@ -18174,16 +18172,16 @@ namespace ts {
         function isValidPropertyAccess(node: PropertyAccessExpression | QualifiedName | ImportTypeNode, propertyName: __String): boolean {
             switch (node.kind) {
                 case SyntaxKind.PropertyAccessExpression:
-                    return isValidPropertyAccessWithType(node, node.expression, propertyName, getWidenedType(checkExpression(node.expression)));
+                    return isValidPropertyAccessWithType(node, node.expression.kind === SyntaxKind.SuperKeyword, propertyName, getWidenedType(checkExpression(node.expression)));
                 case SyntaxKind.QualifiedName:
-                    return isValidPropertyAccessWithType(node, node.left, propertyName, getWidenedType(checkExpression(node.left)));
+                    return isValidPropertyAccessWithType(node, /*isSuper*/ false, propertyName, getWidenedType(checkExpression(node.left)));
                 case SyntaxKind.ImportType:
-                    return isValidPropertyAccessWithType(node, node, propertyName, getTypeFromTypeNode(node));
+                    return isValidPropertyAccessWithType(node, /*isSuper*/ false, propertyName, getTypeFromTypeNode(node));
             }
         }
 
         function isValidPropertyAccessForCompletions(node: PropertyAccessExpression | ImportTypeNode, type: Type, property: Symbol): boolean {
-            return isValidPropertyAccessWithType(node, node.kind === SyntaxKind.ImportType ? node : node.expression, property.escapedName, type)
+            return isValidPropertyAccessWithType(node, node.kind !== SyntaxKind.ImportType && node.expression.kind === SyntaxKind.SuperKeyword, property.escapedName, type)
                 && (!(property.flags & SymbolFlags.Method) || isValidMethodAccess(property, type));
         }
         function isValidMethodAccess(method: Symbol, actualThisType: Type): boolean {
@@ -18206,17 +18204,17 @@ namespace ts {
 
         function isValidPropertyAccessWithType(
             node: PropertyAccessExpression | QualifiedName | ImportTypeNode,
-            left: LeftHandSideExpression | QualifiedName | ImportTypeNode,
+            isSuper: boolean,
             propertyName: __String,
             type: Type): boolean {
 
             if (type === errorType || isTypeAny(type)) {
                 return true;
             }
             const prop = getPropertyOfType(type, propertyName);
-            return prop ? checkPropertyAccessibility(node, left, type, prop)
+            return prop ? checkPropertyAccessibility(node, isSuper, type, prop)
                 // In js files properties of unions are allowed in completion
-                : isInJavaScriptFile(node) && (type.flags & TypeFlags.Union) !== 0 && (<UnionType>type).types.some(elementType => isValidPropertyAccessWithType(node, left, propertyName, elementType));
+                : isInJavaScriptFile(node) && (type.flags & TypeFlags.Union) !== 0 && (<UnionType>type).types.some(elementType => isValidPropertyAccessWithType(node, isSuper, propertyName, elementType));
         }
 
         /**
@@ -21093,19 +21091,19 @@ namespace ts {
             return booleanType;
         }
 
-        function checkObjectLiteralAssignment(node: ObjectLiteralExpression, sourceType: Type): Type {
+        function checkObjectLiteralAssignment(node: ObjectLiteralExpression, sourceType: Type, rightIsThis?: boolean): Type {
             const properties = node.properties;
             if (strictNullChecks && properties.length === 0) {
                 return checkNonNullType(sourceType, node);
             }
             for (const p of properties) {
-                checkObjectLiteralDestructuringPropertyAssignment(sourceType, p, properties);
+                checkObjectLiteralDestructuringPropertyAssignment(sourceType, p, properties, rightIsThis);
             }
             return sourceType;
         }
 
         /** Note: If property cannot be a SpreadAssignment, then allProperties does not need to be provided */
-        function checkObjectLiteralDestructuringPropertyAssignment(objectLiteralType: Type, property: ObjectLiteralElementLike, allProperties?: NodeArray<ObjectLiteralElementLike>) {
+        function checkObjectLiteralDestructuringPropertyAssignment(objectLiteralType: Type, property: ObjectLiteralElementLike, allProperties?: NodeArray<ObjectLiteralElementLike>, rightIsThis = false) {
             if (property.kind === SyntaxKind.PropertyAssignment || property.kind === SyntaxKind.ShorthandPropertyAssignment) {
                 const name = property.name;
                 if (name.kind === SyntaxKind.ComputedPropertyName) {
@@ -21115,20 +21113,10 @@ namespace ts {
                     return undefined;
                 }
 
-                const text = getTextOfPropertyName(name);
-                const type = isTypeAny(objectLiteralType)
-                    ? objectLiteralType
-                    : getTypeOfPropertyOfType(objectLiteralType, text) ||
-                    isNumericLiteralName(text) && getIndexTypeOfType(objectLiteralType, IndexKind.Number) ||
-                    getIndexTypeOfType(objectLiteralType, IndexKind.String);
+                const type = getTypeOfObjectLiteralDestructuringProperty(objectLiteralType, name, property, rightIsThis);
                 if (type) {
-                    if (property.kind === SyntaxKind.ShorthandPropertyAssignment) {
-                        return checkDestructuringAssignment(property, type);
-                    }
-                    else {
-                        // non-shorthand property assignments should always have initializers
-                        return checkDestructuringAssignment(property.initializer, type);
-                    }
+                    // non-shorthand property assignments should always have initializers
+                    return checkDestructuringAssignment(property.kind === SyntaxKind.ShorthandPropertyAssignment ? property : property.initializer, type);
                 }
                 else {
                     error(name, Diagnostics.Type_0_has_no_property_1_and_no_string_index_signature, typeToString(objectLiteralType), declarationNameToString(name));
@@ -21153,6 +21141,25 @@ namespace ts {
             }
         }
 
+        function getTypeOfObjectLiteralDestructuringProperty(objectLiteralType: Type, name: PropertyName, property: PropertyAssignment | ShorthandPropertyAssignment, rightIsThis: boolean) {
+            if (isTypeAny(objectLiteralType)) {
+                return objectLiteralType;
+            }
+
+            let type: Type | undefined;
+            const text = getTextOfPropertyName(name);
+            if (text) { // TODO: GH#26379
+                const prop = getPropertyOfType(objectLiteralType, text);
+                if (prop) {
+                    markPropertyAsReferenced(prop, property, rightIsThis);
+                    checkPropertyAccessibility(property, /*isSuper*/ false, objectLiteralType, prop);
+                    type = getTypeOfSymbol(prop);
+                }
+                type = type || (isNumericLiteralName(text) ? getIndexTypeOfType(objectLiteralType, IndexKind.Number) : undefined);
+            }
+            return type || getIndexTypeOfType(objectLiteralType, IndexKind.String);
+        }
+
         function checkArrayLiteralAssignment(node: ArrayLiteralExpression, sourceType: Type, checkMode?: CheckMode): Type {
             const elements = node.elements;
             if (languageVersion < ScriptTarget.ES2015 && compilerOptions.downlevelIteration) {
@@ -21214,7 +21221,7 @@ namespace ts {
             return undefined;
         }
 
-        function checkDestructuringAssignment(exprOrAssignment: Expression | ShorthandPropertyAssignment, sourceType: Type, checkMode?: CheckMode): Type {
+        function checkDestructuringAssignment(exprOrAssignment: Expression | ShorthandPropertyAssignment, sourceType: Type, checkMode?: CheckMode, rightIsThis?: boolean): Type {
             let target: Expression;
             if (exprOrAssignment.kind === SyntaxKind.ShorthandPropertyAssignment) {
                 const prop = <ShorthandPropertyAssignment>exprOrAssignment;
@@ -21238,7 +21245,7 @@ namespace ts {
                 target = (<BinaryExpression>target).left;
             }
             if (target.kind === SyntaxKind.ObjectLiteralExpression) {
-                return checkObjectLiteralAssignment(<ObjectLiteralExpression>target, sourceType);
+                return checkObjectLiteralAssignment(<ObjectLiteralExpression>target, sourceType, rightIsThis);
             }
             if (target.kind === SyntaxKind.ArrayLiteralExpression) {
                 return checkArrayLiteralAssignment(<ArrayLiteralExpression>target, sourceType, checkMode);
@@ -21337,7 +21344,7 @@ namespace ts {
         function checkBinaryLikeExpression(left: Expression, operatorToken: Node, right: Expression, checkMode?: CheckMode, errorNode?: Node): Type {
             const operator = operatorToken.kind;
             if (operator === SyntaxKind.EqualsToken && (left.kind === SyntaxKind.ObjectLiteralExpression || left.kind === SyntaxKind.ArrayLiteralExpression)) {
-                return checkDestructuringAssignment(left, checkExpression(right, checkMode), checkMode);
+                return checkDestructuringAssignment(left, checkExpression(right, checkMode), checkMode, right.kind === SyntaxKind.ThisKeyword);
             }
             let leftType: Type;
             if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken) {
@@ -24400,7 +24407,7 @@ namespace ts {
                         const property = getPropertyOfType(parentType!, nameText)!; // TODO: GH#18217
                         markPropertyAsReferenced(property, /*nodeForCheckWriteOnly*/ undefined, /*isThisAccess*/ false); // A destructuring is never a write-only reference.
                         if (parent.initializer && property && !isComputedPropertyName(name)) {
-                            checkPropertyAccessibility(parent, parent.initializer, parentType!, property);
+                            checkPropertyAccessibility(parent, parent.initializer.kind === SyntaxKind.SuperKeyword, parentType!, property);
                         }
                     }
                 }

tests/baselines/reference/destructuringAssignment_private.errors.txt

@@ -0,0 +1,17 @@
+tests/cases/compiler/destructuringAssignment_private.ts(6,10): error TS2341: Property 'x' is private and only accessible within class 'C'.
+tests/cases/compiler/destructuringAssignment_private.ts(7,4): error TS2341: Property 'o' is private and only accessible within class 'C'.
+
+
+==== tests/cases/compiler/destructuringAssignment_private.ts (2 errors) ====
+    class C {
+        private x = 0;
+        private o = [{ a: 1 }];
+    }
+    let x: number;
+    ([{ a: { x } }] = [{ a: new C() }]);
+             ~
+!!! error TS2341: Property 'x' is private and only accessible within class 'C'.
+    ({ o: [{ a: x }]} = new C());
+       ~
+!!! error TS2341: Property 'o' is private and only accessible within class 'C'.
+    

tests/baselines/reference/destructuringAssignment_private.js

@@ -0,0 +1,21 @@
+//// [destructuringAssignment_private.ts]
+class C {
+    private x = 0;
+    private o = [{ a: 1 }];
+}
+let x: number;
+([{ a: { x } }] = [{ a: new C() }]);
+({ o: [{ a: x }]} = new C());
+
+
+//// [destructuringAssignment_private.js]
+var C = /** @class */ (function () {
+    function C() {
+        this.x = 0;
+        this.o = [{ a: 1 }];
+    }
+    return C;
+}());
+var x;
+(x = [{ a: new C() }][0].a.x);
+(x = new C().o[0].a);

tests/baselines/reference/destructuringAssignment_private.symbols

@@ -0,0 +1,26 @@
+=== tests/cases/compiler/destructuringAssignment_private.ts ===
+class C {
+>C : Symbol(C, Decl(destructuringAssignment_private.ts, 0, 0))
+
+    private x = 0;
+>x : Symbol(C.x, Decl(destructuringAssignment_private.ts, 0, 9))
+
+    private o = [{ a: 1 }];
+>o : Symbol(C.o, Decl(destructuringAssignment_private.ts, 1, 18))
+>a : Symbol(a, Decl(destructuringAssignment_private.ts, 2, 18))
+}
+let x: number;
+>x : Symbol(x, Decl(destructuringAssignment_private.ts, 4, 3))
+
+([{ a: { x } }] = [{ a: new C() }]);
+>a : Symbol(a, Decl(destructuringAssignment_private.ts, 5, 3))
+>x : Symbol(x, Decl(destructuringAssignment_private.ts, 5, 8))
+>a : Symbol(a, Decl(destructuringAssignment_private.ts, 5, 20))
+>C : Symbol(C, Decl(destructuringAssignment_private.ts, 0, 0))
+
+({ o: [{ a: x }]} = new C());
+>o : Symbol(o, Decl(destructuringAssignment_private.ts, 6, 2))
+>a : Symbol(a, Decl(destructuringAssignment_private.ts, 6, 8))
+>x : Symbol(x, Decl(destructuringAssignment_private.ts, 4, 3))
+>C : Symbol(C, Decl(destructuringAssignment_private.ts, 0, 0))
+

tests/baselines/reference/destructuringAssignment_private.types

@@ -0,0 +1,44 @@
+=== tests/cases/compiler/destructuringAssignment_private.ts ===
+class C {
+>C : C
+
+    private x = 0;
+>x : number
+>0 : 0
+
+    private o = [{ a: 1 }];
+>o : { a: number; }[]
+>[{ a: 1 }] : { a: number; }[]
+>{ a: 1 } : { a: number; }
+>a : number
+>1 : 1
+}
+let x: number;
+>x : number
+
+([{ a: { x } }] = [{ a: new C() }]);
+>([{ a: { x } }] = [{ a: new C() }]) : [{ a: C; }]
+>[{ a: { x } }] = [{ a: new C() }] : [{ a: C; }]
+>[{ a: { x } }] : [{ a: { x: number; }; }]
+>{ a: { x } } : { a: { x: number; }; }
+>a : { x: number; }
+>{ x } : { x: number; }
+>x : number
+>[{ a: new C() }] : [{ a: C; }]
+>{ a: new C() } : { a: C; }
+>a : C
+>new C() : C
+>C : typeof C
+
+({ o: [{ a: x }]} = new C());
+>({ o: [{ a: x }]} = new C()) : C
+>{ o: [{ a: x }]} = new C() : C
+>{ o: [{ a: x }]} : { o: [{ a: number; }]; }
+>o : [{ a: number; }]
+>[{ a: x }] : [{ a: number; }]
+>{ a: x } : { a: number; }
+>a : number
+>x : number
+>new C() : C
+>C : typeof C
+

tests/baselines/reference/noUnusedLocals_destructuringAssignment.errors.txt

@@ -0,0 +1,22 @@
+tests/cases/compiler/noUnusedLocals_destructuringAssignment.ts(10,13): error TS6133: 'f' is declared but its value is never read.
+
+
+==== tests/cases/compiler/noUnusedLocals_destructuringAssignment.ts (1 errors) ====
+    class C {
+        private x = 0;
+    
+        m(): number {
+            let x: number;
+            ({ x } = this);
+            return x;
+        }
+    
+        private f(): Function {
+                ~
+!!! error TS6133: 'f' is declared but its value is never read.
+            let f: Function;
+            ({ f } = this);
+            return f;
+        }
+    }
+    

tests/baselines/reference/noUnusedLocals_destructuringAssignment.js

@@ -0,0 +1,35 @@
+//// [noUnusedLocals_destructuringAssignment.ts]
+class C {
+    private x = 0;
+
+    m(): number {
+        let x: number;
+        ({ x } = this);
+        return x;
+    }
+
+    private f(): Function {
+        let f: Function;
+        ({ f } = this);
+        return f;
+    }
+}
+
+
+//// [noUnusedLocals_destructuringAssignment.js]
+var C = /** @class */ (function () {
+    function C() {
+        this.x = 0;
+    }
+    C.prototype.m = function () {
+        var x;
+        (x = this.x);
+        return x;
+    };
+    C.prototype.f = function () {
+        var f;
+        (f = this.f);
+        return f;
+    };
+    return C;
+}());