Skip to content

Commit

Permalink
Fix:newline-per-chained-call should only warn on methods (fixes #5289)
Browse files Browse the repository at this point in the history
  • Loading branch information
@BYK
BYK committed Feb 22, 2016
1 parent 237fc1e commit cd12a4b
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 116 deletions.
21 changes: 16 additions & 5 deletions docs/rules/newline-per-chained-call.md
View file
Expand Up @@ -57,7 +57,7 @@ _
.filter(bar);

// Or
obj.prop.method().prop
obj.method().method2().method3();
```

Following patterns are not considered problems with default configuration:
Expand All @@ -77,11 +77,21 @@ _
.map(foo)
.filter(bar);

// Or
_.chain({})
.map(foo)
.filter(bar);

// Or
obj
.prop
.method()
.prop
.method().prop;

// Or
obj
.prop.method()
.method2()
.method3().prop;
```

Change the option `ignoreChainWithDepth` value to allow single line chains of that depth.
Expand Down Expand Up @@ -110,12 +120,13 @@ Following patterns are considered problems:
_.chain({}).map(foo).filter(bar);

// Or
obj.prop.method().prop;
obj.prop.method().method2().method3();

// Or
obj
.prop
.method().prop;
.method()
.method2().method3();

```

Expand Down
95 changes: 19 additions & 76 deletions lib/rules/newline-per-chained-call.js
View file
@@ -1,7 +1,9 @@
/**
* @fileoverview Rule to ensure newline per method call when chaining calls
* @author Rajendra Patil
* @author Burak Yigit Kaya
* @copyright 2016 Rajendra Patil. All rights reserved.
* @copyright 2016 Burak Yigit Kaya. All rights reserved.
*/

"use strict";
Expand All @@ -13,90 +15,31 @@
module.exports = function(context) {

var options = context.options[0] || {},
codeStateMap = {},
ignoreChainWithDepth = options.ignoreChainWithDepth || 2;

/**
* Check and Capture State if the chained calls/members
* @param {ASTNode} node The node to check
* @param {object} codeState The state of the code current code to be filled
* @returns {void}
*/
function checkAndCaptureStateRecursively(node, codeState) {
var valid = false,
objectLineNumber,
propertyLineNumber;
if (node.callee) {
node = node.callee;
codeState.hasFunctionCall = true;
}

if (node.object) {
codeState.depth++;
return {
"CallExpression:exit": function(node) {
if (!node.callee || node.callee.type !== "MemberExpression") {
return;
}

objectLineNumber = node.object.loc.end.line;
propertyLineNumber = node.property.loc.end.line;
valid = node.computed || propertyLineNumber > objectLineNumber;
var callee = node.callee;
var parent = callee.object;
var depth = 1;

if (!valid) {
codeState.reports.push({
node: node,
text: "Expected line break after `{{code}}`.",
depth: codeState.depth
});
while (parent && parent.callee) {
depth += 1;
parent = parent.callee.object;
}
// Recurse
checkAndCaptureStateRecursively(node.object, codeState);
}

}
/**
* Verify and report the captured state report
* @param {object} codeState contains the captured state with `hasFunctionCall, reports and depth`
* @returns {void}
*/
function reportState(codeState) {
var report;
if (codeState.hasFunctionCall && codeState.depth > ignoreChainWithDepth && codeState.reports) {
while (codeState.reports.length) {
report = codeState.reports.shift();
context.report(report.node, report.text, {
code: context.getSourceCode().getText(report.node.object).replace(/\r\n|\r|\n/g, "\\n") // Not to print newlines in error report
});
if (depth > ignoreChainWithDepth && callee.property.loc.start.line === callee.object.loc.end.line) {
context.report(
callee.property,
callee.property.loc.start,
"Expected line break after `" + context.getSource(callee.object).replace(/\r\n|\r|\n/g, "\\n") + "`."
);
}
}
}

/**
* Initialize the node state object with default values.
* @returns {void}
*/
function initializeState() {
return {
visited: false,
hasFunctionCall: false,
depth: 1,
reports: []
};
}
/**
* Process the said node and recurse internally
* @param {ASTNode} node The node to check
* @returns {void}
*/
function processNode(node) {
var stateKey = [node.loc.start.line, node.loc.start.column].join("@"),
codeState = codeStateMap[stateKey] = (codeStateMap[stateKey] || initializeState());
if (!codeState.visited) {
codeState.visited = true;
checkAndCaptureStateRecursively(node, codeState);
}
reportState(codeState);
}

return {
"CallExpression": processNode,
"MemberExpression": processNode
};
};

Expand Down
61 changes: 26 additions & 35 deletions tests/lib/rules/newline-per-chained-call.js
View file
Expand Up @@ -27,10 +27,18 @@ ruleTester.run("newline-per-chained-call", rule, {
code: "var a = m1()\n.m2();"
}, {
code: "var a = m1();"
}, {
code: "a()\n.b().c.e.d()"
}, {
code: "a().b().c.e.d()"
}, {
code: "a.b.c.e.d()"
}, {
code: "var a = window\n.location\n.href\n.match(/(^[^#]*)/)[0];"
}, {
code: "var a = window['location']\n.href\n.match(/(^[^#]*)/)[0];"
}, {
code: "var a = window['location'].href.match(/(^[^#]*)/)[0];"
}, {
code: "var a = m1().m2.m3();",
options: [{
Expand All @@ -45,73 +53,56 @@ ruleTester.run("newline-per-chained-call", rule, {
invalid: [{
code: "_\n.chain({}).map(foo).filter(bar).value();",
errors: [{
message: "Expected line break after `_\\n.chain({}).map(foo).filter(bar)`."
}, {
message: "Expected line break after `_\\n.chain({}).map(foo)`."
}, {
message: "Expected line break after `_\\n.chain({})`."
message: "Expected line break after `_\\n.chain({}).map(foo).filter(bar)`."
}]
}, {
code: "_\n.chain({})\n.map(foo)\n.filter(bar).value();",
errors: [{
message: "Expected line break after `_\\n.chain({})\\n.map(foo)\\n.filter(bar)`."
}]
}, {
code: "a()\n.b().c.e.d()",
code: "a().b().c().e.d()",
errors: [{
message: "Expected line break after `a()\\n.b().c.e`."
}, {
message: "Expected line break after `a()\\n.b().c`."
}, {
message: "Expected line break after `a()\\n.b()`."
message: "Expected line break after `a().b()`."
}]
}, {
code: "a().b().c.e.d()",
code: "a().b().c().e.d()",
errors: [{
message: "Expected line break after `a().b().c.e`."
}, {
message: "Expected line break after `a().b().c`."
}, {
message: "Expected line break after `a().b()`."
}, {
message: "Expected line break after `a()`."
}]
}, {
code: "a.b.c.e.d()",
code: "a.b.c().e().d()",
errors: [{
message: "Expected line break after `a.b.c.e`."
}, {
message: "Expected line break after `a.b.c`."
}, {
message: "Expected line break after `a.b`."
}, {
message: "Expected line break after `a`."
message: "Expected line break after `a.b.c().e()`."
}]
}, {
code: "_.chain({}).map(a); ",
code: "_.chain({}).map(a).value(); ",
errors: [{
message: "Expected line break after `_.chain({})`."
}, {
message: "Expected line break after `_`."
message: "Expected line break after `_.chain({}).map(a)`."
}]
}, {
code: "var a = m1.m2();\n var b = m1.m2().m3().m4();",
code: "var a = m1.m2();\n var b = m1.m2().m3().m4().m5();",
errors: [{
message: "Expected line break after `m1.m2().m3()`."
}, {
message: "Expected line break after `m1.m2()`."
}, {
message: "Expected line break after `m1`."
message: "Expected line break after `m1.m2().m3().m4()`."
}]
}, {
code: "var a = m1.m2();\n var b = m1.m2().m3()\n.m4().m5();",
errors: [{
message: "Expected line break after `m1.m2().m3()\\n.m4()`."
}]
}, {
code: "var a = m1().m2\n.m3().m4();",
code: "var a = m1().m2\n.m3().m4().m5().m6().m7();",
options: [{
ignoreChainWithDepth: 3
}],
errors: [{
message: "Expected line break after `m1().m2\\n.m3()`."
message: "Expected line break after `m1().m2\\n.m3().m4().m5()`."
}, {
message: "Expected line break after `m1()`."
message: "Expected line break after `m1().m2\\n.m3().m4().m5().m6()`."
}]
}]

Expand Down

0 comments on commit cd12a4b

Please sign in to comment.