fix: properly detect !() extglob as magic and handle broken symlinks with follow:true

Author: richiemcilroy

src/glob.rs

@@ -1943,8 +1943,11 @@ impl Glob {
                 let full_path = self.cwd.join(&static_path);
 
                 // Check if the file exists
+                // When follow is true, first try metadata() which follows symlinks.
+                // If that fails (e.g., broken symlink), fall back to symlink_metadata().
+                // This matches glob's behavior of returning broken symlinks even with follow: true.
                 let metadata = if self.follow {
-                    fs::metadata(&full_path)
+                    fs::metadata(&full_path).or_else(|_| fs::symlink_metadata(&full_path))
                 } else {
                     fs::symlink_metadata(&full_path)
                 };

src/pattern.rs

@@ -685,8 +685,10 @@ impl Pattern {
                 .collect();
 
             if path_parts.is_empty() {
-                // Pattern is just "." or "/"
-                if self.raw == "." {
+                // Pattern is just "." or "/" or "./"
+                // Check the preprocessed pattern, not the raw pattern
+                let preprocessed = preprocess_pattern(&self.raw);
+                if preprocessed == "." {
                     Some(".".to_string())
                 } else {
                     None
@@ -1399,10 +1401,9 @@ pub fn has_magic_in_pattern(pattern: &str, noext: bool, windows_paths_no_escape:
         // Check for magic characters
         match c {
             '*' | '?' | '[' => return true,
-            // Check for extglob patterns (only + and @ trigger when followed by ()
-            // Note: ! is NOT magic per glob's behavior
-            // Note: * and ? are already caught above
-            '+' | '@' if !noext && i + 1 < chars.len() && chars[i + 1] == '(' => {
+            // Check for extglob patterns when followed by (
+            // All extglob types: +, @, !, *, ? - but * and ? are already caught above
+            '+' | '@' | '!' if !noext && i + 1 < chars.len() && chars[i + 1] == '(' => {
                 return true;
             }
             _ => {}
@@ -3381,8 +3382,8 @@ mod tests {
         // *( and ?( are magic (because * and ? are always magic)
         assert!(Pattern::new("*(a|b)").has_magic());
         assert!(Pattern::new("?(a|b)").has_magic());
-        // !( is NOT magic per glob's behavior
-        assert!(!Pattern::new("!(a|b)").has_magic());
+        // !( IS magic - it's a valid extglob for negation
+        assert!(Pattern::new("!(a|b)").has_magic());
         // With noext, only + and @ lose their magic; * and ? are still magic
         assert!(!Pattern::with_options("+(a|b)", true).has_magic());
         assert!(Pattern::with_options("*(a|b)", true).has_magic());
@@ -3734,8 +3735,8 @@ mod tests {
         assert!(has_magic_in_pattern("+(a|b)", false, false));
         assert!(has_magic_in_pattern("@(a|b)", false, false));
 
-        // !( is NOT magic per glob's behavior
-        assert!(!has_magic_in_pattern("!(a|b)", false, false));
+        // !( IS magic - it's a valid extglob for negation
+        assert!(has_magic_in_pattern("!(a|b)", false, false));
 
         // *( and ?( are magic because * and ? are always magic
         assert!(has_magic_in_pattern("*(a|b)", false, false));

tests/compat/escape.test.ts

@@ -154,9 +154,11 @@ describe('hasMagic', () => {
       expect(hasMagic('@(a|b)')).toBe(true)
     })
 
-    it('should return false for !( per glob behavior', () => {
-      // !( is NOT magic in glob v13
-      expect(hasMagic('!(a|b)')).toBe(false)
+    it('should return true for !( (negation extglob)', () => {
+      // Note: glob v13 returns false for !(a|b) but this is arguably incorrect
+      // since !() IS a valid extglob pattern that affects matching behavior
+      // globlin returns true which is semantically more accurate
+      expect(hasMagic('!(a|b)')).toBe(true)
     })
 
     it('should return true for *( and ?( because * and ? are magic', () => {
@@ -255,6 +257,16 @@ describe('compatibility with glob', () => {
 
   describe('hasMagic compatibility', () => {
     for (const pattern of testPatterns) {
+      // Skip !(a|b) - glob returns false but globlin correctly returns true
+      // since !() IS a valid extglob pattern that affects matching behavior
+      if (pattern === '!(a|b)') {
+        it(`hasMagic("${pattern}") - intentional divergence from glob`, () => {
+          expect(hasMagic(pattern)).toBe(true) // globlin: correct semantic behavior
+          expect(globHasMagic(pattern)).toBe(false) // glob: arguably incorrect
+        })
+        continue
+      }
+
       it(`hasMagic("${pattern}") matches glob`, () => {
         const globlinResult = hasMagic(pattern)
         const globResult = globHasMagic(pattern)