diff --git a/test/.eslintrc.yaml b/test/.eslintrc.yaml index 90bf5f6d88d4e5..5e73630174e75e 100644 --- a/test/.eslintrc.yaml +++ b/test/.eslintrc.yaml @@ -3,4 +3,5 @@ rules: ## common module is mandatory in tests required-modules: [2, common] + prefer-assert-iferror: 2 prefer-assert-methods: 2 diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js index 20ea5c7298199a..9bac769a5cab83 100644 --- a/test/parallel/test-crypto-pbkdf2.js +++ b/test/parallel/test-crypto-pbkdf2.js @@ -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); } diff --git a/test/parallel/test-fs-long-path.js b/test/parallel/test-fs-long-path.js index 63b00118217288..dd091ae1b1a94c 100644 --- a/test/parallel/test-fs-long-path.js +++ b/test/parallel/test-fs-long-path.js @@ -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.'); @@ -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); })); })); diff --git a/test/parallel/test-fs-readfile-pipe.js b/test/parallel/test-fs-readfile-pipe.js index 91465a261c8f96..566dbbe4c84929 100644 --- a/test/parallel/test-fs-readfile-pipe.js +++ b/test/parallel/test-fs-readfile-pipe.js @@ -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; diff --git a/test/parallel/test-fs-symlink-dir-junction-relative.js b/test/parallel/test-fs-symlink-dir-junction-relative.js index 814b0c678eb3ef..166894c97ed846 100644 --- a/test/parallel/test-fs-symlink-dir-junction-relative.js +++ b/test/parallel/test-fs-symlink-dir-junction-relative.js @@ -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); })); diff --git a/test/parallel/test-fs-symlink-dir-junction.js b/test/parallel/test-fs-symlink-dir-junction.js index 58ddb7ca38ae1c..ed0430b947ba0c 100644 --- a/test/parallel/test-fs-symlink-dir-junction.js +++ b/test/parallel/test-fs-symlink-dir-junction.js @@ -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)); })); diff --git a/test/parallel/test-preload.js b/test/parallel/test-preload.js index bf3c4a09cf5600..3413a2a6dc69c8 100644 --- a/test/parallel/test-preload.js +++ b/test/parallel/test-preload.js @@ -34,7 +34,7 @@ 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'); }); @@ -42,7 +42,7 @@ childProcess.exec(nodeBinary + ' ' + preloadOption([fixtureA]) + ' ' + fixtureB, 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'); } ); @@ -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'); } ); @@ -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'); } ); @@ -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)); } ); @@ -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)); } ); diff --git a/test/pummel/test-regress-GH-814.js b/test/pummel/test-regress-GH-814.js index 4633844ee1b65a..4c890e1705acc3 100644 --- a/test/pummel/test-regress-GH-814.js +++ b/test/pummel/test-regress-GH-814.js @@ -2,6 +2,7 @@ // Flags: --expose_gc const common = require('../common'); +const assert = require('assert'); function newBuffer(size, value) { var buffer = Buffer.allocUnsafe(size); @@ -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); } diff --git a/test/pummel/test-regress-GH-814_2.js b/test/pummel/test-regress-GH-814_2.js index 31c18529856da9..afba742e60bb7d 100644 --- a/test/pummel/test-regress-GH-814_2.js +++ b/test/pummel/test-regress-GH-814_2.js @@ -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'); @@ -64,9 +65,7 @@ function writer() { function writerCB(err, written) { //console.error('cb.'); - if (err) { - throw err; - } + assert.ifError(err); } diff --git a/tools/eslint-rules/prefer-assert-iferror.js b/tools/eslint-rules/prefer-assert-iferror.js new file mode 100644 index 00000000000000..0da272d5f670e3 --- /dev/null +++ b/tools/eslint-rules/prefer-assert-iferror.js @@ -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)} + }); + } + } + }; + } +};