Revert "fs: Revert throw on invalid callbacks"

This reverts commit 8250bfd1e5188d5dada58aedf7a991e959d5eaa9.

PR-URL: https://github.com/nodejs/node/pull/18668
Refs: https://github.com/nodejs/node/pull/12562
Refs: https://github.com/nodejs/node/pull/12976
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>

10 files changed, 15 insertions, 35 deletions

diff --git a/test/fixtures/test-fs-readfile-error.js b/test/fixtures/test-fs-readfile-error.js
index 3f8d9a731e..9bac28f8ef 100644
--- a/test/fixtures/test-fs-readfile-error.js
+++ b/test/fixtures/test-fs-readfile-error.js
@@ -19,4 +19,4 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-require('fs').readFile('/');  // throws EISDIR
+require('fs').readFileSync('/');  // throws EISDIR
diff --git a/test/parallel/test-fs-access.js b/test/parallel/test-fs-access.js
index aacf3f1848..81070286eb 100644
--- a/test/parallel/test-fs-access.js
+++ b/test/parallel/test-fs-access.js
@@ -80,7 +80,8 @@ fs.access(readOnlyFile, fs.F_OK | fs.R_OK, common.mustCall((err) => {
   assert.ifError(err);
 }));
 
-fs.access(readOnlyFile, fs.W_OK, common.mustCall((err) => {
+fs.access(readOnlyFile, fs.W_OK, common.mustCall(function(err) {
+  assert.strictEqual(this, undefined);
   if (hasWriteAccessForReadonlyFile) {
     assert.ifError(err);
   } else {
diff --git a/test/parallel/test-fs-append-file.js b/test/parallel/test-fs-append-file.js
index 8e9a0619b0..29ae1b2f3b 100644
--- a/test/parallel/test-fs-append-file.js
+++ b/test/parallel/test-fs-append-file.js
@@ -146,12 +146,9 @@ fs.open(filename5, 'a+', function(e, fd) {
   });
 });
 
-// test that a missing callback emits a warning, even if the last argument is a
-// function.
-const filename6 = join(tmpdir.path, 'append6.txt');
-const warn = 'Calling an asynchronous function without callback is deprecated.';
-common.expectWarning('DeprecationWarning', warn);
-fs.appendFile(filename6, console.log);
+assert.throws(
+  () => fs.appendFile(join(tmpdir.path, 'append6.txt'), console.log),
+  { code: 'ERR_INVALID_CALLBACK' });
 
 process.on('exit', function() {
   assert.strictEqual(12, ncallbacks);
diff --git a/test/parallel/test-fs-exists.js b/test/parallel/test-fs-exists.js
index 0304ca2fb6..cd2d9a712f 100644
--- a/test/parallel/test-fs-exists.js
+++ b/test/parallel/test-fs-exists.js
@@ -26,10 +26,9 @@ const fs = require('fs');
 const { URL } = require('url');
 const f = __filename;
 
-// Only warnings are emitted when the callback is invalid
-fs.exists(f);
-fs.exists();
-fs.exists(f, {});
+assert.throws(() => fs.exists(f), { code: 'ERR_INVALID_CALLBACK' });
+assert.throws(() => fs.exists(), { code: 'ERR_INVALID_CALLBACK' });
+assert.throws(() => fs.exists(f, {}), { code: 'ERR_INVALID_CALLBACK' });
 
 fs.exists(f, common.mustCall(function(y) {
   assert.strictEqual(y, true);
diff --git a/test/parallel/test-fs-fchmod.js b/test/parallel/test-fs-fchmod.js
index 42a11e8a96..22e6a490c9 100644
--- a/test/parallel/test-fs-fchmod.js
+++ b/test/parallel/test-fs-fchmod.js
@@ -44,7 +44,7 @@ const fs = require('fs');
 
 // Check for mode values range
 const modeUpperBoundaryValue = 0o777;
-fs.fchmod(1, modeUpperBoundaryValue);
+fs.fchmod(1, modeUpperBoundaryValue, common.mustCall());
 fs.fchmodSync(1, modeUpperBoundaryValue);
 
 // umask of 0o777 is equal to 775
diff --git a/test/parallel/test-fs-make-callback.js b/test/parallel/test-fs-make-callback.js
index 9d4a6121a3..d3ff165513 100644
--- a/test/parallel/test-fs-make-callback.js
+++ b/test/parallel/test-fs-make-callback.js
@@ -4,7 +4,6 @@ const fs = require('fs');
 const callbackThrowValues = [null, true, false, 0, 1, 'foo', /foo/, [], {}];
 
 const { sep } = require('path');
-const warn = 'Calling an asynchronous function without callback is deprecated.';
 
 const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
@@ -16,11 +15,6 @@ function testMakeCallback(cb) {
   };
 }
 
-common.expectWarning('DeprecationWarning', warn);
-
-// Passing undefined/nothing calls rethrow() internally, which emits a warning
-testMakeCallback()();
-
 function invalidCallbackThrowsTests() {
   callbackThrowValues.forEach((value) => {
     common.expectsError(testMakeCallback(value), {
diff --git a/test/parallel/test-fs-makeStatsCallback.js b/test/parallel/test-fs-makeStatsCallback.js
index ec03b593fa..c8de29e407 100644
--- a/test/parallel/test-fs-makeStatsCallback.js
+++ b/test/parallel/test-fs-makeStatsCallback.js
@@ -2,7 +2,6 @@
 const common = require('../common');
 const fs = require('fs');
 const callbackThrowValues = [null, true, false, 0, 1, 'foo', /foo/, [], {}];
-const warn = 'Calling an asynchronous function without callback is deprecated.';
 
 function testMakeStatsCallback(cb) {
   return function() {
@@ -11,14 +10,9 @@ function testMakeStatsCallback(cb) {
   };
 }
 
-common.expectWarning('DeprecationWarning', warn);
-
 // Verify the case where a callback function is provided
 testMakeStatsCallback(common.mustCall())();
 
-// Passing undefined/nothing calls rethrow() internally, which emits a warning
-testMakeStatsCallback()();
-
 function invalidCallbackThrowsTests() {
   callbackThrowValues.forEach((value) => {
     common.expectsError(testMakeStatsCallback(value), {
diff --git a/test/parallel/test-fs-mkdtemp.js b/test/parallel/test-fs-mkdtemp.js
index 27e2f1c08c..443dafa275 100644
--- a/test/parallel/test-fs-mkdtemp.js
+++ b/test/parallel/test-fs-mkdtemp.js
@@ -29,8 +29,3 @@ fs.mkdtemp(path.join(tmpdir.path, 'bar.'), common.mustCall(handler));
 // Same test as above, but making sure that passing an options object doesn't
 // affect the way the callback function is handled.
 fs.mkdtemp(path.join(tmpdir.path, 'bar.'), {}, common.mustCall(handler));
-
-// Making sure that not passing a callback doesn't crash, as a default function
-// is passed internally.
-fs.mkdtemp(path.join(tmpdir.path, 'bar-'));
-fs.mkdtemp(path.join(tmpdir.path, 'bar-'), {});
diff --git a/test/parallel/test-fs-readfile-error.js b/test/parallel/test-fs-readfile-error.js
index 9b1a06183d..97633c9a88 100644
--- a/test/parallel/test-fs-readfile-error.js
+++ b/test/parallel/test-fs-readfile-error.js
@@ -48,7 +48,7 @@ function test(env, cb) {
 
 test({ NODE_DEBUG: '' }, common.mustCall((data) => {
   assert(/EISDIR/.test(data));
-  assert(!/test-fs-readfile-error/.test(data));
+  assert(/test-fs-readfile-error/.test(data));
 }));
 
 test({ NODE_DEBUG: 'fs' }, common.mustCall((data) => {
@@ -57,7 +57,7 @@ test({ NODE_DEBUG: 'fs' }, common.mustCall((data) => {
 }));
 
 common.expectsError(
-  () => { fs.readFile(() => {}); },
+  () => { fs.readFile(() => {}, common.mustNotCall()); },
   {
     code: 'ERR_INVALID_ARG_TYPE',
     message: 'The "path" argument must be one of type string, Buffer, or URL',
diff --git a/test/parallel/test-fs-write-no-fd.js b/test/parallel/test-fs-write-no-fd.js
index 0aaec4b733..576457203e 100644
--- a/test/parallel/test-fs-write-no-fd.js
+++ b/test/parallel/test-fs-write-no-fd.js
@@ -1,12 +1,12 @@
 'use strict';
-require('../common');
+const common = require('../common');
 const fs = require('fs');
 const assert = require('assert');
 
 assert.throws(function() {
-  fs.write(null, Buffer.allocUnsafe(1), 0, 1);
+  fs.write(null, Buffer.allocUnsafe(1), 0, 1, common.mustNotCall());
 }, /TypeError/);
 
 assert.throws(function() {
-  fs.write(null, '1', 0, 1);
+  fs.write(null, '1', 0, 1, common.mustNotCall());
 }, /TypeError/);