tools: update to Eslint 3.19.0 and replace custom rule #12162

Trott · 2017-04-02T06:12:46Z

Commit 1: Update ESLint to 3.19.0.

Commit 2: ESLint 3.19.0 allows the specification of selectors that represent disallowed syntax. Replace our custom rule for timer arguments with a pair of no-restricted-syntax option objects.

Checklist

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

Affected core subsystem(s)

tools

ESLint 3.19.0 allows the specification of selectors that represent disallowed syntax. Replace our custom rule for timer arguments with a pair of `no-restricted-syntax` option objects.

not-an-aardvark

LGTM

The no-unescaped-regexp-dot custom rule does feature detection here for the sourceCode.getIndexFromLoc and sourceCode.getLocFromIndex methods, which were added to the ESLint rule API in 3.17.0. Since we've now upgraded to ESLint 3.19.0, that feature detection is probably no longer necessary.

mscdex · 2017-04-02T06:34:18Z

@not-an-aardvark Unless the eslint update gets backported to all branches, it might be best to keep it for awhile for better compatibility since the other branches seem to use different versions of eslint compared to master.

not-an-aardvark · 2017-04-02T06:50:03Z

@mscdex Seems reasonable.

If we're interested, almost all of the other custom rules can also be replaced with selectors in no-restricted-syntax:

prefer-common-mustnotcall can be replaced with a selector: CallExpression[callee.object.name="common"][callee.property.name="mustNotCall"] > Literal[value=0]
prefer-assert-methods can be replaced with a selector: ExpressionStatement > CallExpression[callee.name="assert"][arguments.0.operator="==="] (with a few variations for !==, ==, !=)
no-let-in-for-declaration can be replaced with a selector: ForStatement > VariableDeclaration[kind="let"], ForInStatement > VariableDeclaration[kind="let"], ForOfStatement > VariableDeclaration[kind="let"]
new-with-error can be replaced with a selector: ThrowStatement > CallExpression[callee.name=/Error$/]
buffer-constructor can be replaced with a selector: CallExpression[callee.name="Buffer"], NewExpression[callee.name="Buffer"]
assert-throws-arguments can be replaced with a selector: CallExpression[callee.object.name="assert"][callee.property.name="throws"][arguments.1.type="Literal"]:not([arguments.1.regex])
assert-fail-single-argument can be replaced with a selector: CallExpression[callee.object.name="assert"][callee.property.name="fail"][arguments.length=1]
require-buffer can be replaced with a no-restricted-globals configuration

From what I can tell, having these custom rules hasn't been a big maintenance burden, so there's not really any need to switch, but I thought I'd bring up the possibility.

gibfahn · 2017-04-02T13:34:24Z

From what I can tell, having these custom rules hasn't been a big maintenance burden, so there's not really any need to switch, but I thought I'd bring up the possibility.

@not-an-aardvark if we can configure built-in rules rather than creating our own, that seems like a better way to do things. Could you raise a PR so we can see the details of how it would actually work in practice?

jasnell · 2017-04-04T16:36:16Z

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

PR-URL: #12162 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

ESLint 3.19.0 allows the specification of selectors that represent disallowed syntax. Replace our custom rule for timer arguments with a pair of `no-restricted-syntax` option objects. PR-URL: #12162 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

jasnell · 2017-04-04T17:31:04Z

Landed in 3166652...f637703

PR-URL: nodejs#12162 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

italoacasas · 2017-04-10T22:07:18Z

cc @Trott

ESLint 3.19.0 allows the specification of selectors that represent disallowed syntax. Replace our custom rule for timer arguments with a pair of `no-restricted-syntax` option objects. PR-URL: nodejs#12162 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Trott · 2017-04-11T05:44:55Z

@italoacasas #12329

MylesBorins · 2017-04-18T23:36:25Z

rich same for v6

Trott · 2017-04-19T04:05:20Z

@MylesBorins #12504

ESLint 3.19.0 allows the specification of selectors that represent disallowed syntax. Replace our custom rule for timer arguments with a pair of `no-restricted-syntax` option objects. PR-URL: nodejs#12162 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: nodejs#12162 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Backport-PR-URL: #12504 PR-URL: #12162 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

ESLint 3.19.0 allows the specification of selectors that represent disallowed syntax. Replace our custom rule for timer arguments with a pair of `no-restricted-syntax` option objects. PR-URL: #12162 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Backport-PR-URL: #12504 PR-URL: #12162 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Trott · 2017-07-18T06:26:45Z

Removing backport-requested-v6.x because #12504 landed.

Trott · 2017-07-18T06:27:18Z

...and added backported-to-v6.x label.

Backport-PR-URL: nodejs/node#12504 PR-URL: nodejs/node#12162 Reviewed-By: Teddy Katz <teddy.katz@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Trott added 2 commits April 1, 2017 23:12

tools: update ESLint to 3.19.0

18e4ca2

tools: replace custom ESLint timers rule

1381b0a

ESLint 3.19.0 allows the specification of selectors that represent disallowed syntax. Replace our custom rule for timer arguments with a pair of `no-restricted-syntax` option objects.

Trott added the tools Issues and PRs related to the tools directory. label Apr 2, 2017

not-an-aardvark approved these changes Apr 2, 2017

View reviewed changes

targos approved these changes Apr 2, 2017

View reviewed changes

hiroppy approved these changes Apr 2, 2017

View reviewed changes

jasnell approved these changes Apr 3, 2017

View reviewed changes

cjihrig approved these changes Apr 3, 2017

View reviewed changes

jasnell closed this Apr 4, 2017

jasnell mentioned this pull request Apr 4, 2017

8.0.0 Release Proposal #12220

Closed

italoacasas added the backport-to-v7.x label Apr 10, 2017

Trott mentioned this pull request Apr 11, 2017

Backport f637: tools: replace custom ESLint timers rule #12329

Closed

2 tasks

addaleax added backported-to-v7.x and removed backport-requested-v7.x labels Apr 13, 2017

MylesBorins added the backport-requested-v6.x label Apr 18, 2017

Trott mentioned this pull request Apr 19, 2017

Backport 12162 to 6.x #12504

Closed

evanlucas mentioned this pull request May 1, 2017

v7.10.0 proposal #12775

Merged

MylesBorins mentioned this pull request May 23, 2017

v6.11.0 proposal #13059

Merged

Trott added backported-to-v6.x and removed backport-requested-v6.x labels Jul 18, 2017

Trott deleted the eslint-3.19.0 branch January 13, 2022 22:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools: update to Eslint 3.19.0 and replace custom rule #12162

tools: update to Eslint 3.19.0 and replace custom rule #12162

Trott commented Apr 2, 2017

not-an-aardvark left a comment

mscdex commented Apr 2, 2017 •

edited

Loading

not-an-aardvark commented Apr 2, 2017 •

edited

Loading

gibfahn commented Apr 2, 2017

jasnell commented Apr 4, 2017

jasnell commented Apr 4, 2017

italoacasas commented Apr 10, 2017

Trott commented Apr 11, 2017

MylesBorins commented Apr 18, 2017

Trott commented Apr 19, 2017 •

edited

Loading

Trott commented Jul 18, 2017

Trott commented Jul 18, 2017

tools: update to Eslint 3.19.0 and replace custom rule #12162

tools: update to Eslint 3.19.0 and replace custom rule #12162

Conversation

Trott commented Apr 2, 2017

Checklist

Affected core subsystem(s)

not-an-aardvark left a comment

Choose a reason for hiding this comment

mscdex commented Apr 2, 2017 • edited Loading

not-an-aardvark commented Apr 2, 2017 • edited Loading

gibfahn commented Apr 2, 2017

jasnell commented Apr 4, 2017

jasnell commented Apr 4, 2017

italoacasas commented Apr 10, 2017

Trott commented Apr 11, 2017

MylesBorins commented Apr 18, 2017

Trott commented Apr 19, 2017 • edited Loading

Trott commented Jul 18, 2017

Trott commented Jul 18, 2017

mscdex commented Apr 2, 2017 •

edited

Loading

not-an-aardvark commented Apr 2, 2017 •

edited

Loading

Trott commented Apr 19, 2017 •

edited

Loading