fs: runtime deprecate rmdir recursive option

PR-URL: #37302 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Anto Aravinth <[email protected]>

Author: aduh95

doc/api/deprecations.md

@@ -2671,19 +2671,25 @@ The [`crypto.Certificate()` constructor][] is deprecated. Use
 ### DEP0147: `fs.rmdir(path,{recursive: true })`
 <!-- YAML
 changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37302
+ description: Runtime deprecation.
  - version: v15.0.0
  pr-url: https://github.com/nodejs/node/pull/35562
- description: Runtime deprecation.
+ description: Runtime deprecation for permissive behavior.
  - version: v14.14.0
  pr-url: https://github.com/nodejs/node/pull/35579
  description: Documentation-only deprecation.
 -->
 
 Type: Runtime
 
-In future versions of Node.js, `fs.rmdir(path,{recursive: true })` will throw
-if `path` does not exist or is a file.
-Use `fs.rm(path,{recursive: true, force: true })` instead.
+In future versions of Node.js, `recursive` option will be ignored for
+`fs.rmdir`, `fs.rmdirSync`, and `fs.promises.rmdir`.
+
+Use `fs.rm(path,{recursive: true, force: true })`,
+`fs.rmSync(path,{recursive: true, force: true })` or
+`fs.promises.rm(path,{recursive: true, force: true })` instead.
 
 ### DEP0148: Folder mappings in `"exports"` (trailing `"/"`)
 <!-- YAML

doc/api/fs.md

@@ -1048,6 +1048,10 @@ Renames `oldPath` to `newPath`.
 <!-- YAML
 added: v10.0.0
 changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37302
+ description: The `recursive` option is deprecated, using it triggers a
+ deprecation warning.
  - version:
  - v13.3.0
  - v12.16.0
@@ -1072,7 +1076,7 @@ changes:
  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`.
+ 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`.
@@ -1086,9 +1090,8 @@ 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 permissive behavior of the
-`recursive` option is deprecated, `ENOTDIR` and `ENOENT` will be thrown in
-the future.
+that represent files will be deleted. The `recursive` option is deprecated,
+`ENOTDIR` and `ENOENT` will be thrown in the future.
 
 ### `fsPromises.rm(path[, options])`
 <!-- YAML
@@ -3135,6 +3138,10 @@ rename('oldFile.txt', 'newFile.txt', (err) =>{
 <!--YAML
 added: v0.0.2
 changes:
+- version:REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/37302
+ description: The `recursive` option is deprecated, using it triggers a
+ deprecation warning.
 - version:
 - v13.3.0
 - v12.16.0
@@ -3171,7 +3178,7 @@ changes:
  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`.
+ 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`.
@@ -3186,9 +3193,8 @@ 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 permissive behavior of the
-`recursive` option is deprecated, `ENOTDIR` and `ENOENT` will be thrown in
-the future.
+that represent files will be deleted. The`recursive` option is deprecated,
+`ENOTDIR` and `ENOENT` will be thrown in the future.
 
 ### `fs.rm(path[, options], callback)`
 <!--YAML
@@ -4755,6 +4761,10 @@ 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/37302
+ description: The `recursive` option is deprecated, using it triggers a
+ deprecation warning.
  - version:
  - v13.3.0
  - v12.16.0
@@ -4783,7 +4793,7 @@ changes:
  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`.
+ 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`.
@@ -4795,9 +4805,8 @@ 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 permissive behavior of the
-`recursive` option is deprecated, `ENOTDIR` and `ENOENT` will be thrown in
-the future.
+that represent files will be deleted. The `recursive` option is deprecated,
+`ENOTDIR` and `ENOENT` will be thrown in the future.
 
 ### `fs.rmSync(path[, options])`
 <!-- YAML

lib/fs.js

@@ -91,6 +91,7 @@ const internalUtil = require('internal/util');
 const{
  copyObject,
  Dirent,
+ emitRecursiveRmdirWarning,
  getDirents,
  getOptions,
  getValidatedFd,
@@ -893,6 +894,7 @@ function rmdir(path, options, callback){
 path=pathModule.toNamespacedPath(getValidatedPath(path));
 
 if(options?.recursive){
+emitRecursiveRmdirWarning();
 validateRmOptions(
 path,
 { ...options,force: true},
@@ -917,6 +919,7 @@ function rmdirSync(path, options){
 path=getValidatedPath(path);
 
 if(options?.recursive){
+emitRecursiveRmdirWarning();
 options=validateRmOptionsSync(path,{ ...options,force: true},true);
 lazyLoadRimraf();
 returnrimrafSync(pathModule.toNamespacedPath(path),options);

lib/internal/fs/promises.js

@@ -46,6 +46,7 @@ const{isArrayBufferView } = require('internal/util/types');
 const{ rimrafPromises }=require('internal/fs/rimraf');
 const{
  copyObject,
+ emitRecursiveRmdirWarning,
  getDirents,
  getOptions,
  getStatsFromBinding,
@@ -503,6 +504,7 @@ async function rmdir(path, options){
 options=validateRmdirOptions(options);
 
 if(options.recursive){
+emitRecursiveRmdirWarning();
 returnrimrafPromises(path,options);
 }
 

lib/internal/fs/utils.js

@@ -705,18 +705,11 @@ const validateRmOptions = hideStackFrames((path, options, warn, callback) =>{
 lazyLoadFs().stat(path,(err,stats)=>{
 if(err){
 if(options.force&&err.code==='ENOENT'){
-if(warn){
-emitPermissiveRmdirWarning();
-}
 returncallback(null,options);
 }
 returncallback(err,options);
 }
 
-if(warn&&!stats.isDirectory()){
-emitPermissiveRmdirWarning();
-}
-
 if(stats.isDirectory()&&!options.recursive){
 returncallback(newERR_FS_EISDIR({
 code: 'EISDIR',
@@ -738,10 +731,6 @@ const validateRmOptionsSync = hideStackFrames((path, options, warn) =>{
 constisDirectory=lazyLoadFs()
 .statSync(path,{throwIfNoEntry: !options.force})?.isDirectory();
 
-if(warn&&!isDirectory){
-emitPermissiveRmdirWarning();
-}
-
 if(isDirectory&&!options.recursive){
 thrownewERR_FS_EISDIR({
 code: 'EISDIR',
@@ -756,18 +745,16 @@ const validateRmOptionsSync = hideStackFrames((path, options, warn) =>{
 returnoptions;
 });
 
-letpermissiveRmdirWarned=false;
-
-functionemitPermissiveRmdirWarning(){
-if(!permissiveRmdirWarned){
+letrecursiveRmdirWarned=process.noDeprecation;
+functionemitRecursiveRmdirWarning(){
+if(!recursiveRmdirWarned){
 process.emitWarning(
 'In future versions of Node.js, fs.rmdir(path,{recursive: true }) '+
-'will throw if path does not exist or is a file. Use fs.rm(path, '+
-'{recursive: true, force: true }) instead',
+'will be removed. Use fs.rm(path,{recursive: true }) instead',
 'DeprecationWarning',
 'DEP0147'
 );
-permissiveRmdirWarned=true;
+recursiveRmdirWarned=true;
 }
 }
 
@@ -852,6 +839,7 @@ module.exports ={
  BigIntStats,// for testing
  copyObject,
  Dirent,
+ emitRecursiveRmdirWarning,
  getDirent,
  getDirents,
  getOptions,

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

@@ -11,8 +11,7 @@ tmpdir.refresh();
 common.expectWarning(
 'DeprecationWarning',
 'In future versions of Node.js, fs.rmdir(path,{recursive: true }) '+
-'will throw if path does not exist or is a file. Use fs.rm(path, '+
-'{recursive: true, force: true }) instead',
+'will be removed. Use fs.rm(path,{recursive: true }) instead',
 'DEP0147'
 );
 fs.rmdirSync(path.join(tmpdir.path,'noexist.txt'),{recursive: true});

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

@@ -11,8 +11,7 @@ tmpdir.refresh();
 common.expectWarning(
 'DeprecationWarning',
 'In future versions of Node.js, fs.rmdir(path,{recursive: true }) '+
-'will throw if path does not exist or is a file. Use fs.rm(path, '+
-'{recursive: true, force: true }) instead',
+'will be removed. Use fs.rm(path,{recursive: true }) instead',
 'DEP0147'
 );
 constfilePath=path.join(tmpdir.path,'rmdir-recursive.txt');

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

@@ -11,8 +11,7 @@ tmpdir.refresh();
 common.expectWarning(
 'DeprecationWarning',
 'In future versions of Node.js, fs.rmdir(path,{recursive: true }) '+
-'will throw if path does not exist or is a file. Use fs.rm(path, '+
-'{recursive: true, force: true }) instead',
+'will be removed. Use fs.rm(path,{recursive: true }) instead',
 'DEP0147'
 );
 fs.rmdir(

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

@@ -11,8 +11,7 @@ tmpdir.refresh();
 common.expectWarning(
 'DeprecationWarning',
 'In future versions of Node.js, fs.rmdir(path,{recursive: true }) '+
-'will throw if path does not exist or is a file. Use fs.rm(path, '+
-'{recursive: true, force: true }) instead',
+'will be removed. Use fs.rm(path,{recursive: true }) instead',
 'DEP0147'
 );
 constfilePath=path.join(tmpdir.path,'rmdir-recursive.txt');

test/parallel/test-fs-rmdir-recursive.js

@@ -7,6 +7,13 @@ const fs = require('fs');
 constpath=require('path');
 const{ validateRmdirOptions }=require('internal/fs/utils');
 
+common.expectWarning(
+'DeprecationWarning',
+'In future versions of Node.js, fs.rmdir(path,{recursive: true }) '+
+'will be removed. Use fs.rm(path,{recursive: true }) instead',
+'DEP0147'
+);
+
 tmpdir.refresh();
 
 letcount=0;