From b697a596a628a9e92d750e2e59d15b36cfe1550c Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 18 Nov 2022 09:54:23 +0800 Subject: [PATCH] TypeScript: Throw errors for invalid modifier on type parameter (#13853) --- src/language-js/parse/babel.js | 2 - .../parse/postprocess/typescript.js | 33 ++++++++- src/language-js/print/typescript.js | 7 -- tests/config/format-test.js | 5 ++ .../__snapshots__/jsfmt.spec.js.snap | 72 +++++++++++++++++++ .../babel-ts/__snapshots__/jsfmt.spec.js.snap | 64 +++++++++++++++++ .../errors/typescript/babel-ts/jsfmt.spec.js | 18 +++++ .../misc/errors/typescript/jsfmt.spec.js | 18 +++++ .../__snapshots__/jsfmt.spec.js.snap | 20 ------ .../invalid-modifiers.ts | 10 --- .../__snapshots__/jsfmt.spec.js.snap | 20 ------ .../misc/typescript-only/invalid-modifiers.ts | 10 --- .../class/__snapshots__/jsfmt.spec.js.snap | 15 +--- tests/format/typescript/class/jsfmt.spec.js | 2 +- tests/format/typescript/class/methods.ts | 2 +- .../compiler/__snapshots__/jsfmt.spec.js.snap | 5 +- .../modifiersOnInterfaceIndexSignature1.ts | 4 +- 17 files changed, 218 insertions(+), 89 deletions(-) diff --git a/src/language-js/parse/babel.js b/src/language-js/parse/babel.js index 25706b828..f93834041 100644 --- a/src/language-js/parse/babel.js +++ b/src/language-js/parse/babel.js @@ -207,10 +207,8 @@ const allowedMessageCodes = new Set([ "InvalidTupleMemberLabel", "NonClassMethodPropertyHasAbstractModifer", - "ReadonlyForMethodSignature", "ClassMethodHasDeclare", "ClassMethodHasReadonly", - "InvalidModifierOnTypeMember", "DuplicateAccessibilityModifier", "IndexSignatureHasDeclare", diff --git a/src/language-js/parse/postprocess/typescript.js b/src/language-js/parse/postprocess/typescript.js index 1f86d936b..f2a4753bb 100644 --- a/src/language-js/parse/postprocess/typescript.js +++ b/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); }); } diff --git a/src/language-js/print/typescript.js b/src/language-js/print/typescript.js index dadbc8665..d946acb72 100644 --- a/src/language-js/print/typescript.js +++ b/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 ? "]" : "", diff --git a/tests/config/format-test.js b/tests/config/format-test.js index 08b644b8a..dd7ed067b 100644 --- a/tests/config/format-test.js +++ b/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}`}`, diff --git a/tests/format/misc/errors/typescript/__snapshots__/jsfmt.spec.js.snap b/tests/format/misc/errors/typescript/__snapshots__/jsfmt.spec.js.snap index 5a1c8a384..66304e422 100644 --- a/tests/format/misc/errors/typescript/__snapshots__/jsfmt.spec.js.snap +++ b/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 { diff --git a/tests/format/misc/errors/typescript/babel-ts/__snapshots__/jsfmt.spec.js.snap b/tests/format/misc/errors/typescript/babel-ts/__snapshots__/jsfmt.spec.js.snap index c0cb2df8c..839762482 100644 --- a/tests/format/misc/errors/typescript/babel-ts/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/misc/errors/typescript/babel-ts/__snapshots__/jsfmt.spec.js.snap @@ -43,3 +43,67 @@ exports[`snippet: #2 [babel-ts] format 1`] = ` > 1 | 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 | }" +`; diff --git a/tests/format/misc/errors/typescript/babel-ts/jsfmt.spec.js b/tests/format/misc/errors/typescript/babel-ts/jsfmt.spec.js index c4dcd3b77..d418fe69f 100644 --- a/tests/format/misc/errors/typescript/babel-ts/jsfmt.spec.js +++ b/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);", "foo = '100';", + ...[ + "abstract", + "declare", + "export", + "static", + "private", + "protected", + "public", + "readonly", + ].map( + (modifier) => outdent` + interface Foo { + ${modifier} e(); + } + ` + ), ], }, ["babel-ts"] diff --git a/tests/format/misc/errors/typescript/jsfmt.spec.js b/tests/format/misc/errors/typescript/jsfmt.spec.js index 8cfd9c7b4..3c9f2fa56 100644 --- a/tests/format/misc/errors/typescript/jsfmt.spec.js +++ b/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 =
{"foo"}>
;', 'let x5 =
}{"foo"}
;', 'let x6 =
>{"foo"}
;', + ...[ + "abstract", + "declare", + "export", + "static", + "private", + "protected", + "public", + "readonly", + ].map( + (modifier) => outdent` + interface Foo { + ${modifier} e(); + } + ` + ), ], }, ["typescript"] diff --git a/tests/format/misc/typescript-babel-only/__snapshots__/jsfmt.spec.js.snap b/tests/format/misc/typescript-babel-only/__snapshots__/jsfmt.spec.js.snap index 9a2dd84d4..88600baab 100644 --- a/tests/format/misc/typescript-babel-only/__snapshots__/jsfmt.spec.js.snap +++ b/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() {} diff --git a/tests/format/misc/typescript-babel-only/invalid-modifiers.ts b/tests/format/misc/typescript-babel-only/invalid-modifiers.ts index 97fbbb1e8..34b1182c2 100644 --- a/tests/format/misc/typescript-babel-only/invalid-modifiers.ts +++ b/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() {}; diff --git a/tests/format/misc/typescript-only/__snapshots__/jsfmt.spec.js.snap b/tests/format/misc/typescript-only/__snapshots__/jsfmt.spec.js.snap index 17d4458d1..fb9a7c9b1 100644 --- a/tests/format/misc/typescript-only/__snapshots__/jsfmt.spec.js.snap +++ b/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() {} diff --git a/tests/format/misc/typescript-only/invalid-modifiers.ts b/tests/format/misc/typescript-only/invalid-modifiers.ts index 440e6268d..9f287a99f 100644 --- a/tests/format/misc/typescript-only/invalid-modifiers.ts +++ b/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() {}; diff --git a/tests/format/typescript/class/__snapshots__/jsfmt.spec.js.snap b/tests/format/typescript/class/__snapshots__/jsfmt.spec.js.snap index 8725332e6..598e304a2 100644 --- a/tests/format/typescript/class/__snapshots__/jsfmt.spec.js.snap +++ b/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 { -> 6 | export [Symbol.iterator](): Iterator; - | ^ - 7 | } - 8 | - 9 | export class Check {" -`; - exports[`methods.ts format 1`] = ` ====================================options===================================== parsers: ["typescript"] @@ -414,7 +403,7 @@ class X { } interface Iterable { - export [Symbol.iterator](): Iterator; + [Symbol.iterator](): Iterator; } export class Check { @@ -427,7 +416,7 @@ class X { } interface Iterable { - export [Symbol.iterator](): Iterator; + [Symbol.iterator](): Iterator; } export class Check { diff --git a/tests/format/typescript/class/jsfmt.spec.js b/tests/format/typescript/class/jsfmt.spec.js index 72bcd1445..dcdffcf46 100644 --- a/tests/format/typescript/class/jsfmt.spec.js +++ b/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"] }, }); diff --git a/tests/format/typescript/class/methods.ts b/tests/format/typescript/class/methods.ts index 799573c9c..3b81dbedb 100644 --- a/tests/format/typescript/class/methods.ts +++ b/tests/format/typescript/class/methods.ts @@ -3,7 +3,7 @@ class X { } interface Iterable { - export [Symbol.iterator](): Iterator; + [Symbol.iterator](): Iterator; } export class Check { diff --git a/tests/format/typescript/compiler/__snapshots__/jsfmt.spec.js.snap b/tests/format/typescript/compiler/__snapshots__/jsfmt.spec.js.snap index 93d1d0e4d..f788016fc 100644 --- a/tests/format/typescript/compiler/__snapshots__/jsfmt.spec.js.snap +++ b/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; } ================================================================================ diff --git a/tests/format/typescript/compiler/modifiersOnInterfaceIndexSignature1.ts b/tests/format/typescript/compiler/modifiersOnInterfaceIndexSignature1.ts index 212f82bca..d6dc6783f 100644 --- a/tests/format/typescript/compiler/modifiersOnInterfaceIndexSignature1.ts +++ b/tests/format/typescript/compiler/modifiersOnInterfaceIndexSignature1.ts @@ -1,3 +1,3 @@ interface I { - public [a: string]: number; -} \ No newline at end of file + [a: string]: number; +}