vitalif-js
/
prettier
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Javascript: Use function literals in arguments to detect function composition (#6033)

master
Brian Kim 2019-10-01 06:53:45 -04:00 committed by Evilebot Tnawi
parent b4e86b6b9a
commit 38ae5d5210
8 changed files with 172 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
CHANGELOG.unreleased.md
View File

@ -284,6 +284,57 @@ foo;
);
```
### Javascript: Use function literals in arguments to detect function composition ([#6033] by [@brainkim])
Previously, we used a set of hard-coded names related to functional programming
(`compose`, `flow`, `pipe`, etc.) to detect function composition and chaining
patterns in code. This was done so that prettier would not put code like the
following call to `pipe` on the same line even if it fit within the allotted
column budget:
<!-- prettier-ignore -->
```js
source$
.pipe(
filter(x => x % 2 === 0),
map(x => x + x),
scan((acc, x) => acc + x, 0),
)
.subscribe(x => console.log(x));
```
However, this heuristic caused people to complain because of false positives
where calls to functions or methods matching the hard-coded names would always
be split on multiple lines, even if the calls did not contain function
arguments (https://github.com/prettier/prettier/issues/5769,
https://github.com/prettier/prettier/issues/5969). For many, this blanket
decision to split functions based on name was both surprising and sub-optimal.
We now use a refined heuristic which uses the presence of function literals to
detect function composition. This heuristic preserves the line-splitting
behavior above and eliminates many if not all of the false positives caused by
the older heuristic.
We encourage prettier users to try out the new heuristic and provide feedback.
<!-- prettier-ignore -->
```js
// Input
eventStore.update(id, _.flow(updater, incrementVersion));
// Output (Prettier stable)
eventStore.update(
id,
_.flow(
updater,
incrementVersion
)
);
// Output (Prettier master)
eventStore.update(id, _.flow(updater, incrementVersion));
```
### Handlebars: Improve comment formatting ([#6234] by [@gavinjoyce])
Previously, Prettier would incorrectly decode HTML entiites.
@ -696,6 +747,7 @@ Previously, the flag was not applied on html attributes.
````
[#5910]: https://github.com/prettier/prettier/pull/5910
[#6033]: https://github.com/prettier/prettier/pull/6033
[#6186]: https://github.com/prettier/prettier/pull/6186
[#6206]: https://github.com/prettier/prettier/pull/6206
[#6209]: https://github.com/prettier/prettier/pull/6209
@ -704,10 +756,10 @@ Previously, the flag was not applied on html attributes.
[#6236]: https://github.com/prettier/prettier/pull/6236
[#6270]: https://github.com/prettier/prettier/pull/6270
[#6289]: https://github.com/prettier/prettier/pull/6289
[#6332]: https://github.com/prettier/prettier/pull/6332
[#6284]: https://github.com/prettier/prettier/pull/6284
[#6301]: https://github.com/prettier/prettier/pull/6301
[#6307]: https://github.com/prettier/prettier/pull/6307
[#6332]: https://github.com/prettier/prettier/pull/6332
[#6340]: https://github.com/prettier/prettier/pull/6340
[#6412]: https://github.com/prettier/prettier/pull/6412
[#6423]: https://github.com/prettier/prettier/pull/6423
@ -720,6 +772,7 @@ Previously, the flag was not applied on html attributes.
[#6514]: https://github.com/prettier/prettier/pull/6514
[#6467]: https://github.com/prettier/prettier/pull/6467
[#6377]: https://github.com/prettier/prettier/pull/6377
[@brainkim]: https://github.com/brainkim
[@duailibe]: https://github.com/duailibe
[@gavinjoyce]: https://github.com/gavinjoyce
[@sosukesuzuki]: https://github.com/sosukesuzuki

119
src/language-js/printer-estree.js
View File

@ -1210,7 +1210,7 @@ function printPathNoParens(path, options, print, args) {
return printMemberChain(path, options, print);
}
return concat([
const contents = concat([
isNew ? "new " : "",
path.call(print, "callee"),
optional,
@ -1220,6 +1220,14 @@ function printPathNoParens(path, options, print, args) {
printFunctionTypeParameters(path, options, print),
printArgumentsList(path, options, print)
]);
// We group here when the callee is itself a call expression.
// See `isLongCurriedCallExpression` for more info.
if (isCallOrOptionalCallExpression(n.callee)) {
return group(contents);
}
return contents;
}
case "TSInterfaceDeclaration":
if (isNodeStartingWithDeclare(n, options)) {
@ -4006,40 +4014,47 @@ function isSimpleTemplateLiteral(node) {
});
}
const functionCompositionFunctionNames = new Set([
"pipe", // RxJS, Ramda
"pipeP", // Ramda
"pipeK", // Ramda
"compose", // Ramda, Redux
"composeFlipped", // Not from any library, but common in Haskell, so supported
"composeP", // Ramda
"composeK", // Ramda
"flow", // Lodash
"flowRight", // Lodash
"connect", // Redux
"createSelector" // Reselect
]);
const ordinaryMethodNames = new Set([
"connect" // GObject, MongoDB
]);
function isFunctionCompositionFunction(node) {
switch (node.type) {
case "OptionalMemberExpression":
case "MemberExpression": {
return (
isFunctionCompositionFunction(node.property) &&
!ordinaryMethodNames.has(node.property.name)
);
}
case "Identifier": {
return functionCompositionFunctionNames.has(node.name);
}
case "StringLiteral":
case "Literal": {
return functionCompositionFunctionNames.has(node.value);
// Logic to check for args with multiple anonymous functions. For instance,
// the following call should be split on multiple lines for readability:
// source.pipe(map((x) => x + x), filter((x) => x % 2 === 0))
function isFunctionCompositionArgs(args) {
if (args.length <= 1) {
return false;
}
let count = 0;
for (const arg of args) {
if (isFunctionOrArrowExpression(arg)) {
count += 1;
if (count > 1) {
return true;
}
} else if (isCallOrOptionalCallExpression(arg)) {
for (const childArg of arg.arguments) {
if (isFunctionOrArrowExpression(childArg)) {
return true;
}
}
}
}
return false;
}
// Logic to determine if a call is a “long curried function call”.
// See https://github.com/prettier/prettier/issues/1420.
//
// `connect(a, b, c)(d)`
// In the above call expression, the second call is the parent node and the
// first call is the current node.
function isLongCurriedCallExpression(path) {
const node = path.getValue();
const parent = path.getParentNode();
return (
isCallOrOptionalCallExpression(node) &&
isCallOrOptionalCallExpression(parent) &&
parent.callee === node &&
node.arguments.length > parent.arguments.length &&
parent.arguments.length > 0
);
}
function printArgumentsList(path, options, print) {
@ -4143,14 +4158,7 @@ function printArgumentsList(path, options, print) {
);
}
// We want to get
// pipe(
// x => x + 1,
// x => x - 1
// )
// here, but not
// process.stdout.pipe(socket)
if (isFunctionCompositionFunction(node.callee) && args.length > 1) {
if (isFunctionCompositionArgs(args)) {
return allArgsBrokenOut();
}
@ -4217,16 +4225,22 @@ function printArgumentsList(path, options, print) {
]);
}
return group(
concat([
"(",
indent(concat([softline, concat(printedArguments)])),
ifBreak(maybeTrailingComma),
softline,
")"
]),
{ shouldBreak: printedArguments.some(willBreak) || anyArgEmptyLine }
);
const contents = concat([
"(",
indent(concat([softline, concat(printedArguments)])),
ifBreak(maybeTrailingComma),
softline,
")"
]);
if (isLongCurriedCallExpression(path)) {
// By not wrapping the arguments in a group, the printer prioritizes
// breaking up these arguments rather than the args of the parent call.
return contents;
}
return group(contents, {
shouldBreak: printedArguments.some(willBreak) || anyArgEmptyLine
});
}
function printTypeAnnotation(path, options, print) {
@ -5199,6 +5213,9 @@ function printMemberChain(path, options, print) {
// If we only have a single `.`, we shouldn't do anything fancy and just
// render everything concatenated together.
if (groups.length <= cutoff && !hasComment) {
if (isLongCurriedCallExpression(path)) {
return oneLine;
}
return group(oneLine);
}

10
tests/arrows/__snapshots__/jsfmt.spec.js.snap
View File

@ -1084,7 +1084,10 @@ foo(c, a => b, d)
foo(a => b, d)
=====================================output=====================================
promise.then(result => result, err => err);
promise.then(
result => result,
err => err
);
promise.then(
result => {
@ -1132,7 +1135,10 @@ foo(c, a => b, d)
foo(a => b, d)
=====================================output=====================================
promise.then((result) => result, (err) => err);
promise.then(
(result) => result,
(err) => err
);
promise.then(
(result) => {

5
tests/decorators-ts/__snapshots__/jsfmt.spec.js.snap
View File

@ -321,7 +321,10 @@ export class Board {
@Column()
description: string;
@OneToMany(type => Topic, topic => topic.board)
@OneToMany(
type => Topic,
topic => topic.board
)
topics: Topic[];
}

5
tests/decorators/__snapshots__/jsfmt.spec.js.snap
View File

@ -344,10 +344,7 @@ export class MyApp extends React.Component {}
export class Home extends React.Component {}
=====================================output=====================================
@connect(
mapStateToProps,
mapDispatchToProps
)
@connect(mapStateToProps, mapDispatchToProps)
export class MyApp extends React.Component {}
@connect(state => ({ todos: state.todos }))

77
tests/functional_composition/__snapshots__/jsfmt.spec.js.snap
View File

@ -87,12 +87,7 @@ this.a.b.c.compose(
sortBy(x => x),
flatten
);
someObj.someMethod(
this.field.compose(
a,
b
)
);
someObj.someMethod(this.field.compose(a, b));
class A extends B {
compose() {
@ -105,10 +100,7 @@ class A extends B {
this.subscriptions.add(
this.componentUpdates
.pipe(
startWith(this.props),
distinctUntilChanged(isEqual)
)
.pipe(startWith(this.props), distinctUntilChanged(isEqual))
.subscribe(props => {})
);
@ -311,30 +303,25 @@ var followersForUser = R.composeP(lookupFollowers, lookupUser);
followersForUser("JOE").then(followers => console.log("Followers:", followers));
// Followers: ["STEVE","SUZY"]
const mapStateToProps = state => ({
users: R.compose( R.filter(R.propEq('status', 'active')),
R.values)(state.users)
});
=====================================output=====================================
var classyGreeting = (firstName, lastName) =>
"The name's " + lastName + ", " + firstName + " " + lastName;
var yellGreeting = R.compose(
R.toUpper,
classyGreeting
);
var yellGreeting = R.compose(R.toUpper, classyGreeting);
yellGreeting("James", "Bond"); //=> "THE NAME'S BOND, JAMES BOND"
R.compose(
Math.abs,
R.add(1),
R.multiply(2)
)(-4); //=> 7
R.compose(Math.abs, R.add(1), R.multiply(2))(-4); //=> 7
// get :: String -> Object -> Maybe *
var get = R.curry((propName, obj) => Maybe(obj[propName]));
// getStateCode :: Maybe String -> Maybe String
var getStateCode = R.composeK(
R.compose(
Maybe.of,
R.toUpper
),
R.compose(Maybe.of, R.toUpper),
get("state"),
get("address"),
get("user")
@ -357,13 +344,17 @@ var lookupFollowers = user => Promise.resolve(user.followers);
lookupUser("JOE").then(lookupFollowers);
// followersForUser :: String -> Promise [UserId]
var followersForUser = R.composeP(
lookupFollowers,
lookupUser
);
var followersForUser = R.composeP(lookupFollowers, lookupUser);
followersForUser("JOE").then(followers => console.log("Followers:", followers));
// Followers: ["STEVE","SUZY"]
const mapStateToProps = state => ({
users: R.compose(
R.filter(R.propEq("status", "active")),
R.values
)(state.users)
});
================================================================================
`;
@ -398,11 +389,7 @@ getStateCode("[Invalid JSON]");
var followersForUser = R.pipeP(db.getUserById, db.getFollowers);
=====================================output=====================================
var f = R.pipe(
Math.pow,
R.negate,
R.inc
);
var f = R.pipe(Math.pow, R.negate, R.inc);
f(3, 4); // -(3^4) + 1
@ -415,10 +402,7 @@ var getStateCode = R.pipeK(
get("user"),
get("address"),
get("state"),
R.compose(
Maybe.of,
R.toUpper
)
R.compose(Maybe.of, R.toUpper)
);
getStateCode('{"user":{"address":{"state":"ny"}}}');
@ -427,10 +411,7 @@ getStateCode("[Invalid JSON]");
//=> Nothing()
// followersForUser :: String -> Promise [User]
var followersForUser = R.pipeP(
db.getUserById,
db.getFollowers
);
var followersForUser = R.pipeP(db.getUserById, db.getFollowers);
================================================================================
`;
@ -463,10 +444,7 @@ import reducer from "../reducers";
const store = createStore(
reducer,
compose(
applyMiddleware(thunk),
DevTools.instrument()
)
compose(applyMiddleware(thunk), DevTools.instrument())
);
================================================================================
@ -512,15 +490,12 @@ const resolve = createSelector(
=====================================output=====================================
import { createSelector } from "reselect";
const resolve = createSelector(
getIds,
getObjects,
(ids, objects) => ids.map(id => objects[id])
const resolve = createSelector(getIds, getObjects, (ids, objects) =>
ids.map(id => objects[id])
);
const resolve = createSelector(
[getIds, getObjects],
(ids, objects) => ids.map(id => objects[id])
const resolve = createSelector([getIds, getObjects], (ids, objects) =>
ids.map(id => objects[id])
);
================================================================================

5
tests/functional_composition/ramda_compose.js
View File

@ -36,3 +36,8 @@ lookupUser("JOE").then(lookupFollowers);
var followersForUser = R.composeP(lookupFollowers, lookupUser);
followersForUser("JOE").then(followers => console.log("Followers:", followers));
// Followers: ["STEVE","SUZY"]
const mapStateToProps = state => ({
users: R.compose( R.filter(R.propEq('status', 'active')),
R.values)(state.users)
});

7
tests/typescript_arrow/__snapshots__/jsfmt.spec.js.snap
View File

@ -27,7 +27,12 @@ const bar = (...varargs: any[]) => {
console.log(varargs);
};
const foo = (x: string): void => bar(x, () => {}, () => {});
const foo = (x: string): void =>
bar(
x,
() => {},
() => {}
);
app.get("/", (req, res): void => {
res.send("Hello world");