`walk` and `walkSync` skip over symlinks when `followSymlinks: false`

[lucacasonato]

Instead I would expect symlinks to be yielded just like files: if includeSymlinks is set & the symlink matches the exts, match, and skip options.

[iwinux]

Forget the deleted comment, my mistake.

The "if not follow symlink then continue (skip)" logic actually dates back to the initial fs.walk implementation (see https://github.com/denoland/deno_std/pull/192), and gets carried over since then.

Would someone who knows the background explain the decision behind this? It seems like an easy fix (just remove the continue branch) but I'm afraid that something might break if we change that.

[joehillen]

Here is some more context from the duplicate issue I posed:

This is really 2 issues:

1. no symbolic link files are returned when using walk(). See below to reproduce.

2. when using followSymlinks: true, the path to the destination is returned and labeled { isFile: false, isSymlink: true }, even though the destination is a regular file.

Steps to Reproduce

// walkTest.ts

import * as fs from "https://deno.land/[email protected]/fs/mod.ts";

fs.emptyDirSync("dir");

fs.ensureFileSync("dir/files/a.txt");
Deno.writeTextFileSync("dir/files/a.txt", "A");

// FIXME: See https://github.com/denoland/deno_std/issues/2312
// fs.ensureSymlinkSync("../files/a.txt", "dir/links/a.txt");

fs.ensureDirSync("dir/links");
await Deno.run({
  cmd: ["ln", "-s", "../files/a.txt", "dir/links/a.txt"],
}).status();

/*
  dir
  ├── files
  │   └── a.txt
  └── links
      └── a.txt -> ../files/a.txt
*/

console.log(">>> followSymlinks: false >>>>");
for (const f of fs.walkSync("dir", { followSymlinks: false })) {
  console.log(f); // dir/links/a.txt is not returned
}

console.log(">>> followSymlinks: true >>>>");
for (const f of fs.walkSync("dir", { followSymlinks: true })) {
  console.log(f);
  // returns:
  //  {
  //    path: "/home/joe/tmp/walkTest/dir/files/a.txt", <-- this is the path to the destination, not the link file.
  //    name: "a.txt",
  //    isFile: false,
  //    isDirectory: false,
  //    isSymlink: true <-- "path" is a file, not a symlink
  //  }
}

Expected behavior

fs.walkSync("dir", { followSymlinks: false } should include:

{
  path: "dir/links/a.txt",
  name: "a.txt",
  isFile: true,
  isDirectory: false,
  isSymlink: true
}

fs.walkSync("dir", { followSymlinks: true }) should return:

{
  path: "/home/joe/tmp/walkTest/dir/files/a.txt",
  name: "a.txt",
  isFile: true,
  isDirectory: false,
  isSymlink: false
}

[kt3k]

fixed in #3464

Now includeSymlinks defaults to true. Is there any concern on this default?