stream: inline needMoreData function

Inline the needMoreData function since it has only one call place.
Update the related comment.
Add a test for the edge case where HWM=0 and state.length=0.
Add a test for ReadableStream.read(n) method's edge case where
n, HWM and state.length are all zero.
This proves that there is no easy way to simplify the check at
https://github.com/nodejs/node/blob/master/lib/_stream_readable.js#L440

Fixes: https://github.com/nodejs/node/issues/19893
Refs: https://github.com/nodejs/node/pull/19896

PR-URL: https://github.com/nodejs/node/pull/21009
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

lib/_stream_readable.js

@@ -270,7 +270,11 @@ function readableAddChunk(stream, chunk, encoding, addToFront, skipChunkCheck) {
     }
   }
 
-  return needMoreData(state);
+  // We can push more data if we are below the highWaterMark.
+  // Also, if we have no data yet, we can stand some more bytes.
+  // This is to work around cases where hwm=0, such as the repl.
+  return !state.ended &&
+    (state.length < state.highWaterMark || state.length === 0);
 }
 
 function addChunk(stream, state, chunk, addToFront) {
@@ -304,19 +308,6 @@ function chunkInvalid(state, chunk) {
 }
 
 
-// We can push more data if we are below the highWaterMark.
-// Also, if we have no data yet, we can stand some
-// more bytes.  This is to work around cases where hwm=0,
-// such as the repl.  Also, if the push() triggered a
-// readable event, and the user called read(largeNumber) such that
-// needReadable was set, then we ought to push more, so that another
-// 'readable' event will be triggered.
-function needMoreData(state) {
-  return !state.ended &&
-         (state.length < state.highWaterMark ||
-          state.length === 0);
-}
-
 Readable.prototype.isPaused = function() {
   return this._readableState.flowing === false;
 };

test/parallel/test-streams-highwatermark.js

@@ -1,31 +1,58 @@
 'use strict';
 const common = require('../common');
 
-// This test ensures that the stream implementation correctly handles values
-// for highWaterMark which exceed the range of signed 32 bit integers and
-// rejects invalid values.
-
 const assert = require('assert');
 const stream = require('stream');
 
-// This number exceeds the range of 32 bit integer arithmetic but should still
+{
-// be handled correctly.
+  // This test ensures that the stream implementation correctly handles values
-const ovfl = Number.MAX_SAFE_INTEGER;
+  // for highWaterMark which exceed the range of signed 32 bit integers and
+  // rejects invalid values.
 
-const readable = stream.Readable({ highWaterMark: ovfl });
+  // This number exceeds the range of 32 bit integer arithmetic but should still
-assert.strictEqual(readable._readableState.highWaterMark, ovfl);
+  // be handled correctly.
+  const ovfl = Number.MAX_SAFE_INTEGER;
 
-const writable = stream.Writable({ highWaterMark: ovfl });
+  const readable = stream.Readable({ highWaterMark: ovfl });
-assert.strictEqual(writable._writableState.highWaterMark, ovfl);
+  assert.strictEqual(readable._readableState.highWaterMark, ovfl);
 
-for (const invalidHwm of [true, false, '5', {}, -5, NaN]) {
+  const writable = stream.Writable({ highWaterMark: ovfl });
+  assert.strictEqual(writable._writableState.highWaterMark, ovfl);
+
+  for (const invalidHwm of [true, false, '5', {}, -5, NaN]) {
     for (const type of [stream.Readable, stream.Writable]) {
       common.expectsError(() => {
         type({ highWaterMark: invalidHwm });
       }, {
         type: TypeError,
         code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${invalidHwm}" is invalid for option "highWaterMark"`
+        message:
+          `The value "${invalidHwm}" is invalid for option "highWaterMark"`
       });
     }
+  }
+}
+
+{
+  // This test ensures that the push method's implementation
+  // correctly handles the edge case where the highWaterMark and
+  // the state.length are both zero
+
+  const readable = stream.Readable({ highWaterMark: 0 });
+
+  for (let i = 0; i < 3; i++) {
+    const needMoreData = readable.push();
+    assert.strictEqual(needMoreData, true);
+  }
+}
+
+{
+  // This test ensures that the read(n) method's implementation
+  // correctly handles the edge case where the highWaterMark, state.length
+  // and n are all zero
+
+  const readable = stream.Readable({ highWaterMark: 0 });
+
+  readable._read = common.mustCall();
+  readable.read(0);
 }