fs: add directory autodetection to fsPromises.symlink()

LiviaMedeiros · web-flow · commit a4fa526ddcaf · 2022-10-03T01:04:28.000Z
PR-URL: #42894 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -1463,18 +1463,27 @@ changes:
 
 <!-- YAML
 added: v10.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/42894
+    description: If the `type` argument is `null` or omitted, Node.js will
+                 autodetect `target` type and automatically
+                 select `dir` or `file`.
+
 -->
 
 * `target` {string|Buffer|URL}
 * `path` {string|Buffer|URL}
-* `type` {string} **Default:** `'file'`
+* `type` {string|null} **Default:** `null`
 * Returns: {Promise} Fulfills with `undefined` upon success.
 
 Creates a symbolic link.
 
 The `type` argument is only used on Windows platforms and can be one of `'dir'`,
-`'file'`, or `'junction'`. Windows junction points require the destination path
-to be absolute. When using `'junction'`, the `target` argument will
+`'file'`, or `'junction'`. If the `type` argument is not a string, Node.js will
+autodetect `target` type and use `'file'` or `'dir'`. If the `target` does not
+exist, `'file'` will be used. Windows junction points require the destination
+path to be absolute. When using `'junction'`, the `target` argument will
 automatically be normalized to absolute path.
 
 ### `fsPromises.truncate(path[, len])`
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -117,6 +117,8 @@ const {
 const getDirectoryEntriesPromise = promisify(getDirents);
 const validateRmOptionsPromise = promisify(validateRmOptions);
 
+const isWindows = process.platform === 'win32';
+
 let cpPromises;
 function lazyLoadCpPromises() {
   return cpPromises ??= require('internal/fs/cp/cp').cpFn;
@@ -714,7 +716,16 @@ async function readlink(path, options) {
 }
 
 async function symlink(target, path, type_) {
-  const type = (typeof type_ === 'string' ? type_ : null);
+  let type = (typeof type_ === 'string' ? type_ : null);
+  if (isWindows && type === null) {
+    try {
+      const absoluteTarget = pathModule.resolve(`${path}`, '..', `${target}`);
+      type = (await stat(absoluteTarget)).isDirectory() ? 'dir' : 'file';
+    } catch {
+      // Default to 'file' if path is invalid or file does not exist
+      type = 'file';
+    }
+  }
   target = getValidatedPath(target, 'target');
   path = getValidatedPath(path);
   return binding.symlink(preprocessSymlinkDestination(target, type, path),
diff --git a/test/parallel/test-fs-symlink-dir.js b/test/parallel/test-fs-symlink-dir.js
@@ -12,6 +12,7 @@ if (!common.canCreateSymLink())
 const assert = require('assert');
 const path = require('path');
 const fs = require('fs');
+const fsPromises = fs.promises;
 
 const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
@@ -36,11 +37,18 @@ function testAsync(target, path) {
   }));
 }
 
+async function testPromises(target, path) {
+  await fsPromises.symlink(target, path);
+  fs.readdirSync(path);
+}
+
 for (const linkTarget of linkTargets) {
   fs.mkdirSync(path.resolve(tmpdir.path, linkTarget));
   for (const linkPath of linkPaths) {
     testSync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-sync`);
     testAsync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-async`);
+    testPromises(linkTarget, `${linkPath}-${path.basename(linkTarget)}-promises`)
+      .then(common.mustCall());
   }
 }
 
@@ -57,10 +65,17 @@ for (const linkTarget of linkTargets) {
     }));
   }
 
+  async function testPromises(target, path) {
+    await fsPromises.symlink(target, path);
+    assert(!fs.existsSync(path));
+  }
+
   for (const linkTarget of linkTargets.map((p) => p + '-broken')) {
     for (const linkPath of linkPaths) {
       testSync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-sync`);
       testAsync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-async`);
+      testPromises(linkTarget, `${linkPath}-${path.basename(linkTarget)}-promises`)
+        .then(common.mustCall());
     }
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ if (!common.canCreateSymLink())`
`12`	`12`	`const assert = require('assert');`
`13`	`13`	`const path = require('path');`
`14`	`14`	`const fs = require('fs');`
	`15`	`+const fsPromises = fs.promises;`
`15`	`16`
`16`	`17`	`const tmpdir = require('../common/tmpdir');`
`17`	`18`	`tmpdir.refresh();`
`@@ -36,11 +37,18 @@ function testAsync(target, path) {`
`36`	`37`	`}));`
`37`	`38`	`}`
`38`	`39`
	`40`	`+async function testPromises(target, path) {`
	`41`	`+ await fsPromises.symlink(target, path);`
	`42`	`+ fs.readdirSync(path);`
	`43`	`+}`
	`44`	`+`
`39`	`45`	`for (const linkTarget of linkTargets) {`
`40`	`46`	`fs.mkdirSync(path.resolve(tmpdir.path, linkTarget));`
`41`	`47`	`for (const linkPath of linkPaths) {`
`42`	`48`	testSync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-sync`);
`43`	`49`	testAsync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-async`);
	`50`	+ testPromises(linkTarget, `${linkPath}-${path.basename(linkTarget)}-promises`)
	`51`	`+ .then(common.mustCall());`
`44`	`52`	`}`
`45`	`53`	`}`
`46`	`54`
`@@ -57,10 +65,17 @@ for (const linkTarget of linkTargets) {`
`57`	`65`	`}));`
`58`	`66`	`}`
`59`	`67`
	`68`	`+ async function testPromises(target, path) {`
	`69`	`+ await fsPromises.symlink(target, path);`
	`70`	`+ assert(!fs.existsSync(path));`
	`71`	`+ }`
	`72`	`+`
`60`	`73`	`for (const linkTarget of linkTargets.map((p) => p + '-broken')) {`
`61`	`74`	`for (const linkPath of linkPaths) {`
`62`	`75`	testSync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-sync`);
`63`	`76`	testAsync(linkTarget, `${linkPath}-${path.basename(linkTarget)}-async`);
	`77`	+ testPromises(linkTarget, `${linkPath}-${path.basename(linkTarget)}-promises`)
	`78`	`+ .then(common.mustCall());`
`64`	`79`	`}`
`65`	`80`	`}`
`66`	`81`	`}`