Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inconsistencies for ChainElements #15806

Merged
merged 16 commits into from
Jan 3, 2024
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 @@
"InterpreterDirective",
]);

/**
* @param {Node} node
* @returns {boolean}
*/
const isExportDeclaration = createTypeCheckFunction([
"ExportDefaultDeclaration",
"DeclareExportDeclaration",
Expand All @@ -136,19 +132,11 @@
"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 @@
);
}

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

/**
* @param {Node} node
* @returns {boolean}
*/
const isSingleWordType = createTypeCheckFunction([
"Identifier",
"ThisExpression",
Expand All @@ -220,20 +200,12 @@
"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 @@
);
}

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

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

/**
* @param {Node} node
* @returns {boolean}
*/
const isBinaryish = createTypeCheckFunction([
"BinaryExpression",
"LogicalExpression",
Expand Down Expand Up @@ -438,23 +402,22 @@
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 @@ -717,7 +680,7 @@
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 @@ -1050,6 +1013,10 @@
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 @@ -1071,6 +1038,14 @@

function iterateCallArgumentsPath(path, iteratee) {
const { node } = path;

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

Check warning on line 1047 in src/language-js/utils/index.js

View check run for this annotation

Codecov / codecov/patch

src/language-js/utils/index.js#L1043-L1047

Added lines #L1043 - L1047 were not covered by tests

if (node.type === "ImportExpression") {
path.call((sourcePath) => iteratee(sourcePath, 0), "source");

Expand All @@ -1089,17 +1064,22 @@
}

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

Check warning on line 1070 in src/language-js/utils/index.js

View check run for this annotation

Codecov / codecov/patch

src/language-js/utils/index.js#L1069-L1070

Added lines #L1069 - L1070 were not covered by tests

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 @@ -1110,7 +1090,7 @@
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