-
-
Notifications
You must be signed in to change notification settings - Fork 0
chore: Replace custom argument checks with .NET exception helpers #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Replaced all uses of custom Argument.ThrowIfNull and Argument.ThrowIfNullOrWhiteSpace with the standard .NET methods ArgumentNullException.ThrowIfNull and ArgumentException.ThrowIfNullOrWhiteSpace. This update improves code clarity, consistency, and leverages built-in .NET validation utilities across the codebase.
WalkthroughThe pull request replaces custom argument validation calls across multiple builder, model, and factory files with built-in .NET exception types. Specifically, Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks✅ Passed checks (3 passed)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (10)
src/NetEvolve.ProjectBuilders/Builders/GlobalJsonBuilderExtensions.cs (1)
3-3: Remove unused import.The
NetEvolve.Argumentsnamespace is no longer referenced in this file after switching to built-in .NET exception helpers.🔎 Proposed fix
-using NetEvolve.Arguments; using NetEvolve.ProjectBuilders.Abstractions;src/NetEvolve.ProjectBuilders/Builders/TestPackageBuilder.cs (1)
4-4: Remove unused import.The
NetEvolve.Argumentsnamespace is no longer referenced in this file after switching to built-in .NET exception helpers.🔎 Proposed fix
using CliWrap; -using NetEvolve.Arguments; using NetEvolve.ProjectBuilders.Abstractions;src/NetEvolve.ProjectBuilders/Builders/GlobalJsonBuilder.cs (1)
8-8: Remove unused import.The
NetEvolve.Argumentsnamespace is no longer referenced in this file after switching to built-in .NET exception helpers.🔎 Proposed fix
using System.Threading.Tasks; -using NetEvolve.Arguments; using NetEvolve.ProjectBuilders.Abstractions;src/NetEvolve.ProjectBuilders/Builders/ProjectFactoryExtensions.cs (1)
5-5: Remove unused import.The
NetEvolve.Argumentsnamespace is no longer referenced in this file after switching to built-in .NET exception helpers.🔎 Proposed fix
using System.Diagnostics.CodeAnalysis; -using NetEvolve.Arguments; using NetEvolve.ProjectBuilders.Abstractions;src/NetEvolve.ProjectBuilders/Builders/SubdirectoryBuilder.cs (1)
7-7: Remove unused import.The
NetEvolve.Argumentsnamespace is no longer referenced in this file after switching to built-in .NET exception helpers.🔎 Proposed fix
using System.Threading.Tasks; -using NetEvolve.Arguments; using NetEvolve.ProjectBuilders.Abstractions;src/NetEvolve.ProjectBuilders/Models/TargetFramework.cs (1)
128-129: LGTM! Clean migration to .NET built-in validation.The replacement of custom validation with
ArgumentException.ThrowIfNullOrWhiteSpaceis correct and maintains identical behavior.Optional: Consider removing unused import
If
NetEvolve.Argumentsis no longer used in this file after these changes, consider removing the import on line 6:-using NetEvolve.Arguments;src/NetEvolve.ProjectBuilders/Models/PropertyGroup.cs (1)
51-51: LGTM! Consistent with .NET validation patterns.The switch to
ArgumentException.ThrowIfNullOrWhiteSpaceis correct for thekeyparameter validation.Optional: Consider removing unused import
If
NetEvolve.Argumentsis no longer used in this file, consider removing the import on line 5:-using NetEvolve.Arguments;src/NetEvolve.ProjectBuilders/ProjectFactory.cs (1)
137-137: LGTM! Correct null check for array parameter.Using
ArgumentNullException.ThrowIfNullis the appropriate validation for thevariablesarray parameter.Optional: Consider removing unused import
If
NetEvolve.Argumentsis no longer used in this file after these changes, consider removing the import on line 14:-using NetEvolve.Arguments;src/NetEvolve.ProjectBuilders/Builders/ProjectBuilderExtensions.cs (1)
282-282: LGTM! Correct string validation.Using
ArgumentException.ThrowIfNullOrWhiteSpaceis appropriate for the package name parameter.Optional: Consider removing unused import
If
NetEvolve.Argumentsis no longer used in this file after these changes, consider removing the import on line 6:-using NetEvolve.Arguments;src/NetEvolve.ProjectBuilders/Builders/TemporaryDirectoryBuilder.cs (1)
47-47: LGTM! Consistent validation across directory operations.All three string parameter validations correctly use
ArgumentException.ThrowIfNullOrWhiteSpace, maintaining consistency with the relatedSubdirectoryBuilderclass.Optional: Consider removing unused import
If
NetEvolve.Argumentsis no longer used in this file after these changes, consider removing the import on line 7:-using NetEvolve.Arguments;Also applies to: 55-55, 75-75
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (10)
src/NetEvolve.ProjectBuilders/Builders/GlobalJsonBuilder.cssrc/NetEvolve.ProjectBuilders/Builders/GlobalJsonBuilderExtensions.cssrc/NetEvolve.ProjectBuilders/Builders/ProjectBuilderExtensions.cssrc/NetEvolve.ProjectBuilders/Builders/ProjectFactoryExtensions.cssrc/NetEvolve.ProjectBuilders/Builders/SubdirectoryBuilder.cssrc/NetEvolve.ProjectBuilders/Builders/TemporaryDirectoryBuilder.cssrc/NetEvolve.ProjectBuilders/Builders/TestPackageBuilder.cssrc/NetEvolve.ProjectBuilders/Models/PropertyGroup.cssrc/NetEvolve.ProjectBuilders/Models/TargetFramework.cssrc/NetEvolve.ProjectBuilders/ProjectFactory.cs
🧰 Additional context used
🧬 Code graph analysis (2)
src/NetEvolve.ProjectBuilders/Builders/TemporaryDirectoryBuilder.cs (3)
src/NetEvolve.ProjectBuilders/Builders/SubdirectoryBuilder.cs (3)
SubdirectoryBuilder(35-75)SubdirectoryBuilder(42-42)Stream(56-68)src/NetEvolve.ProjectBuilders/Abstractions/ISubdirectoryBuilder.cs (1)
Stream(72-72)src/NetEvolve.ProjectBuilders/Builders/ProjectBuilder.cs (1)
Stream(82-82)
src/NetEvolve.ProjectBuilders/Builders/SubdirectoryBuilder.cs (1)
src/NetEvolve.ProjectBuilders/Builders/TemporaryDirectoryBuilder.cs (1)
Stream(53-70)
🔇 Additional comments (9)
src/NetEvolve.ProjectBuilders/Builders/GlobalJsonBuilderExtensions.cs (1)
40-40: LGTM!The replacement of the custom validation helper with the built-in
ArgumentNullException.ThrowIfNullis correct and improves code clarity.src/NetEvolve.ProjectBuilders/Builders/TestPackageBuilder.cs (1)
90-90: LGTM!The replacement with
ArgumentNullException.ThrowIfNullis correct and maintains the same validation behavior.src/NetEvolve.ProjectBuilders/Builders/ProjectFactoryExtensions.cs (1)
125-125: LGTM!The replacement with
ArgumentException.ThrowIfNullOrWhiteSpaceis correct and improves consistency with the .NET framework.src/NetEvolve.ProjectBuilders/Builders/SubdirectoryBuilder.cs (1)
50-50: LGTM!Both replacements with
ArgumentException.ThrowIfNullOrWhiteSpaceare correct and align with the .NET framework's built-in validation methods.Also applies to: 58-58
src/NetEvolve.ProjectBuilders/Builders/GlobalJsonBuilder.cs (1)
66-66: Verify .NET 8.0+ target framework.The replacement with
ArgumentException.ThrowIfNullOrWhiteSpaceis correct. The project targets net8.0, net9.0, and net10.0, as defined in Directory.Build.props, so this method is available in all target frameworks.src/NetEvolve.ProjectBuilders/ProjectFactory.cs (1)
123-124: LGTM! Appropriate exception types for parameter validation.The validation changes are correct:
ArgumentException.ThrowIfNullOrWhiteSpaceforname(string)ArgumentException.ThrowIfNullOrWhiteSpaceforvalue(string?, requires non-null when explicitly adding)src/NetEvolve.ProjectBuilders/Builders/ProjectBuilderExtensions.cs (3)
56-58: LGTM! Proper validation with .NET exception helpers.The validation pattern is correct:
ArgumentNullException.ThrowIfNullfor the builder objectArgumentException.ThrowIfNullOrWhiteSpacefor fileName and content strings
98-100: LGTM! Consistent validation pattern.Identical validation pattern as
AddCSharpFile, which is correct and consistent.
133-133: LGTM! Appropriate null check for builder parameter.The validation correctly uses
ArgumentNullException.ThrowIfNullfor the builder object parameter.
Replaced all uses of custom Argument.ThrowIfNull and Argument.ThrowIfNullOrWhiteSpace with the standard .NET methods ArgumentNullException.ThrowIfNull and ArgumentException.ThrowIfNullOrWhiteSpace. This update improves code clarity, consistency, and leverages built-in .NET validation utilities across the codebase.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.