tools: lint for function argument alignment #6390

Trott · 2016-04-26T06:45:28Z

Checklist

tests and code linting passes
the commit message follows commit guidelines

Affected core subsystem(s)

tools test

Description of change

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);

jasnell · 2016-04-26T15:34:33Z

LGTM

In preparation for a lint rule enforcing function argument alignment, adjust function arguments to be aligned.

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);

Trott · 2016-04-27T00:17:30Z

CI: https://ci.nodejs.org/job/node-test-pull-request/2404/

jbergstroem · 2016-04-27T00:40:48Z

LGTM

Trott · 2016-04-28T18:00:32Z

Only CI failure is the current problematic tick processor test on OS X. Unrelated.

Trott · 2016-04-28T18:01:28Z

/bump @nodejs/collaborators I'd like to land this, but I definitely would like a few more opinions before doing so.

mscdex · 2016-04-28T18:15:02Z

LGTM

imran-iq · 2016-04-28T18:19:46Z

LGTM

bnoordhuis · 2016-04-28T19:04:02Z

tools/eslint-rules/align-function-arguments.js

+  // 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; })) {


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

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().

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.

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?

bnoordhuis · 2016-04-28T19:04:26Z

LGTM with a suggestion.

rmg · 2016-04-28T19:29:55Z

LGTM

In preparation for a lint rule enforcing function argument alignment, adjust function arguments to be aligned. PR-URL: nodejs#6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

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); PR-URL: nodejs#6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

Trott · 2016-04-28T21:52:47Z

Landed in 8f56958 and ded3aea

If braces are not required for an arrow function body, omit them. Refs: nodejs#6390 (diff)

In preparation for a lint rule enforcing function argument alignment, adjust function arguments to be aligned. PR-URL: #6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

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); PR-URL: #6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

MylesBorins · 2016-06-01T23:48:13Z

@Trott you know the drill 📦

Trott · 2016-06-02T00:02:57Z

@thealphanerd #7100

In preparation for a lint rule enforcing function argument alignment, adjust function arguments to be aligned. PR-URL: nodejs#7100 Refs: nodejs#6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

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); PR-URL: nodejs#7100 Refs: nodejs#6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

In preparation for a lint rule enforcing function argument alignment, adjust function arguments to be aligned. PR-URL: #7100 Refs: #6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

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); PR-URL: #7100 Refs: #6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

In preparation for a lint rule enforcing function argument alignment, adjust function arguments to be aligned. PR-URL: #7100 Refs: #6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

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); PR-URL: #7100 Refs: #6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

In preparation for a lint rule enforcing function argument alignment, adjust function arguments to be aligned. PR-URL: #7100 Refs: #6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

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); PR-URL: #7100 Refs: #6390 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Imran Iqbal <imran@imraniqbal.org> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ryan Graham <r.m.graham@gmail.com>

Trott added test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Apr 26, 2016

Trott mentioned this pull request Apr 26, 2016

tools: lint for function argument alignment #6268

Closed

2 tasks

JacksonTian force-pushed the master branch from bf3ffc2 to 92a02d5 Compare April 26, 2016 06:58

Trott added 2 commits April 26, 2016 17:15

test,tools: adjust function argument alignment

0c69eb9

In preparation for a lint rule enforcing function argument alignment, adjust function arguments to be aligned.

Trott force-pushed the align-again branch from 16562e3 to 4b0c0d8 Compare April 27, 2016 00:16

bnoordhuis reviewed Apr 28, 2016
View reviewed changes

Trott closed this Apr 28, 2016

Trott added a commit to Trott/io.js that referenced this pull request Apr 28, 2016

tools: enforce arrow function brace usage linting

8f7b96d

If braces are not required for an arrow function body, omit them. Refs: nodejs#6390 (diff)

Trott mentioned this pull request Apr 28, 2016

tools: enforce arrow function brace usage linting #6455

Closed

2 tasks

This was referenced May 4, 2016

Propose v6.1.0 wrong base please ignore #6556

Closed

Propose v6.1.0 (Openssl update) #6557

Merged

MylesBorins added the dont-land-on-v4.x label Jun 1, 2016

Trott mentioned this pull request Jun 2, 2016

Backport #6390 #7100

Closed

2 tasks

Trott deleted the align-again branch January 13, 2022 22:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools: lint for function argument alignment #6390

tools: lint for function argument alignment #6390

Trott commented Apr 26, 2016

jasnell commented Apr 26, 2016

Trott commented Apr 27, 2016

jbergstroem commented Apr 27, 2016

Trott commented Apr 28, 2016

Trott commented Apr 28, 2016

mscdex commented Apr 28, 2016

imran-iq commented Apr 28, 2016

bnoordhuis Apr 28, 2016 •

edited

Loading

Trott Apr 28, 2016

Trott Apr 28, 2016

bnoordhuis Apr 28, 2016

bnoordhuis commented Apr 28, 2016

rmg commented Apr 28, 2016

Trott commented Apr 28, 2016

MylesBorins commented Jun 1, 2016

Trott commented Jun 2, 2016

tools: lint for function argument alignment #6390

tools: lint for function argument alignment #6390

Conversation

Trott commented Apr 26, 2016

Checklist

Affected core subsystem(s)

Description of change

jasnell commented Apr 26, 2016

Trott commented Apr 27, 2016

jbergstroem commented Apr 27, 2016

Trott commented Apr 28, 2016

Trott commented Apr 28, 2016

mscdex commented Apr 28, 2016

imran-iq commented Apr 28, 2016

bnoordhuis Apr 28, 2016 • edited Loading

Choose a reason for hiding this comment

Trott Apr 28, 2016

Choose a reason for hiding this comment

Trott Apr 28, 2016

Choose a reason for hiding this comment

bnoordhuis Apr 28, 2016

Choose a reason for hiding this comment

bnoordhuis commented Apr 28, 2016

rmg commented Apr 28, 2016

Trott commented Apr 28, 2016

MylesBorins commented Jun 1, 2016

Trott commented Jun 2, 2016

bnoordhuis Apr 28, 2016 •

edited

Loading