Skip to content

Commit

Permalink
Fix inconsistencies for ChainElements (prettier#15806)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker authored and medikoo committed Feb 17, 2024
1 parent 3288311 commit 15ccb35
Show file tree
Hide file tree
Showing 15 changed files with 420 additions and 68 deletions.
51 changes: 51 additions & 0 deletions changelog_unreleased/javascript/15806.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#### Fix inconsistencies for optional-chaining (#15806 by @fisker)

Only happens when using `typescript`, `meriyah` or other ESTree parsers except `babel`.

<!-- prettier-ignore -->
```js
// 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));
}
theValue = Object.entries(someLongObjectName).filter(
([listingId]) => someListToCompareToHere.includes(listingId),
);
theValue = Object.entries(someLongObjectName).filter(
([listingId]) => someListToCompareToHere?.includes(listingId),
);

// Prettier stable
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));
}
theValue = Object.entries(someLongObjectName).filter(([listingId]) =>
someListToCompareToHere.includes(listingId),
);
theValue = Object.entries(someLongObjectName).filter(
([listingId]) => someListToCompareToHere?.includes(listingId),
);

// Prettier main
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),
);
}
theValue = Object.entries(someLongObjectName).filter(([listingId]) =>
someListToCompareToHere.includes(listingId),
);
theValue = Object.entries(someLongObjectName).filter(([listingId]) =>
someListToCompareToHere?.includes(listingId),
);
```
6 changes: 5 additions & 1 deletion src/language-js/print/call-expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ function printCallExpression(path, options, print) {
!isDynamicImport &&
!isNew &&
isMemberish(node.callee) &&
!path.call((path) => pathNeedsParens(path, options), "callee")
!path.call(
(path) => pathNeedsParens(path, options),
"callee",
...(node.callee.type === "ChainExpression" ? ["expression"] : []),
)
) {
return printMemberChain(path, options, print);
}
Expand Down
13 changes: 13 additions & 0 deletions src/language-js/print/member-chain.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ import {
// 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) {
/* c8 ignore next 6 */
if (path.node.type === "ChainExpression") {
return path.call(
() => printMemberChain(path, options, print),
"expression",
);
}

const { parent } = path;
const isExpressionStatement =
!parent || parent.type === "ExpressionStatement";
Expand Down Expand Up @@ -88,6 +96,11 @@ function printMemberChain(path, options, print) {

function rec(path) {
const { node } = path;

if (node.type === "ChainExpression") {
return path.call(() => rec(path), "expression");
}

if (
isCallExpression(node) &&
(isMemberish(node.callee) || isCallExpression(node.callee))
Expand Down
25 changes: 17 additions & 8 deletions src/language-js/print/member.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,31 @@
import { group, indent, label, softline } from "../../document/builders.js";
import {
getCallArguments,
isCallExpression,
isMemberExpression,
isNumericLiteral,
} from "../utils/index.js";
import { printOptionalToken } from "./misc.js";

const isCallExpressionWithArguments = (node) => {
if (node.type === "ChainExpression" || node.type === "TSNonNullExpression") {
node = node.expression;
}
return isCallExpression(node) && getCallArguments(node).length > 0;
};

function printMemberExpression(path, options, print) {
const objectDoc = print("object");
const lookupDoc = printMemberLookup(path, options, print);
const { node, parent } = path;
const { node } = path;
const firstNonMemberParent = path.findAncestor(
(node) =>
!(isMemberExpression(node) || node.type === "TSNonNullExpression"),
);
const firstNonChainElementWrapperParent = path.findAncestor(
(node) =>
!(node.type === "ChainExpression" || node.type === "TSNonNullExpression"),
);

const shouldInline =
(firstNonMemberParent &&
Expand All @@ -24,13 +36,10 @@ function printMemberExpression(path, options, print) {
node.computed ||
(node.object.type === "Identifier" &&
node.property.type === "Identifier" &&
!isMemberExpression(parent)) ||
((parent.type === "AssignmentExpression" ||
parent.type === "VariableDeclarator") &&
((isCallExpression(node.object) && node.object.arguments.length > 0) ||
(node.object.type === "TSNonNullExpression" &&
isCallExpression(node.object.expression) &&
node.object.expression.arguments.length > 0) ||
!isMemberExpression(firstNonChainElementWrapperParent)) ||
((firstNonChainElementWrapperParent.type === "AssignmentExpression" ||
firstNonChainElementWrapperParent.type === "VariableDeclarator") &&
(isCallExpressionWithArguments(node.object) ||
objectDoc.label?.memberChain));

return label(objectDoc.label, [
Expand Down
11 changes: 9 additions & 2 deletions src/language-js/utils/create-type-check-function.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
function createTypeCheckFunction(types) {
types = new Set(types);
/** @typedef {import("../types/estree.js").Node} Node */
/** @typedef {import("../types/estree.js").Comment} Comment */

/**
* @param {string[]} typesArray
* @returns {(node: Node | Comment) => Boolean}
*/
function createTypeCheckFunction(typesArray) {
const types = new Set(typesArray);
return (node) => types.has(node?.type);
}

Expand Down
94 changes: 37 additions & 57 deletions src/language-js/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,6 @@ const isLineComment = createTypeCheckFunction([
"InterpreterDirective",
]);

/**
* @param {Node} node
* @returns {boolean}
*/
const isExportDeclaration = createTypeCheckFunction([
"ExportDefaultDeclaration",
"DeclareExportDeclaration",
Expand All @@ -136,19 +132,11 @@ const isExportDeclaration = createTypeCheckFunction([
"DeclareExportAllDeclaration",
]);

/**
* @param {Node} node
* @returns {boolean}
*/
const isArrayOrTupleExpression = createTypeCheckFunction([
"ArrayExpression",
"TupleExpression",
]);

/**
* @param {Node} node
* @returns {boolean}
*/
const isObjectOrRecordExpression = createTypeCheckFunction([
"ObjectExpression",
"RecordExpression",
Expand Down Expand Up @@ -191,10 +179,6 @@ function isRegExpLiteral(node) {
);
}

/**
* @param {Node} node
* @returns {boolean}
*/
const isLiteral = createTypeCheckFunction([
"Literal",
"BooleanLiteral",
Expand All @@ -207,10 +191,6 @@ const isLiteral = createTypeCheckFunction([
"StringLiteral",
]);

/**
* @param {Node} node
* @returns {boolean}
*/
const isSingleWordType = createTypeCheckFunction([
"Identifier",
"ThisExpression",
Expand All @@ -220,20 +200,12 @@ const isSingleWordType = createTypeCheckFunction([
"Import",
]);

/**
* @param {Node} node
* @returns {boolean}
*/
const isObjectType = createTypeCheckFunction([
"ObjectTypeAnnotation",
"TSTypeLiteral",
"TSMappedType",
]);

/**
* @param {Node} node
* @returns {boolean}
*/
const isFunctionOrArrowExpression = createTypeCheckFunction([
"FunctionExpression",
"ArrowFunctionExpression",
Expand Down Expand Up @@ -269,10 +241,6 @@ function isAngularTestWrapper(node) {
);
}

/**
* @param {Node} node
* @returns {boolean}
*/
const isJsxElement = createTypeCheckFunction(["JSXElement", "JSXFragment"]);

function isGetterOrSetter(node) {
Expand Down Expand Up @@ -314,10 +282,6 @@ function isTypeAnnotationAFunction(node) {
);
}

/**
* @param {Node} node
* @returns {boolean}
*/
const isBinaryish = createTypeCheckFunction([
"BinaryExpression",
"LogicalExpression",
Expand Down Expand Up @@ -438,23 +402,22 @@ function isTestCall(node, parent) {
return false;
}

/**
* @param {Node} node
* @returns {boolean}
*/
const isCallExpression = createTypeCheckFunction([
"CallExpression",
"OptionalCallExpression",
]);
/** @return {(node: Node) => boolean} */
const skipChainExpression = (fn) => (node) => {
if (node?.type === "ChainExpression") {
node = node.expression;
}

/**
* @param {Node} node
* @returns {boolean}
*/
const isMemberExpression = createTypeCheckFunction([
"MemberExpression",
"OptionalMemberExpression",
]);
return fn(node);
};

const isCallExpression = skipChainExpression(
createTypeCheckFunction(["CallExpression", "OptionalCallExpression"]),
);

const isMemberExpression = skipChainExpression(
createTypeCheckFunction(["MemberExpression", "OptionalMemberExpression"]),
);

/**
*
Expand Down Expand Up @@ -718,7 +681,7 @@ function isFunctionCompositionArgs(args) {
return true;
}
} else if (isCallExpression(arg)) {
for (const childArg of arg.arguments) {
for (const childArg of getCallArguments(arg)) {
if (isFunctionOrArrowExpression(childArg)) {
return true;
}
Expand Down Expand Up @@ -1051,6 +1014,10 @@ function getCallArguments(node) {
return callArgumentsCache.get(node);
}

if (node.type === "ChainExpression") {
return getCallArguments(node.expression);
}

let args = node.arguments;
if (node.type === "ImportExpression") {
args = [node.source];
Expand All @@ -1072,6 +1039,14 @@ 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") {
path.call((sourcePath) => iteratee(sourcePath, 0), "source");

Expand All @@ -1090,17 +1065,22 @@ function iterateCallArgumentsPath(path, iteratee) {
}

function getCallArgumentSelector(node, index) {
const selectors = [];
if (node.type === "ChainExpression") {
selectors.push("expression");
}

if (node.type === "ImportExpression") {
if (index === 0 || index === (node.attributes || node.options ? -2 : -1)) {
return "source";
return [...selectors, "source"];
}
// import attributes
if (node.attributes && (index === 1 || index === -1)) {
return "attributes";
return [...selectors, "attributes"];
}
// deprecated import assertions
if (node.options && (index === 1 || index === -1)) {
return "options";
return [...selectors, "options"];
}
throw new RangeError("Invalid argument index");
}
Expand All @@ -1111,7 +1091,7 @@ function getCallArgumentSelector(node, index) {
if (index < 0 || index >= node.arguments.length) {
throw new RangeError("Invalid argument index");
}
return ["arguments", index];
return [...selectors, "arguments", index];
}

function isPrettierIgnoreComment(comment) {
Expand Down

0 comments on commit 15ccb35

Please sign in to comment.