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,test: enforce assert.ifError with lint rule #10671

Closed
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
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions test/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
rules:
## common module is mandatory in tests
required-modules: [2, common]
prefer-assert-iferror: 2
prefer-assert-methods: 2
2 changes: 1 addition & 1 deletion test/parallel/test-crypto-pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ assert.strictEqual(key.toString('hex'), expected);

crypto.pbkdf2('password', 'salt', 32, 32, 'sha256', common.mustCall(ondone));
function ondone(err, key) {
if (err) throw err;
assert.ifError(err);
assert.strictEqual(key.toString('hex'), expected);
}

Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-fs-long-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
const common = require('../common');
const fs = require('fs');
const path = require('path');
const assert = require('assert');

if (!common.isWindows) {
common.skip('this test is Windows-specific.');
Expand All @@ -21,10 +22,10 @@ console.log({
});

fs.writeFile(fullPath, 'ok', common.mustCall(function(err) {
if (err) throw err;
assert.ifError(err);

fs.stat(fullPath, common.mustCall(function(err, stats) {
if (err) throw err;
assert.ifError(err);
}));
}));

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-readfile-pipe.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const dataExpected = fs.readFileSync(__filename, 'utf8');

if (process.argv[2] === 'child') {
fs.readFile('/dev/stdin', function(er, data) {
if (er) throw er;
assert.ifError(er);
process.stdout.write(data);
});
return;
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-symlink-dir-junction-relative.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ common.refreshTmpDir();

// Test fs.symlink()
fs.symlink(linkData, linkPath1, 'junction', common.mustCall(function(err) {
if (err) throw err;
assert.ifError(err);
verifyLink(linkPath1);
}));

Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-fs-symlink-dir-junction.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ console.log('linkData: ' + linkData);
console.log('linkPath: ' + linkPath);

fs.symlink(linkData, linkPath, 'junction', common.mustCall(function(err) {
if (err) throw err;
assert.ifError(err);

fs.lstat(linkPath, common.mustCall(function(err, stats) {
if (err) throw err;
assert.ifError(err);
assert.ok(stats.isSymbolicLink());

fs.readlink(linkPath, common.mustCall(function(err, destination) {
if (err) throw err;
assert.ifError(err);
assert.strictEqual(destination, linkData);

fs.unlink(linkPath, common.mustCall(function(err) {
if (err) throw err;
assert.ifError(err);
assert(!common.fileExists(linkPath));
assert(common.fileExists(linkData));
}));
Expand Down
12 changes: 6 additions & 6 deletions test/parallel/test-preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ const fixtureThrows = fixture('throws_error4.js');
// test preloading a single module works
childProcess.exec(nodeBinary + ' ' + preloadOption([fixtureA]) + ' ' + fixtureB,
function(err, stdout, stderr) {
if (err) throw err;
assert.ifError(err);
assert.strictEqual(stdout, 'A\nB\n');
});

// test preloading multiple modules works
childProcess.exec(
nodeBinary + ' ' + preloadOption([fixtureA, fixtureB]) + ' ' + fixtureC,
function(err, stdout, stderr) {
if (err) throw err;
assert.ifError(err);
assert.strictEqual(stdout, 'A\nB\nC\n');
}
);
Expand All @@ -63,7 +63,7 @@ childProcess.exec(
childProcess.exec(
nodeBinary + ' ' + preloadOption([fixtureA]) + '-e "console.log(\'hello\');"',
function(err, stdout, stderr) {
if (err) throw err;
assert.ifError(err);
assert.strictEqual(stdout, 'A\nhello\n');
}
);
Expand Down Expand Up @@ -110,7 +110,7 @@ childProcess.exec(
nodeBinary + ' ' + preloadOption([fixtureA]) +
'-e "console.log(\'hello\');" ' + preloadOption([fixtureA, fixtureB]),
function(err, stdout, stderr) {
if (err) throw err;
assert.ifError(err);
assert.strictEqual(stdout, 'A\nB\nhello\n');
}
);
Expand All @@ -131,7 +131,7 @@ childProcess.exec(
nodeBinary + ' ' + '--require ' + fixture('cluster-preload.js') + ' ' +
fixture('cluster-preload-test.js'),
function(err, stdout, stderr) {
if (err) throw err;
assert.ifError(err);
assert.ok(/worker terminated with code 43/.test(stdout));
}
);
Expand All @@ -142,7 +142,7 @@ childProcess.exec(
nodeBinary + ' ' + '--expose_debug_as=v8debug ' + '--require ' +
fixture('cluster-preload.js') + ' ' + 'cluster-preload-test.js',
function(err, stdout, stderr) {
if (err) throw err;
assert.ifError(err);
assert.ok(/worker terminated with code 43/.test(stdout));
}
);
5 changes: 2 additions & 3 deletions test/pummel/test-regress-GH-814.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Flags: --expose_gc

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

function newBuffer(size, value) {
var buffer = Buffer.allocUnsafe(size);
Expand Down Expand Up @@ -59,7 +60,5 @@ var timeToQuit = Date.now() + 8e3; //Test during no more than this seconds.


function cb(err, written) {
if (err) {
throw err;
}
assert.ifError(err);
}
5 changes: 2 additions & 3 deletions test/pummel/test-regress-GH-814_2.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Flags: --expose_gc

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

const fs = require('fs');
const testFileName = require('path').join(common.tmpDir, 'GH-814_test.txt');
Expand Down Expand Up @@ -64,9 +65,7 @@ function writer() {

function writerCB(err, written) {
//console.error('cb.');
if (err) {
throw err;
}
assert.ifError(err);
}


Expand Down
42 changes: 42 additions & 0 deletions tools/eslint-rules/prefer-assert-iferror.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* @fileoverview Prohibit the `if (err) throw err;` pattern
* @author Teddy Katz
*/

'use strict';

module.exports = {
create(context) {
const sourceCode = context.getSourceCode();

function hasSameTokens(nodeA, nodeB) {
const aTokens = sourceCode.getTokens(nodeA);
const bTokens = sourceCode.getTokens(nodeB);

return aTokens.length === bTokens.length &&
aTokens.every((token, index) => {
return token.type === bTokens[index].type &&
token.value === bTokens[index].value;
});
}

return {
IfStatement(node) {
const firstStatement = node.consequent.type === 'BlockStatement' ?
node.consequent.body[0] :
node.consequent;
if (
firstStatement &&
firstStatement.type === 'ThrowStatement' &&
hasSameTokens(node.test, firstStatement.argument)
) {
context.report({
node: firstStatement,
message: 'Use assert.ifError({{argument}}) instead.',
data: {argument: sourceCode.getText(node.test)}
});
}
}
};
}
};