tools: add buffer-constructor eslint rule

[jasnell]

Pull Request check-list

• Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
  this change (including linting)?
• Is the commit message formatted according to [CONTRIBUTING.md][0]?
• If this change fixes a bug (or a performance problem), is a regression
  test (or a benchmark) included?
• Is a documentation update included (if this change modifies
  existing APIs, or introduces new ones)?

Affected core subsystem(s)

tools (eslint)

Description of change

Now that the Buffer.alloc, allocUnsafe, and from methods have landed,
add a linting rule that requires their use within lib. Tests and
benchmarks are explicitly excluded by the rule.

Requires: #4682

/cc @Trott @ChALkeR @nodejs/testing

[Trott]

This custom rule isn't working for me, but maybe I'm misunderstanding what it is supposed to capture. Can you provide a code sample that this should flag? I've been assuming something like this is what you're trying to avoid:

'use strict';

var foo = new Buffer(12).fill('a');
console.log(foo);

[Trott]

I don't think it's possible for node.type === which to be false in this situation. I don't think you need the which variable at all.

[jasnell]

+1

[jasnell]

@Trott .. thank you for that, I'm afraid I'm not that familiar with eslint yet so those comments were very helpful. As for the rule itself, yes, it should catch uses of new Buffer() and Buffer(), but in our source it should only apply to the lib/*.js and src/*.js directory (to catch src/nodej.s).

The rule appears to be working just fine for me on this end. If I add a var foo = new Buffer(10).fill(0) to lib/assert.js for instance, I get:

bash-3.2$ make jslint
./node tools/eslint/bin/eslint.js benchmark lib src test tools/doc \
                tools/eslint-rules --rulesdir tools/eslint-rules

/Users/james/Node/main/node/lib/assert.js
  26:5   error  'foo' is defined but never used                                                                                                no-unused-vars
  26:11  error  Use of the Buffer() constructor has been deprecated. Please use either Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from()  buffer-constructor


✖ 2 problems (2 errors, 0 warnings)

make: *** [jslint] Error 1

Changing that to Buffer.allocUnsafe() removes the buffer-constructor error.

[jasnell]

Updated to address that feedback :-)

[Trott]

LGTM

[jasnell]

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

[jasnell]

CI is green.

[cjihrig]

LGTM, but perhaps you should try getting this upstreamed to help the community.

[jasnell]

That's a possibility. I'd want to get this landed here first tho. If it did hit upstream we could revert and use that later.

[jasnell]

Opened an issue to get this upstreamed. Will land this now. If/When it lands up stream we can update to use that version

[jasnell]

Landed in 9e30129