Skip to content

Fix project glob handling to support just filename filtering#430

Closed
theolivenbaum wants to merge 1 commit into
sensslen:mainfrom
theolivenbaum:main
Closed

Fix project glob handling to support just filename filtering#430
theolivenbaum wants to merge 1 commit into
sensslen:mainfrom
theolivenbaum:main

Conversation

@theolivenbaum
Copy link
Copy Markdown

@theolivenbaum theolivenbaum commented Feb 9, 2026

Hi again @sensslen,

Did another round of testing now that I've the source-code locally to test, and it seems that in 4.0.6 release, passing multiple patterns is failing:

nuget-license -i .\Mosaik.slnx -o JsonPretty -fo .\licenses.json --exclude-projects-matching "*Mosaik.Testing.*;*.pyproj"
>> fails with pyproj-related error

While passing them separately seems to work (in the sense that they don't fail due to the filtered project anymore):

nuget-license -i .\Mosaik.slnx -o JsonPretty -fo .\licenses.json --exclude-projects-matching "*Mosaik.Testing.*"
>> fails with pyproj-related error
nuget-license -i .\Mosaik.slnx -o JsonPretty -fo .\licenses.json --exclude-projects-matching "*.pyproj"`
>> fails with Mosaik.Testing-related error

In the latest preview version, passing multiple patterns work fine:

dotnet tool update --global nuget-license --version 4.0.7-beta.3
Tool 'nuget-license' was successfully updated from version '4.0.6' to version '4.0.7-beta.3'.

nuget-license -i .\Mosaik.slnx -o JsonPretty -fo .\licenses.json --exclude-projects-matching "*Mosaik.Testing.*;*.pyproj"

>> licenses.json file generates correctly

So my guess is that the previous SystemCommandLine parsing was not correctly handling the array (probably the target property should have been a string[]), but the move to a new package handles this fine.

While reviewing it, I noticed that the glob-like matching is not handling only file-name matching, which is a bit surprising. This PR adds a second branch to the check that will also try to match the pattern against the file-name separatelly.

This way, it is also fine to pass a pattern such as Project.Name*, and have that match as one would expect.

Summary by CodeRabbit

  • Bug Fixes
    • Refined project exclusion logic to apply stricter pattern matching criteria, improving filtering accuracy by validating against both full paths and file names.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 9, 2026

Walkthrough

The project filtering logic in LicenseValidationOrchestrator.cs was modified to change how exclusion patterns are applied. Previously, projects were filtered based on matching against the full input string. Now, projects are excluded only when both the full path and file name fail to match any exclusion pattern. The result is materialized as a string[] array instead of remaining as an IEnumerable<string>.

Possibly related PRs

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly addresses the main change in the PR: fixing glob handling to support filename-only patterns in project filtering.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
src/NuGetLicense/LicenseValidationOrchestrator.cs (1)

73-73: GetFileName(p) is recomputed for every pattern — hoist it out of the All predicate.

_fileSystem.Path.GetFileName(p) only depends on p, yet it's evaluated once per exclusion pattern inside All. Extract it in the outer Where lambda. This also helps break up this very long line for readability.

♻️ Proposed refactor
-            string[] projects = (await inputFiles.SelectManyAsync(projectCollector.GetProjectsAsync)).Where(p => excludedProjectsArray.All(m =>  !p.Like(m) && !_fileSystem.Path.GetFileName(p).Like(m))).ToArray();
+            string[] projects = (await inputFiles.SelectManyAsync(projectCollector.GetProjectsAsync))
+                .Where(p =>
+                {
+                    string fileName = _fileSystem.Path.GetFileName(p);
+                    return excludedProjectsArray.All(m => !p.Like(m) && !fileName.Like(m));
+                })
+                .ToArray();

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Feb 9, 2026

@sensslen
Copy link
Copy Markdown
Owner

sensslen commented Feb 9, 2026

@theolivenbaum the multi pattern is a new feature that is not yet in 4.0.6. Therefore it's expected to not work in that version. I'm planning to release 4.0.7 fairly soon. Obviously there may still be issues in the beta version....

@theolivenbaum
Copy link
Copy Markdown
Author

Makes sense why it didn't work then 😄

If I could suggest another small thing, a flag to ignore this kind of errors would be nice:

    "ValidationErrors": [
      {
        "Error": "Unable to determine license from the given license file",
        "Context": "..."
      },

I'm seeing this often for licenses that are correctly extracted as text, but don't match a "known" license - which is fine for my use-case and not an error. If that's ok, happy to submit another PR with the flag

@sensslen
Copy link
Copy Markdown
Owner

sensslen commented Feb 9, 2026

Makes sense why it didn't work then 😄

If I could suggest another small thing, a flag to ignore this kind of errors would be nice:

    "ValidationErrors": [
      {
        "Error": "Unable to determine license from the given license file",
        "Context": "..."
      },

I'm seeing this often for licenses that are correctly extracted as text, but don't match a "known" license - which is fine for my use-case and not an error. If that's ok, happy to submit another PR with the flag

As I already said in my comments to the other PR. I'm very reluctant removing any of the regidity of the project. You can already specify additional licenses the file matcher matches against as well as overwriting licenses by package.....

@sensslen
Copy link
Copy Markdown
Owner

@theolivenbaum thank you so much for your contribution. Your changes have been incooperated into #431 whith additional testing around this topic.

@sensslen sensslen closed this Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants