From 535e9571f5252ea9ce6f5db12f700f73af6df055 Mon Sep 17 00:00:00 2001
From: Lucas Holmquist <lholmqui@redhat.com>
Date: Mon, 14 Oct 2019 11:57:24 -0400
Subject: [PATCH] fs: make FSStatWatcher.start private
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

An instance of FSStatWatcher is returned when a user calls fs.watchFile,
which will call the start method. A user can't create an instance
of a FSStatWatcher directly. If the start method is called by a user
it is a noop since the watcher has already started.

This "Class" is currently undocumented.

PR-URL: https://github.com/nodejs/node/pull/29971
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
---
 lib/fs.js                                 |  3 ++-
 lib/internal/fs/watchers.js               | 20 +++++++++++++++-----
 test/parallel/test-fs-watchfile-bigint.js |  2 --
 test/parallel/test-fs-watchfile.js        |  2 --
 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/lib/fs.js b/lib/fs.js
index 2944a44a317f76..a686b01fff072d 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -1386,7 +1386,8 @@ function watchFile(filename, options, listener) {
     if (!watchers)
       watchers = require('internal/fs/watchers');
     stat = new watchers.StatWatcher(options.bigint);
-    stat.start(filename, options.persistent, options.interval);
+    stat[watchers.kFSStatWatcherStart](filename,
+                                       options.persistent, options.interval);
     statWatchers.set(filename, stat);
   }
 
diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js
index c974f40aae104f..0685bcef4dff58 100644
--- a/lib/internal/fs/watchers.js
+++ b/lib/internal/fs/watchers.js
@@ -26,6 +26,7 @@ const kOldStatus = Symbol('kOldStatus');
 const kUseBigint = Symbol('kUseBigint');
 
 const kFSWatchStart = Symbol('kFSWatchStart');
+const kFSStatWatcherStart = Symbol('kFSStatWatcherStart');
 
 function emitStop(self) {
   self.emit('stop');
@@ -54,13 +55,15 @@ function onchange(newStatus, stats) {
             getStatsFromBinding(stats, kFsStatsFieldsNumber));
 }
 
-// FIXME(joyeecheung): this method is not documented.
 // At the moment if filename is undefined, we
-// 1. Throw an Error if it's the first time .start() is called
-// 2. Return silently if .start() has already been called
+// 1. Throw an Error if it's the first
+//    time Symbol('kFSStatWatcherStart') is called
+// 2. Return silently if Symbol('kFSStatWatcherStart') has already been called
 //    on a valid filename and the wrap has been initialized
 // This method is a noop if the watcher has already been started.
-StatWatcher.prototype.start = function(filename, persistent, interval) {
+StatWatcher.prototype[kFSStatWatcherStart] = function(filename,
+                                                      persistent,
+                                                      interval) {
   if (this._handle !== null)
     return;
 
@@ -88,6 +91,12 @@ StatWatcher.prototype.start = function(filename, persistent, interval) {
   }
 };
 
+// To maximize backward-compatiblity for the end user,
+// a no-op stub method has been added instead of
+// totally removing StatWatcher.prototpye.start.
+// This should not be documented.
+StatWatcher.prototype.start = () => {};
+
 // FIXME(joyeecheung): this method is not documented while there is
 // another documented fs.unwatchFile(). The counterpart in
 // FSWatcher is .close()
@@ -209,5 +218,6 @@ Object.defineProperty(FSEvent.prototype, 'owner', {
 module.exports = {
   FSWatcher,
   StatWatcher,
-  kFSWatchStart
+  kFSWatchStart,
+  kFSStatWatcherStart
 };
diff --git a/test/parallel/test-fs-watchfile-bigint.js b/test/parallel/test-fs-watchfile-bigint.js
index 500a0ef1f64b1b..569c7ad6f57fe4 100644
--- a/test/parallel/test-fs-watchfile-bigint.js
+++ b/test/parallel/test-fs-watchfile-bigint.js
@@ -67,5 +67,3 @@ const watcher =
 // 'stop' should only be emitted once - stopping a stopped watcher should
 // not trigger a 'stop' event.
 watcher.on('stop', common.mustCall(function onStop() {}));
-
-watcher.start();  // Starting a started watcher should be a noop
diff --git a/test/parallel/test-fs-watchfile.js b/test/parallel/test-fs-watchfile.js
index 0b28e0331d3f91..7b3ad4a26db6b5 100644
--- a/test/parallel/test-fs-watchfile.js
+++ b/test/parallel/test-fs-watchfile.js
@@ -82,8 +82,6 @@ const watcher =
 // not trigger a 'stop' event.
 watcher.on('stop', common.mustCall(function onStop() {}));
 
-watcher.start();  // Starting a started watcher should be a noop
-
 // Watch events should callback with a filename on supported systems.
 // Omitting AIX. It works but not reliably.
 if (common.isLinux || common.isOSX || common.isWindows) {