bug: fix contents_first with root symlink

This fixes two incorrect behaviors when `contents_first` is on and the
root path is a symlink:

  1. The root path was returned first in the iterator instead of last.
  2. Some subdirectories were returned out of order.

The issue was that root symlinks were returned immediately rather than
being pushed onto the `deferred_dirs` vec. That lead to `deferred_dirs`
and `depth` being out of sync, which lead to deferred directories being
processed one ascent too late.

TO DO:
  [ ] Better tests (the current ones are not stable)
  [ ] Remove FIXME
  [ ] Consider commenting changed code; it seems somewhat hard to follow
      to my sleep deprived brain.

Fixes: BurntSushi#163

src/lib.rs

@@ -818,19 +818,14 @@ impl IntoIter {
         &mut self,
         mut dent: DirEntry,
     ) -> Option<Result<DirEntry>> {
-        if self.opts.follow_links && dent.file_type().is_symlink() {
+        let should_descend = if !dent.file_type().is_symlink() {
+            dent.is_dir()
+        } else if self.opts.follow_links {
             dent = itry!(self.follow(dent));
-        }
-        let is_normal_dir = !dent.file_type().is_symlink() && dent.is_dir();
-        if is_normal_dir {
-            if self.opts.same_file_system && dent.depth() > 0 {
-                if itry!(self.is_same_file_system(&dent)) {
-                    itry!(self.push(&dent));
-                }
-            } else {
-                itry!(self.push(&dent));
-            }
-        } else if dent.depth() == 0 && dent.file_type().is_symlink() {
+            // FIXME need !dent.file_type().is_symlink()? Seems like follow()
+            // should prevent this from being both a symlink and a dir.
+            !dent.file_type().is_symlink() && dent.is_dir()
+        } else if dent.depth() == 0 {
             // As a special case, if we are processing a root entry, then we
             // always follow it even if it's a symlink and follow_links is
             // false. We are careful to not let this change the semantics of
@@ -841,11 +836,20 @@ impl IntoIter {
             let md = itry!(fs::metadata(dent.path()).map_err(|err| {
                 Error::from_path(dent.depth(), dent.path().to_path_buf(), err)
             }));
-            if md.file_type().is_dir() {
+            md.file_type().is_dir()
+        } else {
+            false
+        };
+        if should_descend {
+            if self.opts.same_file_system && dent.depth() > 0 {
+                if itry!(self.is_same_file_system(&dent)) {
+                    itry!(self.push(&dent));
+                }
+            } else {
                 itry!(self.push(&dent));
             }
         }
-        if is_normal_dir && self.opts.contents_first {
+        if should_descend && self.opts.contents_first {
             self.deferred_dirs.push(dent);
             None
         } else if self.skippable() {

src/tests/recursive.rs

@@ -454,6 +454,76 @@ fn sym_root_dir_follow() {
     assert!(!link_zzz.path_is_symlink());
 }
 
+#[test]
+fn sym_root_dir_nofollow_problem_no() {
+    let dir = Dir::tmp();
+    dir.mkdirp("dir/mdir/ndir");
+    dir.symlink_dir("dir", "link");
+    dir.touch("dir/afile");
+
+    let wd = WalkDir::new(dir.join("link"));
+    let r = dir.run_recursive(wd);
+    r.assert_no_errors();
+
+    let ents = r.ents();
+    assert_eq!(4, ents.len());
+    // DirEntry(".../link")
+    // DirEntry(".../link/mdir")
+    // DirEntry(".../link/mdir/ndir")
+    // DirEntry(".../link/afile")
+
+    let link = &ents[0];
+    assert_eq!(dir.join("link"), link.path());
+    assert!(link.path_is_symlink());
+    assert_eq!(dir.join("dir"), fs::read_link(link.path()).unwrap());
+
+    let link_mdir = &ents[1];
+    assert_eq!(dir.join("link").join("mdir"), link_mdir.path());
+    assert!(!link_mdir.path_is_symlink());
+
+    let link_mdir_ndir = &ents[2];
+    assert_eq!(dir.join("link").join("mdir").join("ndir"), link_mdir_ndir.path());
+    assert!(!link_mdir.path_is_symlink());
+
+    let link_afile = &ents[3];
+    assert_eq!(dir.join("link").join("afile"), link_afile.path());
+    assert!(!link_mdir.path_is_symlink());
+}
+
+#[test]
+fn sym_root_dir_nofollow_problem_yes() {
+    let dir = Dir::tmp();
+    dir.mkdirp("dir/mdir/ndir");
+    dir.symlink_dir("dir", "link");
+    dir.touch("dir/afile");
+
+    let wd = WalkDir::new(dir.join("link")).contents_first(true);
+    let r = dir.run_recursive(wd);
+    r.assert_no_errors();
+
+    let ents = r.ents();
+    assert_eq!(4, ents.len());
+
+    // DirEntry(".../link/mdir/ndir")
+    // DirEntry(".../link/mdir")
+    // DirEntry(".../link/afile")
+    // DirEntry(".../link")
+
+    let link_mdir_ndir = &ents[0];
+    assert_eq!(dir.join("link").join("mdir").join("ndir"), link_mdir_ndir.path());
+
+    let link_mdir = &ents[1];
+    assert_eq!(dir.join("link").join("mdir"), link_mdir.path());
+
+    let link_afile = &ents[2];
+    assert_eq!(dir.join("link").join("afile"), link_afile.path());
+
+    let link = &ents[3];
+    assert_eq!(dir.join("link"), link.path());
+    assert!(link.path_is_symlink());
+    assert_eq!(dir.join("dir"), fs::read_link(link.path()).unwrap());
+}
+
 #[test]
 fn sym_file_nofollow() {
     let dir = Dir::tmp();