fs: remove permissive rmdir recursive

PR-URL: #37216 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Coe <[email protected]> Reviewed-By: Ian Sutherland <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

Author: aduh95

doc/api/fs.md

@@ -1048,6 +1048,16 @@ Renames `oldPath` to `newPath`.
 <!-- YAML
 added: v10.0.0
 changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37216
+ description: "Using `fsPromises.rmdir(path,{recursive: true })` on a `path`
+ that is a file is no longer permitted and results in an
+`ENOENT` error on Windows and an `ENOTDIR` error on POSIX."
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37216
+ description: "Using `fsPromises.rmdir(path,{recursive: true })` on a `path`
+ that does not exist is no longer permitted and results in a
+`ENOENT` error."
  - version: REPLACEME
  pr-url: https://github.com/nodejs/node/pull/37302
  description: The `recursive` option is deprecated, using it triggers a
@@ -1075,8 +1085,8 @@ changes:
  represents the number of retries. This option is ignored if the `recursive`
  option is not `true`. **Default:** `0`.
  * `recursive`{boolean} If `true`, perform a recursive directory removal. In
- recursive mode, errors are not reported if `path` does not exist, and
-operations are retried on failure. **Default:** `false`. **Deprecated**.
+ recursive mode, operations are retried on failure. **Default:** `false`.
+ **Deprecated**.
  * `retryDelay`{integer} The amount of time in milliseconds to wait between
  retries. This option is ignored if the `recursive` option is not `true`.
  **Default:** `100`.
@@ -1088,10 +1098,8 @@ Using `fsPromises.rmdir()` on a file (not a directory) results in the
 promise being rejected with an `ENOENT` error on Windows and an `ENOTDIR`
 error on POSIX.
 
-Setting `recursive` to `true` results in behavior similar to the Unix command
-`rm -rf`: an error will not be raised for paths that do not exist, and paths
-that represent files will be deleted. The `recursive` option is deprecated,
-`ENOTDIR` and `ENOENT` will be thrown in the future.
+To get a behavior similar to the `rm -rf` Unix command, use
+[`fsPromises.rm()`][] with options `{recursive: true, force: true }`.
 
 ### `fsPromises.rm(path[, options])`
 <!-- YAML
@@ -3146,6 +3154,16 @@ rename('oldFile.txt', 'newFile.txt', (err) =>{
 <!--YAML
 added: v0.0.2
 changes:
+- version:REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37216
+ description:"Using `fs.rmdir(path,{recursive: true })` on a `path` that is
+ a file is no longer permitted and results in an `ENOENT` error
+ on Windows and an `ENOTDIR` error on POSIX."
+- version:REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37216
+ description:"Using `fs.rmdir(path,{recursive: true })` on a `path` that
+ does not exist is no longer permitted and results in a `ENOENT`
+ error."
 - version:REPLACEME
  pr-url: https://github.com/nodejs/node/pull/37302
  description: The `recursive` option is deprecated, using it triggers a
@@ -3185,8 +3203,8 @@ changes:
  represents the number ofretries. This option is ignored if the `recursive`
  option is not `true`. **Default:**`0`.
 *`recursive`{boolean} If `true`, perform a recursive directory removal. In
- recursive mode, errors are not reported if`path` does not exist, and
-operations are retried on failure. **Default:**`false`. **Deprecated**.
+ recursive mode, operations are retried on failure. **Default:**`false`.
+**Deprecated**.
 *`retryDelay`{integer} The amount of time in milliseconds to wait between
 retries. This option is ignored if the `recursive` option is not `true`.
 **Default:**`100`.
@@ -3199,10 +3217,8 @@ to the completion callback.
 Using `fs.rmdir()` on a file (not a directory) results in an `ENOENT` error on
 Windows and an `ENOTDIR` error on POSIX.
 
-Setting `recursive` to `true` results in behavior similar to the Unix command
-`rm -rf`: an error will not be raised for paths that do not exist, and paths
-that represent files will be deleted. The`recursive` option is deprecated,
-`ENOTDIR` and `ENOENT` will be thrown in the future.
+To get a behavior similar to the `rm -rf` Unix command, use [`fs.rm()`][]
+with options `{recursive: true, force: true }`.
 
 ### `fs.rm(path[, options], callback)`
 <!--YAML
@@ -4777,6 +4793,16 @@ See the POSIX rename(2) documentation for more details.
 <!-- YAML
 added: v0.1.21
 changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37216
+ description: "Using `fs.rmdirSync(path,{recursive:true })` on a `path`
+ that is a file is no longer permitted and results in an
+`ENOENT` error on Windows and an `ENOTDIR` error on POSIX."
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37216
+ description: "Using `fs.rmdirSync(path,{recursive:true })` on a `path`
+ that does not exist is no longer permitted and results in a
+`ENOENT` error."
  - version: REPLACEME
  pr-url: https://github.com/nodejs/node/pull/37302
  description: The `recursive` option is deprecated, using it triggers a
@@ -4808,8 +4834,8 @@ changes:
  represents the number of retries. This option is ignored if the `recursive`
  option is not `true`. **Default:** `0`.
  * `recursive`{boolean} If `true`, perform a recursive directory removal. In
- recursive mode, errors are not reported if `path` does not exist, and
-operations are retried on failure. **Default:** `false`. **Deprecated**.
+ recursive mode, operations are retried on failure. **Default:** `false`.
+ **Deprecated**.
  * `retryDelay`{integer} The amount of time in milliseconds to wait between
  retries. This option is ignored if the `recursive` option is not `true`.
  **Default:** `100`.
@@ -4819,10 +4845,8 @@ Synchronous rmdir(2). Returns `undefined`.
 Using `fs.rmdirSync()` on a file (not a directory) results in an `ENOENT` error
 on Windows and an `ENOTDIR` error on POSIX.
 
-Setting `recursive` to `true` results in behavior similar to the Unix command
-`rm -rf`: an error will not be raised for paths that do not exist, and paths
-that represent files will be deleted. The `recursive` option is deprecated,
-`ENOTDIR` and `ENOENT` will be thrown in the future.
+To get a behavior similar to the `rm -rf` Unix command, use [`fs.rmSync()`][]
+with options `{recursive:true, force:true }`.
 
 ### `fs.rmSync(path[, options])`
 <!-- YAML
@@ -6670,6 +6694,8 @@ the file contents.
 [`fs.readdirSync()`]: #fs_fs_readdirsync_path_options
 [`fs.readv()`]: #fs_fs_readv_fd_buffers_position_callback
 [`fs.realpath()`]: #fs_fs_realpath_path_options_callback
+[`fs.rm()`]: #fs_fs_rm_path_options_callback
+[`fs.rmSync()`]: #fs_fs_rmsync_path_options
 [`fs.rmdir()`]: #fs_fs_rmdir_path_options_callback
 [`fs.stat()`]: #fs_fs_stat_path_options_callback
 [`fs.symlink()`]: #fs_fs_symlink_target_path_type_callback
@@ -6681,6 +6707,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.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
 [`kqueue(2)`]: https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2

lib/fs.js

@@ -898,15 +898,20 @@ function rmdir(path, options, callback){
 emitRecursiveRmdirWarning();
 validateRmOptions(
 path,
-{ ...options,force: true},
+{ ...options,force: false},
 true,
 (err,options)=>{
+if(err===false){
+constreq=newFSReqCallback();
+req.oncomplete=callback;
+returnbinding.rmdir(path,req);
+}
 if(err){
 returncallback(err);
 }
 
 lazyLoadRimraf();
-returnrimraf(path,options,callback);
+rimraf(path,options,callback);
 });
 }else{
 validateRmdirOptions(options);
@@ -921,12 +926,15 @@ function rmdirSync(path, options){
 
 if(options?.recursive){
 emitRecursiveRmdirWarning();
-options=validateRmOptionsSync(path,{ ...options,force: true},true);
-lazyLoadRimraf();
-returnrimrafSync(pathModule.toNamespacedPath(path),options);
+options=validateRmOptionsSync(path,{ ...options,force: false},true);
+if(options!==false){
+lazyLoadRimraf();
+returnrimrafSync(pathModule.toNamespacedPath(path),options);
+}
+}else{
+validateRmdirOptions(options);
 }
 
-validateRmdirOptions(options);
 constctx={ path };
 binding.rmdir(pathModule.toNamespacedPath(path),undefined,ctx);
 returnhandleErrorFromBinding(ctx);

lib/internal/fs/promises.js

@@ -505,7 +505,10 @@ async function rmdir(path, options){
 
 if(options.recursive){
 emitRecursiveRmdirWarning();
-returnrimrafPromises(path,options);
+conststats=awaitstat(path);
+if(stats.isDirectory()){
+returnrimrafPromises(path,options);
+}
 }
 
 returnbinding.rmdir(path,kUsePromises);

lib/internal/fs/utils.js

@@ -698,39 +698,47 @@ const defaultRmdirOptions ={
 recursive: false,
 };
 
-constvalidateRmOptions=hideStackFrames((path,options,warn,callback)=>{
+constvalidateRmOptions=hideStackFrames((path,options,expectDir,cb)=>{
 options=validateRmdirOptions(options,defaultRmOptions);
 validateBoolean(options.force,'options.force');
 
 lazyLoadFs().stat(path,(err,stats)=>{
 if(err){
 if(options.force&&err.code==='ENOENT'){
-returncallback(null,options);
+returncb(null,options);
 }
-returncallback(err,options);
+returncb(err,options);
+}
+
+if(expectDir&&!stats.isDirectory()){
+returncb(false);
 }
 
 if(stats.isDirectory()&&!options.recursive){
-returncallback(newERR_FS_EISDIR({
+returncb(newERR_FS_EISDIR({
 code: 'EISDIR',
 message: 'is a directory',
  path,
 syscall: 'rm',
 errno: EISDIR
 }));
 }
-returncallback(null,options);
+returncb(null,options);
 });
 });
 
-constvalidateRmOptionsSync=hideStackFrames((path,options,warn)=>{
+constvalidateRmOptionsSync=hideStackFrames((path,options,expectDir)=>{
 options=validateRmdirOptions(options,defaultRmOptions);
 validateBoolean(options.force,'options.force');
 
-if(!options.force||warn||!options.recursive){
+if(!options.force||expectDir||!options.recursive){
 constisDirectory=lazyLoadFs()
 .statSync(path,{throwIfNoEntry: !options.force})?.isDirectory();
 
+if(expectDir&&!isDirectory){
+returnfalse;
+}
+
 if(isDirectory&&!options.recursive){
 thrownewERR_FS_EISDIR({
 code: 'EISDIR',

test/parallel/test-fs-rmdir-recursive-sync-warns-not-found.js

@@ -1,6 +1,7 @@
 'use strict';
 constcommon=require('../common');
 consttmpdir=require('../common/tmpdir');
+constassert=require('assert');
 constfs=require('fs');
 constpath=require('path');
 
@@ -14,5 +15,9 @@ tmpdir.refresh();
 'will be removed. Use fs.rm(path,{recursive: true }) instead',
 'DEP0147'
 );
-fs.rmdirSync(path.join(tmpdir.path,'noexist.txt'),{recursive: true});
+assert.throws(
+()=>fs.rmdirSync(path.join(tmpdir.path,'noexist.txt'),
+{recursive: true}),
+{code: 'ENOENT'}
+);
 }

test/parallel/test-fs-rmdir-recursive-sync-warns-on-file.js

@@ -1,13 +1,13 @@
 'use strict';
 constcommon=require('../common');
 consttmpdir=require('../common/tmpdir');
+constassert=require('assert');
 constfs=require('fs');
 constpath=require('path');
 
 tmpdir.refresh();
 
 {
-// Should warn when trying to delete a file
 common.expectWarning(
 'DeprecationWarning',
 'In future versions of Node.js, fs.rmdir(path,{recursive: true }) '+
@@ -16,5 +16,8 @@ tmpdir.refresh();
 );
 constfilePath=path.join(tmpdir.path,'rmdir-recursive.txt');
 fs.writeFileSync(filePath,'');
-fs.rmdirSync(filePath,{recursive: true});
+assert.throws(
+()=>fs.rmdirSync(filePath,{recursive: true}),
+{code: common.isWindows ? 'ENOENT' : 'ENOTDIR'}
+);
 }

test/parallel/test-fs-rmdir-recursive-throws-not-found.js

@@ -0,0 +1,36 @@
+'use strict';
+constcommon=require('../common');
+consttmpdir=require('../common/tmpdir');
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+
+tmpdir.refresh();
+
+{
+assert.throws(
+()=>
+fs.rmdirSync(path.join(tmpdir.path,'noexist.txt'),{recursive: true}),
+{
+code: 'ENOENT',
+}
+);
+}
+{
+fs.rmdir(
+path.join(tmpdir.path,'noexist.txt'),
+{recursive: true},
+common.mustCall((err)=>{
+assert.strictEqual(err.code,'ENOENT');
+})
+);
+}
+{
+assert.rejects(
+()=>fs.promises.rmdir(path.join(tmpdir.path,'noexist.txt'),
+{recursive: true}),
+{
+code: 'ENOENT',
+}
+).then(common.mustCall());
+}

test/parallel/test-fs-rmdir-recursive-throws-on-file.js

@@ -0,0 +1,29 @@
+'use strict';
+constcommon=require('../common');
+consttmpdir=require('../common/tmpdir');
+constassert=require('assert');
+constfs=require('fs');
+constpath=require('path');
+
+tmpdir.refresh();
+
+constcode=common.isWindows ? 'ENOENT' : 'ENOTDIR';
+
+{
+constfilePath=path.join(tmpdir.path,'rmdir-recursive.txt');
+fs.writeFileSync(filePath,'');
+assert.throws(()=>fs.rmdirSync(filePath,{recursive: true}),{ code });
+}
+{
+constfilePath=path.join(tmpdir.path,'rmdir-recursive.txt');
+fs.writeFileSync(filePath,'');
+fs.rmdir(filePath,{recursive: true},common.mustCall((err)=>{
+assert.strictEqual(err.code,code);
+}));
+}
+{
+constfilePath=path.join(tmpdir.path,'rmdir-recursive.txt');
+fs.writeFileSync(filePath,'');
+assert.rejects(()=>fs.promises.rmdir(filePath,{recursive: true}),
+{ code }).then(common.mustCall());
+}

test/parallel/test-fs-rmdir-recursive-warns-on-file.js

@@ -1,13 +1,13 @@
 'use strict';
 constcommon=require('../common');
 consttmpdir=require('../common/tmpdir');
+constassert=require('assert');
 constfs=require('fs');
 constpath=require('path');
 
 tmpdir.refresh();
 
 {
-// Should warn when trying to delete a file
 common.expectWarning(
 'DeprecationWarning',
 'In future versions of Node.js, fs.rmdir(path,{recursive: true }) '+
@@ -16,5 +16,7 @@ tmpdir.refresh();
 );
 constfilePath=path.join(tmpdir.path,'rmdir-recursive.txt');
 fs.writeFileSync(filePath,'');
-fs.rmdir(filePath,{recursive: true},common.mustSucceed());
+fs.rmdir(filePath,{recursive: true},common.mustCall((err)=>{
+assert.strictEqual(err.code,common.isWindows ? 'ENOENT' : 'ENOTDIR');
+}));
 }