Skip to content

Commit b2aedfe

Browse files
committed
New: Rule to enforce newline after each call in the chain (fixes #4538)
Added new rule (`newline-per-chained-call`) to enforce each call on a new line when chaining the calls.
1 parent 6433b7b commit b2aedfe

File tree

5 files changed

+356
-1
lines changed

5 files changed

+356
-1
lines changed

conf/eslint.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@
164164
"new-cap": 0,
165165
"new-parens": 0,
166166
"newline-after-var": 0,
167-
"object-curly-spacing": 0,
167+
"newline-per-chained-call": 0,
168+
"object-curly-spacing": [0, "never"],
168169
"object-shorthand": 0,
169170
"one-var": 0,
170171
"one-var-declaration-per-line": 0,

docs/rules/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ These rules are purely matters of style and are quite subjective.
172172
* [new-cap](new-cap.md) - require a capital letter for constructors
173173
* [new-parens](new-parens.md) - disallow the omission of parentheses when invoking a constructor with no arguments
174174
* [newline-after-var](newline-after-var.md) - require or disallow an empty newline after variable declarations
175+
* [newline-per-chained-call](newline-per-chained-call.md) - enforce newline after each call when chaining the calls
175176
* [no-array-constructor](no-array-constructor.md) - disallow use of the `Array` constructor
176177
* [no-bitwise](no-bitwise.md) - disallow use of bitwise operators
177178
* [no-continue](no-continue.md) - disallow use of the `continue` statement
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
# Newline Per Chained Method Call (newline-per-chained-call)
2+
3+
Chained method calls on a single line without line breaks are harder to read. This rule enforces new line after each method call in the chain to make it more readable and easy to maintain.
4+
5+
Let's look at the following perfectly valid (but single line) code.
6+
7+
```js
8+
d3.select('body').selectAll('p').data([4, 8, 15, 16, 23, 42 ]).enter().append('p').text(function(d) { return "I'm number " + d + "!"; });
9+
```
10+
11+
However, with appropriate new lines, it becomes easy to read and understand. Look at the same code written below with line breaks after each call.
12+
13+
```js
14+
d3
15+
.select('body')
16+
.selectAll('p')
17+
.data([
18+
4,
19+
8,
20+
15,
21+
16,
22+
23,
23+
42
24+
])
25+
.enter()
26+
.append('p')
27+
.text(function (d) {
28+
return "I'm number " + d + "!";
29+
});
30+
```
31+
32+
This rule reports such code and encourages new lines after each call in the chain as a good practice.
33+
34+
## Rule Details
35+
36+
This rule checks and reports the chained calls if there are no new lines after each call or deep member access.
37+
38+
### Options
39+
40+
The rule takes a single option `ignoreChainWithDepth`. The level/depth to be allowed is configurable through `ignoreChainWithDepth` option. This rule, in its default state, allows 2 levels.
41+
42+
* `ignoreChainWithDepth` Number of depths to be allowed (Default: `2`).
43+
44+
### Usage
45+
46+
Following patterns are considered problems with default configuration:
47+
48+
```js
49+
/*eslint newline-per-chained-call: 2*/
50+
51+
_.chain({}).map(foo).filter(bar).value();
52+
53+
// Or
54+
_.chain({}).map(foo).filter(bar);
55+
56+
// Or
57+
_
58+
.chain({}).map(foo)
59+
.filter(bar);
60+
61+
// Or
62+
obj.prop.method().prop
63+
```
64+
65+
Following patterns are not considered problems with default configuration:
66+
67+
```js
68+
/*eslint newline-per-chained-call: [2]*/
69+
70+
_
71+
.chain({})
72+
.map(foo)
73+
.filter(bar)
74+
.value();
75+
76+
// Or
77+
_
78+
.chain({})
79+
.map(foo)
80+
.filter(bar);
81+
82+
// Or
83+
obj
84+
.prop
85+
.method()
86+
.prop
87+
```
88+
89+
Change the option `ignoreChainWithDepth` value to allow single line chains of that depth.
90+
91+
For example, when configuration is like this:
92+
93+
```js
94+
{
95+
"newline-per-chained-call": [2, {"ignoreChainWithDepth": 3}]
96+
}
97+
```
98+
99+
Following patterns are not considered problems:
100+
101+
```js
102+
_.chain({}).map(foo);
103+
104+
// Or
105+
obj.prop.method();
106+
107+
```
108+
109+
Following patterns are considered problems:
110+
111+
```js
112+
_.chain({}).map(foo).filter(bar);
113+
114+
// Or
115+
obj.prop.method().prop;
116+
117+
// Or
118+
obj
119+
.prop
120+
.method().prop;
121+
122+
```
123+
124+
## When Not To Use It
125+
126+
If you have conflicting rules or when you are fine with chained calls on one line, you can safely turn this rule off.

lib/rules/newline-per-chained-call.js

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/**
2+
* @fileoverview Rule to ensure newline per method call when chaining calls
3+
* @author Rajendra Patil
4+
* @copyright 2016 Rajendra Patil. All rights reserved.
5+
*/
6+
7+
"use strict";
8+
9+
//------------------------------------------------------------------------------
10+
// Rule Definition
11+
//------------------------------------------------------------------------------
12+
13+
module.exports = function(context) {
14+
15+
var options = context.options[0] || {},
16+
codeStateMap = {},
17+
ignoreChainWithDepth = options.ignoreChainWithDepth || 2;
18+
19+
/**
20+
* Check and Capture State if the chained calls/memebers
21+
* @param {ASTNode} node The node to check
22+
* @param {object} codeState The state of the code current code to be filled
23+
* @returns {void}
24+
*/
25+
function checkAndCaptureStateRecursively(node, codeState) {
26+
var valid = false,
27+
objectLineNumber,
28+
propertyLineNumber;
29+
if (node.callee) {
30+
node = node.callee;
31+
codeState.hasFunctionCall = true;
32+
}
33+
34+
if (node.object) {
35+
codeState.depth++;
36+
37+
objectLineNumber = node.object.loc.end.line;
38+
propertyLineNumber = node.property.loc.end.line;
39+
valid = propertyLineNumber > objectLineNumber;
40+
41+
if (!valid) {
42+
codeState.reports.push({
43+
node: node,
44+
text: "Expected line break after `{{code}}`.",
45+
depth: codeState.depth
46+
});
47+
}
48+
// Recurse
49+
checkAndCaptureStateRecursively(node.object, codeState);
50+
}
51+
52+
}
53+
/**
54+
* Verify and report the captured state report
55+
* @param {object} codeState contains the captured state with `hasFunctionCall, reports and depth`
56+
* @returns {void}
57+
*/
58+
function reportState(codeState) {
59+
var report;
60+
if (codeState.hasFunctionCall && codeState.depth > ignoreChainWithDepth && codeState.reports) {
61+
while (codeState.reports.length) {
62+
report = codeState.reports.shift();
63+
context.report(report.node, report.text, {
64+
code: context.getSourceCode().getText(report.node.object).replace(/\r\n|\r|\n/g, "\\n") // Not to print newlines in error report
65+
});
66+
}
67+
}
68+
}
69+
70+
/**
71+
* Initialize the node state object with default values.
72+
* @returns {void}
73+
*/
74+
function initializeState() {
75+
return {
76+
visited: false,
77+
hasFunctionCall: false,
78+
depth: 1,
79+
reports: []
80+
};
81+
}
82+
/**
83+
* Process the said node and recuse internally
84+
* @param {ASTNode} node The node to check
85+
* @returns {void}
86+
*/
87+
function processNode(node) {
88+
var stateKey = [node.loc.start.line, node.loc.start.column].join("@"),
89+
codeState = codeStateMap[stateKey] = (codeStateMap[stateKey] || initializeState());
90+
if (!codeState.visited) {
91+
codeState.visited = true;
92+
checkAndCaptureStateRecursively(node, codeState);
93+
}
94+
reportState(codeState);
95+
}
96+
97+
return {
98+
"CallExpression": processNode,
99+
"MemberExpression": processNode
100+
};
101+
};
102+
103+
module.exports.schema = [{
104+
"type": "object",
105+
"properties": {
106+
"ignoreChainWithDepth": {
107+
"type": "integer",
108+
"minimum": 1,
109+
"maximum": 10
110+
}
111+
},
112+
"additionalProperties": false
113+
}];
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/**
2+
* @fileoverview Tests for newline-per-chained-call rule.
3+
* @author Rajendra Patil
4+
* @copyright 2016 Rajendra Patil. All rights reserved.
5+
*/
6+
7+
"use strict";
8+
9+
//------------------------------------------------------------------------------
10+
// Requirements
11+
//------------------------------------------------------------------------------
12+
13+
var rule = require("../../../lib/rules/newline-per-chained-call"),
14+
RuleTester = require("../../../lib/testers/rule-tester");
15+
16+
var ruleTester = new RuleTester();
17+
ruleTester.run("newline-per-chained-call", rule, {
18+
valid: [{
19+
code: "_\n.chain({})\n.map(foo)\n.filter(bar)\n.value();"
20+
}, {
21+
code: "a.b.c.d.e.f"
22+
}, {
23+
code: "a()\n.b()\n.c\n.e"
24+
}, {
25+
code: "var a = m1.m2(); var b = m1.m2();\nvar c = m1.m2()"
26+
}, {
27+
code: "var a = m1()\n.m2();"
28+
}, {
29+
code: "var a = m1();"
30+
}, {
31+
code: "var a = m1().m2.m3();",
32+
options: [{
33+
ignoreChainWithDepth: 3
34+
}]
35+
}, {
36+
code: "var a = m1().m2.m3().m4.m5().m6.m7().m8;",
37+
options: [{
38+
ignoreChainWithDepth: 8
39+
}]
40+
}],
41+
invalid: [{
42+
code: "_\n.chain({}).map(foo).filter(bar).value();",
43+
errors: [{
44+
message: "Expected line break after `_\\n.chain({}).map(foo).filter(bar)`."
45+
}, {
46+
message: "Expected line break after `_\\n.chain({}).map(foo)`."
47+
}, {
48+
message: "Expected line break after `_\\n.chain({})`."
49+
}]
50+
}, {
51+
code: "_\n.chain({})\n.map(foo)\n.filter(bar).value();",
52+
errors: [{
53+
message: "Expected line break after `_\\n.chain({})\\n.map(foo)\\n.filter(bar)`."
54+
}]
55+
}, {
56+
code: "a()\n.b().c.e.d()",
57+
errors: [{
58+
message: "Expected line break after `a()\\n.b().c.e`."
59+
}, {
60+
message: "Expected line break after `a()\\n.b().c`."
61+
}, {
62+
message: "Expected line break after `a()\\n.b()`."
63+
}]
64+
}, {
65+
code: "a().b().c.e.d()",
66+
errors: [{
67+
message: "Expected line break after `a().b().c.e`."
68+
}, {
69+
message: "Expected line break after `a().b().c`."
70+
}, {
71+
message: "Expected line break after `a().b()`."
72+
}, {
73+
message: "Expected line break after `a()`."
74+
}]
75+
}, {
76+
code: "a.b.c.e.d()",
77+
errors: [{
78+
message: "Expected line break after `a.b.c.e`."
79+
}, {
80+
message: "Expected line break after `a.b.c`."
81+
}, {
82+
message: "Expected line break after `a.b`."
83+
}, {
84+
message: "Expected line break after `a`."
85+
}]
86+
}, {
87+
code: "_.chain({}).map(a); ",
88+
errors: [{
89+
message: "Expected line break after `_.chain({})`."
90+
}, {
91+
message: "Expected line break after `_`."
92+
}]
93+
}, {
94+
code: "var a = m1.m2();\n var b = m1.m2().m3().m4();",
95+
errors: [{
96+
message: "Expected line break after `m1.m2().m3()`."
97+
}, {
98+
message: "Expected line break after `m1.m2()`."
99+
}, {
100+
message: "Expected line break after `m1`."
101+
}]
102+
}, {
103+
code: "var a = m1().m2\n.m3().m4();",
104+
options: [{
105+
ignoreChainWithDepth: 3
106+
}],
107+
errors: [{
108+
message: "Expected line break after `m1().m2\\n.m3()`."
109+
}, {
110+
message: "Expected line break after `m1()`."
111+
}]
112+
}]
113+
114+
});

0 commit comments

Comments
 (0)