fix(eslint-plugin): [prefer-readonly-parameter-types] handle class sh…

…arp 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>
typescript-eslint · Mar 1, 2022 · a65713a · a65713a
1 parent 6c0a777
commit a65713a
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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', () => {