Merge pull request #16895 from tamasvajk/feature/fix-glob-pattern-pro…

…cessing C#: Fix glob pattern processing: allow `**/` to match empty string
github · Jul 4, 2024 · 456c649 · 456c649
2 parents 8e18e7d + 6a036f4
commit 456c649
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 9 deletions.
diff --git a/csharp/extractor/Semmle.Extraction.Tests/FilePathFilter.cs b/csharp/extractor/Semmle.Extraction.Tests/FilePathFilter.cs
@@ -216,9 +216,9 @@ public void TestFiltersWithIncludeExcludeComplexPatterns1()
 
             var expectedRegexMessages = new[]
             {
-                "Filtering in files matching '^c/.*/i\\.[^/]*.*'. Original glob filter: 'include:c/**/i.*'",
-                "Filtering in files matching '^c/d/.*/[^/]*\\.cs.*'. Original glob filter: 'include:c/d/**/*.cs'",
-                "Filtering out files matching '^.*/z/i\\.cs.*'. Original glob filter: 'exclude:**/z/i.cs'"
+                "Filtering in files matching '^c/(.*/|)i\\.[^/]*.*'. Original glob filter: 'include:c/**/i.*'",
+                "Filtering in files matching '^c/d/(.*/|)[^/]*\\.cs.*'. Original glob filter: 'include:c/d/**/*.cs'",
+                "Filtering out files matching '^(.*/|)z/i\\.cs.*'. Original glob filter: 'exclude:**/z/i.cs'"
             };
             Assert.Equivalent(expectedRegexMessages, logger.Messages, strict: false);
         }
@@ -244,10 +244,41 @@ public void TestFiltersWithIncludeExcludeComplexPatterns2()
 
             var expectedRegexMessages = new[]
             {
-                "Filtering in files matching '^.*/i\\.[^/]*.*'. Original glob filter: 'include:**/i.*'",
-                "Filtering out files matching '^.*/z/i\\.cs.*'. Original glob filter: 'exclude:**/z/i.cs'"
+                "Filtering in files matching '^(.*/|)i\\.[^/]*.*'. Original glob filter: 'include:**/i.*'",
+                "Filtering out files matching '^(.*/|)z/i\\.cs.*'. Original glob filter: 'exclude:**/z/i.cs'"
             };
             Assert.Equivalent(expectedRegexMessages, logger.Messages, strict: false);
         }
+
+        [Fact]
+        public void TestFiltersWithIncludeExcludeComplexPatternsRelativeRoot()
+        {
+            (var testSubject, var logger, var files) = TestSetup();
+
+            // 'c' is the start of the relative path so we want to ensure the `**/` glob can match start
+            Environment.SetEnvironmentVariable("LGTM_INDEX_FILTERS", """
+                include:**/c/**/i.*
+                exclude:**/c/**/z/i.cs
+                exclude:**/**/c/**/z/i.cs
+                """);
+
+            var filtered = testSubject.Filter(files);
+
+            var expected = GetExpected(
+                [
+                    "/a/b/c/x/y/i.cs",
+                ]);
+
+            AssertFileInfoEquivalence(expected, filtered);
+            var expectedRegexMessages = new[]
+            {
+                "Filtering in files matching '^(.*/|)c/(.*/|)i\\.[^/]*.*'. Original glob filter: 'include:**/c/**/i.*'",
+                "Filtering out files matching '^(.*/|)c/(.*/|)z/i\\.cs.*'. Original glob filter: 'exclude:**/c/**/z/i.cs'",
+                "Filtering out files matching '^(.*/|)(.*/|)c/(.*/|)z/i\\.cs.*'. Original glob filter: 'exclude:**/**/c/**/z/i.cs'"
+            };
+
+
+            Assert.Equivalent(expectedRegexMessages, logger.Messages, strict: false);
+        }
     }
 }
diff --git a/csharp/extractor/Semmle.Extraction.Tests/FilePattern.cs b/csharp/extractor/Semmle.Extraction.Tests/FilePattern.cs
@@ -10,13 +10,15 @@ public void TestRegexCompilation()
             var fp = new FilePattern("/hadoop*");
             Assert.Equal("^hadoop[^/]*.*", fp.RegexPattern);
             fp = new FilePattern("**/org/apache/hadoop");
-            Assert.Equal("^.*/org/apache/hadoop.*", fp.RegexPattern);
+            Assert.Equal("^(.*/|)org/apache/hadoop.*", fp.RegexPattern);
             fp = new FilePattern("hadoop-common/**/test//    ");
-            Assert.Equal("^hadoop-common/.*/test(?<doubleslash>/).*", fp.RegexPattern);
+            Assert.Equal("^hadoop-common/(.*/|)test(?<doubleslash>/).*", fp.RegexPattern);
             fp = new FilePattern(@"-C:\agent\root\asdf//");
             Assert.Equal("^C:/agent/root/asdf(?<doubleslash>/).*", fp.RegexPattern);
             fp = new FilePattern(@"-C:\agent+\[root]\asdf//");
             Assert.Equal(@"^C:/agent\+/\[root]/asdf(?<doubleslash>/).*", fp.RegexPattern);
+            fp = new FilePattern(@"**/**/abc/**/def/**");
+            Assert.Equal(@"^(.*/|)(.*/|)abc/(.*/|)def/.*", fp.RegexPattern);
         }
 
         [Fact]

diff --git a/csharp/extractor/Semmle.Extraction/FilePattern.cs b/csharp/extractor/Semmle.Extraction/FilePattern.cs
@@ -76,8 +76,20 @@ private static StringBuilder BuildRegex(string pattern)
                             throw new InvalidFilePatternException(pattern, "'**' preceeded by non-`/` character.");
                         if (HasCharAt(i + 2, c => c != '/'))
                             throw new InvalidFilePatternException(pattern, "'**' succeeded by non-`/` character");
-                        sb.Append(".*");
-                        i += 2;
+
+                        if (i + 2 < pattern.Length)
+                        {
+                            // Processing .../**/...
+                            //                ^^^
+                            sb.Append("(.*/|)");
+                            i += 3;
+                        }
+                        else
+                        {
+                            // Processing .../** at the end of the pattern.
+                            // There's no need to add .* because it's anyways added outside the loop.
+                            break;
+                        }
                     }
                     else
                     {