From 7534d5144f5c5e89d67c791e663cdc0bee069b0e Mon Sep 17 00:00:00 2001 From: Andres Suarez Date: Mon, 7 Dec 2020 23:49:29 -0500 Subject: [PATCH] globset: fix recursive suffix over matching Previous, 'foo/**' would match 'foo', but it shouldn't have. In this case, not matching 'foo' is what is documented and also seems consistent with other recursive globbing implementations (like that in zsh). This also updates the prefix extractor to pull 'foo/' out of 'foo/**'. Closes #1756 --- CHANGELOG.md | 2 ++ crates/globset/src/glob.rs | 31 +++++++++++++++++++------------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8c47c1b0..daf68d2f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,8 @@ Bug fixes: Clarify that CLI invocation must always be valid, regardless of config file. * [BUG #1741](https://github.com/BurntSushi/ripgrep/issues/1741): Fix stdin detection when using PowerShell in UNIX environments. +* [BUG #1756](https://github.com/BurntSushi/ripgrep/pull/1756): + Fix bug where `foo/**` would match `foo`, but it shouldn't. * [BUG #1765](https://github.com/BurntSushi/ripgrep/issues/1765): Fix panic when `--crlf` is used in some cases. * [BUG #1638](https://github.com/BurntSushi/ripgrep/issues/1638): diff --git a/crates/globset/src/glob.rs b/crates/globset/src/glob.rs index d02b8c27e..2195f707e 100644 --- a/crates/globset/src/glob.rs +++ b/crates/globset/src/glob.rs @@ -403,7 +403,7 @@ impl Glob { if self.opts.case_insensitive { return None; } - let end = match self.tokens.last() { + let (end, need_sep) = match self.tokens.last() { Some(&Token::ZeroOrMore) => { if self.opts.literal_separator { // If a trailing `*` can't match a `/`, then we can't @@ -414,9 +414,10 @@ impl Glob { // literal prefix. return None; } - self.tokens.len() - 1 + (self.tokens.len() - 1, false) } - _ => self.tokens.len(), + Some(&Token::RecursiveSuffix) => (self.tokens.len() - 1, true), + _ => (self.tokens.len(), false), }; let mut lit = String::new(); for t in &self.tokens[0..end] { @@ -425,6 +426,9 @@ impl Glob { _ => return None, } } + if need_sep { + lit.push('/'); + } if lit.is_empty() { None } else { @@ -683,7 +687,7 @@ impl Tokens { re.push_str("(?:/?|.*/)"); } Token::RecursiveSuffix => { - re.push_str("(?:/?|/.*)"); + re.push_str("/.*"); } Token::RecursiveZeroOrMore => { re.push_str("(?:/|/.*/)"); @@ -1222,9 +1226,9 @@ mod tests { toregex!(re16, "**/**/*", r"^(?:/?|.*/).*$"); toregex!(re17, "**/**/**", r"^.*$"); toregex!(re18, "**/**/**/*", r"^(?:/?|.*/).*$"); - toregex!(re19, "a/**", r"^a(?:/?|/.*)$"); - toregex!(re20, "a/**/**", r"^a(?:/?|/.*)$"); - toregex!(re21, "a/**/**/**", r"^a(?:/?|/.*)$"); + toregex!(re19, "a/**", r"^a/.*$"); + toregex!(re20, "a/**/**", r"^a/.*$"); + toregex!(re21, "a/**/**/**", r"^a/.*$"); toregex!(re22, "a/**/b", r"^a(?:/|/.*/)b$"); toregex!(re23, "a/**/**/b", r"^a(?:/|/.*/)b$"); toregex!(re24, "a/**/**/**/b", r"^a(?:/|/.*/)b$"); @@ -1270,11 +1274,12 @@ mod tests { matches!(matchrec18, "/**/test", "/test"); matches!(matchrec19, "**/.*", ".abc"); matches!(matchrec20, "**/.*", "abc/.abc"); - matches!(matchrec21, ".*/**", ".abc"); + matches!(matchrec21, "**/foo/bar", "foo/bar"); matches!(matchrec22, ".*/**", ".abc/abc"); - matches!(matchrec23, "foo/**", "foo"); - matches!(matchrec24, "**/foo/bar", "foo/bar"); - matches!(matchrec25, "some/*/needle.txt", "some/one/needle.txt"); + matches!(matchrec23, "test/**", "test/"); + matches!(matchrec24, "test/**", "test/one"); + matches!(matchrec25, "test/**", "test/one/two"); + matches!(matchrec26, "some/*/needle.txt", "some/one/needle.txt"); matches!(matchrange1, "a[0-9]b", "a0b"); matches!(matchrange2, "a[0-9]b", "a9b"); @@ -1400,6 +1405,8 @@ mod tests { "some/one/two/three/needle.txt", SLASHLIT ); + nmatches!(matchrec33, ".*/**", ".abc"); + nmatches!(matchrec34, "foo/**", "foo"); macro_rules! extract { ($which:ident, $name:ident, $pat:expr, $expect:expr) => { @@ -1504,7 +1511,7 @@ mod tests { prefix!(extract_prefix1, "/foo", Some(s("/foo"))); prefix!(extract_prefix2, "/foo/*", Some(s("/foo/"))); prefix!(extract_prefix3, "**/foo", None); - prefix!(extract_prefix4, "foo/**", None); + prefix!(extract_prefix4, "foo/**", Some(s("foo/"))); suffix!(extract_suffix1, "**/foo/bar", Some((s("/foo/bar"), true))); suffix!(extract_suffix2, "*/foo/bar", Some((s("/foo/bar"), false)));