[ty] Anchor all exclude patterns (#18685)

Co-authored-by: Andrew Gallant <andrew@astral.sh>

Author: MichaReiser

crates/ty/docs/configuration.md

@@ -5,9 +5,7 @@ use clap::{ArgAction, ArgMatches, Error, Parser};
 use ruff_db::system::SystemPathBuf;
 use ty_project::combine::Combine;
 use ty_project::metadata::options::{EnvironmentOptions, Options, SrcOptions, TerminalOptions};
-use ty_project::metadata::value::{
-    RangedValue, RelativeExcludePattern, RelativePathBuf, ValueSource,
-};
+use ty_project::metadata::value::{RangedValue, RelativeGlobPattern, RelativePathBuf, ValueSource};
 use ty_python_semantic::lint;
 
 #[derive(Debug, Parser)]
@@ -205,12 +203,7 @@ impl CheckCommand {
             src: Some(SrcOptions {
                 respect_ignore_files,
                 exclude: self.exclude.map(|excludes| {
-                    RangedValue::cli(
-                        excludes
-                            .iter()
-                            .map(|exclude| RelativeExcludePattern::cli(exclude))
-                            .collect(),
-                    )
+                    RangedValue::cli(excludes.iter().map(RelativeGlobPattern::cli).collect())
                 }),
                 ..SrcOptions::default()
             }),

crates/ty/src/args.rs

@@ -283,7 +283,7 @@ fn exclude_precedence_over_include() -> anyhow::Result<()> {
         r#"
         [src]
         include = ["src"]
-        exclude = ["test_*.py"]
+        exclude = ["**/test_*.py"]
         "#,
     )?;
 
@@ -404,7 +404,7 @@ fn remove_default_exclude() -> anyhow::Result<()> {
         "ty.toml",
         r#"
         [src]
-        exclude = ["!dist"]
+        exclude = ["!**/dist/"]
         "#,
     )?;
 

crates/ty/tests/cli/file_selection.rs

@@ -2,7 +2,9 @@ use ruff_db::system::SystemPath;
 
 pub(crate) use exclude::{ExcludeFilter, ExcludeFilterBuilder};
 pub(crate) use include::{IncludeFilter, IncludeFilterBuilder};
-pub(crate) use portable::{AbsolutePortableGlobPattern, PortableGlobError, PortableGlobPattern};
+pub(crate) use portable::{
+    AbsolutePortableGlobPattern, PortableGlobError, PortableGlobKind, PortableGlobPattern,
+};
 
 mod exclude;
 mod include;

crates/ty_project/src/glob.rs

@@ -100,16 +100,16 @@ impl ExcludeFilterBuilder {
 /// Matcher for gitignore like globs.
 ///
 /// This code is our own vendored copy of the ignore's crate `Gitignore` type.
-/// The main difference to `ignore`'s version is that it makes use
-/// of the fact that all our globs are absolute. This simplifies the implementation a fair bit.
-/// Making globs absolute is also because the globs can come from both the CLI and configuration files,
-/// where the paths are anchored relative to the current working directory or the project root respectively.
 ///
-/// Vendoring our own copy has the added benefit that we don't need to deal with ignore's `Error` type.
-/// Instead, we can exclusively use [`globset::Error`].
+/// The differences with the ignore's crate version are:
 ///
-/// This implementation also removes supported for comments, because the patterns aren't read
-/// from a `.gitignore` file. This removes the need to escape `#` for file names starting with `#`,
+/// * All globs are anchored. `src` matches `./src` only and not `**/src` to be consistent with `include`.
+/// * It makes use of the fact that all our globs are absolute. This simplifies the implementation a fair bit.
+///   Making globs absolute is also motivated by the fact that the globs can come from both the CLI and configuration files,
+///   where the paths are anchored relative to the current working directory or the project root respectively.
+/// * It uses [`globset::Error`] over the ignore's crate `Error` type.
+/// * Removes supported for commented lines, because the patterns aren't read
+///   from a `.gitignore` file. This removes the need to escape `#` for file names starting with `#`,
 ///
 /// You can find the original source on [GitHub](https://github.com/BurntSushi/ripgrep/blob/cbc598f245f3c157a872b69102653e2e349b6d92/crates/ignore/src/gitignore.rs#L81).
 ///
@@ -267,15 +267,6 @@ impl GitignoreBuilder {
 
         let mut actual = pattern.to_string();
 
-        // If there is a literal slash, then this is a glob that must match the
-        // entire path name. Otherwise, we should let it match anywhere, so use
-        // a **/ prefix.
-        if !pattern.chars().any(|c| c == '/') {
-            // ... but only if we don't already have a **/ prefix.
-            if !pattern.starts_with("**/") {
-                actual = format!("**/{actual}");
-            }
-        }
         // If the glob ends with `/**`, then we should only match everything
         // inside a directory, but not the directory itself. Standard globs
         // will match the directory. So we add `/*` to force the issue.

crates/ty_project/src/glob/exclude.rs

@@ -241,16 +241,16 @@ impl IncludeFilterBuilder {
 mod tests {
     use std::path::{MAIN_SEPARATOR, MAIN_SEPARATOR_STR};
 
-    use crate::glob::PortableGlobPattern;
     use crate::glob::include::{IncludeFilter, IncludeFilterBuilder};
+    use crate::glob::{PortableGlobKind, PortableGlobPattern};
     use ruff_db::system::{MemoryFileSystem, walk_directory::WalkState};
 
     fn create_filter(patterns: impl IntoIterator<Item = &'static str>) -> IncludeFilter {
         let mut builder = IncludeFilterBuilder::new();
         for pattern in patterns {
             builder
                 .add(
-                    &PortableGlobPattern::parse(pattern, false)
+                    &PortableGlobPattern::parse(pattern, PortableGlobKind::Include)
                         .unwrap()
                         .into_absolute(""),
                 )

crates/ty_project/src/glob/include.rs

@@ -32,15 +32,15 @@ use thiserror::Error;
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub(crate) struct PortableGlobPattern<'a> {
     pattern: &'a str,
-    is_exclude: bool,
+    kind: PortableGlobKind,
 }
 
 impl<'a> PortableGlobPattern<'a> {
     /// Parses a portable glob pattern. Returns an error if the pattern isn't valid.
-    pub(crate) fn parse(glob: &'a str, is_exclude: bool) -> Result<Self, PortableGlobError> {
+    pub(crate) fn parse(glob: &'a str, kind: PortableGlobKind) -> Result<Self, PortableGlobError> {
         let mut chars = glob.chars().enumerate().peekable();
 
-        if is_exclude {
+        if matches!(kind, PortableGlobKind::Exclude) {
             chars.next_if(|(_, c)| *c == '!');
         }
 
@@ -124,7 +124,7 @@ impl<'a> PortableGlobPattern<'a> {
         }
         Ok(PortableGlobPattern {
             pattern: glob,
-            is_exclude,
+            kind,
         })
     }
 
@@ -138,21 +138,12 @@ impl<'a> PortableGlobPattern<'a> {
         let mut pattern = self.pattern;
         let mut negated = false;
 
-        if self.is_exclude {
+        if matches!(self.kind, PortableGlobKind::Exclude) {
             // If the pattern starts with `!`, we need to remove it and then anchor the rest.
             if let Some(after) = self.pattern.strip_prefix('!') {
                 pattern = after;
                 negated = true;
             }
-
-            // Patterns that don't contain any `/`, e.g. `.venv` are unanchored patterns
-            // that match anywhere.
-            if !self.chars().any(|c| c == '/') {
-                return AbsolutePortableGlobPattern {
-                    absolute: self.to_string(),
-                    relative: self.pattern.to_string(),
-                };
-            }
         }
 
         if pattern.starts_with('/') {
@@ -242,6 +233,15 @@ impl AbsolutePortableGlobPattern {
     }
 }
 
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
+pub(crate) enum PortableGlobKind {
+    /// An include pattern. Doesn't allow negated patterns.
+    Include,
+
+    /// An exclude pattern. Allows for negated patterns.
+    Exclude,
+}
+
 #[derive(Debug, Error)]
 pub(crate) enum PortableGlobError {
     /// Shows the failing glob in the error message.
@@ -284,15 +284,15 @@ impl std::fmt::Display for InvalidChar {
 #[cfg(test)]
 mod tests {
 
-    use crate::glob::PortableGlobPattern;
+    use crate::glob::{PortableGlobKind, PortableGlobPattern};
     use insta::assert_snapshot;
     use ruff_db::system::SystemPath;
 
     #[test]
     fn test_error() {
         #[track_caller]
         fn parse_err(glob: &str) -> String {
-            let error = PortableGlobPattern::parse(glob, true).unwrap_err();
+            let error = PortableGlobPattern::parse(glob, PortableGlobKind::Exclude).unwrap_err();
             error.to_string()
         }
 
@@ -376,13 +376,13 @@ mod tests {
             r"**/\@test",
         ];
         for case in cases.iter().chain(cases_uv.iter()) {
-            PortableGlobPattern::parse(case, true).unwrap();
+            PortableGlobPattern::parse(case, PortableGlobKind::Exclude).unwrap();
         }
     }
 
     #[track_caller]
     fn assert_absolute_path(pattern: &str, relative_to: impl AsRef<SystemPath>, expected: &str) {
-        let pattern = PortableGlobPattern::parse(pattern, true).unwrap();
+        let pattern = PortableGlobPattern::parse(pattern, PortableGlobKind::Exclude).unwrap();
         let pattern = pattern.into_absolute(relative_to);
         assert_eq!(pattern.absolute(), expected);
     }