Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: enable no-unused-vars linter rule and fix issues #2828

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 2 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ rules:
# list: https://github.com/eslint/eslint/tree/master/docs/rules#variables
## disallow use of undefined variables (globals)
no-undef: 2
## disallow unused variables
no-unused-vars: [2, {vars: "all", args: "none"}]

# Custom rules in tools/eslint-rules
require-buffer: 2
Expand Down
1 change: 0 additions & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const util = require('util');
const net = require('net');
const url = require('url');
const EventEmitter = require('events').EventEmitter;
const HTTPParser = process.binding('http_parser').HTTPParser;
const assert = require('assert').ok;
const common = require('_http_common');
Expand Down
6 changes: 0 additions & 6 deletions lib/internal/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ module.exports.createInternalRepl = createRepl;
// The debounce is to guard against code pasted into the REPL.
const kDebounceHistoryMS = 15;

// XXX(chrisdickinson): hack to make sure that the internal debugger
// uses the original repl.
function replStart() {
return REPL.start.apply(REPL, arguments);
}

function createRepl(env, opts, cb) {
if (typeof opts === 'function') {
cb = opts;
Expand Down
1 change: 0 additions & 1 deletion lib/os.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict';

const binding = process.binding('os');
const util = require('util');
const internalUtil = require('internal/util');
const isWindows = process.platform === 'win32';

Expand Down
2 changes: 1 addition & 1 deletion test/addons/at-exit/test.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
'use strict';
var binding = require('./build/Release/binding');
require('./build/Release/binding');
2 changes: 1 addition & 1 deletion test/addons/repl-domain-abort/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,4 @@ var options = {
};

// Run commands from fake REPL.
var dummy = repl.start(options);
repl.start(options);
8 changes: 0 additions & 8 deletions test/debugger/test-debugger-pid.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,8 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var spawn = require('child_process').spawn;

var port = common.PORT + 1337;
var buffer = '';
var expected = [];
var scriptToDebug = common.fixturesDir + '/empty.js';

function fail() {
assert(0); // `--debug-brk script.js` should not quit
}

// connect to debug agent
var interfacer = spawn(process.execPath, ['debug', '-p', '655555']);
Expand Down
2 changes: 0 additions & 2 deletions test/debugger/test-debugger-remote.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ var common = require('../common');
var assert = require('assert');
var spawn = require('child_process').spawn;

var port = common.PORT + 1337;
var buffer = '';
var expected = [];
var scriptToDebug = common.fixturesDir + '/empty.js';

function fail() {
Expand Down
1 change: 0 additions & 1 deletion test/internet/test-dns-txt-sigsegv.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var dns = require('dns');

Expand Down
5 changes: 2 additions & 3 deletions test/internet/test-dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ var common = require('../common');
var assert = require('assert'),
dns = require('dns'),
net = require('net'),
isIP = net.isIP,
isIPv4 = net.isIPv4,
isIPv6 = net.isIPv6;
var util = require('util');
Expand Down Expand Up @@ -117,7 +116,7 @@ TEST(function test_reverse_bogus(done) {
var error;

try {
var req = dns.reverse('bogus ip', function() {
dns.reverse('bogus ip', function() {
assert.ok(false);
});
} catch (e) {
Expand Down Expand Up @@ -716,7 +715,7 @@ console.log('looking up nodejs.org...');

var cares = process.binding('cares_wrap');
var req = new cares.GetAddrInfoReqWrap();
var err = cares.getaddrinfo(req, 'nodejs.org', 4);
cares.getaddrinfo(req, 'nodejs.org', 4);

req.oncomplete = function(err, domains) {
assert.strictEqual(err, 0);
Expand Down
1 change: 0 additions & 1 deletion test/internet/test-http-dns-fail.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* should trigger the error event after each attempt.
*/

var common = require('../common');
var assert = require('assert');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requiring common actually just does things.

I suppose we don't need to assign it to a variable though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these 'things' relevant to tests that don't use the variable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. See https://github.com/nodejs/node/blob/master/test/common.js#L304-L313 among other things. Actually, all tests should require common. (Perhaps it should just be pre-loaded, not sure.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's just this leaked global check, I see two options:

  • We remove that leaked global check. These can be warned through the linter unless we do something like global.x, but that ought to be intentional.
  • We preload common.js

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's this too but I think that's it:

node/test/common.js

Lines 142 to 151 in 68dc69a

if (process.env.NODE_COMMON_PIPE) {
exports.PIPE = process.env.NODE_COMMON_PIPE;
// Remove manually, the test runner won't do it
// for us like it does for files in test/tmp.
try {
fs.unlinkSync(exports.PIPE);
} catch (e) {
// Ignore.
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the big deal with requiring common? are you trying to shave off ms?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requiring common without actually using it violates the rule I'm trying to add 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw that the rule allows exceptions per varsIgnorePattern, so that could be another option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also change var common = require('../common.js') to just require('../common.js');...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks a bit strange imho and would confuse someone who doesn't know about this 'dependency'.

var http = require('http');

Expand Down
1 change: 0 additions & 1 deletion test/internet/test-net-connect-timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// https://groups.google.com/forum/#!topic/nodejs/UE0ZbfLt6t8
// https://groups.google.com/forum/#!topic/nodejs-dev/jR7-5UDqXkw

var common = require('../common');
var net = require('net');
var assert = require('assert');

Expand Down
1 change: 0 additions & 1 deletion test/internet/test-net-connect-unref.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var net = require('net');

Expand Down
3 changes: 0 additions & 3 deletions test/message/2100bytes.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var util = require('util');

console.log([
'_______________________________________________50',
Expand Down
1 change: 0 additions & 1 deletion test/message/error_exit.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

process.on('exit', function(code) {
Expand Down
3 changes: 0 additions & 3 deletions test/message/eval_messages.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
'use strict';

var common = require('../common');
var assert = require('assert');

var spawn = require('child_process').spawn;

function run(cmd, strict, cb) {
Expand Down
2 changes: 0 additions & 2 deletions test/message/hello_world.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
'use strict';
var common = require('../common');
var assert = require('assert');

console.log('hello world');
1 change: 0 additions & 1 deletion test/message/max_tick_depth.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';
var common = require('../common');

process.maxTickDepth = 10;
var i = 20;
Expand Down
2 changes: 0 additions & 2 deletions test/message/nexttick_throw.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

process.nextTick(function() {
process.nextTick(function() {
Expand Down
2 changes: 0 additions & 2 deletions test/message/stack_overflow.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

Error.stackTraceLimit = 0;

Expand Down
3 changes: 0 additions & 3 deletions test/message/stdin_messages.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
'use strict';

var common = require('../common');
var assert = require('assert');

var spawn = require('child_process').spawn;

function run(cmd, strict, cb) {
Expand Down
2 changes: 0 additions & 2 deletions test/message/throw_custom_error.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

// custom error throwing
throw ({ name: 'MyCustomError', message: 'This is a custom message' });
2 changes: 1 addition & 1 deletion test/message/throw_custom_error.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
*test*message*throw_custom_error.js:6
*test*message*throw_custom_error.js:4
throw ({ name: 'MyCustomError', message: 'This is a custom message' });
^
MyCustomError: This is a custom message
2 changes: 0 additions & 2 deletions test/message/throw_in_line_with_tabs.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
/* eslint-disable indent */
'use strict';
var common = require('../common');
var assert = require('assert');

console.error('before');

Expand Down
2 changes: 1 addition & 1 deletion test/message/throw_in_line_with_tabs.out
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
before
*test*message*throw_in_line_with_tabs.js:10
*test*message*throw_in_line_with_tabs.js:8
throw ({ foo: 'bar' });
^
[object Object]
2 changes: 0 additions & 2 deletions test/message/throw_non_error.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

// custom error throwing
throw ({ foo: 'bar' });
2 changes: 1 addition & 1 deletion test/message/throw_non_error.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
*test*message*throw_non_error.js:6
*test*message*throw_non_error.js:4
throw ({ foo: 'bar' });
^
[object Object]
2 changes: 0 additions & 2 deletions test/message/throw_null.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
'use strict';
var common = require('../common');
var assert = require('assert');

throw null;
2 changes: 1 addition & 1 deletion test/message/throw_null.out
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

*test*message*throw_null.js:5
*test*message*throw_null.js:3
throw null;
^
null
2 changes: 0 additions & 2 deletions test/message/throw_undefined.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
'use strict';
var common = require('../common');
var assert = require('assert');

throw undefined;
2 changes: 1 addition & 1 deletion test/message/throw_undefined.out
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

*test*message*throw_undefined.js:5
*test*message*throw_undefined.js:3
throw undefined;
^
undefined
2 changes: 0 additions & 2 deletions test/message/timeout_throw.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

setTimeout(function() {
undefined_reference_error_maker;
Expand Down
2 changes: 0 additions & 2 deletions test/message/undefined_reference_in_new_context.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var vm = require('vm');

console.error('before');
Expand Down
2 changes: 0 additions & 2 deletions test/message/vm_display_runtime_error.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var vm = require('vm');

console.error('beginning');
Expand Down
2 changes: 0 additions & 2 deletions test/message/vm_display_syntax_error.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var vm = require('vm');

console.error('beginning');
Expand Down
2 changes: 0 additions & 2 deletions test/message/vm_dont_display_runtime_error.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var vm = require('vm');

console.error('beginning');
Expand Down
2 changes: 0 additions & 2 deletions test/message/vm_dont_display_syntax_error.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var vm = require('vm');

console.error('beginning');
Expand Down
2 changes: 0 additions & 2 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,6 @@ assert.throws(makeBlock(a.deepStrictEqual, new Boolean(true), {}),
function thrower(errorConstructor) {
throw new errorConstructor('test');
}
var aethrow = makeBlock(thrower, a.AssertionError);
aethrow = makeBlock(thrower, a.AssertionError);

// the basic calls work
assert.throws(makeBlock(thrower, a.AssertionError),
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-beforeexit-event.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict';
var assert = require('assert');
var net = require('net');
var util = require('util');
var common = require('../common');
var revivals = 0;
var deaths = 0;
Expand Down
2 changes: 0 additions & 2 deletions test/parallel/test-buffer-arraybuffer.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
'use strict';

const common = require('../common');
const assert = require('assert');

const Buffer = require('buffer').Buffer;
const LENGTH = 16;

Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-buffer-ascii.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

// ASCII conversion in node.js simply masks off the high bits,
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-buffer-bytelength.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict';

var common = require('../common');
var assert = require('assert');
var Buffer = require('buffer').Buffer;

Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-buffer-concat.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

var zero = [];
Expand Down
2 changes: 0 additions & 2 deletions test/parallel/test-buffer-fakes.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const Buffer = require('buffer').Buffer;
const Bp = Buffer.prototype;

function FakeBuffer() { }
FakeBuffer.__proto__ = Buffer;
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-buffer-indexof.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

var Buffer = require('buffer').Buffer;
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-buffer-inspect.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

var util = require('util');
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-buffer-iterator.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';
var common = require('../common');
var assert = require('assert');

var buffer = new Buffer([1, 2, 3, 4, 5]);
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-buffer-slow.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const buffer = require('buffer');
const Buffer = buffer.Buffer;
Expand Down
2 changes: 0 additions & 2 deletions test/parallel/test-child-process-buffering.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
var common = require('../common');
var assert = require('assert');

var spawn = require('child_process').spawn;

var pwd_called = false;
var childClosed = false;
var childExited = false;
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-child-process-constructor.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict';

var assert = require('assert');
var common = require('../common');
var child_process = require('child_process');
var ChildProcess = child_process.ChildProcess;
assert.equal(typeof ChildProcess, 'function');
Expand Down
Loading