From eabceb2dcb55c8892c3abe0566eb2970ab2e22d8 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 30 Oct 2020 09:56:47 +0800 Subject: [PATCH] Consistent type print (#9521) --- changelog_unreleased/typescript/pr-9521.md | 25 ++++++++ src/language-js/printer-estree.js | 60 +++++-------------- src/language-js/utils.js | 22 +++++-- .../comments/__snapshots__/jsfmt.spec.js.snap | 4 +- .../__snapshots__/jsfmt.spec.js.snap | 5 +- .../generic/__snapshots__/jsfmt.spec.js.snap | 4 +- .../__snapshots__/jsfmt.spec.js.snap | 4 +- .../__snapshots__/jsfmt.spec.js.snap | 47 +++++++++++++++ .../typeparams/consistent/issue-9501.ts | 3 + .../typeparams/consistent/jsfmt.spec.js | 1 + .../typeparams/consistent/simple-types.ts | 7 +++ 11 files changed, 121 insertions(+), 61 deletions(-) create mode 100644 changelog_unreleased/typescript/pr-9521.md create mode 100644 tests/typescript/typeparams/consistent/__snapshots__/jsfmt.spec.js.snap create mode 100644 tests/typescript/typeparams/consistent/issue-9501.ts create mode 100644 tests/typescript/typeparams/consistent/jsfmt.spec.js create mode 100644 tests/typescript/typeparams/consistent/simple-types.ts diff --git a/changelog_unreleased/typescript/pr-9521.md b/changelog_unreleased/typescript/pr-9521.md new file mode 100644 index 000000000..48ff062d2 --- /dev/null +++ b/changelog_unreleased/typescript/pr-9521.md @@ -0,0 +1,25 @@ +#### Fix inconsistent type format between `typescript` and `flow` (#9521 by @fisker) + + +```ts +// Input +const name: SomeGeneric< + Pick +> = null; + +// Prettier master (--parser=typescript) +const name: SomeGeneric> = null; + +// Prettier master (--parser=flow) +const name: SomeGeneric< + Pick +> = null; + +// Prettier stable (typescript and flow parser) +const name: SomeGeneric< + Pick +> = null; +``` diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 2f8af23d2..506bcd03e 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -95,7 +95,7 @@ const { isNumericLiteral, isObjectType, isObjectTypePropertyAFunction, - isSimpleFlowType, + isSimpleType, isSimpleNumber, isSimpleTemplateLiteral, isStringLiteral, @@ -2390,14 +2390,10 @@ function printPathNoParens(path, options, print, args) { return "*"; case "EmptyTypeAnnotation": return "empty"; - case "AnyTypeAnnotation": - return "any"; case "MixedTypeAnnotation": return "mixed"; case "ArrayTypeAnnotation": return concat([path.call(print, "elementType"), "[]"]); - case "BooleanTypeAnnotation": - return "boolean"; case "BooleanLiteralTypeAnnotation": return "" + n.value; case "DeclareClass": @@ -2634,11 +2630,6 @@ function printPathNoParens(path, options, print, args) { path.call(print, "typeAnnotation"), ]); } - case "GenericTypeAnnotation": - return concat([ - path.call(print, "id"), - path.call(print, "typeParameters"), - ]); case "DeclareInterface": case "InterfaceDeclaration": @@ -2833,15 +2824,6 @@ function printPathNoParens(path, options, print, args) { } case "NullableTypeAnnotation": return concat(["?", path.call(print, "typeAnnotation")]); - case "TSNullKeyword": - case "NullLiteralTypeAnnotation": - return "null"; - case "ThisTypeAnnotation": - return "this"; - case "NumberTypeAnnotation": - return "number"; - case "SymbolTypeAnnotation": - return "symbol"; case "ObjectTypeCallProperty": if (n.static) { parts.push("static "); @@ -2899,8 +2881,6 @@ function printPathNoParens(path, options, print, args) { } return printNumber(n.raw); - case "StringTypeAnnotation": - return "string"; case "DeclareTypeAlias": case "TypeAlias": { if (n.type === "DeclareTypeAlias" || n.declare) { @@ -3018,8 +2998,6 @@ function printPathNoParens(path, options, print, args) { } case "TypeofTypeAnnotation": return concat(["typeof ", path.call(print, "argument")]); - case "VoidTypeAnnotation": - return "void"; case "InferredPredicate": return "%checks"; // Unhandled types below. If encountered, nodes of these types should @@ -3029,10 +3007,12 @@ function printPathNoParens(path, options, print, args) { return concat(["%checks(", path.call(print, "value"), ")"]); case "TSAbstractKeyword": return "abstract"; + case "AnyTypeAnnotation": case "TSAnyKeyword": return "any"; case "TSAsyncKeyword": return "async"; + case "BooleanTypeAnnotation": case "TSBooleanKeyword": return "boolean"; case "TSBigIntKeyword": @@ -3043,8 +3023,12 @@ function printPathNoParens(path, options, print, args) { return "declare"; case "TSExportKeyword": return "export"; + case "NullLiteralTypeAnnotation": + case "TSNullKeyword": + return "null"; case "TSNeverKeyword": return "never"; + case "NumberTypeAnnotation": case "TSNumberKeyword": return "number"; case "TSObjectKeyword": @@ -3057,16 +3041,19 @@ function printPathNoParens(path, options, print, args) { return "public"; case "TSReadonlyKeyword": return "readonly"; + case "SymbolTypeAnnotation": case "TSSymbolKeyword": return "symbol"; case "TSStaticKeyword": return "static"; + case "StringTypeAnnotation": case "TSStringKeyword": return "string"; case "TSUndefinedKeyword": return "undefined"; case "TSUnknownKeyword": return "unknown"; + case "VoidTypeAnnotation": case "TSVoidKeyword": return "void"; case "TSAsExpression": @@ -3125,9 +3112,10 @@ function printPathNoParens(path, options, print, args) { parts.push(path.call(print, "parameter")); return concat(parts); + case "GenericTypeAnnotation": case "TSTypeReference": return concat([ - path.call(print, "typeName"), + path.call(print, n.type === "TSTypeReference" ? "typeName" : "id"), printTypeParameters(path, options, print, "typeParameters"), ]); case "TSTypeQuery": @@ -3180,6 +3168,7 @@ function printPathNoParens(path, options, print, args) { ]); case "TSNonNullExpression": return concat([path.call(print, "expression"), "!"]); + case "ThisTypeAnnotation": case "TSThisType": return "this"; case "TSImportType": @@ -4012,7 +4001,7 @@ function printFunctionParameters( functionNode.this !== parameters[0] && parameters[0].typeAnnotation && functionNode.typeParameters === null && - isSimpleFlowType(parameters[0].typeAnnotation) && + isSimpleType(parameters[0].typeAnnotation) && !functionNode.rest; if (isFlowShorthandWithOneArg) { @@ -4217,9 +4206,6 @@ function printTypeParameters(path, options, print, paramsKey) { } const grandparent = path.getNode(2); - const greatGrandParent = path.getNode(3); - const greatGreatGrandParent = path.getNode(4); - const isParameterInTestCall = grandparent != null && isTestCall(grandparent); const shouldInline = @@ -4231,21 +4217,7 @@ function printTypeParameters(path, options, print, paramsKey) { shouldHugType(n[paramsKey][0].id)) || (n[paramsKey][0].type === "TSTypeReference" && shouldHugType(n[paramsKey][0].typeName)) || - n[paramsKey][0].type === "NullableTypeAnnotation" || - // See https://github.com/prettier/prettier/pull/6467 for the context. - (greatGreatGrandParent && - greatGreatGrandParent.type === "VariableDeclarator" && - grandparent.type === "TSTypeAnnotation" && - greatGrandParent.type !== "ArrowFunctionExpression" && - n[paramsKey][0].type !== "TSUnionType" && - n[paramsKey][0].type !== "UnionTypeAnnotation" && - n[paramsKey][0].type !== "TSIntersectionType" && - n[paramsKey][0].type !== "IntersectionTypeAnnotation" && - n[paramsKey][0].type !== "TSConditionalType" && - n[paramsKey][0].type !== "TSMappedType" && - n[paramsKey][0].type !== "TSTypeOperator" && - n[paramsKey][0].type !== "TSIndexedAccessType" && - n[paramsKey][0].type !== "TSArrayType"))); + n[paramsKey][0].type === "NullableTypeAnnotation")); function printDanglingCommentsForInline(n) { if (!hasDanglingComments(n)) { @@ -5130,7 +5102,7 @@ function stmtNeedsASIProtection(path, options) { } function shouldHugType(node) { - if (isSimpleFlowType(node) || isObjectType(node)) { + if (isSimpleType(node) || isObjectType(node)) { return true; } diff --git a/src/language-js/utils.js b/src/language-js/utils.js index b2abb7bf4..cfa43d91d 100644 --- a/src/language-js/utils.js +++ b/src/language-js/utils.js @@ -430,7 +430,7 @@ function isMemberish(node) { ); } -const flowTypeAnnotations = new Set([ +const simpleFlowTypeAnnotations = new Set([ "AnyTypeAnnotation", "NullLiteralTypeAnnotation", "GenericTypeAnnotation", @@ -443,16 +443,28 @@ const flowTypeAnnotations = new Set([ "BooleanLiteralTypeAnnotation", "StringTypeAnnotation", ]); +const simpleTypeScriptTypeAnnotations = new Set([ + "TSAnyKeyword", + "TSNullKeyword", + "TSTypeReference", + "TSThisType", + "TSNumberKeyword", + "TSVoidKeyword", + "TSBooleanKeyword", + "TSLiteralType", +]); /** * @param {Node} node * @returns {boolean} */ -function isSimpleFlowType(node) { +function isSimpleType(node) { return ( node && - flowTypeAnnotations.has(node.type) && - !(node.type === "GenericTypeAnnotation" && node.typeParameters) + ((simpleFlowTypeAnnotations.has(node.type) && + !(node.type === "GenericTypeAnnotation" && node.typeParameters)) || + (simpleTypeScriptTypeAnnotations.has(node.type) && + !(node.type === "TSTypeReference" && node.typeParameters))) ); } @@ -1475,7 +1487,7 @@ module.exports = { isNumericLiteral, isObjectType, isObjectTypePropertyAFunction, - isSimpleFlowType, + isSimpleType, isSimpleNumber, isSimpleTemplateLiteral, isStringLiteral, diff --git a/tests/typescript/comments/__snapshots__/jsfmt.spec.js.snap b/tests/typescript/comments/__snapshots__/jsfmt.spec.js.snap index f0088c2db..b20ab8598 100644 --- a/tests/typescript/comments/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript/comments/__snapshots__/jsfmt.spec.js.snap @@ -702,9 +702,7 @@ interface Foo { } type T = (arg) => any; -functionName< - A // comment ->(); +functionName(); // comment const a: T< // comment > = 1; diff --git a/tests/typescript/decorators/__snapshots__/jsfmt.spec.js.snap b/tests/typescript/decorators/__snapshots__/jsfmt.spec.js.snap index 774d1014e..1c61830d0 100644 --- a/tests/typescript/decorators/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript/decorators/__snapshots__/jsfmt.spec.js.snap @@ -389,9 +389,8 @@ class Bar { } class MyContainerComponent { - @ContentChildren(MyComponent) components: QueryListSomeBigName< - MyComponentThat - >; + @ContentChildren(MyComponent) + components: QueryListSomeBigName; } ================================================================================ diff --git a/tests/typescript/generic/__snapshots__/jsfmt.spec.js.snap b/tests/typescript/generic/__snapshots__/jsfmt.spec.js.snap index 470b1ece1..38e2ba125 100644 --- a/tests/typescript/generic/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript/generic/__snapshots__/jsfmt.spec.js.snap @@ -73,9 +73,7 @@ export const getVehicleDescriptor = async ( export const getVehicleDescriptor = async ( vehicleId: string -): Promise< - Collections.Parts.PrintedCircuitBoardAssemblyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy -> => {}; +): Promise => {}; export const getVehicleDescriptor = async ( vehicleId: string diff --git a/tests/typescript/template-literals/__snapshots__/jsfmt.spec.js.snap b/tests/typescript/template-literals/__snapshots__/jsfmt.spec.js.snap index 1153f60f2..f56f6190c 100644 --- a/tests/typescript/template-literals/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript/template-literals/__snapshots__/jsfmt.spec.js.snap @@ -39,9 +39,7 @@ printWidth: 80 =====================================input====================================== const bar = tag\`but where will prettier wrap such a long tagged template literal? \${foo.bar.baz} long long long long long long long long long long long long long long\`; =====================================output===================================== -const bar = tag< - number ->\`but where will prettier wrap such a long tagged template literal? \${foo.bar.baz} long long long long long long long long long long long long long long\`; +const bar = tag\`but where will prettier wrap such a long tagged template literal? \${foo.bar.baz} long long long long long long long long long long long long long long\`; ================================================================================ `; diff --git a/tests/typescript/typeparams/consistent/__snapshots__/jsfmt.spec.js.snap b/tests/typescript/typeparams/consistent/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000..9d867e023 --- /dev/null +++ b/tests/typescript/typeparams/consistent/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,47 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`issue-9501.ts format 1`] = ` +====================================options===================================== +parsers: ["typescript", "flow", "babel-flow", "babel"] +printWidth: 80 + | printWidth +=====================================input====================================== +const name: SomeGeneric< + Pick +> = null; + +=====================================output===================================== +const name: SomeGeneric< + Pick +> = null; + +================================================================================ +`; + +exports[`simple-types.ts format 1`] = ` +====================================options===================================== +parsers: ["typescript", "flow", "babel-flow", "babel"] +printWidth: 80 + | printWidth +=====================================input====================================== +const foo1: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo2: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo3: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo4: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo5: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo6: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo7: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; + +=====================================output===================================== +const foo1: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo2: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo3: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo4: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo5: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo6: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo7: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo< + symbol +> = a; + +================================================================================ +`; diff --git a/tests/typescript/typeparams/consistent/issue-9501.ts b/tests/typescript/typeparams/consistent/issue-9501.ts new file mode 100644 index 000000000..c5cbe7386 --- /dev/null +++ b/tests/typescript/typeparams/consistent/issue-9501.ts @@ -0,0 +1,3 @@ +const name: SomeGeneric< + Pick +> = null; diff --git a/tests/typescript/typeparams/consistent/jsfmt.spec.js b/tests/typescript/typeparams/consistent/jsfmt.spec.js new file mode 100644 index 000000000..61cd597f9 --- /dev/null +++ b/tests/typescript/typeparams/consistent/jsfmt.spec.js @@ -0,0 +1 @@ +run_spec(__dirname, ["typescript", "flow", "babel-flow", "babel"]); diff --git a/tests/typescript/typeparams/consistent/simple-types.ts b/tests/typescript/typeparams/consistent/simple-types.ts new file mode 100644 index 000000000..9238642c3 --- /dev/null +++ b/tests/typescript/typeparams/consistent/simple-types.ts @@ -0,0 +1,7 @@ +const foo1: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo2: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo3: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo4: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo5: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo6: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a; +const foo7: Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo = a;