You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Zero code-analysis warnings on dotnet build Ploch.Data.slnx -c Release — all 31 unique analyzer warnings (9 in shipping src/, 22 in tests) resolved. Remaining NU1603 lines are tracked by Fix sample app NuGet dependency versions for clean-machine builds #68 (out of scope per the issue).
Describe your changes
Shipping src/ (real fixes, no behaviour change):
Data.EFCore/DbContextExtensions.cs — GetStaticPropertyValue gained full XML docs (CS1591) and moved below the Set overloads so they are adjacent (S4136); reflection lookup uses nameof(Set) (CC0021); S3011 suppressed inline with justification — reading non-public static properties is the helper's documented purpose and is covered by GetStaticPropertyValueTests.
Data.GenericRepository.EFCore/ReadRepositoryAsync.cs — new protected AuditEntityHandler property eliminates the CS9107 double capture (primary-ctor parameter was captured in the derived type and passed to base). Additive member; no API breakage.
Data.EFCore.SqLite/SqLiteDbContextConfigurator.cs — primary constructor (IDE0290); <param> docs moved to class level.
Data.EFCore.SqLite/SqLiteDbContextFactory.cs — CA1000 suppressed with justification: CreationLifecycle is intentionally accessed via concrete non-generic derived factories (e.g. MyDbContextFactory.CreationLifecycle), so callers never supply type arguments — the rule's rationale does not apply. Moving it off the generic type would be an unnecessary breaking change.
Culture-explicit calls (CA1305/CA1304/CA1311). Deliberate choice: the converter tests use CultureInfo.CurrentCulture because CollectionStringSplitConverter currently writes with current culture — using invariant would break the tests on non-invariant machines. The converter's write/read culture asymmetry is a real latent bug, filed as fix(data-efcore): CollectionStringSplitConverter writes with CurrentCulture but reads with InvariantCulture #97; these call sites flip to invariant when it's fixed.
Removed dead Data property (SA1137 — nothing references it via [MemberData]), commented-out code and the TODO comment (S125/SA1005/S1135 — the TODO's substance is preserved in follow-up issue test(sqlserver): Restore broken SQL Server container integration tests (currently skipped) #98), renamed builder → modelBuilder (CA1725/S927), sealed/static-ified test helper types (CA1852/RCS1102/S1118), fixed invalid pragma text (CS1696), generic FluentAssertions overload (CA2263), ToUpperInvariant for duplicate detection, MyMethod renamed per the test naming convention.
Reflection-target members (PrivateValue, NullValue) carry [SuppressMessage] with justification — they are read via GetStaticPropertyValue reflection, which the analyzers cannot see.
tests/.editorconfig — VSTHRD200 disabled for tests with the same rationale as the pre-existing s4261 disable directly above it: the repository's test naming convention (Method_should_do_X) intentionally has no Async suffix, and xUnit test methods are never awaited by user code.
I have updated documentation. (XML docs added where CS1591 flagged; no markdown docs describe the touched internals.)
If applicable, I have updated the sample application (not applicable — no API changes; SampleApp re-run end-to-end, exit 0)
📐 Design Decisions
Suppress CA1000 instead of moving CreationLifecycle — moving it off the generic factory would break the public API for zero practical gain; access via non-generic derived factories already avoids the problem CA1000 exists to prevent.
VSTHRD200 disabled at tests scope, not per-method renames — renaming tests to *_Async would violate the repo's documented naming convention; the disable sits beside the identical, pre-existing s4261 disable.
Second-opinion review note: the usual pre-push Codex review could not run (the configured models are rejected for this account) and the local Gemini fallback has no API quota. Validation relied on a zero-warning build, the full test suite, and this PR's automated reviewers — all of whose findings will be addressed before merge.
eeb1425 — int-list converter test now mirrors CollectionStringSplitConverter's exact write format (Uri.EscapeDataString per element, string.Empty for default values) per Copilot review; the expected query string can no longer diverge from the stored value.
9e01c61 — docs typo fix ("baase" -> "base") in the converter's XML docs.
d32310b — reflection lookup in DbContextExtensions.GetEntitySet now uses nameof(DbContext.Set) (was nameof(Set), which bound to the local extension method by coincidence) per Copilot review; identical compiled string, no behaviour change.
All review threads resolved; SonarCloud platform: 0 issues, 0 hotspots, quality gate OK (89.2% new-code coverage). Codacy's check suite has been stuck queued since eeb1425 (integration stalled on Codacy's side); completing without it was explicitly approved, its last verdict being a pass on 7757250.
Refactored SqLiteDbContextConfigurator to use primary constructor and simplified options handling.
Moved GetStaticPropertyValue extension method to DbContextExtensions and added support for non-public properties.
Improved ReadRepositoryAsync to use a protected property for the audit handler and simplified RepositoriesConfiguration.
Updated various tests to improve code quality, including fixing test naming conventions and adding culture-aware string conversions.
Added threading analyzer suppression to .editorconfig for xUnit test methods.
CodeAnt-AI Description
Resolve analyzer warnings across code and tests
What Changed
Removed the remaining code-analysis warnings in the solution, including cleanup in data access, SQLite, and SQL Server setup code
Added support for reading static properties through the helper again, including private ones used by tests, with clearer error messages when a property is missing or the wrong type
Kept repository auditing behavior intact while removing a redundant capture in the async read path
Updated tests to match current culture-aware formatting, removed dead sample code, and cleaned up test naming and model-building code
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
@codeant-ai ask: Your question here
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
@codeant-ai ask: Can you suggest a safer alternative to storing this secret?
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
@codeant-ai: Your feedback here
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
@codeant-ai: Do not flag unused imports.
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
@codeant-ai: review
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Summary by CodeRabbit
Refactor
Streamlined SQLite configuration and lifecycle test factories.
Added a protected audit handler property for repository access tracking.
Minor formatting/style updates across EF Core and SQL Server.
Bug Fixes
Improved argument validation and reflection-based behavior for static property access.
Adjusted default behavior for the audit access option.
Tests
Expanded null-argument test coverage.
Updated culture-aware serialization and assertion patterns.
Refreshed test naming and connection-string setup.
…ests
Eliminates every code-analysis warning from the solution build ahead of
the v4.0.0 release, so future regressions stand out. Highlights:
- DbContextExtensions: XML docs for GetStaticPropertyValue (CS1591),
Set overloads made adjacent (S4136), nameof(Set) (CC0021), justified
S3011 suppression (non-public access is the helper's documented
purpose).
- ReadRepositoryAsync: new protected AuditEntityHandler property
removes the CS9107 double capture of the primary-ctor parameter
(additive, non-breaking).
- SqLiteDbContextConfigurator converted to a primary constructor
(IDE0290); SqlServerDbContextFactory ctor reformatted (SA1003).
- CA1000 on SqLiteDbContextFactory.CreationLifecycle suppressed with
justification: intended access is via concrete non-generic derived
factories, so no type arguments are ever needed at call sites.
- Tests: culture-explicit ToString/ToUpperInvariant (CA1305/CA1304/
CA1311 - CurrentCulture chosen deliberately to match the converter's
current write behaviour, see #97), dead Data property removed
(SA1137), commented-out code and TODO removed (S125/SA1005/S1135,
substance preserved in #98), builder renamed to modelBuilder
(CA1725/S927), test types sealed/static (CA1852/RCS1102/S1118),
reflection-target members suppressed with justification
(IDE0051/S1144), MyMethod renamed per test naming convention.
- tests/.editorconfig: VSTHRD200 disabled for tests with the same
rationale as the existing s4261 disable (Method_should_do_X naming
convention).
NU1603 warnings remain and are tracked separately by #68.
Refs: #90
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This PR resolves all 31 code-analysis warnings in src and tests by tightening reflection usage, modernizing constructors, eliminating redundant state, adjusting test code to match analyzer expectations, and adding/suppressing diagnostics with explicit justifications — all without changing runtime behavior.
Sequence diagram for async read repository auditing flow
sequenceDiagram
actor RepositoryClient
participant ReadRepositoryAsync
participant DbContext
participant DbSet
participant AuditEntityHandler
RepositoryClient->>ReadRepositoryAsync: GetAllAsync(query, cancellationToken)
ReadRepositoryAsync->>DbSet: ToListAsync(cancellationToken)
DbSet-->>ReadRepositoryAsync: result
loop for each entity in result
ReadRepositoryAsync->>AuditEntityHandler: HandleAccess(entity)
end
ReadRepositoryAsync-->>RepositoryClient: result
Loading
File-Level Changes
Change
Details
Files
Refined DbContext extension helpers and documented/suppressed reflection-based access to static properties.
Use nameof(Set) instead of string literal when locating DbContext.Set via reflection.
Move GetStaticPropertyValue below Set overloads and add complete XML documentation.
Switch reflection binding-flags usage to imported BindingFlags and add inline suppression for S3011 with justification.
Update FindEntityType test expectation to use Be() and adjust NonExistingEntity test type to be sealed with clarified pragma.
Aligned test code with globalization and naming analyzers while preserving current behavior, and added targeted suppressions for reflection-only members.
Use CultureInfo.CurrentCulture in converter tests when formatting int and DateTime values, matching current converter write behavior.
Rename OnModelCreating parameter from builder to modelBuilder and propagate usage.
Add SuppressMessage attributes to reflection-only test members and switch helper type to static.
Replace ToLower duplicate detection with ToUpperInvariant for case-insensitive comparison.
Seal or mark helper test types as static where appropriate and fix invalid pragma text and other minor analyzer findings.
Resolve all 9 analyzer warnings in shipping src libraries through code changes, documentation, or narrowly justified suppressions, without changing runtime behaviour.
Resolve all 22 analyzer warnings in test projects (Data.EFCore.Tests, Data.EFCore.SqLite.Tests, Data.EFCore.SqlServer.Tests, Data.StandardDataSets.Tests) via test code updates or test-scope configuration changes.
Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!
@kploch, you've reached your PR review limit, so we couldn't start this review.
Next review available in:22 minutes
Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).
How can I continue?
After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.
To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.
How do review limits work?
CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.
For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.
The pull request resolves analyzer findings through C# primary constructors, reflection cleanup, repository handler storage, default-value simplification, targeted suppressions, and test updates covering null arguments, culture-aware formatting, naming, and normalization.
Changes
Analyzer cleanup
Layer / File(s)
Summary
Source API and reflection cleanup src/Data.EFCore.SqLite/..., src/Data.EFCore.SqlServer/..., src/Data.EFCore/DbContextExtensions.cs, src/Data.EFCore/CollectionStringSplitConverter.cs
SQLite configurator and factory declarations are updated for analyzer compliance, SQL Server constructor formatting is corrected, reflection lookup uses nameof with explicit null checks, and converter XML documentation is corrected.
Repository state and auditing wiring src/Data.GenericRepository.Data.GenericRepository.EFCore/ReadRepositoryAsync.cs, src/Data.GenericRepository/.../RepositoriesConfiguration.cs
The audit handler is stored in a protected property and reused by access calls; AuditAccess no longer has an explicit false initializer.
EF Core and factory test alignment tests/.editorconfig, tests/Data.EFCore.SqLite.Tests/..., tests/Data.EFCore.Tests/...
Tests and analyzer settings are updated for primary constructors, culture-aware conversion, reflection argument validation, entity assertions, and related code-analysis suppressions.
SQL Server and dataset test cleanup tests/Data.EFCore.SqlServer.Tests/..., tests/Data.StandardDataSets.Tests/CountriesTests.cs
A connection-string test was renamed and simplified, commented SQL Server guidance was removed, and country duplicate detection now uses ToUpperInvariant().
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors several classes to use primary constructors, cleans up test code, and addresses static analysis warnings across the codebase. Key changes include updating DbContextExtensions.GetStaticPropertyValue with warning suppressions for reflection, storing the audit handler in a protected property in ReadRepositoryAsync, and adjusting various test assertions and configurations. Feedback focuses on improving robustness by adding null and whitespace validation to GetStaticPropertyValue and ensuring the auditEntityHandler dependency is validated against null during construction.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer TIP This summary will be updated as you push new changes.
GetStaticPropertyValue now validates type and propertyName with
ArgumentNullException.ThrowIfNull, consistent with the sibling
extension methods in DbContextExtensions, and documents the exception.
ReadRepositoryAsync.AuditEntityHandler fails fast at construction when
the injected handler is null instead of throwing NullReferenceException
at first use. Added two regression tests for the new guards.
Refs: #90
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nameof(Set) resolves to the local extension methods in DbContextExtensions, not to DbContext.Set<T>(). The string value is "Set" either way so this is functionally correct, but it's semantically misleading. Since nameof(DbContext.Set) requires specifying a type argument, the current approach is a reasonable workaround — just noting for awareness.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Data.EFCore/DbContextExtensions.cs` at line 61, Clarify the method-name
lookup in the reflection logic around setMethod so it explicitly communicates
that the target is DbContext.Set<T>(), while preserving the existing
zero-parameter generic-method selection and behavior. Avoid relying on an
unqualified nameof(Set) that can resolve to DbContextExtensions methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Data.EFCore/DbContextExtensions.cs`:
- Line 61: Clarify the method-name lookup in the reflection logic around
setMethod so it explicitly communicates that the target is DbContext.Set<T>(),
while preserving the existing zero-parameter generic-method selection and
behavior. Avoid relying on an unqualified nameof(Set) that can resolve to
DbContextExtensions methods.
ℹ️ Review info⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8477657-a074-4cc5-b740-aa1a44d23ed7
📥 Commits
Reviewing files that changed from the base of the PR and between ef0c106 and 76738c9.
… (IDE0058)
SonarCloud imported IDE0058 external Roslyn diagnostics for the lines
changed in this PR: the fluent UseSqlite return values in
SqLiteDbContextConfigurator and the bool result of
IAuditEntityHandler.HandleAccess in ReadRepositoryAsync. Explicit
discards document that the return values are intentionally unused.
Refs: #90
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CollectionStringSplitConverter_should_handle_int_list searched the
serialised column for a single element such as "4", which can also
match inside another entity's values (e.g. "147") depending on the
AutoFixture-generated data - a latent data-dependent flake that
surfaced on CI for PR #99. The query now searches for the complete
serialised second list, which uniquely identifies the target entity.
Verified stable across five consecutive runs with fresh random data.
Refs: #90
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR targets a clean dotnet build Ploch.Data.slnx -c Release by resolving the remaining analyzer warnings across shipping libraries (src/) and the test suite, primarily through small refactors, targeted suppressions with justification, and test cleanups.
Changes:
Refactored and documented reflection/static-access helpers (including justified suppression for non-public reflection) and improved argument validation via tests.
Adjusted EF Core repository and provider-specific configurator/factory code to address analyzer warnings without intended behavior changes.
Cleaned up tests and test configuration to eliminate analyzer noise (culture-explicit conversions, naming, dead code removal, and test-scope analyzer settings).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
The int-list converter test built its expected query substring with
plain culture-formatted ToString, while CollectionStringSplitConverter
writes each element as Uri.EscapeDataString(value.ToString()) and
string.Empty for default values. Mirror the converter's write format
exactly so the expected string cannot diverge from the stored value
for any generated data. Addresses the Copilot review thread on PR #99.
Refs: #90
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LGTM! The changes are well-structured, clean, and idiomatic. The null guards and corresponding regression tests for GetStaticPropertyValue address the previous feedback, and the primary constructor refactorings across the codebase are clean.
🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does.
nameof(Set) resolved against the local DbContextExtensions.Set
extension method, which only matched the reflected DbContext.Set
method by coincidence of naming. Qualify it as nameof(DbContext.Set)
so a rename of the extension method cannot silently break the lookup.
Identical compile-time string; no behaviour change. Addresses the
Copilot review thread on PR #99.
Refs: #90
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🤖 Review skipped: Repository Owner rate limit exceeded. Free accounts are limited to 3 reviews per 4 hours across all repositories. Upgrade to a paid plan for unlimited reviews.
Codacy dropped the push events for the last three commits while its
plan gate misclassified this public repo as private. The gate is now
cleared; empty commit generates a fresh push event so Codacy analyses
the current PR #99 head.
Refs: #90
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LGTM! The changes are well-structured and clean. Recent fixes for parameter null guards in DbContextExtensions.GetStaticPropertyValue and serializer alignment in CollectionStringSplitConverterTests look solid and well-tested.
🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does.
Copilot review on PR #99: .Contains(serialisedSecondList) could still
match the wrong row if another entity's serialised list contains the
expected list as a substring. The expected value mirrors the converter's
write format exactly, so equality is the deterministic comparison.
Refs: #90
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Added explicit null guards (ArgumentNullException.ThrowIfNull) and XML documentation to DbContextExtensions.GetStaticPropertyValue, with comprehensive unit tests for null arguments.
Refactored ReadRepositoryAsync and SqLiteDbContextConfigurator to primary constructor syntax cleanly.
Fixed test flakiness in CollectionStringSplitConverterTests and updated string operations to be culture-invariant in CountriesTests.
Cleaned up obsolete comments and redundant initializations across tests and repository configuration.
🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does.
Copilot review on PR #99: replace per-element ToUpperInvariant plus
Distinct with Distinct(StringComparer.OrdinalIgnoreCase) — same
case-insensitive semantics without the intermediate string allocations.
Refs: #90
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.
Copilot review on PR #99 (outdated thread, still valid): the DateTime
test located the second entity via Contains on a single element, which
is nondeterministic if the generated lists share a value. Mirror the
converter's write format and compare the full serialised string for
equality, matching the int-list test pattern.
Refs: #90
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.
// Match the complete serialised list exactly rather than searching for a single
// element: a short digit substring such as "4" can also match inside another
// entity's values (e.g. "147"), which made this test fail intermittently.
// Mirror the converter's write format exactly (Uri.EscapeDataString per element,
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
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.
codeant-ai
Bot
added
the
size:M
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
User description
Pull Request Description
Issue ticket number and link
Closes #90
Pull Request Changes Summary
🌿 Other
dotnet build Ploch.Data.slnx -c Release— all 31 unique analyzer warnings (9 in shippingsrc/, 22 in tests) resolved. Remaining NU1603 lines are tracked by Fix sample app NuGet dependency versions for clean-machine builds #68 (out of scope per the issue).Describe your changes
Shipping
src/(real fixes, no behaviour change):Data.EFCore/DbContextExtensions.cs—GetStaticPropertyValuegained full XML docs (CS1591) and moved below theSetoverloads so they are adjacent (S4136); reflection lookup usesnameof(Set)(CC0021); S3011 suppressed inline with justification — reading non-public static properties is the helper's documented purpose and is covered byGetStaticPropertyValueTests.Data.GenericRepository.EFCore/ReadRepositoryAsync.cs— newprotected AuditEntityHandlerproperty eliminates the CS9107 double capture (primary-ctor parameter was captured in the derived type and passed to base). Additive member; no API breakage.Data.EFCore.SqLite/SqLiteDbContextConfigurator.cs— primary constructor (IDE0290);<param>docs moved to class level.Data.EFCore.SqLite/SqLiteDbContextFactory.cs— CA1000 suppressed with justification:CreationLifecycleis intentionally accessed via concrete non-generic derived factories (e.g.MyDbContextFactory.CreationLifecycle), so callers never supply type arguments — the rule's rationale does not apply. Moving it off the generic type would be an unnecessary breaking change.Data.EFCore.SqlServer/SqlServerDbContextFactory.cs— SA1003 formatting.Data.GenericRepository/RepositoriesConfiguration.cs— CA1805 redundant= falseremoved.Tests:
CultureInfo.CurrentCulturebecauseCollectionStringSplitConvertercurrently writes with current culture — using invariant would break the tests on non-invariant machines. The converter's write/read culture asymmetry is a real latent bug, filed as fix(data-efcore): CollectionStringSplitConverter writes with CurrentCulture but reads with InvariantCulture #97; these call sites flip to invariant when it's fixed.Dataproperty (SA1137 — nothing references it via[MemberData]), commented-out code and the TODO comment (S125/SA1005/S1135 — the TODO's substance is preserved in follow-up issue test(sqlserver): Restore broken SQL Server container integration tests (currently skipped) #98), renamedbuilder→modelBuilder(CA1725/S927), sealed/static-ified test helper types (CA1852/RCS1102/S1118), fixed invalid pragma text (CS1696), generic FluentAssertions overload (CA2263),ToUpperInvariantfor duplicate detection,MyMethodrenamed per the test naming convention.PrivateValue,NullValue) carry[SuppressMessage]with justification — they are read viaGetStaticPropertyValuereflection, which the analyzers cannot see.tests/.editorconfig— VSTHRD200 disabled for tests with the same rationale as the pre-existings4261disable directly above it: the repository's test naming convention (Method_should_do_X) intentionally has noAsyncsuffix, and xUnit test methods are never awaited by user code.Checklist before requesting a review
main.)📐 Design Decisions
CreationLifecycle— moving it off the generic factory would break the public API for zero practical gain; access via non-generic derived factories already avoids the problem CA1000 exists to prevent.CurrentCulture(not invariant) in converter tests — matches the code under test's actual write behaviour; the underlying converter bug is tracked in fix(data-efcore): CollectionStringSplitConverter writes with CurrentCulture but reads with InvariantCulture #97 rather than silently changing shipping behaviour in a warnings-cleanup PR.*_Asyncwould violate the repo's documented naming convention; the disable sits beside the identical, pre-existings4261disable.Testing
dotnet build Ploch.Data.slnx -c Release: 0 errors, 0 code-analysis warnings (down from 31 unique); only Fix sample app NuGet dependency versions for clean-machine builds #68-tracked NU1603 remains.dotnet test: 239 passed / 1 failed — the failure is pre-existing test: UnitOfWorkRepositoryAsyncSQLiteInMemoryTests fails locally — Ploch.Common 4.0.7 / 3.x assembly version skew from sibling ploch-common checkout #95 (Ploch.Common assembly-version skew from the sibling checkout), identical onmain.=== Sample Application Complete ===, exit 0.Post-review updates (2026-07-24)
CollectionStringSplitConverter's exact write format (Uri.EscapeDataStringper element,string.Emptyfor default values) per Copilot review; the expected query string can no longer diverge from the stored value.DbContextExtensions.GetEntitySetnow usesnameof(DbContext.Set)(wasnameof(Set), which bound to the local extension method by coincidence) per Copilot review; identical compiled string, no behaviour change.queuedsince eeb1425 (integration stalled on Codacy's side); completing without it was explicitly approved, its last verdict being a pass on 7757250.Related
Summary by Bito
CodeAnt-AI Description
Resolve analyzer warnings across code and tests
What Changed
Impact
✅ Cleaner release builds✅ Fewer analyzer regressions✅ More reliable reflection-based tests💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Summary by CodeRabbit
Refactor
Bug Fixes
Tests