fix(quality): resolve Domain and ServiceDefaults CA warnings - #156
Closed
mpaulosky wants to merge 1 commit into
Closed
fix(quality): resolve Domain and ServiceDefaults CA warnings#156mpaulosky wants to merge 1 commit into
mpaulosky wants to merge 1 commit into
Conversation
github-actions
Bot
added
the
squad
Contributor
🏗️ PR Added to Squad Triage QueueThis PR has been labeled with Next steps:
|
…140 - Remove duplicate copyright header in Result.cs - Remove implicit operator overloads from Result<T> (CA2225) - Add InternalsVisibleTo entries in Domain.csproj (CA1014) - Add ConfigureAwait(false) in ValidationBehavior.cs (CA2007) - Add StringComparison.OrdinalIgnoreCase to StartsWithSegments calls (CA1307) - Validate app parameter null-guard in Extensions.MapDefaultEndpoints (CA1062) - Update Domain.Tests to remove implicit operator usage; fix namespaces and test names
Contributor
Test Results Summary0 tests 0 ✅ 0s ⏱️ Results for commit 7d03aac. ♻️ This comment has been updated with latest results. |
mpaulosky
force-pushed
the
squad/140-domain-servicedefaults-ca-warnings
branch
from
April 24, 2026 23:37
576779e to
7d03aac
Compare
There was a problem hiding this comment.
Pull request overview
This PR focuses on resolving .NET analyzer (CA*) warnings across the Domain, ServiceDefaults, and Domain.Tests projects, primarily by adjusting Result<T> APIs, improving null-validation / string-comparison correctness, and updating related tests.
Changes:
- Removed
Result<T>implicit conversion operators (CA2225) and updated Domain tests to useToValue()/FromValue(...). - Updated
ServiceDefaults/Extensions.csto useStringComparison.OrdinalIgnoreCaseforStartsWithSegments(CA1307) and reformatted code. - Added/updated compliance and visibility settings (CLS compliance +
InternalsVisibleTo) and addedConfigureAwait(false)inValidationBehavior(CA2007).
Reviewed changes
Copilot reviewed 10 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Domain.Tests/GlobalUsings.cs | Formatting-only adjustments to global usings. |
| tests/Domain.Tests/Entities/BlogPostTests.cs | Aligns namespace with project root namespace. |
| tests/Domain.Tests/Behaviors/ValidationBehaviorTests.cs | Aligns namespace with project root namespace. |
| tests/Domain.Tests/Abstractions/ResultTests.cs | Updates tests to avoid implicit conversions; adds coverage for FromValue/ToValue and operator absence. |
| tests/Web.Tests/Properties/AssemblyInfo.cs | Intended CLS compliance change per PR description, but file is empty in current state. |
| tests/Web.Tests.Bunit/Properties/AssemblyInfo.cs | Intended CLS compliance change per PR description, but file is empty in current state. |
| tests/Web.Tests.Integration/Properties/AssemblyInfo.cs | Intended CLS compliance change per PR description, but file is empty in current state. |
| src/ServiceDefaults/Extensions.cs | Adds StringComparison overload usage for path checks; refactoring/formatting. |
| src/Domain/Interfaces/IBlogPostRepository.cs | Formatting-only (tab indentation). |
| src/Domain/Entities/BlogPost.cs | Formatting-only (tab indentation). |
| src/Domain/Domain.csproj | Adds additional InternalsVisibleTo entries for test assemblies. |
| src/Domain/Behaviors/ValidationBehavior.cs | Adds ConfigureAwait(false) and minor LINQ predicate refactor. |
| src/Domain/Abstractions/Result.cs | Removes implicit operators from Result<T> to satisfy CA2225. |
Comment on lines
131
to
135
| } | ||
| #pragma warning restore CA1000 // Do not declare static members on generic types | ||
|
|
||
| public static implicit operator T?(Result<T>? result) | ||
| { | ||
| if (result is null) | ||
| { | ||
| // Return the language default for T? when the Result is null. For value types this will | ||
| // be the underlying default (e.g., 0 for int) which matches existing behavior. | ||
| return default; | ||
| } | ||
|
|
||
| return result.Value; | ||
| } | ||
|
|
||
| public static implicit operator Result<T>(T? value) | ||
| { | ||
| return Ok(value); | ||
| } | ||
| } |
|
|
||
| <ItemGroup> | ||
| <InternalsVisibleTo Include="Architecture.Tests"/> | ||
| <InternalsVisibleTo Include="Domain.Tests"/> |
| @@ -0,0 +1,10 @@ | |||
| //======================================================= | |||
| @@ -0,0 +1,10 @@ | |||
| //======================================================= | |||
| @@ -0,0 +1,10 @@ | |||
| //======================================================= | |||
mpaulosky
added a commit
that referenced
this pull request
May 6, 2026
…emote branch prune (#237) ## Summary Working as Ralph (Meta) Closes #236 ## Changes - **Merged PR #235** — squash merged with all 23 CI checks passing - **Pruned 6 stale remote branches** (Sprint 6–8 orphans, all associated issues/PRs closed since April 2026): - `origin/squad/140-domain-servicedefaults-ca-warnings` (PR #156) - `origin/squad/153-web-infrastructure-warnings` (PR #157) - `origin/squad/154-webtests-bunit-warnings` (PR #158) - `origin/squad/155-test-assembly-ca1014` (PR #159) - `origin/squad/164-domain-tests-xunit-v3-fixes` (PR #171) - `origin/sprint/8-xunit-v3-pilot` (PR #188) - **Updated `identity/now.md`**: Sprint 16 ready, remote hygiene milestone noted - **Updated `ralph/history.md`**: 2026-05-06 follow-up session log appended ## Verification - Pre-commit gate: ✅ 0 markdownlint errors - Pre-push gate (build + tests + integration): ✅ all green Co-authored-by: Boromir <boromir@squad.dev> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #140
Resolves all analyzer warnings in Domain, ServiceDefaults projects, and Domain.Tests.
Changes
CA2225 — Operator overloads without named alternates (
Result.cs)Result<T>(no named alternates)result.ToValue()andResult<T>.FromValue(value)insteadCA1307 — Missing StringComparison overload (
ServiceDefaults/Extensions.cs)StringComparison.OrdinalIgnoreCaseto bothStartsWithSegmentscallsCA1062 — Parameter null validation (
ServiceDefaults/Extensions.cs)ArgumentNullException.ThrowIfNull(app)inMapDefaultEndpointsCA2007 — ConfigureAwait (
ValidationBehavior.cs)ConfigureAwait(false)toawait next(cancellationToken)CA1014 — CLSCompliant assemblies (
Domain.csproj+ test AssemblyInfo files)InternalsVisibleToentries for all test projects inDomain.csproj[assembly: CLSCompliant(false)]toWeb.Tests,Web.Tests.Bunit,Web.Tests.IntegrationDomain.Tests cleanup
Tests.Domain→MyBlog.Domain.Tests)FromValue,ToValue, and absence of implicit operatorsWorking as Sam (Backend / .NET)