diff --git a/changelog_unreleased/typescript/18654.md b/changelog_unreleased/typescript/18654.md new file mode 100644 index 000000000..829896fa5 --- /dev/null +++ b/changelog_unreleased/typescript/18654.md @@ -0,0 +1,53 @@ +#### Consistent print non-null assertions (#18654 by @fisker) + + +```tsx +// Input +const corners = baseCorners.map((corner) => + curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset))!, +); +const corners = baseCorners.map((corner) => + curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset)), +); + +// Prettier stable +const corners = baseCorners.map( + (corner) => + curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset))!, +); +const corners = baseCorners.map((corner) => + curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset)), +); + +// Prettier main +const corners = baseCorners.map((corner) => + curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset))!, +); +const corners = baseCorners.map((corner) => + curveCatmullRomCubicApproxPoints(curveOffsetPoints(corner, offset)), +); +``` + + +```tsx +// Input +isEqual(a?.map(([t, _]) => t?.id)!, b?.map(([t, _]) => t?.id)); +isEqual(a?.map(([t, _]) => t?.id)!, b?.map(([t, _]) => t?.id)!); + +// Prettier stable +isEqual( + a?.map(([t, _]) => t?.id)!, + b?.map(([t, _]) => t?.id), +); +isEqual(a?.map(([t, _]) => t?.id)!, b?.map(([t, _]) => t?.id)!); + +// Prettier main +isEqual( + a?.map(([t, _]) => t?.id)!, + b?.map(([t, _]) => t?.id), +); +isEqual( + a?.map(([t, _]) => t?.id)!, + b?.map(([t, _]) => t?.id)!, +); +``` diff --git a/src/language-js/parentheses/needs-parentheses.js b/src/language-js/parentheses/needs-parentheses.js index 99d047156..c000e28d9 100644 --- a/src/language-js/parentheses/needs-parentheses.js +++ b/src/language-js/parentheses/needs-parentheses.js @@ -1263,6 +1263,7 @@ function canDecoratorExpressionUnparenthesized(node) { return ( isDecoratorMemberExpression(node) || (isCallExpression(node) && + // @ts-expect-error -- doesn't exists on `CallExpression` !node.optional && isDecoratorMemberExpression(node.callee)) ); diff --git a/src/language-js/print/assignment.js b/src/language-js/print/assignment.js index 774f9de6d..28112f96d 100644 --- a/src/language-js/print/assignment.js +++ b/src/language-js/print/assignment.js @@ -16,6 +16,7 @@ import { isBinaryish, isBooleanLiteral, isCallExpression, + isChainElementWrapper, isIntersectionType, isLoneShortArgument, isMemberExpression, @@ -365,7 +366,7 @@ function isPoorlyBreakableMemberOrCallChain( const goDeeper = () => isPoorlyBreakableMemberOrCallChain(path, options, print, true); - if (node.type === "ChainExpression" || node.type === "TSNonNullExpression") { + if (isChainElementWrapper(node)) { return path.call(goDeeper, "expression"); } diff --git a/src/language-js/print/binaryish.js b/src/language-js/print/binaryish.js index 6f1640fa9..13ecc2bac 100644 --- a/src/language-js/print/binaryish.js +++ b/src/language-js/print/binaryish.js @@ -243,6 +243,7 @@ function printBinaryishExpressions( const shouldInline = shouldInlineLogicalExpression(node); const rightNodeToCheckComments = + // TODO[@fisker]: Use `stripChainElementWrappers` node.right.type === "ChainExpression" ? node.right.expression : node.right; const lineBeforeOperator = (node.type === "NGPipeExpression" || diff --git a/src/language-js/print/call-arguments.js b/src/language-js/print/call-arguments.js index eae83ee58..d45458521 100644 --- a/src/language-js/print/call-arguments.js +++ b/src/language-js/print/call-arguments.js @@ -33,6 +33,7 @@ import { isStringLiteral, iterateCallArgumentsPath, shouldPrintComma, + stripChainElementWrappers, } from "../utilities/index.js"; import { isConciselyPrintedArray } from "./array.js"; import { printDanglingCommentsInList } from "./miscellaneous.js"; @@ -201,26 +202,60 @@ function printCallArguments(path, options, print) { } function couldExpandArg(arg, arrowChainRecursion = false) { - return ( - (isObjectExpression(arg) && - (arg.properties.length > 0 || hasComment(arg))) || - (isArrayExpression(arg) && (arg.elements.length > 0 || hasComment(arg))) || - (arg.type === "TSTypeAssertion" && couldExpandArg(arg.expression)) || - (isBinaryCastExpression(arg) && couldExpandArg(arg.expression)) || + if ( + isObjectExpression(arg) && + (arg.properties.length > 0 || hasComment(arg)) + ) { + return true; + } + + if (isArrayExpression(arg) && (arg.elements.length > 0 || hasComment(arg))) { + return true; + } + + if ( + (isBinaryCastExpression(arg) || arg.type === "TSTypeAssertion") && + couldExpandArg(arg.expression) + ) { + return true; + } + + if ( arg.type === "FunctionExpression" || - (arg.type === "ArrowFunctionExpression" && - (arg.body.type === "BlockStatement" || - (arg.body.type === "ArrowFunctionExpression" && - couldExpandArg(arg.body, true)) || - isObjectExpression(arg.body) || - isArrayExpression(arg.body) || - (!arrowChainRecursion && - (isCallExpression(arg.body) || - arg.body.type === "ConditionalExpression")) || - isJsxElement(arg.body))) || arg.type === "DoExpression" || arg.type === "ModuleExpression" - ); + ) { + return true; + } + + if (arg.type === "ArrowFunctionExpression") { + const { body } = arg; + + if ( + body.type === "BlockStatement" || + isJsxElement(body) || + isObjectExpression(body) || + isArrayExpression(body) + ) { + return true; + } + + if (body.type === "ArrowFunctionExpression" && couldExpandArg(body, true)) { + return true; + } + + if (!arrowChainRecursion) { + if (body.type === "ConditionalExpression") { + return true; + } + + if (isCallExpression(stripChainElementWrappers(body))) { + return true; + } + } + } + + return false; } function shouldExpandLastArg(args, argDocs, options) { diff --git a/src/language-js/print/class.js b/src/language-js/print/class.js index 13b2f4290..c0cabba86 100644 --- a/src/language-js/print/class.js +++ b/src/language-js/print/class.js @@ -18,6 +18,7 @@ import { CommentCheckFlags, createTypeCheckFunction, hasComment, + isChainElementWrapper, isMemberExpression, } from "../utilities/index.js"; import { printAssignment } from "./assignment.js"; @@ -169,10 +170,8 @@ function hasMultipleHeritage(node) { } function isMemberish(node) { - if ( - node?.type === "ChainExpression" || - node?.type === "TSNonNullExpression" - ) { + // TODO[@fisker]: Should use `stripChainElementWrappers` instead, since it can be in multiple wrappers + if (isChainElementWrapper(node)) { return isMemberish(node.expression); } diff --git a/src/language-js/print/jsx.js b/src/language-js/print/jsx.js index a9462bf24..3e68134f9 100644 --- a/src/language-js/print/jsx.js +++ b/src/language-js/print/jsx.js @@ -588,6 +588,7 @@ function printJsxExpressionContainer(path, options, print) { (shouldInline(node.argument, node) || node.argument.type === "JSXElement")) || isCallExpression(node) || + // TODO[@fisker]: Use `stripChainElementWrappers` (node.type === "ChainExpression" && isCallExpression(node.expression)) || node.type === "FunctionExpression" || diff --git a/src/language-js/print/member-chain.js b/src/language-js/print/member-chain.js index 3915a034a..357e238f2 100644 --- a/src/language-js/print/member-chain.js +++ b/src/language-js/print/member-chain.js @@ -47,6 +47,7 @@ import { printOptionalToken } from "./miscellaneous.js"; // MemberExpression and CallExpression. We need to traverse the AST // and make groups out of it to print it in the desired way. function printMemberChain(path, options, print) { + // TODO[@fisker]: skip `TSNonNullExpression` /* c8 ignore next 6 */ if (path.node.type === "ChainExpression") { return path.call( diff --git a/src/language-js/print/member.js b/src/language-js/print/member.js index ecc90a4ac..115a24b1e 100644 --- a/src/language-js/print/member.js +++ b/src/language-js/print/member.js @@ -8,13 +8,15 @@ import { import { getCallArguments, isCallExpression, + isChainElementWrapper, isMemberExpression, isNumericLiteral, } from "../utilities/index.js"; import { printOptionalToken } from "./miscellaneous.js"; const isCallExpressionWithArguments = (node) => { - if (node.type === "ChainExpression" || node.type === "TSNonNullExpression") { + // TODO[@fisker]: Use `stripChainElementWrappers` + if (isChainElementWrapper(node)) { node = node.expression; } return isCallExpression(node) && getCallArguments(node).length > 0; @@ -48,8 +50,7 @@ function printMemberExpression(path, options, print) { !(isMemberExpression(node) || node.type === "TSNonNullExpression"), ); const firstNonChainElementWrapperParent = path.findAncestor( - (node) => - !(node.type === "ChainExpression" || node.type === "TSNonNullExpression"), + (node) => !isChainElementWrapper(node), ); const shouldInline = diff --git a/src/language-js/print/ternary-old.js b/src/language-js/print/ternary-old.js index d6d2ade93..290506037 100644 --- a/src/language-js/print/ternary-old.js +++ b/src/language-js/print/ternary-old.js @@ -10,6 +10,7 @@ import { import { isBinaryCastExpression, isCallExpression, + isChainElementWrapper, isJsxElement, isMemberExpression, } from "../utilities/index.js"; @@ -146,9 +147,7 @@ function shouldExtraIndentForConditionalExpression(path) { const node = path.getParentNode(ancestorCount); if ( - ((node.type === "ChainExpression" || - node.type === "TSNonNullExpression") && - node.expression === child) || + (isChainElementWrapper(node) && node.expression === child) || (isCallExpression(node) && node.callee === child) || (isMemberExpression(node) && node.object === child) ) { diff --git a/src/language-js/print/ternary.js b/src/language-js/print/ternary.js index 597a100d0..32fc1fa2c 100644 --- a/src/language-js/print/ternary.js +++ b/src/language-js/print/ternary.js @@ -18,6 +18,7 @@ import { hasComment, isBinaryCastExpression, isCallExpression, + isChainElementWrapper, isConditionalType, isJsxElement, isLoneShortArgument, @@ -103,9 +104,7 @@ function shouldExtraIndentForConditionalExpression(path) { const node = path.getParentNode(ancestorCount); if ( - ((node.type === "ChainExpression" || - node.type === "TSNonNullExpression") && - node.expression === child) || + (isChainElementWrapper(node) && node.expression === child) || (isCallExpression(node) && node.callee === child) || (isMemberExpression(node) && node.object === child) ) { diff --git a/src/language-js/utilities/index.js b/src/language-js/utilities/index.js index fc3658bc5..f80aced29 100644 --- a/src/language-js/utilities/index.js +++ b/src/language-js/utilities/index.js @@ -64,8 +64,7 @@ function hasNakedLeftSide(node) { node.type === "BindExpression" || (node.type === "UpdateExpression" && !node.prefix) || isBinaryCastExpression(node) || - node.type === "TSNonNullExpression" || - node.type === "ChainExpression" + isChainElementWrapper(node) ); } @@ -364,21 +363,10 @@ function isTestCall(node, parent) { return false; } -/** -@template {ReturnType} TypeCheckFunction -@param {TypeCheckFunction} fn -@return {(node: Node) => boolean} */ -const skipChainExpression = (fn) => (node) => { - if (node?.type === "ChainExpression") { - node = node.expression; - } - - return fn(node); -}; - -const isCallExpression = skipChainExpression( - createTypeCheckFunction(["CallExpression", "OptionalCallExpression"]), -); +const isCallExpression = createTypeCheckFunction([ + "CallExpression", + "OptionalCallExpression", +]); const isMemberExpression = createTypeCheckFunction([ "MemberExpression", @@ -524,16 +512,19 @@ function isFunctionCompositionArgs(args) { return false; } let count = 0; - for (const arg of args) { + for (let arg of args) { if (isFunctionOrArrowExpression(arg)) { count += 1; if (count > 1) { return true; } - } else if (isCallExpression(arg)) { - for (const childArg of getCallArguments(arg)) { - if (isFunctionOrArrowExpression(childArg)) { - return true; + } else { + arg = stripChainElementWrappers(arg); + if (isCallExpression(arg)) { + for (const childArg of getCallArguments(arg)) { + if (isFunctionOrArrowExpression(childArg)) { + return true; + } } } } @@ -574,12 +565,10 @@ function isSimpleCallArgument(node, depth = 2) { return false; } - if (node.type === "ChainExpression" || node.type === "TSNonNullExpression") { - return isSimpleCallArgument(node.expression, depth); - } - const isChildSimple = (child) => isSimpleCallArgument(child, depth - 1); + node = stripChainElementWrappers(node); + if (isRegExpLiteral(node)) { return getStringWidth(node.pattern ?? node.regex.pattern) <= 5; } @@ -705,9 +694,9 @@ function startsWithNoLookaheadToken(node, predicate) { case "SequenceExpression": return startsWithNoLookaheadToken(node.expressions[0], predicate); case "ChainExpression": + case "TSNonNullExpression": case "TSSatisfiesExpression": case "TSAsExpression": - case "TSNonNullExpression": case "AsExpression": case "AsConstExpression": case "SatisfiesExpression": @@ -856,10 +845,6 @@ function getCallArguments(node) { return callArgumentsCache.get(node); } - if (node.type === "ChainExpression") { - return getCallArguments(node.expression); - } - let args; if (node.type === "ImportExpression" || node.type === "TSImportType") { args = [node.source]; @@ -880,13 +865,6 @@ function getCallArguments(node) { function iterateCallArgumentsPath(path, iteratee) { const { node } = path; - if (node.type === "ChainExpression") { - return path.call( - () => iterateCallArgumentsPath(path, iteratee), - "expression", - ); - } - if (node.type === "ImportExpression" || node.type === "TSImportType") { path.call(() => iteratee(path, 0), "source"); @@ -901,30 +879,24 @@ function iterateCallArgumentsPath(path, iteratee) { } function getCallArgumentSelector(node, index) { - const selectors = []; - if (node.type === "ChainExpression") { - node = node.expression; - selectors.push("expression"); - } - if (node.type === "ImportExpression" || node.type === "TSImportType") { if (index === 0 || index === (node.options ? -2 : -1)) { - return [...selectors, "source"]; + return ["source"]; } if (node.options && (index === 1 || index === -1)) { - return [...selectors, "options"]; + return ["options"]; } throw new RangeError("Invalid argument index"); } else if (node.type === "TSExternalModuleReference") { if (index === 0 || index === -1) { - return [...selectors, "expression"]; + return ["expression"]; } } else { if (index < 0) { index = node.arguments.length + index; } if (index >= 0 && index < node.arguments.length) { - return [...selectors, "arguments", index]; + return ["arguments", index]; } } @@ -1114,6 +1086,23 @@ function isIifeCalleeOrTaggedTemplateExpressionTag(path) { ); } +const isChainElementWrapper = createTypeCheckFunction([ + "ChainExpression", + "TSNonNullExpression", +]); + +/** +@param {Node} node +@returns {Exclude} +*/ +function stripChainElementWrappers(node) { + while (isChainElementWrapper(node)) { + node = node.expression; + } + + return node; +} + export { CommentCheckFlags, createTypeCheckFunction, @@ -1138,6 +1127,7 @@ export { isBooleanTypeCoercion, isCallExpression, isCallLikeExpression, + isChainElementWrapper, isConditionalType, isExportDeclaration, isFlowObjectTypePropertyAFunction, @@ -1181,6 +1171,7 @@ export { shouldPrintComma, shouldUnionTypePrintOwnComments, startsWithNoLookaheadToken, + stripChainElementWrappers, }; export { default as isMeaningfulEmptyStatement } from "./is-meaningful-empty-statement.js"; export { default as isNodeMatches } from "./is-node-matches.js"; diff --git a/tests/format/js/chain-expression/__snapshots__/format.test.js.snap b/tests/format/js/chain-expression/__snapshots__/format.test.js.snap index 5ee91af26..6fefb1888 100644 --- a/tests/format/js/chain-expression/__snapshots__/format.test.js.snap +++ b/tests/format/js/chain-expression/__snapshots__/format.test.js.snap @@ -61,19 +61,13 @@ printWidth: 80 | printWidth =====================================input====================================== function someFunctionName( - someLongBreakingParameterName, - anotherLongParameterName, ) { return isEqual(a?.map(([t, _]) => t?.id), b?.map(([t, _]) => t?.id)); return isEqual(a.map(([t, _]) => t?.id), b.map(([t, _]) => t?.id)); } - =====================================output===================================== -function someFunctionName( - someLongBreakingParameterName, - anotherLongParameterName, -) { +function someFunctionName() { return isEqual( a?.map(([t, _]) => t?.id), b?.map(([t, _]) => t?.id), diff --git a/tests/format/js/chain-expression/issue-15785-1.js b/tests/format/js/chain-expression/issue-15785-1.js index b712e1101..1281ee76a 100644 --- a/tests/format/js/chain-expression/issue-15785-1.js +++ b/tests/format/js/chain-expression/issue-15785-1.js @@ -1,8 +1,5 @@ function someFunctionName( - someLongBreakingParameterName, - anotherLongParameterName, ) { return isEqual(a?.map(([t, _]) => t?.id), b?.map(([t, _]) => t?.id)); return isEqual(a.map(([t, _]) => t?.id), b.map(([t, _]) => t?.id)); } - diff --git a/tests/format/typescript/chain-expression/__snapshots__/format.test.js.snap b/tests/format/typescript/chain-expression/__snapshots__/format.test.js.snap index 578675106..72df887c2 100644 --- a/tests/format/typescript/chain-expression/__snapshots__/format.test.js.snap +++ b/tests/format/typescript/chain-expression/__snapshots__/format.test.js.snap @@ -140,6 +140,73 @@ foo(a?.b!); ================================================================================ `; +exports[`issue-15785-1.ts format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +function someFunctionName( +) { + return isEqual(a?.map(([t, _]) => t?.id), b?.map(([t, _]) => t?.id)); + return isEqual(a?.map(([t, _]) => t?.id)!, b?.map(([t, _]) => t?.id)!); + return isEqual(a.map(([t, _]) => t?.id), b.map(([t, _]) => t?.id)); + return isEqual(a.map(([t, _]) => t?.id)!, b.map(([t, _]) => t?.id)!); +} + +=====================================output===================================== +function someFunctionName() { + return isEqual( + a?.map(([t, _]) => t?.id), + b?.map(([t, _]) => t?.id), + ); + return isEqual( + a?.map(([t, _]) => t?.id)!, + b?.map(([t, _]) => t?.id)!, + ); + return isEqual( + a.map(([t, _]) => t?.id), + b.map(([t, _]) => t?.id), + ); + return isEqual( + a.map(([t, _]) => t?.id)!, + b.map(([t, _]) => t?.id)!, + ); +} + +================================================================================ +`; + +exports[`issue-15785-2.ts format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const theValue = Object.entries(someLongObjectName).filter( + ([listingId]) => someListToCompareToHere?.includes(listingId), +); +const theValue2 = Object.entries(someLongObjectName).filter( + ([listingId]) => someListToCompareToHere.includes(listingId), +); +const theValue3 = Object.entries(someLongObjectName).filter( + ([listingId]) => someListToCompareToHere.includes(listingId)!, +); + +=====================================output===================================== +const theValue = Object.entries(someLongObjectName).filter(([listingId]) => + someListToCompareToHere?.includes(listingId), +); +const theValue2 = Object.entries(someLongObjectName).filter(([listingId]) => + someListToCompareToHere.includes(listingId), +); +const theValue3 = Object.entries(someLongObjectName).filter(([listingId]) => + someListToCompareToHere.includes(listingId)!, +); + +================================================================================ +`; + exports[`member-expression.ts format 1`] = ` ====================================options===================================== parsers: ["typescript"] diff --git a/tests/format/typescript/chain-expression/issue-15785-1.ts b/tests/format/typescript/chain-expression/issue-15785-1.ts new file mode 100644 index 000000000..1116ac4e3 --- /dev/null +++ b/tests/format/typescript/chain-expression/issue-15785-1.ts @@ -0,0 +1,7 @@ +function someFunctionName( +) { + return isEqual(a?.map(([t, _]) => t?.id), b?.map(([t, _]) => t?.id)); + return isEqual(a?.map(([t, _]) => t?.id)!, b?.map(([t, _]) => t?.id)!); + return isEqual(a.map(([t, _]) => t?.id), b.map(([t, _]) => t?.id)); + return isEqual(a.map(([t, _]) => t?.id)!, b.map(([t, _]) => t?.id)!); +} diff --git a/tests/format/typescript/chain-expression/issue-15785-2.ts b/tests/format/typescript/chain-expression/issue-15785-2.ts new file mode 100644 index 000000000..2e6dfdcf5 --- /dev/null +++ b/tests/format/typescript/chain-expression/issue-15785-2.ts @@ -0,0 +1,9 @@ +const theValue = Object.entries(someLongObjectName).filter( + ([listingId]) => someListToCompareToHere?.includes(listingId), +); +const theValue2 = Object.entries(someLongObjectName).filter( + ([listingId]) => someListToCompareToHere.includes(listingId), +); +const theValue3 = Object.entries(someLongObjectName).filter( + ([listingId]) => someListToCompareToHere.includes(listingId)!, +); diff --git a/tests/format/typescript/non-null/__snapshots__/format.test.js.snap b/tests/format/typescript/non-null/__snapshots__/format.test.js.snap index fe0a17628..69f69d452 100644 --- a/tests/format/typescript/non-null/__snapshots__/format.test.js.snap +++ b/tests/format/typescript/non-null/__snapshots__/format.test.js.snap @@ -58,6 +58,11 @@ this.foo.get("bar")!.doThings().more(); foo!.bar().baz().what(); +xyz.a(b).a(b).a(b) +xyz.a(b!).a(b!).a(b!) +xyz.a(/a/).a(/a/).a(/a/) +xyz.a(/a/!).a(/a/!).a(/a/!) + =====================================output===================================== const { somePropThatHasAReallyLongName, anotherPropThatHasALongName } = this.props.imReallySureAboutThis!; @@ -69,6 +74,11 @@ this.foo.get("bar")!.doThings().more(); foo!.bar().baz().what(); +xyz.a(b).a(b).a(b); +xyz.a(b!).a(b!).a(b!); +xyz.a(/a/).a(/a/).a(/a/); +xyz.a(/a/!).a(/a/!).a(/a/!); + ================================================================================ `; diff --git a/tests/format/typescript/non-null/member-chain.ts b/tests/format/typescript/non-null/member-chain.ts index 753ae6695..9283df3e9 100644 --- a/tests/format/typescript/non-null/member-chain.ts +++ b/tests/format/typescript/non-null/member-chain.ts @@ -5,3 +5,8 @@ const { somePropThatHasAReallyLongName2, anotherPropThatHasALongName2 } = this.p this.foo.get("bar")!.doThings().more(); foo!.bar().baz().what(); + +xyz.a(b).a(b).a(b) +xyz.a(b!).a(b!).a(b!) +xyz.a(/a/).a(/a/).a(/a/) +xyz.a(/a/!).a(/a/!).a(/a/!)