Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- 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.
- Also fire the "search pattern contains a path separator" diagnostic for `--and` patterns, not only the primary positional pattern. `--and` patterns are matched against the file name just like the primary pattern, so a path separator in them silently returned zero results. See #1873.
- Fix bug where passing "-" as a directory argument didn't actually search that directory, see #849 (@Sean-Kenneth-Doherty).
- Fix `--glob` patterns containing a path separator never matching on Windows, where candidate paths keep their backslashes at match time, see #2067.

# 10.4.2

Expand Down
92 changes: 90 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,60 @@ fn ensure_single_search_pattern_is_not_a_path(pattern: &str) -> Result<()> {
}
}

/// Rewrite a globset-generated regex so that either `/` or `\` is accepted wherever
/// globset emitted a path separator. Windows candidate paths keep their backslashes
/// at match time, so a regex that only accepts `/` never matches.
///
/// Separators inside a character class have to be handled differently from the ones
/// outside it: inside, the backslash joins the existing members (`[^/]` -> `[^/\\]`),
/// outside, the separator becomes a class of its own (`a/b` -> `a[/\\]b`).
fn accept_either_separator(regex: &str) -> String {
let mut out = String::with_capacity(regex.len());
let mut chars = regex.chars();
let mut in_class = false;

while let Some(c) = chars.next() {
match c {
// Keep escape sequences intact so an escaped `[` doesn't open a class.
'\\' => {
out.push(c);
if let Some(escaped) = chars.next() {
out.push(escaped);
}
}
'[' if !in_class => {
in_class = true;
out.push(c);
}
']' if in_class => {
in_class = false;
out.push(c);
}
'/' if in_class => out.push_str("/\\\\"),
'/' => out.push_str("[/\\\\]"),
_ => out.push(c),
}
}

out
}

fn build_pattern_regex(pattern: &str, opts: &Opts) -> Result<String> {
Ok(if opts.glob && !pattern.is_empty() {
let glob = GlobBuilder::new(pattern).literal_separator(true).build()?;
glob.regex().to_owned()
let pattern_str = if cfg!(windows) {
pattern.replace('\\', "/")
} else {
pattern.to_string()
};
let glob = GlobBuilder::new(&pattern_str)
.literal_separator(true)
.build()?;
let regex_str = glob.regex();
if cfg!(windows) {
accept_either_separator(regex_str)
} else {
regex_str.to_owned()
}
} else if opts.exact {
// Anchor the escaped pattern so the full filename (or path) must match exactly.
// Literal. No substring matching.
Expand Down Expand Up @@ -553,3 +603,41 @@ fn build_regex(pattern_regex: String, config: &Config) -> Result<regex::bytes::R
)
})
}

#[cfg(test)]
mod tests {
use super::{GlobBuilder, accept_either_separator};

#[test]
fn accept_either_separator_rewrites_literal_separators() {
assert_eq!(accept_either_separator(r"^a/b$"), r"^a[/\\]b$");
assert_eq!(
accept_either_separator(r"^(?:/?|.*/)foo$"),
r"^(?:[/\\]?|.*[/\\])foo$"
);
}

#[test]
fn accept_either_separator_rewrites_separators_inside_a_class() {
assert_eq!(accept_either_separator(r"^[^/]*$"), r"^[^/\\]*$");
assert_eq!(accept_either_separator(r"^[/x]$"), r"^[/\\x]$");
}

#[test]
fn accept_either_separator_leaves_escaped_brackets_alone() {
assert_eq!(accept_either_separator(r"^a\[/b$"), r"^a\[[/\\]b$");
}

#[test]
fn rewritten_glob_regex_matches_backslash_paths() {
let glob = GlobBuilder::new("**/src/**/*.spec.ts")
.literal_separator(true)
.build()
.unwrap();
let regex = regex::bytes::Regex::new(&accept_either_separator(glob.regex())).unwrap();

assert!(regex.is_match(br"fixture\src\foo\a.spec.ts"));
assert!(regex.is_match(b"fixture/src/foo/a.spec.ts"));
assert!(!regex.is_match(br"fixture\lib\foo\a.spec.ts"));
}
}
28 changes: 14 additions & 14 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,7 @@ fn test_absolute_path() {
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo/
{abs_path}/symlink",
abs_path = &abs_path
abs_path = abs_path
),
);

Expand All @@ -1287,7 +1287,7 @@ fn test_absolute_path() {
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo/",
abs_path = &abs_path
abs_path = abs_path
),
);
}
Expand All @@ -1306,7 +1306,7 @@ fn test_implicit_absolute_path() {
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo/",
abs_path = &abs_path
abs_path = abs_path
),
);
}
Expand All @@ -1326,7 +1326,7 @@ fn test_normalized_absolute_path() {
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo/",
abs_path = &abs_path
abs_path = abs_path
),
);
}
Expand Down Expand Up @@ -1560,7 +1560,7 @@ fn test_symlink_as_root() {
{dir}/one/two/three/d.foo
{dir}/one/two/three/directory_foo/
{dir}/symlink",
dir = &parent_parent
dir = parent_parent
),
);
}
Expand All @@ -1580,7 +1580,7 @@ fn test_symlink_and_absolute_path() {
{abs_path}/{expected_path}/three/
{abs_path}/{expected_path}/three/d.foo
{abs_path}/{expected_path}/three/directory_foo/",
abs_path = &abs_path,
abs_path = abs_path,
expected_path = expected_path
),
);
Expand All @@ -1598,7 +1598,7 @@ fn test_symlink_as_absolute_root() {
{abs_path}/symlink/three/
{abs_path}/symlink/three/d.foo
{abs_path}/symlink/three/directory_foo/",
abs_path = &abs_path
abs_path = abs_path
),
);
}
Expand All @@ -1622,7 +1622,7 @@ fn test_symlink_and_full_path() {
"{abs_path}/{expected_path}/three/
{abs_path}/{expected_path}/three/d.foo
{abs_path}/{expected_path}/three/directory_foo/",
abs_path = &abs_path,
abs_path = abs_path,
expected_path = expected_path
),
);
Expand All @@ -1643,7 +1643,7 @@ fn test_symlink_and_full_path_abs_path() {
"{abs_path}/symlink/three/
{abs_path}/symlink/three/d.foo
{abs_path}/symlink/three/directory_foo/",
abs_path = &abs_path
abs_path = abs_path
),
);
}
Expand Down Expand Up @@ -1772,7 +1772,7 @@ fn test_exec() {
{abs_path}/one/two/c.foo
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo",
abs_path = &abs_path
abs_path = abs_path
),
);

Expand Down Expand Up @@ -1871,7 +1871,7 @@ fn test_exec_multi() {
test c.foo
test d.foo
test directory_foo",
abs_path = &abs_path
abs_path = abs_path
),
);

Expand Down Expand Up @@ -1927,7 +1927,7 @@ fn test_exec_batch() {
&["--absolute-path", "foo", "--exec-batch", "echo"],
&format!(
"{abs_path}/a.foo {abs_path}/one/b.foo {abs_path}/one/two/C.Foo2 {abs_path}/one/two/c.foo {abs_path}/one/two/three/d.foo {abs_path}/one/two/three/directory_foo",
abs_path = &abs_path
abs_path = abs_path
),
);

Expand Down Expand Up @@ -2539,7 +2539,7 @@ fn test_base_directory() {

// Ignore base directory when absolute path is used
let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES);
let abs_base_dir = &format!("{abs_path}/one/two/", abs_path = &abs_path);
let abs_base_dir = &format!("{abs_path}/one/two/", abs_path = abs_path);
te.assert_output(
&["--base-directory", abs_base_dir, "foo", &abs_path],
&format!(
Expand All @@ -2549,7 +2549,7 @@ fn test_base_directory() {
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo/",
abs_path = &abs_path
abs_path = abs_path
),
);
}
Expand Down