tools: enforce consistent operator linebreak style by targos · Pull Request #10178 · nodejs/node · GitHub

Checklist

make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

lib, test, tools

Description of change

Commit 1:

lib,test: use consistent operator linebreak style

We have a tacit rule that for multiline statements, the operator should
be placed before the linebreak. This commit commit fixes the few
violations of this rule in the code base.
This allows us to enable the corresponding ESLint rule.

Commit 2:

tools: enforce consistent operator linebreak style

Adds the `operator-linebreak` rule to our ESLint config.

I disabled the check for the ternary operators because ESLint has a special case for it (rule is inverted) and it's not obvious what we want. There are about 30 violations for each case (linebreak before or after the operator).
I personally would like it to be after, like everything else.

There are about 30 violations for each case (linebreak before or after the operator).

I always put the linebreak after the ? because you miss context if you are grepping through code and the operator is on the next line.

¯\_(ツ)_/¯

LGTM

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

LGTM

I disabled the check for the ternary operators because ESLint has a special case for it (rule is inverted) and it's not obvious what we want.

Actually, ESLint doesn't have a special case for those operators, but it looks like ESLint's docs are currently a bit misleading on that point. See eslint/eslint#7726 for an explanation of the actual behavior. (tl;dr: If we just use operator-linebreak: [2, after], the rule will be enforced as expected for all operators without inverting ternary operators.)

@not-an-aardvark

If we just use operator-linebreak: [2, after], the rule will be enforced as expected for all operators without inverting ternary operators.)

I figured this out while playing with the options. What I meant is that if ESLint's default behavior is to give a special treatment to ternary operators, maybe people here feel the same about it.

LGTM if CI is happy.

We have a tacit rule that for multiline statements, the operator should be placed before the linebreak. This commit commit fixes the few violations of this rule in the code base. This allows us to enable the corresponding ESLint rule. PR-URL: nodejs#10178 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

Adds the `operator-linebreak` rule to our ESLint config. PR-URL: nodejs#10178 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

Landed in 0cd1f54...7c2dbd1

We have a tacit rule that for multiline statements, the operator should be placed before the linebreak. This commit commit fixes the few violations of this rule in the code base. This allows us to enable the corresponding ESLint rule. PR-URL: #10178 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

Adds the `operator-linebreak` rule to our ESLint config. PR-URL: #10178 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>