fs: make open and close stream override optional when unused

When using `createReadStream` or `createWriteStream` with a specific
file descriptor or `FileHandle` instead of a path, `open` method is not
used, there is no point in forcing users to provide it.
When using `createReadStream` or `createWriteStream` with `autoClose`
set to false, `close` method is not used, there is no point in forcing
users to provide it.
PR-URL: #40013
Reviewed-By: James M Snell <jasnell@gmail.com>

Author: aduh95

doc/api/fs.md

@@ -1887,6 +1887,12 @@ behavior is similar to `cp dir1/ dir2/`.
 <!-- YAML
 added: v0.1.31
 changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/40013
+ description: The `fs` option does not need `open` method if an `fd` was provided.
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/40013
+ description: The `fs` option does not need `close` method if `autoClose` is `false`.
  - version:
  - v15.4.0
  pr-url: https://github.com/nodejs/node/pull/35922
@@ -1962,7 +1968,9 @@ destroyed, like most `Readable` streams. Set the `emitClose` option to
 
 By providing the `fs` option, it is possible to override the corresponding `fs`
 implementations for `open`, `read`, and `close`. When providing the `fs` option,
-overrides for `open`, `read`, and `close` are required.
+an override for `read` is required. If no `fd` is provided, an override for
+`open` is also required. If `autoClose` is `true`, an override for `close` is
+also required.
 
 ```mjs
 import { createReadStream } from 'fs';
@@ -2004,6 +2012,12 @@ If `options` is a string, then it specifies the encoding.
 <!-- YAML
 added: v0.1.31
 changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/40013
+ description: The `fs` option does not need `open` method if an `fd` was provided.
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/40013
+ description: The `fs` option does not need `close` method if `autoClose` is `false`.
  - version:
  - v15.4.0
  pr-url: https://github.com/nodejs/node/pull/35922
@@ -2067,8 +2081,10 @@ destroyed, like most `Writable` streams. Set the `emitClose` option to
 By providing the `fs` option it is possible to override the corresponding `fs`
 implementations for `open`, `write`, `writev` and `close`. Overriding `write()`
 without `writev()` can reduce performance as some optimizations (`_writev()`)
-will be disabled. When providing the `fs` option, overrides for `open`,
-`close`, and at least one of `write` and `writev` are required.
+will be disabled. When providing the `fs` option, overrides for at least one of
+`write` and `writev` are required. If no `fd` option is supplied, an override
+for `open` is also required. If `autoClose` is `true`, an override for `close`
+is also required.
 
 Like {fs.ReadStream}, if `fd` is specified, {fs.WriteStream} will ignore the
 `path` argument and will use the specified file descriptor. This means that no

lib/internal/fs/streams.js

@@ -120,29 +120,29 @@ function close(stream, err, cb) {
 }
 
 functionimportFd(stream,options){
-stream.fd=null;
-if(options.fd!=null){
-if(typeofoptions.fd==='number'){
-// When fd is a raw descriptor, we must keep our fingers crossed
-// that the descriptor won't get closed, or worse, replaced with
-// another one
-// https://github.com/nodejs/node/issues/35862
-stream.fd=options.fd;
-}elseif(typeofoptions.fd==='object'&&
-options.fdinstanceofFileHandle){
-// When fd is a FileHandle we can listen for 'close' events
-if(options.fs)
-// FileHandle is not supported with custom fs operations
-thrownewERR_METHOD_NOT_IMPLEMENTED('FileHandle with fs');
-stream[kHandle]=options.fd;
-stream.fd=options.fd.fd;
-stream[kFs]=FileHandleOperations(stream[kHandle]);
-stream[kHandle][kRef]();
-options.fd.on('close',FunctionPrototypeBind(stream.close,stream));
-}else
-throwERR_INVALID_ARG_TYPE('options.fd',
-['number','FileHandle'],options.fd);
+if(typeofoptions.fd==='number'){
+// When fd is a raw descriptor, we must keep our fingers crossed
+// that the descriptor won't get closed, or worse, replaced with
+// another one
+// https://github.com/nodejs/node/issues/35862
+stream[kFs]=options.fs||fs;
+returnoptions.fd;
+}elseif(typeofoptions.fd==='object'&&
+options.fdinstanceofFileHandle){
+// When fd is a FileHandle we can listen for 'close' events
+if(options.fs){
+// FileHandle is not supported with custom fs operations
+thrownewERR_METHOD_NOT_IMPLEMENTED('FileHandle with fs');
+}
+stream[kHandle]=options.fd;
+stream[kFs]=FileHandleOperations(stream[kHandle]);
+stream[kHandle][kRef]();
+options.fd.on('close',FunctionPrototypeBind(stream.close,stream));
+returnoptions.fd.fd;
 }
+
+throwERR_INVALID_ARG_TYPE('options.fd',
+['number','FileHandle'],options.fd);
 }
 
 functionReadStream(path,options){
@@ -158,21 +158,29 @@ function ReadStream(path, options) {
 options.autoDestroy=false;
 }
 
-this[kFs]=options.fs||fs;
+if(options.fd==null){
+this.fd=null;
+this[kFs]=options.fs||fs;
+validateFunction(this[kFs].open,'options.fs.open');
 
-validateFunction(this[kFs].open,'options.fs.open');
-validateFunction(this[kFs].read,'options.fs.read');
-validateFunction(this[kFs].close,'options.fs.close');
+// Path will be ignored when fd is specified, so it can be falsy
+this.path=toPathIfFileURL(path);
+this.flags=options.flags===undefined ? 'r' : options.flags;
+this.mode=options.mode===undefined ? 0o666 : options.mode;
+
+validatePath(this.path);
+}else{
+this.fd=getValidatedFd(importFd(this,options));
+}
 
 options.autoDestroy=options.autoClose===undefined ?
 true : options.autoClose;
 
-// Path will be ignored when fd is specified, so it can be falsy
-this.path=toPathIfFileURL(path);
-this.flags=options.flags===undefined ? 'r' : options.flags;
-this.mode=options.mode===undefined ? 0o666 : options.mode;
+validateFunction(this[kFs].read,'options.fs.read');
 
-importFd(this,options);
+if(options.autoDestroy){
+validateFunction(this[kFs].close,'options.fs.close');
+}
 
 this.start=options.start;
 this.end=options.end;
@@ -187,12 +195,6 @@ function ReadStream(path, options) {
 this.pos=this.start;
 }
 
-// If fd has been set, validate, otherwise validate path.
-if(this.fd!=null){
-this.fd=getValidatedFd(this.fd);
-}else{
-validatePath(this.path);
-}
 
 if(this.end===undefined){
 this.end=Infinity;
@@ -310,9 +312,23 @@ function WriteStream(path, options) {
 // Only buffers are supported.
 options.decodeStrings=true;
 
-this[kFs]=options.fs||fs;
+if(options.fd==null){
+this.fd=null;
+this[kFs]=options.fs||fs;
+validateFunction(this[kFs].open,'options.fs.open');
+
+// Path will be ignored when fd is specified, so it can be falsy
+this.path=toPathIfFileURL(path);
+this.flags=options.flags===undefined ? 'w' : options.flags;
+this.mode=options.mode===undefined ? 0o666 : options.mode;
+
+validatePath(this.path);
+}else{
+this.fd=getValidatedFd(importFd(this,options));
+}
 
-validateFunction(this[kFs].open,'options.fs.open');
+options.autoDestroy=options.autoClose===undefined ?
+true : options.autoClose;
 
 if(!this[kFs].write&&!this[kFs].writev){
 thrownewERR_INVALID_ARG_TYPE('options.fs.write','function',
@@ -327,7 +343,9 @@ function WriteStream(path, options) {
 validateFunction(this[kFs].writev,'options.fs.writev');
 }
 
-validateFunction(this[kFs].close,'options.fs.close');
+if(options.autoDestroy){
+validateFunction(this[kFs].close,'options.fs.close');
+}
 
 // It's enough to override either, in which case only one will be used.
 if(!this[kFs].write){
@@ -337,28 +355,12 @@ function WriteStream(path, options) {
 this._writev=null;
 }
 
-options.autoDestroy=options.autoClose===undefined ?
-true : options.autoClose;
-
-// Path will be ignored when fd is specified, so it can be falsy
-this.path=toPathIfFileURL(path);
-this.flags=options.flags===undefined ? 'w' : options.flags;
-this.mode=options.mode===undefined ? 0o666 : options.mode;
-
-importFd(this,options);
-
 this.start=options.start;
 this.pos=undefined;
 this.bytesWritten=0;
 this.closed=false;
 this[kIsPerformingIO]=false;
 
-// If fd has been set, validate, otherwise validate path.
-if(this.fd!=null){
-this.fd=getValidatedFd(this.fd);
-}else{
-validatePath(this.path);
-}
 
 if(this.start!==undefined){
 validateInteger(this.start,'start',0);