Make FilePatternMatch.Stem non-nullable.#118410
Conversation
- Add an ArgumentNullException if stem is null in Success method. - Add tests for null checks.
There was a problem hiding this comment.
Pull Request Overview
This pull request makes the FilePatternMatch.Stem property non-nullable to improve type safety. The change ensures that the Stem property always has a valid string value, preventing potential null reference exceptions when consumers use this property.
Key changes:
- Converted
Stemproperty from nullable string to non-nullable string - Added null validation in the constructor to enforce the contract
- Added comprehensive tests to verify null argument validation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| FilePatternMatch.cs | Changed Stem property to non-nullable and added null validation in constructor |
| PatternTestResult.cs | Added MemberNotNullWhen attribute to support nullable analysis when IsSuccessful is true |
| FilePatternMatchTests.cs | Added unit tests to verify ArgumentNullException is thrown for null path and stem parameters |
Comments suppressed due to low confidence (2)
src/libraries/Microsoft.Extensions.FileSystemGlobbing/tests/FilePatternMatchTests.cs:32
- The test should verify that the exception is thrown for the correct parameter name. Consider using Assert.Throws("path", () => new FilePatternMatch(null, "sub2/bar/baz/three.txt")) to ensure the parameter name is validated.
Assert.Throws<ArgumentNullException>(() => new FilePatternMatch(null, "sub2/bar/baz/three.txt"));
src/libraries/Microsoft.Extensions.FileSystemGlobbing/tests/FilePatternMatchTests.cs:38
- The test should verify that the exception is thrown for the correct parameter name. Consider using Assert.Throws("stem", () => new FilePatternMatch("sub2/bar/baz/three.txt", null)) to ensure the parameter name is validated.
Assert.Throws<ArgumentNullException>(() => new FilePatternMatch("sub2/bar/baz/three.txt", null));
dotnet-policy-service
bot
added
the
community-contribution
|
@dotnet-policy-service agree company="Microsoft" |
jeffhandley
left a comment
jeffhandley
left a comment
There was a problem hiding this comment.
Looks good to me; thanks, @MitchBodmer! I will mark this as a
breaking-change
jeffhandley
added
the
breaking-change
|
Added When you commit this breaking change:
Tagging @dotnet/compat for awareness of the breaking change. |
dotnet-policy-service
bot
added
the
needs-breaking-change-doc-created
|
@jeffhandley it looks like the build hit some unrelated failures. Is there something I should do here? |
|
Thanks, @MitchBodmer. No worries; I'll review the failures further and the build analysis shortly and will update the build analysis with the justification for override, and then I'll merge this after I create the breaking-change notification. I'll let you know if I need anything further from you on it. Thanks for the contribution! |
jeffhandley
removed
the
needs-breaking-change-doc-created
|
Breaking change documentation issue created: [Breaking change]: FilePatternMatch.Stem annotated as non-nullable (dotnet/docs#47914). PR is pending review from @artl93 for .NET 10 RC1 consideration. |
Implements #116982