fix(quality): address Domain and ServiceDefaults analyzer warnings by mpaulosky · Pull Request #150 · mpaulosky/MyBlog

mpaulosky · 2026-04-24T18:58:30Z

Closes #140

Working as Sam (Backend Developer)

Addresses five analyzer rules across Domain, ServiceDefaults, and Web:

CA1000 — Static members on generic types (Result<T>)

Wrapped static new Fail overloads in Result<T> with #pragma warning disable/restore CA1000

CA2225 — Named alternates for implicit operators in Result<T>

Added ToValue() instance method (alternate for implicit operator T?)
Added static FromValue(T? value) method (alternate for implicit operator Result<T>)

CA1062 — Validate parameter next in ValidationBehavior

Added ArgumentNullException.ThrowIfNull(next); at top of Handle

CA1307 — String comparison in ServiceDefaults/Extensions.cs

Suppressed with #pragma warning disable/restore CA1307 around the two StartsWithSegments calls (PathString API, not a regular string comparison)

CA1014 — CLSCompliant attribute

Created Properties/AssemblyInfo.cs for Domain, ServiceDefaults, and Domain.Tests with [assembly: CLSCompliant(false)]
Added [assembly: CLSCompliant(false)] to Web's existing AssemblyInfo.cs

All 27 unit/arch tests pass.

CA1000 — suppress static members on generic type Result<T> CA2225 — add ToValue() and FromValue() named alternates for implicit operators in Result<T> CA1062 — add ArgumentNullException.ThrowIfNull(next) in ValidationBehavior.Handle CA1307 — suppress StartsWithSegments calls in ServiceDefaults/Extensions.cs CA1014 — add [assembly: CLSCompliant(false)] for Domain, ServiceDefaults, Domain.Tests, and Web projects Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-04-24T18:58:40Z

🏗️ PR Added to Squad Triage Queue

This PR has been labeled with squad and added to the triage queue.

Next steps:

The squad Lead will review and assign to an appropriate team member
A squad:member label will be added after triage

If you know which squad member should handle this, you can add the appropriate squad:member label yourself.

Copilot

Pull request overview

Resolves .NET analyzer warnings across core projects (Domain, ServiceDefaults, Web) by adding CLS compliance attributes, adjusting Result<T> APIs to satisfy operator/factory rules, and adding missing null-argument validation.

Changes:

Added [assembly: CLSCompliant(false)] across Domain, ServiceDefaults, Web, and Domain.Tests projects.
Added named alternates for Result<T> implicit operators and suppressed CA1000 for selected static members.
Added ArgumentNullException.ThrowIfNull(next) to ValidationBehavior and suppressed CA1307 in ServiceDefaults tracing filter.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/Domain.Tests/Properties/AssemblyInfo.cs	Marks test assembly as not CLS-compliant to satisfy CA1014.
src/Web/Properties/AssemblyInfo.cs	Adds CLS compliance attribute to satisfy CA1014.
src/ServiceDefaults/Properties/AssemblyInfo.cs	Adds CLS compliance attribute to satisfy CA1014.
src/ServiceDefaults/Extensions.cs	Suppresses CA1307 around `StartsWithSegments` calls in OpenTelemetry filter.
src/Domain/Properties/AssemblyInfo.cs	Adds CLS compliance attribute and `InternalsVisibleTo` for Domain.Tests.
src/Domain/Behaviors/ValidationBehavior.cs	Adds null validation for `next` to satisfy CA1062.
src/Domain/Abstractions/Result.cs	Adds named alternates for implicit operators and suppresses CA1000 for `Fail` overloads.

Copilot · 2026-04-24T19:02:28Z

+#pragma warning disable CA1000
 	public static new Result<T> Fail(string errorMessage)


If you keep the CA1000 suppression approach, please add a brief justification comment next to the #pragma warning disable/restore so future maintainers understand why static factories on Result<T> are intentional here.

Copilot · 2026-04-24T19:02:28Z

+#pragma warning disable CA1307
                            !context.Request.Path.StartsWithSegments(HealthEndpointPath)
                            && !context.Request.Path.StartsWithSegments(AlivenessEndpointPath)
+#pragma warning restore CA1307


CA1307 is being suppressed here, but PathString.StartsWithSegments has overloads that accept a StringComparison. Prefer calling the appropriate overload (e.g., StartsWithSegments("/health", StringComparison.OrdinalIgnoreCase) or using PathString constants) and remove the pragma so the analyzer remains effective.

Suggested change

#pragma warning disable CA1307

!context.Request.Path.StartsWithSegments(HealthEndpointPath)

&& !context.Request.Path.StartsWithSegments(AlivenessEndpointPath)

#pragma warning restore CA1307

!context.Request.Path.StartsWithSegments(HealthEndpointPath, System.StringComparison.OrdinalIgnoreCase)

&& !context.Request.Path.StartsWithSegments(AlivenessEndpointPath, System.StringComparison.OrdinalIgnoreCase)

Copilot · 2026-04-24T19:02:28Z

+	public T? ToValue() => Value;
+
+	public static Result<T> FromValue(T? value) => Ok(value);
+


Result<T>.FromValue(T? value) is a new public static member on a generic type and will also trigger CA1000 (the pragma currently only covers the Fail overloads). Either include this method in the CA1000 suppression with a justification, or move the named alternate to the non-generic Result type and have the operator delegate to it.

Copilot · 2026-04-24T19:02:29Z


+	public T? ToValue() => Value;
+
+	public static Result<T> FromValue(T? value) => Ok(value);


There are now two similarly named APIs with different null semantics: Result.FromValue<T>(null) returns a failure, but Result<T>.FromValue(null) returns a success (via Ok(null)). This is easy to misuse—consider aligning behavior or renaming one of the methods to make the null-handling contract explicit.

Suggested change

public static Result<T> FromValue(T? value) => Ok(value);

public static Result<T> FromValue(T? value)

{

return value is null

? Fail("Value cannot be null.")

: Ok(value);

}

github-actions · 2026-04-24T19:02:48Z

Test Results Summary

206 tests ±0 206 ✅ ±0 14s ⏱️ ±0s
5 suites ±0 0 💤 ±0
5 files ±0 0 ❌ ±0

Results for commit 961e2b9. ± Comparison against base commit 475185e.

♻️ This comment has been updated with latest results.

codecov · 2026-04-24T19:03:04Z

Codecov Report

❌ Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.51%. Comparing base (475185e) to head (961e2b9).
⚠️ Report is 15 commits behind head on sprint/6-code-quality.

Files with missing lines	Patch %	Lines
src/Domain/Abstractions/Result.cs	0.00%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@                    Coverage Diff                    @@
##           sprint/6-code-quality     #150      +/-   ##
=========================================================
- Coverage                  76.93%   76.51%   -0.43%     
=========================================================
  Files                         43       43              
  Lines                        672      677       +5     
  Branches                     111      112       +1     
=========================================================
+ Hits                         517      518       +1     
- Misses                       105      108       +3     
- Partials                      50       51       +1

Files with missing lines	Coverage Δ
src/Domain/Behaviors/ValidationBehavior.cs	`100.00% <100.00%> (ø)`
src/Domain/Abstractions/Result.cs	`72.22% <0.00%> (-9.03%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Move Result<T>.FromValue inside CA1000 pragma block with justification comment - Align Result<T>.FromValue(null) to return Fail (consistent with Result.FromValue<T>(null)) - Replace CA1307 pragma in Extensions.cs with StartsWithSegments StringComparison overload Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mpaulosky · 2026-04-24T19:27:21Z

✅ Aragorn Review — Copilot Comments Addressed

All 4 actionable Copilot review suggestions have been implemented in commit 961e2b9:

Result.cs — Justification comment: Added a // Suppress CA1000: comment block above the #pragma warning disable line explaining the design rationale.
Result.cs — Inline pragma text: Added // Do not declare static members on generic types text to both disable and restore lines.
Result.cs — FromValue moved inside pragma block: The method now lives within the CA1000 suppression scope rather than outside it (where it would still trigger the analyzer).
Result.cs — Null semantics aligned: Result<T>.FromValue(null) now returns Fail("Value cannot be null.") — consistent with the non-generic Result.FromValue<T>(null) behavior. Previously it returned Ok(null).
Extensions.cs — CA1307 pragma removed: Both StartsWithSegments calls now pass System.StringComparison.OrdinalIgnoreCase as the second argument, eliminating the need for the suppression pragma.

All gates passed on push (build ✅, unit tests ✅, architecture tests ✅, bUnit tests ✅, integration tests ✅).

## Sprint 6: Code Quality **Goal:** Eliminate analyzer warnings, enforce internal visibility on Web feature types, expand test coverage, and migrate Unit.Tests into Web.Tests. --- ## Merged Feature PRs - Closes #137 — PR #144 — fix(quality): rename `ct` → `cancellationToken` in all MediatR handlers - Closes #138 — PR #145 — test: add unit tests for BlogPost, Result, and ValidationBehavior (42 tests) - Closes #139 — PR #146 — test: add unit tests for UserManagementHandler (16 tests) - Closes #140 — PR #147 — fix(quality): make Web feature types `internal` (CA1515, ~28 types) - Closes #141 — PR #148 — test: add unit tests for BlogPostMappings (22 tests) - Closes #142 — PR #149 — fix(quality): add ConfigureAwait(false) and specific exception catches - Closes #143 — PR #150 — fix(quality): address Domain and ServiceDefaults analyzer warnings - Closes #151 — PR #152 — feat(tests): migrate Unit.Tests → Web.Tests and remove Unit.Tests project --- ## Test Summary - **80+ new tests** added across all PRs - **105 Web.Tests passing** (0 failures) - `DynamicProxyGenAssembly2` added to `InternalsVisibleTo` in Web.csproj for NSubstitute support ## Notable Changes - `Unit.Tests` project removed; all tests now live in `Web.Tests` - ~28 Web feature types changed from `public` to `internal` (CA1515 compliance) - All MediatR handler parameters renamed from `ct` to `cancellationToken` for clarity - `ConfigureAwait(false)` applied to all async calls in service layer - Domain and ServiceDefaults analyzer warnings resolved --- ## Checklist - [x] All sprint issues closed (#137–#143, #151) - [x] CI green (0 build errors) - [x] 105 Web.Tests passing - [x] Milestone at 100% --------- Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 24, 2026 18:58

github-actions Bot added the squad Squad triage inbox — Lead will assign to a member label Apr 24, 2026

Copilot started reviewing on behalf of mpaulosky April 24, 2026 18:58 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

mpaulosky merged commit 8979828 into sprint/6-code-quality Apr 24, 2026
12 checks passed

mpaulosky deleted the squad/140-domain-servicedefaults-warnings branch April 24, 2026 19:30

mpaulosky mentioned this pull request Apr 25, 2026

sprint(6): merge Sprint 6 — Code Quality #160

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(quality): address Domain and ServiceDefaults analyzer warnings#150

fix(quality): address Domain and ServiceDefaults analyzer warnings#150
mpaulosky merged 2 commits into
sprint/6-code-qualityfrom
squad/140-domain-servicedefaults-warnings

mpaulosky commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Apr 24, 2026 •

edited

Loading

Uh oh!

mpaulosky commented Apr 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		#pragma warning disable CA1000
		public static new Result<T> Fail(string errorMessage)

		public T? ToValue() => Value;

		public static Result<T> FromValue(T? value) => Ok(value);

-	public static Result<T> FromValue(T? value) => Ok(value);
+	public static Result<T> FromValue(T? value)
+	{
+		return value is null
+			? Fail("Value cannot be null.")
+			: Ok(value);
+	}

Conversation

mpaulosky commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

🏗️ PR Added to Squad Triage Queue

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results Summary

Uh oh!

codecov Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mpaulosky commented Apr 24, 2026

✅ Aragorn Review — Copilot Comments Addressed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Apr 24, 2026 •

edited

Loading

codecov Bot commented Apr 24, 2026 •

edited

Loading