From 91f30548374bf80c285d894f8cbbdcb1026ef149 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= <targos@protonmail.com>
Date: Sat, 27 Mar 2021 11:36:45 +0100
Subject: [PATCH 1/2] fs: introduce readJSON functions

This adds `fs.readJSON`, `fs.readJSONSync`, `fsPromises.readJSON`, and
`fileHandle.readJSON`.
All of them take the usual parameters of their respective `readFile`
equivalents, with the addition of `JSON.parse`'s `reviver` option.
The file will be read as a string, with the encoding defaulting to
`'utf8'`, and if the read succeeds, `JSON.parse` will be called on the
result. Parsing errors are propagated the same way as reading errors.
---
 doc/api/fs.md                               | 118 ++++++++++++++++++++
 lib/fs.js                                   |  35 ++++++
 lib/internal/fs/promises.js                 |  19 ++++
 lib/internal/source_map/source_map_cache.js |   3 +-
 test/common/wpt.js                          |   2 +-
 test/internet/test-trace-events-dns.js      |   3 +-
 test/parallel/test-fs-read-json.mjs         | 116 +++++++++++++++++++
 7 files changed, 291 insertions(+), 5 deletions(-)
 create mode 100644 test/parallel/test-fs-read-json.mjs

diff --git a/doc/api/fs.md b/doc/api/fs.md
index e11c057802e763..fceb333f5d0935 100644
--- a/doc/api/fs.md
+++ b/doc/api/fs.md
@@ -332,6 +332,33 @@ If one or more `filehandle.read()` calls are made on a file handle and then a
 position till the end of the file. It doesn't always read from the beginning
 of the file.
 
+#### `filehandle.readJSON(options)`
+<!-- YAML
+added: REPLACEME
+-->
+
+* `options` {Object|string}
+  * `encoding` {string|null} **Default:** `'utf8'`
+  * `reviver` {Function} Reviver function passed to [`JSON.parse`][].
+  * `signal` {AbortSignal} allows aborting an in-progress readJSON
+* Returns: {Promise} Fulfills upon a successful read with the contents of the
+  file parsed with [`JSON.parse`][].
+
+Asynchronously reads the contents of a file and call [`JSON.parse`][] on it.
+
+The `encoding` and `signal` options are passed to the underlying
+[`fsPromises.readFile()`][] call and the `reviver` option is passed to
+[`JSON.parse`][].
+
+If `options` is a string, then it specifies the `encoding`.
+
+The {FileHandle} has to support reading.
+
+If one or more `filehandle.read()` calls are made on a file handle and then a
+`filehandle.readJSON()` call is made, the data will be read from the current
+position till the end of the file. It doesn't always read from the beginning
+of the file.
+
 #### `filehandle.readv(buffers[, position])`
 <!-- YAML
 added:
@@ -987,6 +1014,39 @@ system requests but rather the internal buffering `fs.readFile` performs.
 
 Any specified {FileHandle} has to support reading.
 
+### `fsPromises.readJSON(path[, options])`
+<!-- YAML
+added: REPLACEME
+-->
+
+* `path` {string|Buffer|URL|FileHandle} filename or `FileHandle`
+* `options` {Object|string}
+  * `encoding` {string|null} **Default:** `'utf8'`
+  * `flag` {string} See [support of file system `flags`][]. **Default:** `'r'`.
+  * `reviver` {Function} Reviver function passed to [`JSON.parse`][].
+  * `signal` {AbortSignal} allows aborting an in-progress readFile
+* Returns: {Promise} Fulfills with the contents of the file parsed with
+  [`JSON.parse`][].
+
+Asynchronously reads the contents of a file and call [`JSON.parse`][] on it.
+
+The `path` parameter, and the `encoding`, `flag` and `signal` options are passed
+to the underlying [`fsPromises.readFile()`][] call and the `reviver` option is
+passed to [`JSON.parse`][].
+
+If `options` is a string, then it specifies the encoding.
+
+It is possible to abort an ongoing `readJSON` using an {AbortSignal}. If a
+request is aborted the promise returned is rejected with an `AbortError`.
+
+Any specified {FileHandle} has to support reading.
+
+```mjs
+import { readJSON } from 'fs/promises';
+
+const json = await readJSON('package.json');
+```
+
 ### `fsPromises.readlink(path[, options])`
 <!-- YAML
 added: v10.0.0
@@ -2952,6 +3012,35 @@ The Node.js GitHub issue [#25741][] provides more information and a detailed
 analysis on the performance of `fs.readFile()` for multiple file sizes in
 different Node.js versions.
 
+### `fs.readJSON(path[, options], callback)`
+<!-- YAML
+added: REPLACEME
+-->
+
+* `path` {string|Buffer|URL|integer} filename or file descriptor
+* `options` {Object|string}
+  * `encoding` {string|null} **Default:** `'utf8'`
+  * `flag` {string} See [support of file system `flags`][]. **Default:** `'r'`.
+  * `reviver` {Function} Reviver function passed to [`JSON.parse`][].
+  * `signal` {AbortSignal} allows aborting an in-progress readJSON
+* `callback` {Function}
+  * `err` {Error|AggregateError}
+  * `data` {any}
+
+Asynchronously reads the contents of a file and call [`JSON.parse`][] on it.
+
+The `path` parameter, and the `encoding`, `flag` and `signal` options are passed
+to the underlying [`fs.readFileSync()`][] call and the `reviver` option is
+passed to to [`JSON.parse`][].
+
+For detailed information, see the documentation of [`fs.readFile()`][].
+
+```mjs
+import { readJSON } from 'fs';
+
+readJSON('package.json', callback);
+```
+
 ### `fs.readlink(path[, options], callback)`
 <!-- YAML
 added: v0.1.31
@@ -4618,6 +4707,33 @@ readFileSync('<directory>');
 readFileSync('<directory>'); // => <data>
 ```
 
+### `fs.readJSONSync(path[, options])`
+<!-- YAML
+added: REPLACEME
+-->
+
+* `path` {string|Buffer|URL|integer} filename or file descriptor
+* `options` {Object|string}
+  * `encoding` {string|null} **Default:** `'utf8'`
+  * `flag` {string} See [support of file system `flags`][]. **Default:** `'r'`.
+  * `reviver` {Function} Reviver function passed to [`JSON.parse`][].
+* Returns: {any}
+
+Reads the contents of the `path` and returns the result of calling
+[`JSON.parse`][] on it.
+
+The `path` parameter, and the `encoding` and `flag` options are passed to the
+underlying [`fs.readFileSync()`][] call and the `reviver` option is passed to
+to [`JSON.parse`][].
+
+For detailed information, see the documentation of [`fs.readFile()`][].
+
+```mjs
+import { readJSONSync } from 'fs';
+
+const json = readJSONSync('package.json');
+```
+
 ### `fs.readlinkSync(path[, options])`
 <!-- YAML
 added: v0.1.31
@@ -6659,6 +6775,7 @@ the file contents.
 [`AHAFS`]: https://www.ibm.com/developerworks/aix/library/au-aix_event_infrastructure/
 [`Buffer.byteLength`]: buffer.md#buffer_static_method_buffer_bytelength_string_encoding
 [`FSEvents`]: https://developer.apple.com/documentation/coreservices/file_system_events
+[`JSON.Parse`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse
 [`Number.MAX_SAFE_INTEGER`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
 [`ReadDirectoryChangesW`]: https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-readdirectorychangesw
 [`UV_THREADPOOL_SIZE`]: cli.md#cli_uv_threadpool_size_size
@@ -6701,6 +6818,7 @@ the file contents.
 [`fs.writev()`]: #fs_fs_writev_fd_buffers_position_callback
 [`fsPromises.open()`]: #fs_fspromises_open_path_flags_mode
 [`fsPromises.opendir()`]: #fs_fspromises_opendir_path_options
+[`fsPromises.readFile()`]: #fs_fspromises_readfile_path_options
 [`fsPromises.rm()`]: #fs_fspromises_rm_path_options
 [`fsPromises.utimes()`]: #fs_fspromises_utimes_path_atime_mtime
 [`inotify(7)`]: https://man7.org/linux/man-pages/man7/inotify.7.html
diff --git a/lib/fs.js b/lib/fs.js
index d396d3a80c99c0..59794ee41cf7b8 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -35,6 +35,7 @@ const kIoMaxLength = 2 ** 31 - 1;
 const {
   ArrayPrototypePush,
   BigIntPrototypeToString,
+  JSONParse,
   MathMax,
   Number,
   ObjectCreate,
@@ -360,6 +361,27 @@ function readFile(path, options, callback) {
                req);
 }
 
+function readJSON(path, options, callback) {
+  callback = maybeCallback(callback || options);
+  options = getOptions(options, { encoding: 'utf8', flag: 'r' });
+
+  const { reviver } = options;
+  if (typeof reviver !== 'undefined') {
+    validateFunction(reviver, 'options.reviver');
+  }
+
+  readFile(path, options, (readFileError, file) => {
+    if (readFileError) return callback(readFileError);
+    let json;
+    try {
+      json = JSONParse(file, reviver);
+    } catch (parseError) {
+      return callback(parseError);
+    }
+    callback(null, json);
+  });
+}
+
 function tryStatSync(fd, isUserFd) {
   const ctx = {};
   const stats = binding.fstat(fd, false, undefined, ctx);
@@ -448,6 +470,17 @@ function readFileSync(path, options) {
   return buffer;
 }
 
+function readJSONSync(path, options) {
+  options = getOptions(options, { encoding: 'utf8', flag: 'r' });
+
+  const { reviver } = options;
+  if (typeof reviver !== 'undefined') {
+    validateFunction(reviver, 'options.reviver');
+  }
+
+  return JSONParse(readFileSync(path, options), reviver);
+}
+
 function defaultCloseCallback(err) {
   if (err != null) throw err;
 }
@@ -2157,6 +2190,8 @@ module.exports = fs = {
   readvSync,
   readFile,
   readFileSync,
+  readJSON,
+  readJSONSync,
   readlink,
   readlinkSync,
   realpath,
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
index 87704a60074f18..ccee121dfa3c98 100644
--- a/lib/internal/fs/promises.js
+++ b/lib/internal/fs/promises.js
@@ -11,6 +11,7 @@ const kWriteFileMaxChunkSize = 512 * 1024;
 const {
   ArrayPrototypePush,
   Error,
+  JSONParse,
   MathMax,
   MathMin,
   NumberIsSafeInteger,
@@ -71,6 +72,7 @@ const {
   validateAbortSignal,
   validateBoolean,
   validateBuffer,
+  validateFunction,
   validateInteger,
   validateUint32
 } = require('internal/validators');
@@ -146,6 +148,10 @@ class FileHandle extends EventEmitterMixin(JSTransferable) {
     return fsCall(readFile, this, options);
   }
 
+  readJSON(options) {
+    return fsCall(readJSON, this, options);
+  }
+
   stat(options) {
     return fsCall(fstat, this, options);
   }
@@ -718,6 +724,18 @@ async function readFile(path, options) {
   return PromisePrototypeFinally(readFileHandle(fd, options), fd.close);
 }
 
+async function readJSON(path, options) {
+  options = getOptions(options, { encoding: 'utf8', flag: 'r' });
+
+  const { reviver } = options;
+  if (typeof reviver !== 'undefined') {
+    validateFunction(reviver, 'options.reviver');
+  }
+
+  const file = await readFile(path, options);
+  return JSONParse(file, reviver);
+}
+
 module.exports = {
   exports: {
     access,
@@ -747,6 +765,7 @@ module.exports = {
     writeFile,
     appendFile,
     readFile,
+    readJSON,
     watch,
   },
 
diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js
index f0d911f78fad8b..55772ac4c09e21 100644
--- a/lib/internal/source_map/source_map_cache.js
+++ b/lib/internal/source_map/source_map_cache.js
@@ -126,8 +126,7 @@ function lineLengths(content) {
 
 function sourceMapFromFile(mapURL) {
   try {
-    const content = fs.readFileSync(fileURLToPath(mapURL), 'utf8');
-    const data = JSONParse(content);
+    const data = fs.readJSONSync(fileURLToPath(mapURL));
     return sourcesToAbsolute(mapURL, data);
   } catch (err) {
     debug(err.stack);
diff --git a/test/common/wpt.js b/test/common/wpt.js
index 137ea59e0e6114..1021cb9179e8b1 100644
--- a/test/common/wpt.js
+++ b/test/common/wpt.js
@@ -261,7 +261,7 @@ class StatusLoader {
   load() {
     const dir = path.join(__dirname, '..', 'wpt');
     const statusFile = path.join(dir, 'status', `${this.path}.json`);
-    const result = JSON.parse(fs.readFileSync(statusFile, 'utf8'));
+    const result = fs.readJSONSync(statusFile);
     this.rules.addRules(result);
 
     const subDir = fixtures.path('wpt', this.path);
diff --git a/test/internet/test-trace-events-dns.js b/test/internet/test-trace-events-dns.js
index 8c6bd249cbf2a7..90d5ffcde113e5 100644
--- a/test/internet/test-trace-events-dns.js
+++ b/test/internet/test-trace-events-dns.js
@@ -59,8 +59,7 @@ for (const tr in tests) {
 
   const file = path.join(tmpdir.path, traceFile);
 
-  const data = fs.readFileSync(file);
-  const traces = JSON.parse(data.toString()).traceEvents
+  const traces = fs.readJSONSync(file).traceEvents
         .filter((trace) => trace.cat !== '__metadata');
 
   assert(traces.length > 0);
diff --git a/test/parallel/test-fs-read-json.mjs b/test/parallel/test-fs-read-json.mjs
new file mode 100644
index 00000000000000..f5237311ce665a
--- /dev/null
+++ b/test/parallel/test-fs-read-json.mjs
@@ -0,0 +1,116 @@
+import { expectsError, mustCall } from '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+
+import assert from 'assert';
+import { readJSON, readJSONSync } from 'fs';
+import { open, readJSON as readJSONPromises } from 'fs/promises';
+
+const invalidJsonFile = fixtures.path('invalid.json');
+const validJsonFile = fixtures.path('experimental.json');
+const expectedValid = { ofLife: 42 };
+const invalidPath = fixtures.path('i/do/not/exist');
+
+function reviver(key, value) {
+  if (key === 'ofLife') {
+    return value / 2;
+  }
+  return value;
+}
+const expectedWithReviver = { ofLife: 21 };
+
+// fs.readJSON()
+{
+  readJSON(
+    validJsonFile,
+    mustCall((error, json) => {
+      assert.strictEqual(error, null);
+      assert.deepStrictEqual(json, expectedValid);
+    })
+  );
+
+  readJSON(
+    validJsonFile,
+    { reviver },
+    mustCall((error, json) => {
+      assert.strictEqual(error, null);
+      assert.deepStrictEqual(json, expectedWithReviver);
+    })
+  );
+
+  readJSON(
+    invalidJsonFile,
+    expectsError({
+      name: 'SyntaxError',
+    })
+  );
+
+  readJSON(
+    invalidPath,
+    expectsError({
+      code: 'ENOENT',
+    })
+  );
+
+  assert.throws(() => readJSON(null, () => {}), {
+    code: 'ERR_INVALID_ARG_TYPE',
+  });
+  assert.throws(() => readJSON(validJsonFile), {
+    code: 'ERR_INVALID_CALLBACK',
+  });
+  assert.throws(() => readJSON(validJsonFile, { reviver: true }, () => {}), {
+    code: 'ERR_INVALID_ARG_TYPE',
+  });
+} // fs.readJSON()
+
+// fs.readJSONSync()
+{
+  assert.deepStrictEqual(readJSONSync(validJsonFile), expectedValid);
+
+  assert.deepStrictEqual(
+    readJSONSync(validJsonFile, { reviver }),
+    expectedWithReviver
+  );
+
+  assert.throws(() => readJSONSync(invalidJsonFile), { name: 'SyntaxError' });
+
+  assert.throws(() => readJSONSync(invalidPath), { code: 'ENOENT' });
+
+  assert.throws(() => readJSONSync(null), { code: 'ERR_INVALID_ARG_TYPE' });
+  assert.throws(() => readJSONSync(validJsonFile, { reviver: true }), {
+    code: 'ERR_INVALID_ARG_TYPE',
+  });
+} // fs.readJSONSync()
+
+// fsPromises.readJSON()
+{
+  assert.deepStrictEqual(await readJSONPromises(validJsonFile), expectedValid);
+
+  assert.deepStrictEqual(
+    await readJSONPromises(validJsonFile, { reviver }),
+    expectedWithReviver
+  );
+
+  await assert.rejects(readJSONPromises(invalidJsonFile), {
+    name: 'SyntaxError',
+  });
+
+  await assert.rejects(readJSONPromises(invalidPath), { code: 'ENOENT' });
+
+  await assert.rejects(readJSONPromises(null), {
+    code: 'ERR_INVALID_ARG_TYPE',
+  });
+  await assert.rejects(readJSONPromises(validJsonFile, { reviver: true }), {
+    code: 'ERR_INVALID_ARG_TYPE',
+  });
+} // fsPromises.readJSON()
+
+// fileHandle.readJSON()
+{
+  let fileHandle = await open(validJsonFile);
+  assert.deepStrictEqual(await fileHandle.readJSON(), expectedValid);
+  await fileHandle.close();
+
+  fileHandle = await open(invalidJsonFile);
+  await assert.rejects(fileHandle.readJSON(), { name: 'SyntaxError' });
+  await fileHandle.close();
+} // fileHandle.readJSON()

From dfe261c7063cf5350ceba1567a68a42619d3f0a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= <targos@protonmail.com>
Date: Wed, 31 Mar 2021 18:49:54 +0200
Subject: [PATCH 2/2] fixup! fs: introduce readJSON functions

test error in callback
---
 test/parallel/test-fs-read-json.mjs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/test/parallel/test-fs-read-json.mjs b/test/parallel/test-fs-read-json.mjs
index f5237311ce665a..706e8ebad27335 100644
--- a/test/parallel/test-fs-read-json.mjs
+++ b/test/parallel/test-fs-read-json.mjs
@@ -51,6 +51,17 @@ const expectedWithReviver = { ofLife: 21 };
     })
   );
 
+  process.once(
+    'uncaughtException',
+    mustCall((error) => {
+      assert.strictEqual(error.message, 'error in callback');
+    })
+  );
+
+  readJSON(validJsonFile, () => {
+    throw new Error('error in callback');
+  });
+
   assert.throws(() => readJSON(null, () => {}), {
     code: 'ERR_INVALID_ARG_TYPE',
   });