Merge pull request #1975 from SAY-5/fix/pattern-path-separator-v2

fix: flag any pattern containing a path separator, not just ones naming a directory

Author: tmccombs

CHANGELOG.md

@@ -5,6 +5,7 @@
 
 ## Bugfixes
 - Handle invalid working directories gracefully when using `--full-path`, see #1900 (@Xavrir).
+- Fire the "search pattern contains a path separator" diagnostic for any pattern containing `/`, not just patterns that happen to name an existing directory. Preserves the legacy Windows behaviour that also flags native `\` separators when the pattern resolves to a real directory. See #1873.
 
 # 10.4.2
 

src/main.rs

@@ -145,21 +145,56 @@ fn set_working_dir(opts: &Opts) -> Result<()> {
     Ok(())
 }
 
-/// Detect if the user accidentally supplied a path instead of a search pattern
+/// Detect if the user accidentally supplied a path instead of a search pattern.
+///
+/// Without `--full-path`, fd matches patterns against file names, so any pattern
+/// containing a path separator can never match. Two cases are worth a friendly
+/// error rather than silent "no results":
+///
+/// 1. The pattern contains '/'. '/' is always a path separator (including on
+///    Windows) and has no regex meaning, so flagging it is safe and catches the
+///    common Linux/macOS mistake of pasting a full path as the pattern.
+/// 2. On Windows only, the pattern contains the native `\` separator *and*
+///    names an existing directory on disk. We can't treat `\` as a pure
+///    path-separator signal there because it is also the regex escape char,
+///    so valid regex patterns like `\Ac` or `\d+` must still run. Requiring
+///    that the pattern resolves to a real directory avoids those false
+///    positives while preserving the legacy diagnostic for operators who
+///    literally typed a directory path.
+///
+/// See https://github.com/sharkdp/fd/issues/1873.
 fn ensure_search_pattern_is_not_a_path(opts: &Opts) -> Result<()> {
-    if !opts.full_path
-        && opts.pattern.contains(std::path::MAIN_SEPARATOR)
-        && Path::new(&opts.pattern).is_dir()
+    if opts.full_path {
+        return Ok(());
+    }
+
+    // Start with the cheap check: '/' is always a path separator, including on
+    // Windows, and has no regex meaning, so flagging it is safe and catches the
+    // Linux/macOS mistake of pasting a full path as the pattern.
+    #[cfg_attr(not(windows), allow(unused_mut))]
+    let mut should_warn = opts.pattern.contains('/');
+
+    // On Windows we additionally accept the native `\` separator, but only when
+    // the pattern actually resolves to an existing directory - `\` is also the
+    // regex escape char there, so valid patterns like `\Ac` or `\d+` must still
+    // run. The is_dir syscall is only needed when `should_warn` is still false,
+    // so short-circuit via `||` to avoid the stat call on the happy path.
+    #[cfg(windows)]
     {
+        should_warn = should_warn
+            || (opts.pattern.contains(std::path::MAIN_SEPARATOR)
+                && Path::new(&opts.pattern).is_dir());
+    }
+
+    if should_warn {
         Err(anyhow!(
-            "The search pattern '{pattern}' contains a path-separation character ('{sep}') \
+            "The search pattern '{pattern}' contains a path-separation character \
              and will not lead to any search results.\n\n\
              If you want to search for all files inside the '{pattern}' directory, use a match-all pattern:\n\n  \
              fd . '{pattern}'\n\n\
              Instead, if you want your pattern to match the full file path, use:\n\n  \
              fd --full-path '{pattern}'",
             pattern = &opts.pattern,
-            sep = std::path::MAIN_SEPARATOR,
         ))
     } else {
         Ok(())

tests/tests.rs

@@ -374,6 +374,49 @@ fn test_multi_file_with_missing() {
     );
 }
 
+/// Without --full-path, a pattern containing '/' should always produce the
+/// path-separator diagnostic, even if the pattern does not name an existing
+/// directory. Before the fix for sharkdp/fd#1873 this only fired when the
+/// pattern happened to resolve to a real directory, so the common typo of
+/// pasting a full path silently returned zero matches.
+#[test]
+fn test_pattern_with_forward_slash_is_rejected() {
+    let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);
+
+    // Pattern that is NOT a real directory; old behaviour: no warning.
+    te.assert_failure_with_error(
+        &["nonexistent/path"],
+        "[fd error]: The search pattern 'nonexistent/path' contains a path-separation character and will not lead to any search results.",
+    );
+
+    // Pattern that IS a real directory; old behaviour: warning. Must still fire.
+    te.assert_failure_with_error(
+        &["one/two/three"],
+        "[fd error]: The search pattern 'one/two/three' contains a path-separation character and will not lead to any search results.",
+    );
+}
+
+/// --full-path is the user's explicit opt-in to regex-over-full-path matching,
+/// so a path-separation character in the pattern is expected and must not
+/// trigger the diagnostic.
+///
+/// Gated off Windows: the actual match is regex-over-the-full-path, so a
+/// forward-slash pattern only matches Unix-style paths. On Windows the OS
+/// uses `\` and `one/two/c` (as a literal regex) does not match a real
+/// entry — the behaviour this test is pinning (the diagnostic does not
+/// fire) is covered by the fact that the invocation does not error.
+#[test]
+#[cfg(not(windows))]
+fn test_pattern_with_forward_slash_allowed_with_full_path() {
+    let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);
+
+    te.assert_output(
+        &["--full-path", "one/two/c"],
+        "one/two/c.foo
+        one/two/C.Foo2",
+    );
+}
+
 /// Explicit root path
 #[test]
 fn test_explicit_root_path() {