From 89c4e8ca1c8410823393525841f8911354bdb20c Mon Sep 17 00:00:00 2001
From: Gabriela Araujo Britto <gabrielaa@microsoft.com>
Date: Fri, 30 Jul 2021 13:57:51 -0700
Subject: [PATCH 1/9] add test for completions crash

---
 .../cases/fourslash/completionsWrappedClass.ts | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 tests/cases/fourslash/completionsWrappedClass.ts

diff --git a/tests/cases/fourslash/completionsWrappedClass.ts b/tests/cases/fourslash/completionsWrappedClass.ts
new file mode 100644
index 0000000000000..2e7105dc9fa67
--- /dev/null
+++ b/tests/cases/fourslash/completionsWrappedClass.ts
@@ -0,0 +1,18 @@
+/// <reference path="fourslash.ts" />
+
+////class Client {
+////    private close() { }
+////    public open() { }
+////}
+////type Wrap<T> = T &
+////{
+////    [K in Extract<keyof T, string> as `${K}Wrapped`]: T[K];
+////};
+////class Service {
+////    method() {
+////        let service = undefined as unknown as Wrap<Client>;
+////        const { /*a*/ } = service;
+////    }
+////}
+
+verify.completions({ marker: "a", exact: ["open", "openWrapped"] });

From 85e1f26d24cd1e69bcebb03bf707fb166d10b9a5 Mon Sep 17 00:00:00 2001
From: Gabriela Araujo Britto <gabrielaa@microsoft.com>
Date: Fri, 6 Aug 2021 18:34:58 -0700
Subject: [PATCH 2/9] WIP: experiment

---
 src/compiler/checker.ts     | 120 ++++++++++++++++++++++++++----------
 src/compiler/types.ts       |   1 +
 src/services/completions.ts |  19 +++---
 3 files changed, 102 insertions(+), 38 deletions(-)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index de7c514576408..54bae3d3b7787 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -263,6 +263,30 @@ namespace ts {
         Uncapitalize: IntrinsicTypeKind.Uncapitalize
     }));
 
+    enum TypeChecks {
+        Ok,
+        HasDiagnostic
+    }
+
+    interface CheckerResultOk {
+        result: TypeChecks.Ok
+    }
+
+    interface CheckerResultHasDiagnostic {
+        result: TypeChecks.HasDiagnostic,
+        message: DiagnosticMessage;
+        args:
+            | never[]
+            | [string | number]
+            | [string | number, string | number]
+            | [string | number, string | number, string | number]
+            | [string | number, string | number, string | number, string | number];
+    }
+
+    type CheckerResult =
+        | CheckerResultOk
+        | CheckerResultHasDiagnostic;
+
     function SymbolLinks(this: SymbolLinks) {
     }
 
@@ -712,6 +736,9 @@ namespace ts {
 
             getLocalTypeParametersOfClassOrInterfaceOrTypeAlias,
             isDeclarationVisible,
+            isPropertyAccessible: (node, isSuper, writing, type, prop) => {
+                return checkPropertyAccessibilityAtNode(node, isSuper, writing, type, prop).result === TypeChecks.Ok;
+            }
         };
 
         function getResolvedSignatureWorker(nodeIn: CallLikeExpression, candidatesOutArray: Signature[] | undefined, argumentCount: number | undefined, checkMode: CheckMode): Signature | undefined {
@@ -11775,7 +11802,7 @@ namespace ts {
             return getReducedType(getApparentType(getReducedType(type)));
         }
 
-        function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
+        function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined { // candidate
             let singleProp: Symbol | undefined;
             let propSet: ESMap<SymbolId, Symbol> | undefined;
             let indexTypes: Type[] | undefined;
@@ -27275,11 +27302,36 @@ namespace ts {
             node: PropertyAccessExpression | QualifiedName | PropertyAccessExpression | VariableDeclaration | ParameterDeclaration | ImportTypeNode | PropertyAssignment | ShorthandPropertyAssignment | BindingElement,
             isSuper: boolean, writing: boolean, type: Type, prop: Symbol, reportError = true): boolean {
 
-            const flags = getDeclarationModifierFlagsFromSymbol(prop, writing);
             const errorNode = node.kind === SyntaxKind.QualifiedName ? node.right :
                 node.kind === SyntaxKind.ImportType ? node :
                 node.kind === SyntaxKind.BindingElement && node.propertyName ? node.propertyName : node.name;
 
+            const typeChecks = checkPropertyAccessibilityAtNode(node, isSuper, writing, type, prop);
+            switch (typeChecks.result) {
+                case (TypeChecks.Ok):
+                    return true;
+                case (TypeChecks.HasDiagnostic):
+                    if (reportError) {
+                        error(errorNode, typeChecks.message, ...typeChecks.args);
+                    }
+                    return false;
+            }
+        }
+
+        // Possible concerns:
+        // (1) I'm not sure if this function has the right behavior if `node` is allowed to be any node,
+        // although we only use `node` for its location in the parse tree.
+        // (2) Does it even make sense to check for property accessibility at a certain arbitrary node?
+        // Maybe this function should be called "checkPropertyVisibilityAtNode" or something else.
+        function checkPropertyAccessibilityAtNode(node: Node,
+            isSuper: boolean, writing: boolean,
+            type: Type, prop: Symbol): CheckerResult {
+            const flags = getDeclarationModifierFlagsFromSymbol(prop, writing);
+
+            const checkerOk: CheckerResultOk = {
+                result: TypeChecks.Ok
+            };
+
             if (isSuper) {
                 // TS 1.0 spec (April 2014): 4.8.2
                 // - In a constructor, instance member function, instance member accessor, or
@@ -27290,10 +27342,11 @@ namespace ts {
                 //   a super property access is permitted and must specify a public static member function of the base class.
                 if (languageVersion < ScriptTarget.ES2015) {
                     if (symbolHasNonMethodDeclaration(prop)) {
-                        if (reportError) {
-                            error(errorNode, Diagnostics.Only_public_and_protected_methods_of_the_base_class_are_accessible_via_the_super_keyword);
-                        }
-                        return false;
+                        return {
+                            result: TypeChecks.HasDiagnostic,
+                            message: Diagnostics.Only_public_and_protected_methods_of_the_base_class_are_accessible_via_the_super_keyword,
+                            args: emptyArray
+                        };
                     }
                 }
                 if (flags & ModifierFlags.Abstract) {
@@ -27301,10 +27354,11 @@ namespace ts {
                     // This error could mask a private property access error. But, a member
                     // cannot simultaneously be private and abstract, so this will trigger an
                     // additional error elsewhere.
-                    if (reportError) {
-                        error(errorNode, Diagnostics.Abstract_method_0_in_class_1_cannot_be_accessed_via_super_expression, symbolToString(prop), typeToString(getDeclaringClass(prop)!));
-                    }
-                    return false;
+                    return {
+                        result: TypeChecks.HasDiagnostic,
+                        message: Diagnostics.Abstract_method_0_in_class_1_cannot_be_accessed_via_super_expression,
+                        args: [symbolToString(prop), typeToString(getDeclaringClass(prop)!)]
+                    };
                 }
             }
 
@@ -27313,16 +27367,17 @@ namespace ts {
                 (isThisProperty(node) || isThisInitializedObjectBindingExpression(node) || isObjectBindingPattern(node.parent) && isThisInitializedDeclaration(node.parent.parent))) {
                 const declaringClassDeclaration = getClassLikeDeclarationOfSymbol(getParentOfSymbol(prop)!);
                 if (declaringClassDeclaration && isNodeUsedDuringClassInitialization(node)) {
-                    if (reportError) {
-                        error(errorNode, Diagnostics.Abstract_property_0_in_class_1_cannot_be_accessed_in_the_constructor, symbolToString(prop), getTextOfIdentifierOrLiteral(declaringClassDeclaration.name!)); // TODO: GH#18217
-                    }
-                    return false;
+                    return {
+                        result: TypeChecks.HasDiagnostic,
+                        message: Diagnostics.Abstract_property_0_in_class_1_cannot_be_accessed_in_the_constructor,
+                        args: [symbolToString(prop), getTextOfIdentifierOrLiteral(declaringClassDeclaration.name!)]
+                    };
                 }
             }
 
             // Public properties are otherwise accessible.
             if (!(flags & ModifierFlags.NonPublicAccessibilityModifier)) {
-                return true;
+                return checkerOk;
             }
 
             // Property is known to be private or protected at this point
@@ -27331,19 +27386,20 @@ namespace ts {
             if (flags & ModifierFlags.Private) {
                 const declaringClassDeclaration = getClassLikeDeclarationOfSymbol(getParentOfSymbol(prop)!)!;
                 if (!isNodeWithinClass(node, declaringClassDeclaration)) {
-                    if (reportError) {
-                        error(errorNode, Diagnostics.Property_0_is_private_and_only_accessible_within_class_1, symbolToString(prop), typeToString(getDeclaringClass(prop)!));
-                    }
-                    return false;
+                    return {
+                        result: TypeChecks.HasDiagnostic,
+                        message: Diagnostics.Property_0_is_private_and_only_accessible_within_class_1,
+                        args: [symbolToString(prop), typeToString(getDeclaringClass(prop)!)]
+                    };
                 }
-                return true;
+                return checkerOk;
             }
 
             // Property is known to be protected at this point
 
             // All protected properties of a supertype are accessible in a super access
             if (isSuper) {
-                return true;
+                return checkerOk;
             }
 
             // Find the first enclosing class that has the declaring classes of the protected constituents
@@ -27358,10 +27414,11 @@ namespace ts {
                 // static member access is disallow
                 let thisParameter: ParameterDeclaration | undefined;
                 if (flags & ModifierFlags.Static || !(thisParameter = getThisParameterFromNodeContext(node)) || !thisParameter.type) {
-                    if (reportError) {
-                        error(errorNode, Diagnostics.Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses, symbolToString(prop), typeToString(getDeclaringClass(prop) || type));
-                    }
-                    return false;
+                    return {
+                        result: TypeChecks.HasDiagnostic,
+                        message: Diagnostics.Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses,
+                        args: [symbolToString(prop), typeToString(getDeclaringClass(prop) || type)]
+                    };
                 }
 
                 const thisType = getTypeFromTypeNode(thisParameter.type);
@@ -27369,19 +27426,20 @@ namespace ts {
             }
             // No further restrictions for static properties
             if (flags & ModifierFlags.Static) {
-                return true;
+                return checkerOk;
             }
             if (type.flags & TypeFlags.TypeParameter) {
                 // get the original type -- represented as the type constraint of the 'this' type
                 type = (type as TypeParameter).isThisType ? getConstraintOfTypeParameter(type as TypeParameter)! : getBaseConstraintOfType(type as TypeParameter)!; // TODO: GH#18217 Use a different variable that's allowed to be undefined
             }
             if (!type || !hasBaseType(type, enclosingClass)) {
-                if (reportError) {
-                    error(errorNode, Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1_This_is_an_instance_of_class_2, symbolToString(prop), typeToString(enclosingClass), typeToString(type));
-                }
-                return false;
+                return {
+                    result: TypeChecks.HasDiagnostic,
+                    message: Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1_This_is_an_instance_of_class_2,
+                    args: [symbolToString(prop), typeToString(enclosingClass), typeToString(type)]
+                };
             }
-            return true;
+            return checkerOk;
         }
 
         function getThisParameterFromNodeContext(node: Node) {
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
index bf50717690756..5a5c638d31872 100644
--- a/src/compiler/types.ts
+++ b/src/compiler/types.ts
@@ -4354,6 +4354,7 @@ namespace ts {
 
         /* @internal */ getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol: Symbol): readonly TypeParameter[] | undefined;
         /* @internal */ isDeclarationVisible(node: Declaration | AnyImportSyntax): boolean;
+        /* @internal */ isPropertyAccessible(node: Node, isSuper: boolean, writing: boolean, type: Type, prop: Symbol): boolean;
     }
 
     /* @internal */
diff --git a/src/services/completions.ts b/src/services/completions.ts
index a762645ea6de3..cedd4d3e4c18a 100644
--- a/src/services/completions.ts
+++ b/src/services/completions.ts
@@ -2207,17 +2207,22 @@ namespace ts.Completions {
                         canGetType = isExpression(rootDeclaration.parent.parent) && !!typeChecker.getContextualType(rootDeclaration.parent.parent as Expression);
                     }
                 }
-                if (canGetType) {
+                if (canGetType) { // here
                     const typeForObject = typeChecker.getTypeAtLocation(objectLikeContainer);
                     if (!typeForObject) return GlobalsSearch.Fail;
                     // In a binding pattern, get only known properties (unless in the same scope).
                     // Everywhere else we will get all possible properties.
-                    const containerClass = getContainingClass(objectLikeContainer);
-                    typeMembers = typeChecker.getPropertiesOfType(typeForObject).filter(symbol =>
-                        // either public
-                        !(getDeclarationModifierFlagsFromSymbol(symbol) & ModifierFlags.NonPublicAccessibilityModifier)
-                        // or we're in it
-                        || containerClass && contains(typeForObject.symbol.declarations, containerClass));
+
+                    // const containerClass = getContainingClass(objectLikeContainer);
+                    typeMembers = typeChecker.getPropertiesOfType(typeForObject).filter(propertySymbol => {
+                        return typeChecker.isPropertyAccessible(objectLikeContainer, /*isSuper*/ false, /*writing*/ false, typeForObject, propertySymbol);
+                        // return !(getDeclarationModifierFlagsFromSymbol(propertySymbol) & ModifierFlags.NonPublicAccessibilityModifier)
+                        //     || containerClass && contains(typeForObject.symbol.declarations, containerClass);
+                    });
+                        // // either public
+                        // !(getDeclarationModifierFlagsFromSymbol(symbol) & ModifierFlags.NonPublicAccessibilityModifier)
+                        // // or we're in it
+                        // || containerClass && contains(typeForObject.symbol.declarations, containerClass));
                     existingMembers = objectLikeContainer.elements;
                 }
             }

From 6136c7c0565a859df70f5cf5eba08825275ee9b1 Mon Sep 17 00:00:00 2001
From: Gabriela Araujo Britto <gabrielaa@microsoft.com>
Date: Mon, 9 Aug 2021 15:39:45 -0700
Subject: [PATCH 3/9] remove comments

---
 src/compiler/checker.ts     |  2 +-
 src/services/completions.ts | 12 +-----------
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 5656e5e22c2db..b0a74e778fa7d 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -11802,7 +11802,7 @@ namespace ts {
             return getReducedType(getApparentType(getReducedType(type)));
         }
 
-        function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined { // candidate
+        function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
             let singleProp: Symbol | undefined;
             let propSet: ESMap<SymbolId, Symbol> | undefined;
             let indexTypes: Type[] | undefined;
diff --git a/src/services/completions.ts b/src/services/completions.ts
index a1cf6eb49a6f3..52c9b3325a9f8 100644
--- a/src/services/completions.ts
+++ b/src/services/completions.ts
@@ -2213,22 +2213,12 @@ namespace ts.Completions {
                         canGetType = isExpression(rootDeclaration.parent.parent) && !!typeChecker.getContextualType(rootDeclaration.parent.parent as Expression);
                     }
                 }
-                if (canGetType) { // here
+                if (canGetType) {
                     const typeForObject = typeChecker.getTypeAtLocation(objectLikeContainer);
                     if (!typeForObject) return GlobalsSearch.Fail;
-                    // In a binding pattern, get only known properties (unless in the same scope).
-                    // Everywhere else we will get all possible properties.
-
-                    // const containerClass = getContainingClass(objectLikeContainer);
                     typeMembers = typeChecker.getPropertiesOfType(typeForObject).filter(propertySymbol => {
                         return typeChecker.isPropertyAccessible(objectLikeContainer, /*isSuper*/ false, /*writing*/ false, typeForObject, propertySymbol);
-                        // return !(getDeclarationModifierFlagsFromSymbol(propertySymbol) & ModifierFlags.NonPublicAccessibilityModifier)
-                        //     || containerClass && contains(typeForObject.symbol.declarations, containerClass);
                     });
-                        // // either public
-                        // !(getDeclarationModifierFlagsFromSymbol(symbol) & ModifierFlags.NonPublicAccessibilityModifier)
-                        // // or we're in it
-                        // || containerClass && contains(typeForObject.symbol.declarations, containerClass));
                     existingMembers = objectLikeContainer.elements;
                 }
             }

From 8dd1cf67cfa869efefd99fc7bdad5697e6418ac2 Mon Sep 17 00:00:00 2001
From: Gabriela Araujo Britto <gabrielaa@microsoft.com>
Date: Mon, 9 Aug 2021 17:03:56 -0700
Subject: [PATCH 4/9] add call to getParseTreeNode

---
 src/compiler/checker.ts | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index b0a74e778fa7d..7e21b6fcf67c1 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -736,8 +736,10 @@ namespace ts {
 
             getLocalTypeParametersOfClassOrInterfaceOrTypeAlias,
             isDeclarationVisible,
-            isPropertyAccessible: (node, isSuper, writing, type, prop) => {
-                return checkPropertyAccessibilityAtNode(node, isSuper, writing, type, prop).result === TypeChecks.Ok;
+            isPropertyAccessible: (nodeIn, isSuper, writing, type, prop) => {
+                const node = getParseTreeNode(nodeIn);
+                return node ?
+                    checkPropertyAccessibilityAtNode(node, isSuper, writing, type, prop).result === TypeChecks.Ok : false;
             }
         };
 

From 2546be8dd20b837aa3edd3d43bec9c231d60e661 Mon Sep 17 00:00:00 2001
From: Gabriela Araujo Britto <gabrielaa@microsoft.com>
Date: Mon, 9 Aug 2021 17:07:22 -0700
Subject: [PATCH 5/9] Revert "add call to getParseTreeNode"

This reverts commit 8dd1cf67cfa869efefd99fc7bdad5697e6418ac2.
---
 src/compiler/checker.ts | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 7e21b6fcf67c1..b0a74e778fa7d 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -736,10 +736,8 @@ namespace ts {
 
             getLocalTypeParametersOfClassOrInterfaceOrTypeAlias,
             isDeclarationVisible,
-            isPropertyAccessible: (nodeIn, isSuper, writing, type, prop) => {
-                const node = getParseTreeNode(nodeIn);
-                return node ?
-                    checkPropertyAccessibilityAtNode(node, isSuper, writing, type, prop).result === TypeChecks.Ok : false;
+            isPropertyAccessible: (node, isSuper, writing, type, prop) => {
+                return checkPropertyAccessibilityAtNode(node, isSuper, writing, type, prop).result === TypeChecks.Ok;
             }
         };
 

From 4ffa81069ebe2eff3c466106ff2f8940f5ac8d52 Mon Sep 17 00:00:00 2001
From: Gabriela Araujo Britto <gabrielaa@microsoft.com>
Date: Fri, 13 Aug 2021 13:23:38 -0700
Subject: [PATCH 6/9] Fix comments

---
 src/compiler/checker.ts | 211 +++++++++++++++++++++-------------------
 src/compiler/types.ts   |   2 +-
 2 files changed, 114 insertions(+), 99 deletions(-)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index b0a74e778fa7d..79d82162edbb1 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -263,30 +263,6 @@ namespace ts {
         Uncapitalize: IntrinsicTypeKind.Uncapitalize
     }));
 
-    enum TypeChecks {
-        Ok,
-        HasDiagnostic
-    }
-
-    interface CheckerResultOk {
-        result: TypeChecks.Ok
-    }
-
-    interface CheckerResultHasDiagnostic {
-        result: TypeChecks.HasDiagnostic,
-        message: DiagnosticMessage;
-        args:
-            | never[]
-            | [string | number]
-            | [string | number, string | number]
-            | [string | number, string | number, string | number]
-            | [string | number, string | number, string | number, string | number];
-    }
-
-    type CheckerResult =
-        | CheckerResultOk
-        | CheckerResultHasDiagnostic;
-
     function SymbolLinks(this: SymbolLinks) {
     }
 
@@ -736,9 +712,7 @@ namespace ts {
 
             getLocalTypeParametersOfClassOrInterfaceOrTypeAlias,
             isDeclarationVisible,
-            isPropertyAccessible: (node, isSuper, writing, type, prop) => {
-                return checkPropertyAccessibilityAtNode(node, isSuper, writing, type, prop).result === TypeChecks.Ok;
-            }
+            isPropertyAccessible,
         };
 
         function getResolvedSignatureWorker(nodeIn: CallLikeExpression, candidatesOutArray: Signature[] | undefined, argumentCount: number | undefined, checkMode: CheckMode): Signature | undefined {
@@ -27293,35 +27267,28 @@ namespace ts {
             node: PropertyAccessExpression | QualifiedName | PropertyAccessExpression | VariableDeclaration | ParameterDeclaration | ImportTypeNode | PropertyAssignment | ShorthandPropertyAssignment | BindingElement,
             isSuper: boolean, writing: boolean, type: Type, prop: Symbol, reportError = true): boolean {
 
-            const errorNode = node.kind === SyntaxKind.QualifiedName ? node.right :
+            const errorNode = !reportError ? undefined : node.kind === SyntaxKind.QualifiedName ? node.right :
                 node.kind === SyntaxKind.ImportType ? node :
                 node.kind === SyntaxKind.BindingElement && node.propertyName ? node.propertyName : node.name;
 
-            const typeChecks = checkPropertyAccessibilityAtNode(node, isSuper, writing, type, prop);
-            switch (typeChecks.result) {
-                case (TypeChecks.Ok):
-                    return true;
-                case (TypeChecks.HasDiagnostic):
-                    if (reportError) {
-                        error(errorNode, typeChecks.message, ...typeChecks.args);
-                    }
-                    return false;
-            }
+            return checkPropertyAccessibilityAtLocation(node, isSuper, writing, type, prop, errorNode);
         }
 
-        // Possible concerns:
-        // (1) I'm not sure if this function has the right behavior if `node` is allowed to be any node,
-        // although we only use `node` for its location in the parse tree.
-        // (2) Does it even make sense to check for property accessibility at a certain arbitrary node?
-        // Maybe this function should be called "checkPropertyVisibilityAtNode" or something else.
-        function checkPropertyAccessibilityAtNode(node: Node,
+        /**
+         * Check whether the requested property can be accessed at the requested location.
+         * Returns true if node is a valid property access, and false otherwise.
+         * @param location The location node where we want to check if the property is accessible.
+         * @param isSuper True if the access is from `super.`.
+         * @param writing True if this is a write property access, false if it is a read property access.
+         * @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.
+         * @param errorNode The node where we should report an invalid property access error, or undefined if we should not report errors.
+         */
+        function checkPropertyAccessibilityAtLocation(location: Node,
             isSuper: boolean, writing: boolean,
-            type: Type, prop: Symbol): CheckerResult {
-            const flags = getDeclarationModifierFlagsFromSymbol(prop, writing);
+            type: Type, prop: Symbol, errorNode?: Node): boolean {
 
-            const checkerOk: CheckerResultOk = {
-                result: TypeChecks.Ok
-            };
+            const flags = getDeclarationModifierFlagsFromSymbol(prop, writing);
 
             if (isSuper) {
                 // TS 1.0 spec (April 2014): 4.8.2
@@ -27333,11 +27300,10 @@ namespace ts {
                 //   a super property access is permitted and must specify a public static member function of the base class.
                 if (languageVersion < ScriptTarget.ES2015) {
                     if (symbolHasNonMethodDeclaration(prop)) {
-                        return {
-                            result: TypeChecks.HasDiagnostic,
-                            message: Diagnostics.Only_public_and_protected_methods_of_the_base_class_are_accessible_via_the_super_keyword,
-                            args: emptyArray
-                        };
+                        if (errorNode) {
+                            error(errorNode, Diagnostics.Only_public_and_protected_methods_of_the_base_class_are_accessible_via_the_super_keyword);
+                        }
+                        return false;
                     }
                 }
                 if (flags & ModifierFlags.Abstract) {
@@ -27345,30 +27311,34 @@ namespace ts {
                     // This error could mask a private property access error. But, a member
                     // cannot simultaneously be private and abstract, so this will trigger an
                     // additional error elsewhere.
-                    return {
-                        result: TypeChecks.HasDiagnostic,
-                        message: Diagnostics.Abstract_method_0_in_class_1_cannot_be_accessed_via_super_expression,
-                        args: [symbolToString(prop), typeToString(getDeclaringClass(prop)!)]
-                    };
+                    if (errorNode) {
+                        error(errorNode,
+                            Diagnostics.Abstract_method_0_in_class_1_cannot_be_accessed_via_super_expression,
+                            symbolToString(prop),
+                            typeToString(getDeclaringClass(prop)!));
+                    }
+                    return false;
                 }
             }
 
             // Referencing abstract properties within their own constructors is not allowed
             if ((flags & ModifierFlags.Abstract) && symbolHasNonMethodDeclaration(prop) &&
-                (isThisProperty(node) || isThisInitializedObjectBindingExpression(node) || isObjectBindingPattern(node.parent) && isThisInitializedDeclaration(node.parent.parent))) {
+                (isThisProperty(location) || isThisInitializedObjectBindingExpression(location) || isObjectBindingPattern(location.parent) && isThisInitializedDeclaration(location.parent.parent))) {
                 const declaringClassDeclaration = getClassLikeDeclarationOfSymbol(getParentOfSymbol(prop)!);
-                if (declaringClassDeclaration && isNodeUsedDuringClassInitialization(node)) {
-                    return {
-                        result: TypeChecks.HasDiagnostic,
-                        message: Diagnostics.Abstract_property_0_in_class_1_cannot_be_accessed_in_the_constructor,
-                        args: [symbolToString(prop), getTextOfIdentifierOrLiteral(declaringClassDeclaration.name!)]
-                    };
+                if (declaringClassDeclaration && isNodeUsedDuringClassInitialization(location)) {
+                    if (errorNode) {
+                        error(errorNode,
+                            Diagnostics.Abstract_property_0_in_class_1_cannot_be_accessed_in_the_constructor,
+                            symbolToString(prop),
+                            getTextOfIdentifierOrLiteral(declaringClassDeclaration.name!));
+                    }
+                    return false;
                 }
             }
 
             // Public properties are otherwise accessible.
             if (!(flags & ModifierFlags.NonPublicAccessibilityModifier)) {
-                return checkerOk;
+                return true;
             }
 
             // Property is known to be private or protected at this point
@@ -27376,26 +27346,28 @@ namespace ts {
             // Private property is accessible if the property is within the declaring class
             if (flags & ModifierFlags.Private) {
                 const declaringClassDeclaration = getClassLikeDeclarationOfSymbol(getParentOfSymbol(prop)!)!;
-                if (!isNodeWithinClass(node, declaringClassDeclaration)) {
-                    return {
-                        result: TypeChecks.HasDiagnostic,
-                        message: Diagnostics.Property_0_is_private_and_only_accessible_within_class_1,
-                        args: [symbolToString(prop), typeToString(getDeclaringClass(prop)!)]
-                    };
+                if (!isNodeWithinClass(location, declaringClassDeclaration)) {
+                    if (errorNode) {
+                        error(errorNode,
+                            Diagnostics.Property_0_is_private_and_only_accessible_within_class_1,
+                            symbolToString(prop),
+                            typeToString(getDeclaringClass(prop)!));
+                    }
+                    return false;
                 }
-                return checkerOk;
+                return true;
             }
 
             // Property is known to be protected at this point
 
             // All protected properties of a supertype are accessible in a super access
             if (isSuper) {
-                return checkerOk;
+                return true;
             }
 
             // Find the first enclosing class that has the declaring classes of the protected constituents
             // of the property as base classes
-            let enclosingClass = forEachEnclosingClass(node, enclosingDeclaration => {
+            let enclosingClass = forEachEnclosingClass(location, enclosingDeclaration => {
                 const enclosingClass = getDeclaredTypeOfSymbol(getSymbolOfNode(enclosingDeclaration)!) as InterfaceType;
                 return isClassDerivedFromDeclaringClasses(enclosingClass, prop, writing) ? enclosingClass : undefined;
             });
@@ -27404,12 +27376,14 @@ namespace ts {
                 // allow PropertyAccessibility if context is in function with this parameter
                 // static member access is disallow
                 let thisParameter: ParameterDeclaration | undefined;
-                if (flags & ModifierFlags.Static || !(thisParameter = getThisParameterFromNodeContext(node)) || !thisParameter.type) {
-                    return {
-                        result: TypeChecks.HasDiagnostic,
-                        message: Diagnostics.Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses,
-                        args: [symbolToString(prop), typeToString(getDeclaringClass(prop) || type)]
-                    };
+                if (flags & ModifierFlags.Static || !(thisParameter = getThisParameterFromNodeContext(location)) || !thisParameter.type) {
+                    if (errorNode) {
+                        error(errorNode,
+                            Diagnostics.Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses,
+                            symbolToString(prop),
+                            typeToString(getDeclaringClass(prop) || type));
+                    }
+                    return false;
                 }
 
                 const thisType = getTypeFromTypeNode(thisParameter.type);
@@ -27417,20 +27391,21 @@ namespace ts {
             }
             // No further restrictions for static properties
             if (flags & ModifierFlags.Static) {
-                return checkerOk;
+                return true;
             }
             if (type.flags & TypeFlags.TypeParameter) {
                 // get the original type -- represented as the type constraint of the 'this' type
                 type = (type as TypeParameter).isThisType ? getConstraintOfTypeParameter(type as TypeParameter)! : getBaseConstraintOfType(type as TypeParameter)!; // TODO: GH#18217 Use a different variable that's allowed to be undefined
             }
             if (!type || !hasBaseType(type, enclosingClass)) {
-                return {
-                    result: TypeChecks.HasDiagnostic,
-                    message: Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1_This_is_an_instance_of_class_2,
-                    args: [symbolToString(prop), typeToString(enclosingClass), typeToString(type)]
-                };
+                if (errorNode) {
+                    error(errorNode,
+                        Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1_This_is_an_instance_of_class_2,
+                        symbolToString(prop), typeToString(enclosingClass), typeToString(type));
+                }
+                return false;
             }
-            return checkerOk;
+            return true;
         }
 
         function getThisParameterFromNodeContext(node: Node) {
@@ -28150,8 +28125,22 @@ namespace ts {
             }
         }
 
+        /**
+         * Checks if an existing property access is valid for completions purposes.
+         * @param node a property access-like node where we want to check if we can access a property.
+         * This node does not need to be an access of the property we are checking.
+         * e.g. in completions, this node will often be an incomplete property access node, as in `foo.`.
+         * Besides providing a location (i.e. scope) used to check property accessibility, we use this node for
+         * computing whether this is a `super` property access.
+         * @param type the type whose property we are checking
+         * @param property the accessed property's symbol
+         */
         function isValidPropertyAccessForCompletions(node: PropertyAccessExpression | ImportTypeNode | QualifiedName, type: Type, property: Symbol): boolean {
-            return isValidPropertyAccessWithType(node, node.kind === SyntaxKind.PropertyAccessExpression && node.expression.kind === SyntaxKind.SuperKeyword, property.escapedName, type);
+            return isPropertyAccessible(node,
+                node.kind === SyntaxKind.PropertyAccessExpression && node.expression.kind === SyntaxKind.SuperKeyword,
+                /* isWrite */ false,
+                type,
+                property);
             // Previously we validated the 'this' type of methods but this adversely affected performance. See #31377 for more context.
         }
 
@@ -28161,19 +28150,45 @@ namespace ts {
             propertyName: __String,
             type: Type): boolean {
 
+            // Short-circuiting for improved performance.
             if (type === errorType || isTypeAny(type)) {
                 return true;
             }
+
             const prop = getPropertyOfType(type, propertyName);
-            if (prop) {
-                if (prop.valueDeclaration && isPrivateIdentifierClassElementDeclaration(prop.valueDeclaration)) {
-                    const declClass = getContainingClass(prop.valueDeclaration);
-                    return !isOptionalChain(node) && !!findAncestor(node, parent => parent === declClass);
-                }
-                return checkPropertyAccessibility(node, isSuper, /*writing*/ false, type, prop, /* reportError */ false);
+            return !!prop && isPropertyAccessible(node, isSuper, /* isWrite */ false, type, prop);
+        }
+
+        /**
+         * Checks if a property can be accessed in a location.
+         * The location is given by the `node` parameter.
+         * The node does not need to be a property access.
+         * @param node location where to check property accessibility
+         * @param isSuper whether to consider this a `super` property access, e.g. `super.foo`.
+         * @param isWrite whether this is a write access, e.g. `++foo.x`.
+         * @param type type where the property comes from.
+         * @param property property symbol.
+         */
+        function isPropertyAccessible(
+            node: Node,
+            isSuper: boolean,
+            isWrite: boolean,
+            type: Type,
+            property: Symbol): boolean {
+
+            // Short-circuiting for improved performance.
+            if (type === errorType || isTypeAny(type)) {
+                return true;
+            }
+
+            // A #private property access in an optional chain is an error dealt with by the parser.
+             // The checker does not check for it, so we need to do our own check here.
+             if (property.valueDeclaration && isPrivateIdentifierClassElementDeclaration(property.valueDeclaration)) {
+                const declClass = getContainingClass(property.valueDeclaration);
+                return !isOptionalChain(node) && !!findAncestor(node, parent => parent === declClass);
             }
-            // In js files properties of unions are allowed in completion
-            return isInJSFile(node) && (type.flags & TypeFlags.Union) !== 0 && (type as UnionType).types.some(elementType => isValidPropertyAccessWithType(node, isSuper, propertyName, elementType));
+
+            return checkPropertyAccessibilityAtLocation(node, isSuper, isWrite, type, property);
         }
 
         /**
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
index e1b3c873f45cc..61a118382da86 100644
--- a/src/compiler/types.ts
+++ b/src/compiler/types.ts
@@ -4216,7 +4216,7 @@ namespace ts {
 
         getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): string | number | undefined;
         isValidPropertyAccess(node: PropertyAccessExpression | QualifiedName | ImportTypeNode, propertyName: string): boolean;
-        /** Exclude accesses to private properties or methods with a `this` parameter that `type` doesn't satisfy. */
+        /** Exclude accesses to private properties. */
         /* @internal */ isValidPropertyAccessForCompletions(node: PropertyAccessExpression | ImportTypeNode | QualifiedName, type: Type, property: Symbol): boolean;
         /** Follow all aliases to get the original symbol. */
         getAliasedSymbol(symbol: Symbol): Symbol;

From 7c3d29ccd0429337d88d3b0ff13b8b9d357ff234 Mon Sep 17 00:00:00 2001
From: Gabriela Araujo Britto <gabrielaa@microsoft.com>
Date: Mon, 16 Aug 2021 15:27:51 -0700
Subject: [PATCH 7/9] minor fixes

---
 src/compiler/checker.ts | 4 ++--
 src/compiler/types.ts   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 50c747f45f44e..1a7ff0a54a889 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -28157,8 +28157,8 @@ namespace ts {
          * e.g. in completions, this node will often be an incomplete property access node, as in `foo.`.
          * Besides providing a location (i.e. scope) used to check property accessibility, we use this node for
          * computing whether this is a `super` property access.
-         * @param type the type whose property we are checking
-         * @param property the accessed property's symbol
+         * @param type the type whose property we are checking.
+         * @param property the accessed property's symbol.
          */
         function isValidPropertyAccessForCompletions(node: PropertyAccessExpression | ImportTypeNode | QualifiedName, type: Type, property: Symbol): boolean {
             return isPropertyAccessible(node,
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
index fdbf2dd07a27d..2125dc81f7678 100644
--- a/src/compiler/types.ts
+++ b/src/compiler/types.ts
@@ -4355,7 +4355,7 @@ namespace ts {
 
         /* @internal */ getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol: Symbol): readonly TypeParameter[] | undefined;
         /* @internal */ isDeclarationVisible(node: Declaration | AnyImportSyntax): boolean;
-        /* @internal */ isPropertyAccessible(node: Node, isSuper: boolean, writing: boolean, type: Type, prop: Symbol): boolean;
+        /* @internal */ isPropertyAccessible(node: Node, isSuper: boolean, isWrite: boolean, type: Type, property: Symbol): boolean;
     }
 
     /* @internal */

From 996bce571ccc43b3579543812ad5169838228c7f Mon Sep 17 00:00:00 2001
From: Gabriela Araujo Britto <gabrielaa@microsoft.com>
Date: Thu, 19 Aug 2021 11:01:33 -0700
Subject: [PATCH 8/9] fix formatting

---
 src/compiler/checker.ts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 1a7ff0a54a889..6dafc2403154e 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -27292,7 +27292,8 @@ namespace ts {
             node: PropertyAccessExpression | QualifiedName | PropertyAccessExpression | VariableDeclaration | ParameterDeclaration | ImportTypeNode | PropertyAssignment | ShorthandPropertyAssignment | BindingElement,
             isSuper: boolean, writing: boolean, type: Type, prop: Symbol, reportError = true): boolean {
 
-            const errorNode = !reportError ? undefined : node.kind === SyntaxKind.QualifiedName ? node.right :
+            const errorNode = !reportError ? undefined :
+                node.kind === SyntaxKind.QualifiedName ? node.right :
                 node.kind === SyntaxKind.ImportType ? node :
                 node.kind === SyntaxKind.BindingElement && node.propertyName ? node.propertyName : node.name;
 

From 7ca48dc09b92ac03a4749646977438a0333ed234 Mon Sep 17 00:00:00 2001
From: Gabriela Araujo Britto <gabrielaa@microsoft.com>
Date: Thu, 19 Aug 2021 12:32:58 -0700
Subject: [PATCH 9/9] rename type to containingType

---
 src/compiler/checker.ts | 22 +++++++++++-----------
 src/compiler/types.ts   |  2 +-
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 6dafc2403154e..8b62c8a89304c 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -27306,13 +27306,13 @@ namespace ts {
          * @param location The location node where we want to check if the property is accessible.
          * @param isSuper True if the access is from `super.`.
          * @param writing True if this is a write property access, false if it is a read property access.
-         * @param type The type of the object whose property is being accessed. (Not the type of the property.)
+         * @param containingType 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.
          * @param errorNode The node where we should report an invalid property access error, or undefined if we should not report errors.
          */
         function checkPropertyAccessibilityAtLocation(location: Node,
             isSuper: boolean, writing: boolean,
-            type: Type, prop: Symbol, errorNode?: Node): boolean {
+            containingType: Type, prop: Symbol, errorNode?: Node): boolean {
 
             const flags = getDeclarationModifierFlagsFromSymbol(prop, writing);
 
@@ -27407,7 +27407,7 @@ namespace ts {
                         error(errorNode,
                             Diagnostics.Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses,
                             symbolToString(prop),
-                            typeToString(getDeclaringClass(prop) || type));
+                            typeToString(getDeclaringClass(prop) || containingType));
                     }
                     return false;
                 }
@@ -27419,15 +27419,15 @@ namespace ts {
             if (flags & ModifierFlags.Static) {
                 return true;
             }
-            if (type.flags & TypeFlags.TypeParameter) {
+            if (containingType.flags & TypeFlags.TypeParameter) {
                 // get the original type -- represented as the type constraint of the 'this' type
-                type = (type as TypeParameter).isThisType ? getConstraintOfTypeParameter(type as TypeParameter)! : getBaseConstraintOfType(type as TypeParameter)!; // TODO: GH#18217 Use a different variable that's allowed to be undefined
+                containingType = (containingType as TypeParameter).isThisType ? getConstraintOfTypeParameter(containingType as TypeParameter)! : getBaseConstraintOfType(containingType as TypeParameter)!; // TODO: GH#18217 Use a different variable that's allowed to be undefined
             }
-            if (!type || !hasBaseType(type, enclosingClass)) {
+            if (!containingType || !hasBaseType(containingType, enclosingClass)) {
                 if (errorNode) {
                     error(errorNode,
                         Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1_This_is_an_instance_of_class_2,
-                        symbolToString(prop), typeToString(enclosingClass), typeToString(type));
+                        symbolToString(prop), typeToString(enclosingClass), typeToString(containingType));
                 }
                 return false;
             }
@@ -28192,18 +28192,18 @@ namespace ts {
          * @param node location where to check property accessibility
          * @param isSuper whether to consider this a `super` property access, e.g. `super.foo`.
          * @param isWrite whether this is a write access, e.g. `++foo.x`.
-         * @param type type where the property comes from.
+         * @param containingType type where the property comes from.
          * @param property property symbol.
          */
         function isPropertyAccessible(
             node: Node,
             isSuper: boolean,
             isWrite: boolean,
-            type: Type,
+            containingType: Type,
             property: Symbol): boolean {
 
             // Short-circuiting for improved performance.
-            if (type === errorType || isTypeAny(type)) {
+            if (containingType === errorType || isTypeAny(containingType)) {
                 return true;
             }
 
@@ -28214,7 +28214,7 @@ namespace ts {
                 return !isOptionalChain(node) && !!findAncestor(node, parent => parent === declClass);
             }
 
-            return checkPropertyAccessibilityAtLocation(node, isSuper, isWrite, type, property);
+            return checkPropertyAccessibilityAtLocation(node, isSuper, isWrite, containingType, property);
         }
 
         /**
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
index 2125dc81f7678..6efadc8b48630 100644
--- a/src/compiler/types.ts
+++ b/src/compiler/types.ts
@@ -4355,7 +4355,7 @@ namespace ts {
 
         /* @internal */ getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol: Symbol): readonly TypeParameter[] | undefined;
         /* @internal */ isDeclarationVisible(node: Declaration | AnyImportSyntax): boolean;
-        /* @internal */ isPropertyAccessible(node: Node, isSuper: boolean, isWrite: boolean, type: Type, property: Symbol): boolean;
+        /* @internal */ isPropertyAccessible(node: Node, isSuper: boolean, isWrite: boolean, containingType: Type, property: Symbol): boolean;
     }
 
     /* @internal */