TypeScript: Throw errors for invalid modifier on type parameter (#13853)

src/language-js/parse/babel.js

@@ -207,10 +207,8 @@ const allowedMessageCodes = new Set([
   "InvalidTupleMemberLabel",
 
   "NonClassMethodPropertyHasAbstractModifer",
-  "ReadonlyForMethodSignature",
   "ClassMethodHasDeclare",
   "ClassMethodHasReadonly",
-  "InvalidModifierOnTypeMember",
   "DuplicateAccessibilityModifier",
   "IndexSignatureHasDeclare",
 

src/language-js/parse/postprocess/typescript.js

@@ -8,6 +8,9 @@ const SyntaxKind = {
   SourceFile: 305,
   DeclareKeyword: 135,
   PropertyDeclaration: 167,
+  InKeyword: 101,
+  OutKeyword: 144,
+  TypeParameter: 168,
 };
 
 function getTsNodeLocation(nodeOrToken) {
@@ -93,6 +96,32 @@ function throwErrorForInvalidDeclare(tsNode, esTreeNode) {
   );
 }
 
+function throwErrorForInvalidModifierOnTypeParameter(tsNode, esTreeNode) {
+  if (
+    esTreeNode.type !== "TSMethodSignature" ||
+    tsNode.kind !== SyntaxKind.TypeParameter
+  ) {
+    return;
+  }
+
+  const invalidModifier = tsNode.modifiers?.find(
+    (modifier) =>
+      !(
+        modifier.kind === SyntaxKind.InKeyword ||
+        modifier.kind === SyntaxKind.OutKeyword
+      )
+  );
+
+  if (!invalidModifier) {
+    return;
+  }
+
+  throwTsSyntaxError(
+    { loc: getTsNodeLocation(invalidModifier) },
+    "This modifier cannot appear on a type member"
+  );
+}
+
 function getTsNode(node, tsParseResult) {
   const { esTreeNodeToTSNodeMap, tsNodeToESTreeNodeMap } = tsParseResult;
   const tsNode = esTreeNodeToTSNodeMap.get(node);
@@ -113,7 +142,8 @@ function throwErrorForInvalidNodes(tsParseResult, options) {
     // decorators
     // abstract properties
     // declare in accessor
-    !/@|abstract|declare/.test(options.originalText)
+    // modifiers on type parameter
+    !/@|abstract|declare|interface/.test(options.originalText)
   ) {
     return;
   }
@@ -127,6 +157,7 @@ function throwErrorForInvalidNodes(tsParseResult, options) {
     throwErrorForInvalidDeclare(tsNode, esTreeNode);
     throwErrorForInvalidDecorator(tsNode);
     throwErrorForInvalidAbstractProperty(tsNode, esTreeNode);
+    throwErrorForInvalidModifierOnTypeParameter(tsNode, esTreeNode);
   });
 }
 

src/language-js/print/typescript.js

@@ -320,13 +320,6 @@ function printTypescript(path, options, print) {
       parts.push(
         node.accessibility ? [node.accessibility, " "] : "",
         kind,
-        node.export ? "export " : "",
-        node.static ? "static " : "",
-        node.readonly ? "readonly " : "",
-        // "abstract" and "declare" are supported by only "babel-ts"
-        // https://github.com/prettier/prettier/issues/9760
-        node.abstract ? "abstract " : "",
-        node.declare ? "declare " : "",
         node.computed ? "[" : "",
         print("key"),
         node.computed ? "]" : "",

tests/config/format-test.js

@@ -166,6 +166,11 @@ function runSpec(fixtures, parsers, options) {
 
   snippets = snippets.map((test, index) => {
     test = typeof test === "string" ? { code: test } : test;
+
+    if (typeof test.code !== "string") {
+      throw Object.assign(new Error("Invalid test"), { test });
+    }
+
     return {
       ...test,
       name: `snippet: ${test.name || `#${index}`}`,

tests/format/misc/errors/typescript/__snapshots__/jsfmt.spec.js.snap

@@ -94,6 +94,78 @@ exports[`snippet: #5 [typescript] format 1`] = `
     |               ^"
 `;
 
+exports[`snippet: #6 [typescript] format 1`] = `
+"This modifier cannot appear on a type member (1:16)
+> 1 | interface Foo {
+    |                ^
+> 2 |   abstract e();
+    | ^^^^^^^^^^^
+  3 | }"
+`;
+
+exports[`snippet: #7 [typescript] format 1`] = `
+"This modifier cannot appear on a type member (1:16)
+> 1 | interface Foo {
+    |                ^
+> 2 |   declare e();
+    | ^^^^^^^^^^
+  3 | }"
+`;
+
+exports[`snippet: #8 [typescript] format 1`] = `
+"This modifier cannot appear on a type member (1:16)
+> 1 | interface Foo {
+    |                ^
+> 2 |   export e();
+    | ^^^^^^^^^
+  3 | }"
+`;
+
+exports[`snippet: #9 [typescript] format 1`] = `
+"This modifier cannot appear on a type member (1:16)
+> 1 | interface Foo {
+    |                ^
+> 2 |   static e();
+    | ^^^^^^^^^
+  3 | }"
+`;
+
+exports[`snippet: #10 [typescript] format 1`] = `
+"This modifier cannot appear on a type member (1:16)
+> 1 | interface Foo {
+    |                ^
+> 2 |   private e();
+    | ^^^^^^^^^^
+  3 | }"
+`;
+
+exports[`snippet: #11 [typescript] format 1`] = `
+"This modifier cannot appear on a type member (1:16)
+> 1 | interface Foo {
+    |                ^
+> 2 |   protected e();
+    | ^^^^^^^^^^^^
+  3 | }"
+`;
+
+exports[`snippet: #12 [typescript] format 1`] = `
+"This modifier cannot appear on a type member (1:16)
+> 1 | interface Foo {
+    |                ^
+> 2 |   public e();
+    | ^^^^^^^^^
+  3 | }"
+`;
+
+exports[`snippet: #13 [typescript] format 1`] = `
+"This modifier cannot appear on a type member (1:16)
+> 1 | interface Foo {
+    |                ^
+> 2 |   readonly e();
+    | ^^^^^^^^^^^
+  3 | }"
+`;
+
 exports[`value-of-abstract-property.ts [typescript] format 1`] = `
 "Abstract property cannot have an initializer (2:3)
   1 | abstract class Foo {

tests/format/misc/errors/typescript/babel-ts/__snapshots__/jsfmt.spec.js.snap

@@ -43,3 +43,67 @@ exports[`snippet: #2 [babel-ts] format 1`] = `
 > 1 | <string>foo = '100';
     |         ^"
 `;
+
+exports[`snippet: #3 [babel-ts] format 1`] = `
+"'abstract' modifier cannot appear on a type member. (2:3)
+  1 | interface Foo {
+> 2 |   abstract e();
+    |   ^
+  3 | }"
+`;
+
+exports[`snippet: #4 [babel-ts] format 1`] = `
+"'declare' modifier cannot appear on a type member. (2:3)
+  1 | interface Foo {
+> 2 |   declare e();
+    |   ^
+  3 | }"
+`;
+
+exports[`snippet: #5 [babel-ts] format 1`] = `
+"Unexpected token, expected ";" (2:10)
+  1 | interface Foo {
+> 2 |   export e();
+    |          ^
+  3 | }"
+`;
+
+exports[`snippet: #6 [babel-ts] format 1`] = `
+"'static' modifier cannot appear on a type member. (2:3)
+  1 | interface Foo {
+> 2 |   static e();
+    |   ^
+  3 | }"
+`;
+
+exports[`snippet: #7 [babel-ts] format 1`] = `
+"'private' modifier cannot appear on a type member. (2:3)
+  1 | interface Foo {
+> 2 |   private e();
+    |   ^
+  3 | }"
+`;
+
+exports[`snippet: #8 [babel-ts] format 1`] = `
+"'protected' modifier cannot appear on a type member. (2:3)
+  1 | interface Foo {
+> 2 |   protected e();
+    |   ^
+  3 | }"
+`;
+
+exports[`snippet: #9 [babel-ts] format 1`] = `
+"'public' modifier cannot appear on a type member. (2:3)
+  1 | interface Foo {
+> 2 |   public e();
+    |   ^
+  3 | }"
+`;
+
+exports[`snippet: #10 [babel-ts] format 1`] = `
+"'readonly' modifier can only appear on a property declaration or index signature. (2:3)
+  1 | interface Foo {
+> 2 |   readonly e();
+    |   ^
+  3 | }"
+`;

tests/format/misc/errors/typescript/babel-ts/jsfmt.spec.js

@@ -1,3 +1,5 @@
+import { outdent } from "outdent";
+
 run_spec(
   {
     importMeta: import.meta,
@@ -5,6 +7,22 @@ run_spec(
       "foo as any = 10;",
       "({ a: b as any = 2000 } = x);",
       "<string>foo = '100';",
+      ...[
+        "abstract",
+        "declare",
+        "export",
+        "static",
+        "private",
+        "protected",
+        "public",
+        "readonly",
+      ].map(
+        (modifier) => outdent`
+          interface Foo {
+            ${modifier} e();
+          }
+        `
+      ),
     ],
   },
   ["babel-ts"]

tests/format/misc/errors/typescript/jsfmt.spec.js

@@ -1,3 +1,5 @@
+import { outdent } from "outdent";
+
 run_spec(
   {
     importMeta: import.meta,
@@ -8,6 +10,22 @@ run_spec(
       'let x4 = <div>{"foo"}></div>;',
       'let x5 = <div>}{"foo"}</div>;',
       'let x6 = <div>>{"foo"}</div>;',
+      ...[
+        "abstract",
+        "declare",
+        "export",
+        "static",
+        "private",
+        "protected",
+        "public",
+        "readonly",
+      ].map(
+        (modifier) => outdent`
+          interface Foo {
+            ${modifier} e();
+          }
+        `
+      ),
     ],
   },
   ["typescript"]

tests/format/misc/typescript-babel-only/__snapshots__/jsfmt.spec.js.snap

@@ -26,32 +26,12 @@ parsers: ["babel-ts"]
 printWidth: 80
                                                                                 | printWidth
 =====================================input======================================
-interface Foo {
-  private a();
-  public b();
-  protected c();
-  static d();
-  declare e();
-  abstract f();
-  readonly g();
-}
-
 class Bar {
   declare e() {};
   readonly g() {};
 }
 
 =====================================output=====================================
-interface Foo {
-  private a();
-  public b();
-  protected c();
-  static d();
-  declare e();
-  abstract f();
-  readonly g();
-}
-
 class Bar {
   declare e() {}
   readonly g() {}

tests/format/misc/typescript-babel-only/invalid-modifiers.ts

@@ -1,13 +1,3 @@
-interface Foo {
-  private a();
-  public b();
-  protected c();
-  static d();
-  declare e();
-  abstract f();
-  readonly g();
-}
-
 class Bar {
   declare e() {};
   readonly g() {};

tests/format/misc/typescript-only/__snapshots__/jsfmt.spec.js.snap

@@ -57,16 +57,6 @@ parsers: ["typescript"]
 printWidth: 80
                                                                                 | printWidth
 =====================================input======================================
-interface Foo {
-  private a();
-  public b();
-  protected c();
-  static d();
-  declare e();
-  abstract f();
-  readonly g();
-}
-
 class Bar {
   declare e() {};
   abstract f() {};
@@ -74,16 +64,6 @@ class Bar {
 }
 
 =====================================output=====================================
-interface Foo {
-  private a();
-  public b();
-  protected c();
-  static d();
-  e();
-  f();
-  readonly g();
-}
-
 class Bar {
   e() {}
   abstract f() {}

tests/format/misc/typescript-only/invalid-modifiers.ts

@@ -1,13 +1,3 @@
-interface Foo {
-  private a();
-  public b();
-  protected c();
-  static d();
-  declare e();
-  abstract f();
-  readonly g();
-}
-
 class Bar {
   declare e() {};
   abstract f() {};

tests/format/typescript/class/__snapshots__/jsfmt.spec.js.snap

@@ -392,17 +392,6 @@ interface AudioBufferList {
 ================================================================================
 `;
 
-exports[`methods.ts [babel-ts] format 1`] = `
-"Unexpected token, expected ";" (6:10)
-  4 |
-  5 | interface Iterable<T> {
-> 6 |   export [Symbol.iterator](): Iterator<T>;
-    |          ^
-  7 | }
-  8 |
-  9 | export class Check {"
-`;
-
 exports[`methods.ts format 1`] = `
 ====================================options=====================================
 parsers: ["typescript"]
@@ -414,7 +403,7 @@ class X {
 }
 
 interface Iterable<T> {
-  export [Symbol.iterator](): Iterator<T>;
+  [Symbol.iterator](): Iterator<T>;
 }
 
 export class Check {
@@ -427,7 +416,7 @@ class X {
 }
 
 interface Iterable<T> {
-  export [Symbol.iterator](): Iterator<T>;
+  [Symbol.iterator](): Iterator<T>;
 }
 
 export class Check {

tests/format/typescript/class/jsfmt.spec.js

@@ -1,3 +1,3 @@
 run_spec(import.meta, ["typescript"], {
-  errors: { "babel-ts": ["constructor.ts", "generics.ts", "methods.ts"] },
+  errors: { "babel-ts": ["constructor.ts", "generics.ts"] },
 });

tests/format/typescript/class/methods.ts

@@ -3,7 +3,7 @@ class X {
 }
 
 interface Iterable<T> {
-  export [Symbol.iterator](): Iterator<T>;
+  [Symbol.iterator](): Iterator<T>;
 }
 
 export class Check {

tests/format/typescript/compiler/__snapshots__/jsfmt.spec.js.snap

@@ -668,11 +668,12 @@ printWidth: 80
                                                                                 | printWidth
 =====================================input======================================
 interface I {
-  public [a: string]: number;
+  [a: string]: number;
 }
+
 =====================================output=====================================
 interface I {
-  public [a: string]: number;
+  [a: string]: number;
 }
 
 ================================================================================

tests/format/typescript/compiler/modifiersOnInterfaceIndexSignature1.ts

@@ -1,3 +1,3 @@
 interface I {
-  public [a: string]: number;
-}
+  [a: string]: number;
+}