From a65713ae138e56555d01a9e8e5179221a2f39e75 Mon Sep 17 00:00:00 2001
From: Taeheon Kim <skyrabbits1@gmail.com>
Date: Tue, 1 Mar 2022 13:08:50 +0900
Subject: [PATCH] fix(eslint-plugin): [prefer-readonly-parameter-types] handle
 class sharp private field and member without throwing error (#4343)

* fix: handle class sharp private field and member

* fix: handle class sharp private field and member

* feat: exempt private identifier from the rule

* chore: shorten unnecessary logic

* test: add unit test

* test: cover case that crashed before

* test: add readonly test case

* Update packages/type-utils/src/propertyTypes.ts

Co-authored-by: Josh Goldberg <me@joshuakgoldberg.com>
---
 .../prefer-readonly-parameter-types.test.ts   | 20 ++++++++++++++++++-
 packages/type-utils/src/isTypeReadonly.ts     |  5 +++++
 packages/type-utils/src/propertyTypes.ts      | 19 +++++++++++++++++-
 .../type-utils/tests/isTypeReadonly.test.ts   |  8 ++++++++
 4 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts
index f17a19f1f6d..eadd526ca09 100644
--- a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts
+++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts
@@ -169,6 +169,25 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
       };
       function bar(arg: MyType) {}
     `,
+    // PrivateIdentifier is exempt from this rule
+    {
+      code: `
+        class Foo {
+          #privateField = 'foo';
+          #privateMember() {}
+        }
+        function foo(arg: Foo) {}
+      `,
+    },
+    {
+      code: `
+        class HasText {
+          readonly #text: string;
+        }
+
+        export function onDone(task: HasText): void {}
+      `,
+    },
     // methods treated as readonly
     {
       code: `
@@ -222,7 +241,6 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
         },
       ],
     },
-
     // parameter properties should work fine
     {
       code: `
diff --git a/packages/type-utils/src/isTypeReadonly.ts b/packages/type-utils/src/isTypeReadonly.ts
index 0ec24438943..3098df3c435 100644
--- a/packages/type-utils/src/isTypeReadonly.ts
+++ b/packages/type-utils/src/isTypeReadonly.ts
@@ -157,6 +157,11 @@ function isTypeReadonlyObject(
         continue;
       }
 
+      const name = ts.getNameOfDeclaration(property.valueDeclaration);
+      if (name && ts.isPrivateIdentifier(name)) {
+        continue;
+      }
+
       return Readonlyness.Mutable;
     }
 
diff --git a/packages/type-utils/src/propertyTypes.ts b/packages/type-utils/src/propertyTypes.ts
index 5e2f1054239..7e064ea0ace 100644
--- a/packages/type-utils/src/propertyTypes.ts
+++ b/packages/type-utils/src/propertyTypes.ts
@@ -7,7 +7,7 @@ export function getTypeOfPropertyOfName(
   escapedName?: ts.__String,
 ): ts.Type | undefined {
   // Most names are directly usable in the checker and aren't different from escaped names
-  if (!escapedName || !name.startsWith('__')) {
+  if (!escapedName || !isSymbol(escapedName)) {
     return checker.getTypeOfPropertyOfType(type, name);
   }
 
@@ -34,3 +34,20 @@ export function getTypeOfPropertyOfType(
     property.getEscapedName(),
   );
 }
+
+// Symbolic names need to be specially handled because TS api is not sufficient for these cases.
+// Source based on:
+// https://github.com/microsoft/TypeScript/blob/0043abe982aae0d35f8df59f9715be6ada758ff7/src/compiler/utilities.ts#L3388-L3402
+function isSymbol(escapedName: string): boolean {
+  return isKnownSymbol(escapedName) || isPrivateIdentifierSymbol(escapedName);
+}
+
+// case for escapedName: "__@foo@10", name: "__@foo@10"
+function isKnownSymbol(escapedName: string): boolean {
+  return escapedName.startsWith('__@');
+}
+
+// case for escapedName: "__#1@#foo", name: "#foo"
+function isPrivateIdentifierSymbol(escapedName: string): boolean {
+  return escapedName.startsWith('__#');
+}
diff --git a/packages/type-utils/tests/isTypeReadonly.test.ts b/packages/type-utils/tests/isTypeReadonly.test.ts
index f6f2cebd12b..0cbda39727a 100644
--- a/packages/type-utils/tests/isTypeReadonly.test.ts
+++ b/packages/type-utils/tests/isTypeReadonly.test.ts
@@ -82,6 +82,14 @@ describe('isTypeReadonly', () => {
             ['type Test = Readonly<ReadonlySet<string>>;'],
             ['type Test = Readonly<ReadonlyMap<string, string>>;'],
           ])('handles fully readonly sets and maps', runTests);
+
+          // Private Identifier.
+          // Note: It can't be accessed from outside of class thus exempt from the checks.
+          it.each([
+            ['class Foo { readonly #readonlyPrivateField = "foo"; }'],
+            ['class Foo { #privateField = "foo"; }'],
+            ['class Foo { #privateMember() {}; }'],
+          ])('treat private identifier as readonly', runTests);
         });
 
         describe('is not readonly', () => {