chore: switch from standard to eslint

[trentm]

This switches from using standard to eslint for linting/formatting.
It is currently setup to use the same "standard" rules. This change
allows us to add/try custom rules, should we want to, or eventually to
diverge on style. The config setup was helped by:
standard/standard#1016 (comment)

Checklist

• solution for it not working on 8.6
• discuss if we want to switch
• consider update to eslint@latest, eslint-config-standard@latest. I've moved this to upgrade to eslint@8 #2583 to update to eslint@7 + eslint-config-standard@16 (or eventually to eslint@8 + eslint-config-standard@17 if the latter is released by then)

[trentm]

This fails with node 8.6 at least, so we'll need to cope with that:

% make check
npm run lint

> [email protected] lint /Users/trentm/el/apm-agent-nodejs8
> eslint .

/Users/trentm/el/apm-agent-nodejs8/node_modules/eslint/lib/rules/no-octal-escape.js:41
                    /^(?:[^\\]|\\.)*?\\([0-3][0-7]{1,2}|[4-7][0-7]|0(?=[89])|[1-7])/su
                    ^

SyntaxError: Invalid regular expression flags
    at NativeCompileCache._moduleCompile (/Users/trentm/el/apm-agent-nodejs8/node_modules/v8-compile-cache/v8-compile-cache.js:240:18)
    at Module._compile (/Users/trentm/el/apm-agent-nodejs8/node_modules/v8-compile-cache/v8-compile-cache.js:184:36)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)
    at Function.Module._load (module.js:500:3)
    at Module.require (module.js:568:17)
    at require (/Users/trentm/el/apm-agent-nodejs8/node_modules/v8-compile-cache/v8-compile-cache.js:159:20)
    at no-octal-escape (/Users/trentm/el/apm-agent-nodejs8/node_modules/eslint/lib/rules/index.js:171:30)
    at Map.get (/Users/trentm/el/apm-agent-nodejs8/node_modules/eslint/lib/rules/utils/lazy-loading-rule-map.js:68:24)
npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! [email protected] lint: `eslint .`
npm ERR! Exit status 2
npm ERR!
npm ERR! Failed at the [email protected] lint script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/trentm/.npm/_logs/2022-02-17T19_16_21_702Z-debug.log
make: *** [check] Error 2

standard just "passes" but doesn't run checks for versions of node it doesn't support:

% make check
npm run lint

> [email protected] lint /Users/trentm/el/apm-agent-nodejs
> standard

standard: Node 8.10.0 or greater is required. `standard` did not run.

% echo $?
0

[ghost]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2022-02-24T22:16:46.986+0000

• Duration: 21 min 48 sec

Test stats 🧪

Test	Results
Failed	0
Passed	241793
Skipped	0
Total	241793

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• run module tests for <modules> : Run TAV tests for one or more modules, where <modules> can be either a comma separated list of modules (e.g. memcached,redis) or the string literal ALL to test all modules

• run benchmark tests : Run the benchmark test only.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[trentm]

@astorm Thoughts on doing this? No rush at all.

[astorm]

Thoughts on doing this? No rush at all.

I'm all for it.

[astorm]

Looks great and should let us evolve out coding standards as time goes on (i.e. I'll never forget a use strict again)

I'd be fine with this going in as is.

One improvement we might make is having it ignore javascript from builds. Specifically, our .gitignore includes the following lines

/test/babel/out.js
/test/types/transpile/index.js
/test/types/transpile-default/index.js

//... 

/test/babel/out.js
/test/types/transpile/index.js
/test/types/transpile-default/index.js

These files produce javascript that eslint's unhappy with. If we could get the make check/npm run lint commands to ignore these entries that'd be great. Put another way, the first time I ran the linter it complained hard about all these build files :)

[trentm]

One improvement we might make is having it ignore javascript from builds.

Hrm. I guess I don't have local builds of those files in my working copy. Did the existing npm run lint using standard also break on those generated files for you?

[trentm]

Well that is a "fun" pickle. One recommended way to do this is to use the --ignore-path ... option to eslint, so that we run eslint --ignore-path .gitignore ..

However, adding that makes eslint no longer pick up these ignore patterns from "package.json":

  "eslintIgnore": [
    "/test/sourcemaps/fixtures/lib",
    "/test/stacktraces/fixtures/dist"
  ]

so we then get linter errors on those files that are in git.

[trentm]

^ @astorm I've switched to having a .eslintrc.json file that explicitly lists the ignore patterns. Please give that a try.

[astorm]

^ @astorm I've switched to having a .eslintrc.json file that explicitly lists the ignore patterns. Please give that a try.

💥 that did it -- the 5000 or so linting errors in the built javascript are no longer reported. We appear to be only linting the files that we edit 👍