chore: bump ANcpLua.NET.Sdk to 3.4.29, delete WarningsNotAsErrors carve-out#15
Conversation
…ve-out
global.json msbuild-sdks pinned to 3.4.29 (analyzer relaxations land:
14 NetAnalyzers + 11 AL style rules go warning → suggestion globally).
With that floor lowered, the 25-rule Directory.Build.props carve-out is
no longer needed — file deleted instead of trimmed.
Surfaced 76 real bug-class errors that the carve-out was hiding. Each
fixed at the code level, not re-suppressed:
TimeProvider migration (AL0026 + RS0030):
- Production: RichProblemDetailsFactory uses TimeProvider.System.GetUtcNow()
- Tests: replaced DateTime/DateTimeOffset.UtcNow / DateTime.Now across
PaperlessServices.Tests and PaperlessREST.Tests integration + unit suites
- PaperlessUI.Blazor Weather demo uses TimeProvider for current date
Disposable hygiene (CA2000):
- MinioOptions / ElasticsearchOptions: client construction inlined into
DI factory so disposal lifetime is owned by the container, not the
options-record helper
- ReportProcessor: XmlReader wrapped in using
- GlobalExceptionHandlerTests: Activity wrapped in using var
- Integration test fixtures: nullable backing fields with property
accessors (replaces null!/default! initializers that triggered IDE0370)
- DocumentEndpointTests: nested try/catch covers pre-Add() window for
ByteArrayContent; SuppressMessage justifies the standard HttpClient
content-ownership-transfer pattern
ValueTask hygiene (CA2012):
- TypedErrorOrAsyncExtensions.ToOkOr404 now extends ErrorOr<T> directly
(was ValueTask<ErrorOr<T>>), so callers await first and the analyzer
sees a direct extension call instead of a ValueTask pipeline
- DocumentEndpoints GetDocumentById/GetSummary updated to await-first
Cryptographic randomness (CA5394):
- Blazor Weather sample: RandomNumberGenerator.GetInt32 instead of Random
Naming (IDE1006):
- Test method renames from snake_case to PascalCase_When_Then style
(matches existing test naming convention in PaperlessREST.Tests/Unit)
Suppression hygiene (IDE0370):
- Removed redundant null-forgiving operators identified by the analyzer
Drive-by fix to Pipeline/Components/ITest.cs: the existing namespace filter
`*.Unit.*` / `*.Integration.*` matched zero tests because the project's
test namespaces are exactly `*.Tests.Unit` / `*.Tests.Integration` (no
sub-namespace). Filter now correctly discovers all 273 unit tests +
integration tests.
Verification (local, against packed 3.4.29):
- `./build.sh Compile --configuration Release`: 0 errors, 0 warnings
- `dotnet build Paperless.slnx -c Release`: 0 errors, 0 warnings
- `./build.sh UnitTests --configuration Release`:
PaperlessServices.Tests.Unit: 59/59 pass
PaperlessREST.Tests.Unit: 214/214 pass
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (35)
📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Summary by CodeRabbitRelease Notes
WalkthroughDie Änderung entfernt die Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (6 passed)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the repo to use ANcpLua.NET.Sdk 3.4.29 and removes the previous WarningsNotAsErrors carve-out by fixing the newly-surfaced analyzer issues directly in code. It also corrects the NUKE/MTP namespace filters so unit/integration targets actually match the existing test namespaces.
Changes:
- Bump
ANcpLua.NET.Sdk{,.Web,.Test}to3.4.29inglobal.jsonand deleteDirectory.Build.props(warnings carve-out removal). - Address analyzer findings across production and tests (TimeProvider migration, disposal fixes, ValueTask/extension usage updates, naming fixes).
- Fix test discovery filters for unit/integration test targets in the pipeline.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Pipeline/Components/ITest.cs | Fix namespace filters for Unit/Integration test targets. |
| PaperlessUI.Blazor/Components/Pages/Weather.razor | Replace DateTime.Now/Random usage with TimeProvider + cryptographic RNG. |
| PaperlessServices.Tests/Unit/SearchIndexServiceTests.cs | TimeProvider updates; settings disposal adjustments. |
| PaperlessServices.Tests/Unit/OcrWorkerTests.cs | Ensure OcrWorker is disposed in tests. |
| PaperlessServices.Tests/Unit/OcrProcessorTests.cs | TimeProvider updates; naming fixes (s_ statics). |
| PaperlessServices.Tests/Integration/WorkerTestBase.cs | TimeProvider updates; Minio client setup/disposal changes. |
| PaperlessServices.Tests/Integration/SearchIndexIntegrationTests.cs | TimeProvider updates; constant naming normalization. |
| PaperlessServices.Tests/Integration/OcrIntegrationTests.cs | TimeProvider updates; constant naming normalization. |
| PaperlessServices.Tests/Integration/FakeTextSummarizer.cs | Simplify fake summarizer behavior/cancellation handling. |
| PaperlessServices.Tests/GlobalUsings.cs | Add System.Diagnostics.CodeAnalysis for suppressions. |
| PaperlessREST/Host/Extensions/TypedErrorOrAsyncExtensions.cs | Refactor ToOkOr404/ToNoContentOr404 to avoid ValueTask analyzer issues. |
| PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs | Inline Minio/Elasticsearch client construction in DI factories. |
| PaperlessREST/Host/Extensions/RichProblemDetailsFactory.cs | Use TimeProvider.System.GetUtcNow() for metadata timestamps. |
| PaperlessREST/Features/DocumentManagement/Presentation/Endpoints/DocumentEndpoints.cs | Await-first pattern to work with updated ErrorOr extensions. |
| PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs | Ensure XmlReader is disposed before schema compilation. |
| PaperlessREST/Configuration/MinioOptions.cs | Tighten string comparison; remove client factory helper. |
| PaperlessREST/Configuration/ElasticsearchOptions.cs | Remove client factory extension helper. |
| PaperlessREST.Tests/Unit/ReportProcessorTests.cs | Naming update (s_ static). |
| PaperlessREST.Tests/Unit/OcrResultListenerTests.cs | Dispose listener in tests; TimeProvider updates. |
| PaperlessREST.Tests/Unit/MappingTests.cs | TimeProvider updates. |
| PaperlessREST.Tests/Unit/GlobalExceptionHandlerTests.cs | Activity disposal pattern update; naming updates. |
| PaperlessREST.Tests/Unit/GenAiResultListenerTests.cs | Dispose listener in tests; TimeProvider updates. |
| PaperlessREST.Tests/Unit/ExceptionHandlerTests.cs | Use a more specific exception type for analyzer compliance. |
| PaperlessREST.Tests/Unit/EndpointsTests.cs | TimeProvider updates; naming for static builders. |
| PaperlessREST.Tests/Unit/DocumentTests.cs | Naming updates; TimeProvider usage. |
| PaperlessREST.Tests/Unit/DocumentServiceTests.cs | TimeProvider updates. |
| PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs | TimeProvider updates in mocks. |
| PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs | Refactor fixture init; add configured factory class; Minio setup changes. |
| PaperlessREST.Tests/Integration/DocumentRepositoryIntegrationTests.cs | Replace null-forgiving fields with guarded accessors; TimeProvider updates. |
| PaperlessREST.Tests/Integration/DocumentEndpointTests.cs | Dispose multipart content; add CA2000 suppression + safer ownership transfer handling. |
| PaperlessREST.Tests/Integration/DocumentAccessRepositoryIntegrationTests.cs | Replace null-forgiving fields with guarded accessors; TimeProvider updates; naming. |
| PaperlessREST.Tests/Integration/BatchOrchestratorIntegrationTests.cs | Naming updates; TimeProvider updates; nullability cleanup. |
| PaperlessREST.Tests/GlobalUsings.cs | Add System.Diagnostics.CodeAnalysis for suppressions. |
| PaperlessREST.Tests/DocumentBuilder.cs | Default timestamps via TimeProvider. |
| global.json | Bump ANcpLua.NET.Sdk versions to 3.4.29. |
| Directory.Build.props | Delete warnings carve-out file. |
Comments suppressed due to low confidence (1)
PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs:155
NpgsqlDataSourceis created here but not registered with DI or disposed.UseNpgsql(dataSource)does not make EF/Core own the data source lifetime, so this can leak connections/resources across test runs. Prefer registering theNpgsqlDataSourceas a singleton inConfigureTestServices(so the container disposes it), or avoid manually creating it and use a connection string overload instead.
NpgsqlDataSource dataSource = new NpgsqlDataSourceBuilder(postgresConnectionString)
.MapEnum<DocumentStatus>("document_status")
.Build();
services.AddPooledDbContextFactory<DocumentPersistence>(opts =>
opts.UseNpgsql(dataSource));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using MinioClient minioClient = new(); | ||
| minioClient | ||
| .WithEndpoint(minioEndpoint) | ||
| .WithCredentials(_minio.GetAccessKey(), _minio.GetSecretKey()) | ||
| .Build(); | ||
|
|
||
| await minioClient.MakeBucketAsync(new MakeBucketArgs().WithBucket(_bucketName)); |
| using MinioClient minioClient = new(); | ||
| minioClient | ||
| .WithEndpoint(minioEndpoint) | ||
| .WithCredentials(_minio.GetAccessKey(), _minio.GetSecretKey()) | ||
| .Build(); | ||
| await minioClient.MakeBucketAsync(new MakeBucketArgs().WithBucket(_bucketName)); |
Summary
ANcpLua.NET.Sdk{,.Web,.Test}to 3.4.29 inglobal.json. The new SDK demotes 14 NetAnalyzers + 11 AL style rules fromwarning→suggestionglobally, so the 25-ruleWarningsNotAsErrorscarve-out becomes deletable.Directory.Build.props(it was 100% the carve-out — no other content).Pipeline/Components/ITest.cs: the existing*.Unit.*/*.Integration.*namespace filters matched zero tests against*.Tests.Unit/*.Tests.Integration(no sub-namespace). Filter now correctly discovers all 273 unit tests.What broke (and how each was fixed)
TimeProvider migration (AL0026 + RS0030):
RichProblemDetailsFactoryswitched toTimeProvider.System.GetUtcNow()DateTime/DateTimeOffset.UtcNow/DateTime.Nowreplaced throughoutPaperlessServices.TestsandPaperlessREST.TestsPaperlessUI.BlazorWeather sample uses TimeProvider for the date columnDisposable hygiene (CA2000):
MinioOptions/ElasticsearchOptions: client construction inlined into the DI factory so disposal lifetime is container-owned, not options-record-ownedReportProcessorwrapsXmlReader.Create(...)inusingnull!/default!to nullable backing fields + property accessor with throw-on-uninitialized (this is also what cleared IDE0370 — see below)GlobalExceptionHandlerTestsActivity wrapped inusing varDocumentEndpointTests.CreatePdfUploadAsyncuses nested try/catch to cover the pre-Add()window; a single[SuppressMessage]justifies the standard HttpClient content-ownership-transfer patternValueTask hygiene (CA2012):
TypedErrorOrAsyncExtensions.ToOkOr404now extendsErrorOr<T>directly (wasValueTask<ErrorOr<T>>). Callersawaitfirst, then call the extension on the value — the analyzer sees a direct extension call instead of a ValueTask pipelineDocumentEndpoints.GetDocumentById/GetSummaryupdated to await-firstCryptographic randomness (CA5394):
RandomNumberGenerator.GetInt32(...)instead ofRandomNaming (IDE1006):
snake_casetoPascalCase_When_Thenstyle (matches existing convention inPaperlessREST.Tests/Unit/)Suppression hygiene (IDE0370):
Test plan
./build.sh Compile --configuration Release— 0 errors, 0 warningsdotnet build Paperless.slnx -c Release— 0 errors, 0 warnings./build.sh UnitTests --configuration Release— 273/273 pass (59 PaperlessServices.Tests.Unit + 214 PaperlessREST.Tests.Unit)Notes
PaperlessREST.Tests.Integrationnot run locally (needs Testcontainers); CI exercises it.🤖 Generated with Claude Code