Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
tools: lint for function argument alignment
In function calls that span multiple lines, apply a custom lint rule to
enforce argument alignment.

With this rule, the following code will be flagged as an error by the
linter because the arguments on the second line start in a different
column than on the first line:

    myFunction(a, b,
      c, d);

The following code will not be flagged as an error by the linter:

    myFunction(a, b,
               c, d);
  • Loading branch information
Trott committed Apr 27, 2016
commit 4b0c0d865c4750168975544103ce4e06e947d35f
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ rules:
prefer-const: 2

# Custom rules in tools/eslint-rules
align-function-arguments: 2
align-multiline-assignment: 2
assert-fail-single-argument: 2
new-with-error: [2, "Error", "RangeError", "TypeError", "SyntaxError", "ReferenceError"]
Expand Down
70 changes: 70 additions & 0 deletions tools/eslint-rules/align-function-arguments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/**
* @fileoverview Align arguments in multiline function calls
* @author Rich Trott
*/
'use strict';

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

function checkArgumentAlignment(context, node) {

function isNodeFirstInLine(node, byEndLocation) {
const firstToken = byEndLocation === true ? context.getLastToken(node, 1) :
context.getTokenBefore(node);
const startLine = byEndLocation === true ? node.loc.end.line :
node.loc.start.line;
const endLine = firstToken ? firstToken.loc.end.line : -1;

return startLine !== endLine;
}

if (node.arguments.length === 0)
return;

var msg = '';
const first = node.arguments[0];
var currentLine = first.loc.start.line;
const firstColumn = first.loc.start.column;

const ignoreTypes = [
'ArrowFunctionExpression',
'CallExpression',
'FunctionExpression',
'ObjectExpression',
'TemplateLiteral'
];

const args = node.arguments;

// For now, don't bother trying to validate potentially complicating things
// like closures. Different people will have very different ideas and it's
// probably best to implement configuration options.
if (args.some((node) => { return ignoreTypes.indexOf(node.type) !== -1; })) {

@bnoordhuis bnoordhuis Apr 28, 2016 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (args.some((node) => ignoreTypes.includes(node.type))) {?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least for the moment, that will break on CI. For faster results, the VM that runs the linter uses the system Node.js (currently 5.x) and doesn't compile from source. 5.x does not have Array.prototype.includes().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I could get rid of the braces and return keyword... is "no braces for arrow functions where they can be omitted" a style we want to standardize on? ESLint has a built-in rule for that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the short form because why many words when can terse, right? @jbergstroem Are there any downsides to upgrading the node that runs the linter to v6?

return;
}

if (!isNodeFirstInLine(node)) {
return;
}

args.slice(1).forEach((argument) => {
if (argument.loc.start.line === currentLine + 1) {
if (argument.loc.start.column !== firstColumn) {
msg = 'Function called with argument in column ' +
`${argument.loc.start.column}, expected in ${firstColumn}`;
}
}
currentLine = argument.loc.start.line;
});

if (msg)
context.report(node, msg);
}

module.exports = function(context) {
return {
'CallExpression': (node) => checkArgumentAlignment(context, node)
};
};