From d36234625f39d6c47d41e23a6c520ca794ae2040 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Sun, 12 Jul 2026 16:20:57 -0700 Subject: [PATCH] Standardize argument-name validation in OutputBuilderFactory The five public methods each duplicated the same `string name` guard, split across two styles: an `if (string.IsNullOrWhiteSpace(name)) throw` block in `Create`/`CreateTests` and a ternary-throw form in `GetOutputBuilder`/`GetTestOutputBuilder`/`TryGetOutputBuilder`. Factor the check into a single private `ValidateName` helper and call it from every method so the guard lives in one place. Preserve exception semantics exactly. The existing guard throws `ArgumentNullException(nameof(name))` for null, empty, and whitespace names, so `ValidateName` retains the `string.IsNullOrWhiteSpace` check rather than switching to `ArgumentNullException.ThrowIfNull` (which would not throw for non-null empty/whitespace) or `ArgumentException.ThrowIfNullOrWhiteSpace` (which would throw `ArgumentException` instead). The distinct `ArgumentException` for a non-test builder in `GetTestOutputBuilder` is left unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OutputBuilderFactory.cs | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/sources/ClangSharp.PInvokeGenerator/OutputBuilderFactory.cs b/sources/ClangSharp.PInvokeGenerator/OutputBuilderFactory.cs index de4693e7..b7621d38 100644 --- a/sources/ClangSharp.PInvokeGenerator/OutputBuilderFactory.cs +++ b/sources/ClangSharp.PInvokeGenerator/OutputBuilderFactory.cs @@ -20,10 +20,7 @@ internal sealed class OutputBuilderFactory(PInvokeGenerator generator) public IOutputBuilder Create(string name) { - if (string.IsNullOrWhiteSpace(name)) - { - throw new ArgumentNullException(nameof(name)); - } + ValidateName(name); var outputBuilder = generator.Config.OutputMode switch { @@ -38,10 +35,7 @@ public IOutputBuilder Create(string name) public CSharpOutputBuilder CreateTests(string name) { - if (string.IsNullOrWhiteSpace(name)) - { - throw new ArgumentNullException(nameof(name)); - } + ValidateName(name); var outputBuilder = new CSharpOutputBuilder(name, generator, isTestOutput: true, writeSourceLocation: _writeSourceLocation); @@ -49,21 +43,31 @@ public CSharpOutputBuilder CreateTests(string name) return outputBuilder; } - public IOutputBuilder GetOutputBuilder(string name) => string.IsNullOrWhiteSpace(name) ? throw new ArgumentNullException(nameof(name)) : _outputBuilders[name]; + public IOutputBuilder GetOutputBuilder(string name) + { + ValidateName(name); + return _outputBuilders[name]; + } public CSharpOutputBuilder GetTestOutputBuilder(string name) { - return string.IsNullOrWhiteSpace(name) - ? throw new ArgumentNullException(nameof(name)) - : _outputBuilders[name] is CSharpOutputBuilder csharpOutputBuilder && csharpOutputBuilder.IsTestOutput + ValidateName(name); + return _outputBuilders[name] is CSharpOutputBuilder csharpOutputBuilder && csharpOutputBuilder.IsTestOutput ? csharpOutputBuilder : throw new ArgumentException("A test output builder was not found with the given name", nameof(name)); } public bool TryGetOutputBuilder(string name, [MaybeNullWhen(false)] out IOutputBuilder outputBuilder) { - return string.IsNullOrWhiteSpace(name) - ? throw new ArgumentNullException(nameof(name)) - : _outputBuilders.TryGetValue(name, out outputBuilder); + ValidateName(name); + return _outputBuilders.TryGetValue(name, out outputBuilder); + } + + private static void ValidateName([NotNull] string name) + { + if (string.IsNullOrWhiteSpace(name)) + { + throw new ArgumentNullException(nameof(name)); + } } }