Address follow-up review feedback for matcher reuse and null-pattern validation#1770
Open
Address follow-up review feedback for matcher reuse and null-pattern validation#1770
Conversation
…ob patterns efficiently. And align on one implementation of pattern/file matching.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates file name pattern matching to a centralized implementation (PatternMatchingUtility), removing legacy matching logic from PathUtilityService and updating the directory/file enumeration code paths and tests to use the new matcher.
Changes:
- Introduced span-based pattern matching using
System.IO.Enumeration.FileSystemName.MatchesSimpleExpressionand a reusableCompiledMatcher. - Removed
MatchesPatternfromIPathUtilityService/PathUtilityServiceand updated callers to usePatternMatchingUtility. - Expanded/updated unit tests to cover the new matching behavior and APIs.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.ComponentDetection.TestsUtilities/DetectorTestUtilityBuilder.cs | Updates test utility pattern selection to use PatternMatchingUtility.GetMatchingPattern. |
| test/Microsoft.ComponentDetection.Common.Tests/SafeFileEnumerableTests.cs | Removes mocking for the deleted IPathUtilityService.MatchesPattern API. |
| test/Microsoft.ComponentDetection.Common.Tests/PatternMatchingUtilityTests.cs | Updates tests to the new Compile/CompiledMatcher APIs and adds more matching cases. |
| src/Microsoft.ComponentDetection.Contracts/IPathUtilityService.cs | Removes MatchesPattern from the path utility contract. |
| src/Microsoft.ComponentDetection.Common/SafeFileEnumerable.cs | Switches file inclusion + matched-pattern selection to a compiled matcher. |
| src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs | Replaces custom wildcard logic with FileSystemName.MatchesSimpleExpression; adds CompiledMatcher. |
| src/Microsoft.ComponentDetection.Common/PathUtilityService.cs | Removes legacy pattern matching implementations. |
| src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs | Uses CompiledMatcher for file filtering and matched-pattern resolution; removes MatchesAnyPattern. |
Copilot's findings
Comments suppressed due to low confidence (2)
src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs:214
patternsis enumerated multiple times here (string.Joinand thenPatternMatchingUtility.Compile(patterns)), which can be problematic for lazy/non-repeatableIEnumerable<string>inputs and adds overhead. Consider converting to an array once (e.g.,var patternArray = patterns.ToArray()) and using it for both logging and compilation.
this.logger.LogDebug("Logging patterns {Patterns} for {Root}", string.Join(":", patterns), root.FullName);
var compiled = PatternMatchingUtility.Compile(patterns);
src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs:236
- This method compiles
patternsand also callsSubscribe(root, patterns), which currently re-enumerates and recompilespatternsagain. Materializingpatternsonce (array) and reusing a single compiled matcher for both filtering andMatchedPatternselection would avoid redundant work and prevents issues ifpatternsis a single-pass/lazy enumerable.
public IObservable<ProcessRequest> GetFilteredComponentStreamObservable(DirectoryInfo root, IEnumerable<string> patterns, IComponentRecorder componentRecorder)
{
var compiled = PatternMatchingUtility.Compile(patterns);
var observable = this.Subscribe(root, patterns).OfType<FileInfo>()
.Select(f => new
- Files reviewed: 8/8 changed files
- Comments generated: 1
|
👋 Hi! It looks like you modified some files in the
If none of the above scenarios apply, feel free to ignore this comment 🙂 |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR centralizes file-pattern matching by replacing custom PathUtilityService logic with a standardized PatternMatchingUtility implementation backed by System.IO.Enumeration.FileSystemName, and updates directory enumeration code paths and tests accordingly.
Changes:
- Introduces
PatternMatchingUtility.CompiledMatcherand updates consumers to use compiled span-based pattern matching. - Removes
MatchesPatternfromIPathUtilityService/PathUtilityServiceand updates call sites. - Expands/updates pattern matching tests to cover more patterns and case-insensitive matching.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.ComponentDetection.TestsUtilities/DetectorTestUtilityBuilder.cs | Switches test helper pattern selection to PatternMatchingUtility.GetMatchingPattern. |
| test/Microsoft.ComponentDetection.Common.Tests/SafeFileEnumerableTests.cs | Removes mocks for the deleted IPathUtilityService.MatchesPattern API. |
| test/Microsoft.ComponentDetection.Common.Tests/PatternMatchingUtilityTests.cs | Updates tests to use compiled matcher + adds coverage for case-insensitivity and pattern selection. |
| src/Microsoft.ComponentDetection.Contracts/IPathUtilityService.cs | Removes MatchesPattern from the contract. |
| src/Microsoft.ComponentDetection.Common/SafeFileEnumerable.cs | Uses a precompiled matcher for filtering and for retrieving the matched pattern. |
| src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs | Replaces manual matching with FileSystemName.MatchesSimpleExpression and adds CompiledMatcher. |
| src/Microsoft.ComponentDetection.Common/PathUtilityService.cs | Deletes legacy pattern matching implementations. |
| src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs | Converts file scanning/filtering to use compiled matcher (including matched-pattern selection). |
Copilot's findings
- Files reviewed: 8/8 changed files
- Comments generated: 3
src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs
Outdated
Show resolved
Hide resolved
Member
Author
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/microsoft/component-detection/sessions/d0bc12a0-7074-4cf3-9e91-a11be40e4d3a Co-authored-by: jpinz <8357054+jpinz@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/component-detection/sessions/d0bc12a0-7074-4cf3-9e91-a11be40e4d3a Co-authored-by: jpinz <8357054+jpinz@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/component-detection/sessions/d0bc12a0-7074-4cf3-9e91-a11be40e4d3a Co-authored-by: jpinz <8357054+jpinz@users.noreply.github.com>
Contributor
Implemented the requested updates from that review thread in commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
Migrate to efficient glob pattern matching implementation
Address review feedback on pattern matcher API, logic deduplication, and precedence docs
Apr 8, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the centralized pattern-matching API in Microsoft.ComponentDetection.Common to align visibility with intended usage, deduplicate matching behavior across entry points, and document/enforce first-match-wins precedence when multiple patterns match the same filename.
Changes:
- Made
PatternMatchingUtility.Compile(...)/CompiledMatcherpublic and consolidatedMatchesPattern,GetMatchingPattern, and compiled matching to a single shared implementation. - Updated file enumeration and directory walking to use the compiled matcher and to apply explicit first-match-wins pattern precedence.
- Updated test utilities and unit tests to use the new API and removed usage of
IPathUtilityService.MatchesPattern(including removing it from the interface).
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.ComponentDetection.TestsUtilities/DetectorTestUtilityBuilder.cs | Uses PatternMatchingUtility.GetMatchingPattern instead of instantiating PathUtilityService for test pattern selection. |
| test/Microsoft.ComponentDetection.Common.Tests/SafeFileEnumerableTests.cs | Removes mocks for IPathUtilityService.MatchesPattern since enumeration no longer depends on it. |
| test/Microsoft.ComponentDetection.Common.Tests/PatternMatchingUtilityTests.cs | Expands/updates coverage for compile/match APIs, case-insensitivity, and precedence behavior. |
| src/Microsoft.ComponentDetection.Contracts/IPathUtilityService.cs | Removes MatchesPattern from the public Contracts interface. |
| src/Microsoft.ComponentDetection.Common/SafeFileEnumerable.cs | Switches enumeration matching + selected-pattern reporting to CompiledMatcher, documenting first-match-wins behavior. |
| src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs | Introduces public Compile, MatchesPattern, GetMatchingPattern, and shared internal matching helpers. |
| src/Microsoft.ComponentDetection.Common/PathUtilityService.cs | Removes MatchesPattern implementations now replaced by PatternMatchingUtility. |
| src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs | Replaces prior per-pattern matching logic with a compiled matcher and dedupes per-file matching in filtered observables. |
Copilot's findings
- Files reviewed: 8/8 changed files
- Comments generated: 3
Comment on lines
+14
to
+20
| private readonly IPathUtilityService pathUtilityService; | ||
| private readonly bool recursivelyScanDirectories; | ||
| private readonly Func<FileInfo, bool> fileMatchingPredicate; | ||
| private readonly PatternMatchingUtility.CompiledMatcher compiledMatcher; |
There was a problem hiding this comment.
searchPatterns is now assigned but never used after switching to compiledMatcher. With TreatWarningsAsErrors enabled, this will raise CS0414 and fail the build. Remove the searchPatterns field/assignment (or use it for logging/debugging) so the file compiles cleanly.
Comment on lines
8
to
23
| public static class PatternMatchingUtility | ||
| { | ||
| public delegate bool FilePatternMatcher(ReadOnlySpan<char> span); | ||
| public static bool MatchesPattern(string pattern, string fileName) => IsPatternMatch(pattern, fileName.AsSpan()); | ||
|
|
||
| public static FilePatternMatcher GetFilePatternMatcher(IEnumerable<string> patterns) | ||
| /// <summary> | ||
| /// Returns the first matching pattern for <paramref name="fileName"/>. | ||
| /// Earlier patterns in <paramref name="patterns"/> have higher priority when multiple match. | ||
| /// </summary> | ||
| /// <returns>The first matching pattern, or <see langword="null"/> if no patterns match.</returns> | ||
| public static string? GetMatchingPattern(string fileName, IEnumerable<string> patterns) | ||
| => GetFirstMatchingPattern(fileName.AsSpan(), patterns); | ||
|
|
||
| public static CompiledMatcher Compile(IEnumerable<string> patterns) => new(patterns); | ||
|
|
||
| private static string? GetFirstMatchingPattern(ReadOnlySpan<char> fileName, IEnumerable<string> patterns) | ||
| { |
There was a problem hiding this comment.
Now that Compile(...) / CompiledMatcher are public, passing null for patterns will currently throw a NullReferenceException (via patterns.ToArray() / foreach). Add explicit ArgumentNullException.ThrowIfNull(patterns) (and consider similar validation for pattern/fileName) to make failures deterministic and user-friendly.
Comment on lines
23
to
29
| /// <summary> | ||
| /// Returns true when the below file path exists under the above file path. | ||
| /// </summary> | ||
| /// <param name="aboveFilePath">The top file path.</param> | ||
| /// <param name="belowFilePath">The file path to find within the top file path.</param> | ||
| /// <returns>Returns true if the below file path exists under the above file path, otherwise false.</returns> | ||
| bool IsFileBelowAnother(string aboveFilePath, string belowFilePath); |
There was a problem hiding this comment.
Removing MatchesPattern from the public IPathUtilityService interface is a breaking change for any external implementers/consumers of the Contracts package. If compatibility is required, consider keeping it (e.g., [Obsolete] and forwarding to PatternMatchingUtility.MatchesPattern) or introducing a new interface rather than deleting the member.
Member
Author
|
@copilot apply changes based on the comments in this thread |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR follows up on the centralized pattern-matching migration by hardening public pattern-matching APIs against null inputs, restoring contract compatibility for IPathUtilityService.MatchesPattern(...) via an obsolete shim, and cleaning up pattern-matching call sites to use the new compiled matcher approach.
Changes:
- Added null-argument guards and a compiled matcher API to
PatternMatchingUtility, plus expanded unit test coverage for new behaviors. - Reintroduced
IPathUtilityService.MatchesPattern(string, string)as an[Obsolete]compatibility shim and updatedPathUtilityServiceto forward toPatternMatchingUtility. - Updated filesystem enumeration code paths (SafeFileEnumerable / FastDirectoryWalkerFactory / test utilities) to use compiled matching and removed now-unused pattern state/mocking.
Show a summary per file
| File | Description |
|---|---|
src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs |
Introduces null-guarded public APIs, compiled matcher type, and uses FileSystemName.MatchesSimpleExpression for matching. |
src/Microsoft.ComponentDetection.Contracts/IPathUtilityService.cs |
Restores MatchesPattern(pattern, fileName) on the contract as [Obsolete] for compatibility. |
src/Microsoft.ComponentDetection.Common/PathUtilityService.cs |
Adds obsolete forwarding implementation to PatternMatchingUtility.MatchesPattern and removes old matching logic. |
src/Microsoft.ComponentDetection.Common/SafeFileEnumerable.cs |
Removes stored search patterns and switches to a compiled matcher for include + matched-pattern selection. |
src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs |
Switches filtering and pattern attribution to compiled matcher usage; removes old per-pattern fanout logic. |
test/Microsoft.ComponentDetection.Common.Tests/PatternMatchingUtilityTests.cs |
Updates matcher tests to new APIs and adds coverage for matching, compilation, GetMatchingPattern, and null guards. |
test/Microsoft.ComponentDetection.Common.Tests/PathUtilityServiceTests.cs |
Adds coverage to ensure obsolete PathUtilityService.MatchesPattern forwards correctly. |
test/Microsoft.ComponentDetection.Common.Tests/SafeFileEnumerableTests.cs |
Removes now-obsolete mocking of MatchesPattern since enumeration no longer uses it. |
test/Microsoft.ComponentDetection.TestsUtilities/DetectorTestUtilityBuilder.cs |
Updates test pattern selection to use PatternMatchingUtility.GetMatchingPattern. |
Copilot's findings
- Files reviewed: 9/9 changed files
- Comments generated: 2
Member
Author
|
@copilot apply changes based on the comments in this thread |
JamieMagee
reviewed
Apr 8, 2026
Comment on lines
+61
to
+64
| // Pattern priority is first-match-wins: earlier entries in searchPatterns | ||
| // are treated as higher priority when multiple patterns match. | ||
| var foundPattern = this.compiledMatcher.GetMatchingPattern(entry.FileName) | ||
| ?? entry.FileName.ToString(); |
Member
There was a problem hiding this comment.
I'd mention it in the PR description alongside the case sensitivity note.
Agent-Logs-Url: https://github.com/microsoft/component-detection/sessions/cb82505f-3618-459b-8175-59d5aaf87c85 Co-authored-by: jpinz <8357054+jpinz@users.noreply.github.com>
Contributor
Applied in commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
Address follow-up review feedback for centralized pattern matching migration
Address follow-up review feedback for matcher reuse and null-pattern validation
Apr 8, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the centralized file pattern-matching pipeline to reuse compiled matchers across filtering and matched-pattern selection, while making null-pattern handling deterministic and updating test coverage accordingly.
Changes:
- Reuse a single compiled matcher in
FastDirectoryWalkerFactoryfor both inclusion filtering and selecting the matched pattern. - Introduce deterministic
ArgumentNullExceptionbehavior when any pattern element isnull, and standardize matching viaFileSystemName.MatchesSimpleExpression(..., ignoreCase: true). - Update and expand tests to cover matcher compilation, matching behavior, and null input validation.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs | Reuses one compiled matcher per subscription path and avoids recompiling patterns during filtered observable construction. |
| src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs | Adds Compile, MatchesPattern, GetMatchingPattern, and CompiledMatcher with deterministic null validation. |
| src/Microsoft.ComponentDetection.Common/SafeFileEnumerable.cs | Switches to a compiled matcher for file inclusion and pattern attribution (and removes per-file pattern iteration). |
| src/Microsoft.ComponentDetection.Common/PathUtilityService.cs | Removes local pattern matching logic and forwards (obsolete) MatchesPattern to PatternMatchingUtility. |
| src/Microsoft.ComponentDetection.Contracts/IPathUtilityService.cs | Marks MatchesPattern obsolete and updates docs/parameter naming accordingly. |
| test/Microsoft.ComponentDetection.Common.Tests/PatternMatchingUtilityTests.cs | Adds/updates tests for compiled matcher behavior, case-insensitivity, and null validation scenarios. |
| test/Microsoft.ComponentDetection.Common.Tests/PathUtilityServiceTests.cs | Adds a test verifying obsolete PathUtilityService.MatchesPattern forwards to the new utility logic. |
| test/Microsoft.ComponentDetection.Common.Tests/SafeFileEnumerableTests.cs | Removes mocks that are no longer relevant after SafeFileEnumerable no longer calls IPathUtilityService.MatchesPattern. |
| test/Microsoft.ComponentDetection.TestsUtilities/DetectorTestUtilityBuilder.cs | Switches pattern selection to PatternMatchingUtility.GetMatchingPattern for consistency with production matching. |
Copilot's findings
- Files reviewed: 9/9 changed files
- Comments generated: 2
src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR centralizes wildcard pattern matching by introducing a compiled matcher API, reusing compiled matchers in directory scanning, and adding deterministic validation for null patterns (with tests updated accordingly).
Changes:
- Replace ad-hoc/path-service pattern checks with
PatternMatchingUtility+CompiledMatcherreuse to reduce repeated compilation and matching work. - Add deterministic
ArgumentNullExceptionbehavior for null pattern inputs / null pattern elements. - Expand and adjust unit tests to cover
Compile(...),MatchesPattern(...), andGetMatchingPattern(...)(including case-insensitivity and null validation).
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.ComponentDetection.TestsUtilities/DetectorTestUtilityBuilder.cs | Swaps local matching logic to PatternMatchingUtility.GetMatchingPattern(...). |
| test/Microsoft.ComponentDetection.Common.Tests/SafeFileEnumerableTests.cs | Updates tests to reflect matcher no longer calling IPathUtilityService.MatchesPattern(...). |
| test/Microsoft.ComponentDetection.Common.Tests/PatternMatchingUtilityTests.cs | Adds coverage for compiled matcher, matching pattern selection, case-insensitivity, and null validation. |
| test/Microsoft.ComponentDetection.Common.Tests/PathUtilityServiceTests.cs | Verifies obsolete PathUtilityService.MatchesPattern forwards to PatternMatchingUtility. |
| src/Microsoft.ComponentDetection.Contracts/IPathUtilityService.cs | Updates doc/comments and marks MatchesPattern obsolete. |
| src/Microsoft.ComponentDetection.Common/SafeFileEnumerable.cs | Replaces per-pattern iteration with a compiled matcher and pattern selection. |
| src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs | Introduces MatchesPattern, GetMatchingPattern, Compile, and CompiledMatcher using FileSystemName.MatchesSimpleExpression. |
| src/Microsoft.ComponentDetection.Common/PathUtilityService.cs | Removes custom matching implementation; forwards obsolete API to PatternMatchingUtility. |
| src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs | Reuses a single compiled matcher for filtering + matched-pattern selection; adds subscribe overload. |
Copilot's findings
- Files reviewed: 9/9 changed files
- Comments generated: 3
src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors file pattern matching across the repo by centralizing matching logic in PatternMatchingUtility (with a reusable CompiledMatcher) and updating file enumeration to process each matched file once, using first-match pattern priority.
Changes:
- Introduces
PatternMatchingUtility.CompiledMatcherand span-based matching powered byFileSystemName.MatchesSimpleExpression(case-insensitive). - Updates directory walking/enumeration to select the first matching pattern per file and avoid duplicate
ProcessRequestgeneration. - Modernizes and expands test coverage for matching semantics and null validation; marks
IPathUtilityService.MatchesPatternas obsolete and forwards it.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.ComponentDetection.TestsUtilities/DetectorTestUtilityBuilder.cs | Uses PatternMatchingUtility.GetMatchingPattern instead of PathUtilityService.MatchesPattern in test utilities. |
| test/Microsoft.ComponentDetection.Common.Tests/SafeFileEnumerableTests.cs | Removes mocks for MatchesPattern since matching no longer uses IPathUtilityService. |
| test/Microsoft.ComponentDetection.Common.Tests/PatternMatchingUtilityTests.cs | Updates matcher tests to use Compile/CompiledMatcher and adds additional cases + null validation tests. |
| test/Microsoft.ComponentDetection.Common.Tests/PathUtilityServiceTests.cs | Adds a regression test verifying the obsolete MatchesPattern forwards to PatternMatchingUtility. |
| src/Microsoft.ComponentDetection.Contracts/IPathUtilityService.cs | Updates docs and marks MatchesPattern obsolete in favor of PatternMatchingUtility. |
| src/Microsoft.ComponentDetection.Contracts/FileComponentDetector.cs | Documents first-match-wins priority for SearchPatterns. |
| src/Microsoft.ComponentDetection.Common/SafeFileEnumerable.cs | Compiles patterns once and uses first-match-wins to assign a single pattern per matched file. |
| src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs | Adds MatchesPattern, GetMatchingPattern, and Compile APIs + CompiledMatcher implementation. |
| src/Microsoft.ComponentDetection.Common/PathUtilityService.cs | Removes legacy matching implementations; obsolete method forwards to PatternMatchingUtility. |
| src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs | Reuses compiled matchers and emits one ProcessRequest per matching file with the selected pattern. |
Copilot's findings
- Files reviewed: 10/10 changed files
- Comments generated: 1
src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request continues the repo-wide refactor of file pattern matching to use PatternMatchingUtility.CompiledMatcher for reusable, case-insensitive matching, while updating callers/tests to align with the new “first match wins” semantics and improved null validation.
Changes:
- Updates directory/file enumeration to compile and reuse matchers instead of re-evaluating patterns repeatedly.
- Updates tests/utilities to use
PatternMatchingUtilityAPIs and adds coverage for compilation, matching, and null-input validation. - Marks legacy
IPathUtilityService.MatchesPatternas obsolete and forwards it toPatternMatchingUtility.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.ComponentDetection.TestsUtilities/DetectorTestUtilityBuilder.cs | Uses PatternMatchingUtility.GetMatchingPattern to determine the pattern associated with a file in tests. |
| test/Microsoft.ComponentDetection.Common.Tests/SafeFileEnumerableTests.cs | Removes mocking of legacy MatchesPattern now that matching is handled via compiled matcher. |
| test/Microsoft.ComponentDetection.Common.Tests/PatternMatchingUtilityTests.cs | Adds/updates tests for compiled matcher behavior, case-insensitivity, and null argument validation. |
| test/Microsoft.ComponentDetection.Common.Tests/PathUtilityServiceTests.cs | Verifies obsolete PathUtilityService.MatchesPattern forwards to the new utility. |
| src/Microsoft.ComponentDetection.Contracts/IPathUtilityService.cs | Updates docs and marks MatchesPattern obsolete in favor of PatternMatchingUtility. |
| src/Microsoft.ComponentDetection.Contracts/FileComponentDetector.cs | Documents “first match wins” priority expectation for SearchPatterns. |
| src/Microsoft.ComponentDetection.Common/SafeFileEnumerable.cs | Replaces per-pattern matching loops with a compiled matcher and uses first-match priority for MatchedFile.Pattern. |
| src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs | Introduces MatchesPattern, GetMatchingPattern, and CompiledMatcher backed by FileSystemName.MatchesSimpleExpression. |
| src/Microsoft.ComponentDetection.Common/PathUtilityService.cs | Removes legacy matching logic and forwards obsolete MatchesPattern to the new utility. |
| src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs | Compiles patterns once per call and reuses the matcher for filtering and matched-pattern selection (avoids duplicate compilation). |
Copilot's findings
- Files reviewed: 10/10 changed files
- Comments generated: 2
src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ComponentDetection.Contracts/FileComponentDetector.cs
Outdated
Show resolved
Hide resolved
JamieMagee
approved these changes
Apr 9, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the pattern-matching refactor by introducing a reusable compiled matcher abstraction and wiring it through file enumeration and test utilities, while also tightening null validation and clarifying first-match priority semantics.
Changes:
- Add
PatternMatchingUtility.CompiledMatcher(span-based) and new helpers (Compile,MatchesPattern,GetMatchingPattern) backed byFileSystemName.MatchesSimpleExpression(ignoreCase: true). - Update file enumeration/filtering to compile patterns once and reuse the matcher;
GetFilteredComponentStreamObservablenow emits oneProcessRequestper file with the first matching pattern. - Update/expand unit tests and forward the obsolete
IPathUtilityService.MatchesPatterntoPatternMatchingUtility.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.ComponentDetection.Common/PatternMatchingUtility.cs | Introduces CompiledMatcher + null validation and span-based matching via FileSystemName.MatchesSimpleExpression. |
| src/Microsoft.ComponentDetection.Common/SafeFileEnumerable.cs | Compiles patterns once per enumerable and uses first-match-wins for MatchedFile.Pattern. |
| src/Microsoft.ComponentDetection.Common/FastDirectoryWalkerFactory.cs | Reuses a single compiled matcher and returns a single ProcessRequest per matching file (first matching pattern). |
| src/Microsoft.ComponentDetection.Common/PathUtilityService.cs | Removes legacy matcher logic; obsolete MatchesPattern now forwards to PatternMatchingUtility. |
| src/Microsoft.ComponentDetection.Contracts/IPathUtilityService.cs | Marks MatchesPattern obsolete and updates XML docs/parameter naming. |
| src/Microsoft.ComponentDetection.Contracts/FileComponentDetector.cs | Updates SearchPatterns docs to reflect first-match priority and wildcard usage. |
| test/Microsoft.ComponentDetection.Common.Tests/PatternMatchingUtilityTests.cs | Expands matching, case-insensitivity, null validation, and compiled matcher behaviors. |
| test/Microsoft.ComponentDetection.Common.Tests/SafeFileEnumerableTests.cs | Adjusts mocks to reflect removal of IPathUtilityService.MatchesPattern usage. |
| test/Microsoft.ComponentDetection.Common.Tests/PathUtilityServiceTests.cs | Verifies obsolete MatchesPattern forwarding behavior. |
| test/Microsoft.ComponentDetection.TestsUtilities/DetectorTestUtilityBuilder.cs | Uses PatternMatchingUtility.GetMatchingPattern to select the effective pattern in tests. |
Copilot's findings
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
JamieMagee
approved these changes
Apr 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors and modernizes the file pattern matching logic throughout the codebase. The main improvements are the introduction of a new
PatternMatchingUtility.CompiledMatcherclass for efficient, reusable pattern matching, the removal of legacy pattern matching code, and the simplification of APIs that deal with file pattern matching. These changes improve maintainability, performance, and code clarity.Worth noting is that as a part of this change, the ordering of the
SearchPatternsin each detector is now ordered so that earlier patterns have a higher priority when multiple patterns match a file.Additionally, only one
ProcessRequestis returned per matching file, previously we returned one per matching pattern.Old behavior (one per matching pattern): If a file like
package.jsonmatched both*.jsonandpackage.json, the detector'sOnFileFoundAsyncwould be called twice for the same file. Both calls would parse the same content and register the same components into the sameSingleFileComponentRecorder(since it's keyed by file path viaCreateSingleFileComponentRecorder(lazyComponentStream.Location)). The second call is pure waste.New behavior (one per file): Each file is processed exactly once. No duplicate parsing, no redundant
RegisterUsagecalls.