Skip to content

Commit 456c649

Browse files
authored
Merge pull request github#16895 from tamasvajk/feature/fix-glob-pattern-processing
C#: Fix glob pattern processing: allow `**/` to match empty string
2 parents 8e18e7d + 6a036f4 commit 456c649

File tree

3 files changed

+54
-9
lines changed

3 files changed

+54
-9
lines changed

csharp/extractor/Semmle.Extraction.Tests/FilePathFilter.cs

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -216,9 +216,9 @@ public void TestFiltersWithIncludeExcludeComplexPatterns1()
216216

217217
var expectedRegexMessages = new[]
218218
{
219-
"Filtering in files matching '^c/.*/i\\.[^/]*.*'. Original glob filter: 'include:c/**/i.*'",
220-
"Filtering in files matching '^c/d/.*/[^/]*\\.cs.*'. Original glob filter: 'include:c/d/**/*.cs'",
221-
"Filtering out files matching '^.*/z/i\\.cs.*'. Original glob filter: 'exclude:**/z/i.cs'"
219+
"Filtering in files matching '^c/(.*/|)i\\.[^/]*.*'. Original glob filter: 'include:c/**/i.*'",
220+
"Filtering in files matching '^c/d/(.*/|)[^/]*\\.cs.*'. Original glob filter: 'include:c/d/**/*.cs'",
221+
"Filtering out files matching '^(.*/|)z/i\\.cs.*'. Original glob filter: 'exclude:**/z/i.cs'"
222222
};
223223
Assert.Equivalent(expectedRegexMessages, logger.Messages, strict: false);
224224
}
@@ -244,10 +244,41 @@ public void TestFiltersWithIncludeExcludeComplexPatterns2()
244244

245245
var expectedRegexMessages = new[]
246246
{
247-
"Filtering in files matching '^.*/i\\.[^/]*.*'. Original glob filter: 'include:**/i.*'",
248-
"Filtering out files matching '^.*/z/i\\.cs.*'. Original glob filter: 'exclude:**/z/i.cs'"
247+
"Filtering in files matching '^(.*/|)i\\.[^/]*.*'. Original glob filter: 'include:**/i.*'",
248+
"Filtering out files matching '^(.*/|)z/i\\.cs.*'. Original glob filter: 'exclude:**/z/i.cs'"
249249
};
250250
Assert.Equivalent(expectedRegexMessages, logger.Messages, strict: false);
251251
}
252+
253+
[Fact]
254+
public void TestFiltersWithIncludeExcludeComplexPatternsRelativeRoot()
255+
{
256+
(var testSubject, var logger, var files) = TestSetup();
257+
258+
// 'c' is the start of the relative path so we want to ensure the `**/` glob can match start
259+
Environment.SetEnvironmentVariable("LGTM_INDEX_FILTERS", """
260+
include:**/c/**/i.*
261+
exclude:**/c/**/z/i.cs
262+
exclude:**/**/c/**/z/i.cs
263+
""");
264+
265+
var filtered = testSubject.Filter(files);
266+
267+
var expected = GetExpected(
268+
[
269+
"/a/b/c/x/y/i.cs",
270+
]);
271+
272+
AssertFileInfoEquivalence(expected, filtered);
273+
var expectedRegexMessages = new[]
274+
{
275+
"Filtering in files matching '^(.*/|)c/(.*/|)i\\.[^/]*.*'. Original glob filter: 'include:**/c/**/i.*'",
276+
"Filtering out files matching '^(.*/|)c/(.*/|)z/i\\.cs.*'. Original glob filter: 'exclude:**/c/**/z/i.cs'",
277+
"Filtering out files matching '^(.*/|)(.*/|)c/(.*/|)z/i\\.cs.*'. Original glob filter: 'exclude:**/**/c/**/z/i.cs'"
278+
};
279+
280+
281+
Assert.Equivalent(expectedRegexMessages, logger.Messages, strict: false);
282+
}
252283
}
253284
}

csharp/extractor/Semmle.Extraction.Tests/FilePattern.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,15 @@ public void TestRegexCompilation()
1010
var fp = new FilePattern("/hadoop*");
1111
Assert.Equal("^hadoop[^/]*.*", fp.RegexPattern);
1212
fp = new FilePattern("**/org/apache/hadoop");
13-
Assert.Equal("^.*/org/apache/hadoop.*", fp.RegexPattern);
13+
Assert.Equal("^(.*/|)org/apache/hadoop.*", fp.RegexPattern);
1414
fp = new FilePattern("hadoop-common/**/test// ");
15-
Assert.Equal("^hadoop-common/.*/test(?<doubleslash>/).*", fp.RegexPattern);
15+
Assert.Equal("^hadoop-common/(.*/|)test(?<doubleslash>/).*", fp.RegexPattern);
1616
fp = new FilePattern(@"-C:\agent\root\asdf//");
1717
Assert.Equal("^C:/agent/root/asdf(?<doubleslash>/).*", fp.RegexPattern);
1818
fp = new FilePattern(@"-C:\agent+\[root]\asdf//");
1919
Assert.Equal(@"^C:/agent\+/\[root]/asdf(?<doubleslash>/).*", fp.RegexPattern);
20+
fp = new FilePattern(@"**/**/abc/**/def/**");
21+
Assert.Equal(@"^(.*/|)(.*/|)abc/(.*/|)def/.*", fp.RegexPattern);
2022
}
2123

2224
[Fact]

csharp/extractor/Semmle.Extraction/FilePattern.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,20 @@ bool HasCharAt(int i, Predicate<char> p) =>
7676
throw new InvalidFilePatternException(pattern, "'**' preceeded by non-`/` character.");
7777
if (HasCharAt(i + 2, c => c != '/'))
7878
throw new InvalidFilePatternException(pattern, "'**' succeeded by non-`/` character");
79-
sb.Append(".*");
80-
i += 2;
79+
80+
if (i + 2 < pattern.Length)
81+
{
82+
// Processing .../**/...
83+
// ^^^
84+
sb.Append("(.*/|)");
85+
i += 3;
86+
}
87+
else
88+
{
89+
// Processing .../** at the end of the pattern.
90+
// There's no need to add .* because it's anyways added outside the loop.
91+
break;
92+
}
8193
}
8294
else
8395
{

0 commit comments

Comments
 (0)