chore(security): InspectCode noise-floor .DotSettings (#361) - #378
Closed
Chris-Wolfgang wants to merge 5 commits into
Closed
Chris-Wolfgang wants to merge 5 commits into
Chris-Wolfgang wants to merge 5 commits into
Conversation
Silence 1,960 of the 1,980 open InspectCode Code Scanning alerts as demonstrable false positives / duplicate reports: - PublicAPI analyzer FPs (RS0016/RS0036/RS0037 — 1,491): InspectCode runs the analyzer WITHOUT the PublicAPI.*.txt AdditionalFiles, so it thinks nothing is declared and flags every public member. The in-build analyzer gates this correctly via Directory.Build.props Exists() guards. - Multi-TFM FP (RedundantUsingDirective — 198): src multi-targets net462/netstandard2.0/2.1/net8.0/net10.0 with ImplicitUsings only on net10.0, so explicit System usings are REQUIRED on the other TFMs but look redundant to InspectCode (single-TFM analysis). - Roslynator-covered redundancy (92): RedundantCast, RedundantNameQualifier, RedundantExtendsListEntry, RedundantSuppressNullableWarningExpression, RedundantTypeArgumentsOfMethod, RedundantNullableDirective, RedundantWithCancellation, RedundantAssignment, RedundantArgumentDefaultValue. - Nullability defensive checks (6): library defensive null-checks against nullable-oblivious / older-TFM callers. - Test-policy conflicts (xUnit1030 — 32): .editorconfig sets CA2007=none under tests/; the src stack mandates it. Settled policy. - Dup Sonar/Roslyn re-reports (S1939, MA0009, S3220, S2068 — 16+). - Example-project pedagogical patterns: AccessToModifiedClosure (36 — Volatile.Read pattern from Timer callbacks), RCS1102 (23 — Program.cs as static), S125 (12 — illustrative comments), S2930 (10 — short-lived cts in demo Program), S4456 (18 — iterator arg-validation split), S108 (4), S2326 (3). - Test-code intentional patterns: MA0055 (3 — FinalizationSuppressionTests intentionally defines finalizers), S3877 (2 — throwing test doubles), S3871 (1 — internal test-only exception). - Roslynator/IDE00xx-covered unused-variable duplicates (4). Real findings (StaticMemberInGenericType, MA0158, S6966, S5034, S2699, S6608, S4487, S1994, S3267, RCS1194, InvalidXmlDocComment) stay visible so the base library errs on the side of surfacing signal — consistent with the umbrella issue guidance for fleet-critical repos. Expected post-merge Code Scanning count: ~20 (from 1,980), well below the < 25 target in #361. Scorecard (14) already meets < 25. Refs: #361, repo-template#431. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
3 tasks
Fixes and per-pattern suppressions covering the 20 findings that stayed visible after the initial noise-floor drop. Real code fixes (7): - Extract shared abort item-error policy delegate to a non-generic ItemErrorPolicyDefaults helper class, so ExtractorBase / LoaderBase / TransformerBase all reference the same Func instance instead of paying an allocation per closed generic type (fixes 3× StaticMemberInGenericType). - Add the standard exception constructors (parameterless, (string, Exception)) to WrongOverloadCalledException (fixes RCS1194). - Remove the unused `_intervalMs` field from SystemProgressTimerTests' CapturingExtractor test double (fixes S4487). - Add explicit Record.Exception / ExceptionAsync assertions to the DisposableStageContractTests Dispose_is_idempotent tests so they document the assertion contract instead of relying on "no throw" (fixes 2× S2699). - Replace `reports.Last()` with `reports[reports.Count - 1]` in EtlPipelineTests to satisfy the perf micro-opt on List<T> (fixes S6608). - Fix stale <see cref="…"/> targets: EtlPipeline.From → the actual host EtlPipelineSourceExtensions.From in TestDoubles.cs, and add the missing `using System.Collections.Generic;` in PipelineBenchmarks.cs so IAsyncEnumerable<T> resolves in the type's XML docs (fixes 2× InvalidXmlDocComment). Recurring-pattern suppressions added to .DotSettings: - S1994 — infinite `for (var i = 0; ; i++)` generator loops in TestExtractor.Generate* and `for (var attempt = 1; ; attempt++)` retry loops in RetryingExtractor / RetrySeamTests. Loops break internally. - S1215 — GC.Collect + WaitForPendingFinalizers in AllocationBudgetContractTests. Settling the heap is the whole point of the measurement. - MA0158 — System.Threading.Lock is net9.0+; `new object()` for a lock is required on 4 of the 5 TFMs (net462/netstandard2.0/2.1/net8.0) and a #if NET9_0_OR_GREATER split isn't worth the ceremony for a test-only progress-capture double. - S3267 — MiddlewareExtensions validates with fail-fast throw at the first null; LINQ Where + Any would lose enumeration-position semantics and require two passes. - S5034 — TestKitFuzzTests.Drain is the deliberate sync-over-async bridge for the synchronous CsCheck sampler. - S6966 — cts.Cancel() from inside an `await foreach` in ExtractorBase / TransformerBase contract tests. CancelAsync() would race the "the enumerator sees cancel on the next MoveNextAsync" assertion. Extends the family already suppressed by the surrounding `#pragma warning disable CA1849, VSTHRD103`. Post-merge expected InspectCode count: 0 (from 20). Scorecard already under target at 14. Refs: #361. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
BenchmarkDotNet
Details
| Benchmark suite | Current: 0ded397 | Previous: 9f7a223 | Ratio |
|---|---|---|---|
Wolfgang.Etl.Abstractions.Benchmarks.ExtractorBenchmarks.Extract_NoProgress(RecordCount: 1000) |
36441.04443359375 ns (± 1025.9621470216405) |
32261.896341959637 ns (± 364.15799521714877) |
1.13 |
Wolfgang.Etl.Abstractions.Benchmarks.ExtractorBenchmarks.Extract_WithProgress(RecordCount: 1000) |
36426.62489827474 ns (± 56.66652039101131) |
35381.66305541992 ns (± 290.01444589193056) |
1.03 |
Wolfgang.Etl.Abstractions.Benchmarks.ExtractorBenchmarks.Extract_NoProgress(RecordCount: 100000) |
3127929.6770833335 ns (± 3277.654428092272) |
3147298.1979166665 ns (± 4080.1891642462997) |
0.99 |
Wolfgang.Etl.Abstractions.Benchmarks.ExtractorBenchmarks.Extract_WithProgress(RecordCount: 100000) |
3469765.9388020835 ns (± 5068.8504903200665) |
3460913.9778645835 ns (± 12980.283555153379) |
1.00 |
Wolfgang.Etl.Abstractions.Benchmarks.PipelineBenchmarks.FluentPipeline(RecordCount: 1000) |
29487.31086222331 ns (± 94.37021654431574) |
30769.176920572918 ns (± 67.1035212022148) |
0.96 |
Wolfgang.Etl.Abstractions.Benchmarks.PipelineBenchmarks.ManualComposition(RecordCount: 1000) |
29055.817286173504 ns (± 85.8678606969175) |
29653.87747701009 ns (± 69.75862046871877) |
0.98 |
Wolfgang.Etl.Abstractions.Benchmarks.PipelineBenchmarks.BaseClassComposition(RecordCount: 1000) |
78573.76778157552 ns (± 363.5809967670395) |
84119.70609537761 ns (± 199.43699171620807) |
0.93 |
Wolfgang.Etl.Abstractions.Benchmarks.PipelineBenchmarks.FluentPipeline(RecordCount: 100000) |
2874997.6471354165 ns (± 6521.503927211178) |
2858407.7447916665 ns (± 718.8118438209126) |
1.01 |
Wolfgang.Etl.Abstractions.Benchmarks.PipelineBenchmarks.ManualComposition(RecordCount: 100000) |
2853502.6178385415 ns (± 2065.759483467366) |
2921320.4205729165 ns (± 6424.736099927367) |
0.98 |
Wolfgang.Etl.Abstractions.Benchmarks.PipelineBenchmarks.BaseClassComposition(RecordCount: 100000) |
8297563.677083333 ns (± 22105.094951581654) |
8379163.734375 ns (± 2341.3678513388713) |
0.99 |
This comment was automatically generated by workflow using github-action-benchmark.
Owner
Author
Reconciliation — this PR overlaps ~80% with the vNext code-scanning cleanup stackHeads-up: this #361 work was done in parallel with an 8-PR cleanup stack on The code changes here are fully duplicated by the vNext stack
Same intent, different type name ( The
|
…ine-2223a8 # Conflicts: # src/Wolfgang.Etl.Abstractions/ExtractorBase.cs # src/Wolfgang.Etl.Abstractions/LoaderBase.cs # src/Wolfgang.Etl.Abstractions/TransformerBase.cs # src/Wolfgang.Etl.TestKit.Xunit/DisposableStageContractTests.cs
Owner
Author
|
Superseded — closing. The trimmed structural-only |
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
Add
ETL-Abstractions.sln.DotSettingsto silence 1,960 of 1,980 open InspectCode Code Scanning alerts as false positives / duplicate reports of rules the in-build analyzer stack already gates.Addresses the InspectCode half of #361. Scorecard side is already at 14 (below the < 25 target) and needs no action here.
Alert reduction
AdditionalFilesin InspectCode)using System;)Redundant*rules.editorconfigCA2007=noneundertests/)Expected post-merge count: ~20 (target: < 25 per #361).
What stays visible (deliberately)
Because ETL-Abstractions is the base library of the ETL fleet, real findings are NOT suppressed — the umbrella guidance is "err on the side of fixing real findings rather than blanket-suppressing":
StaticMemberInGenericType(3) — static field in generic base classMA0158(1) — useSystem.Threading.Lockin ProgressCaptureS6966(2) —await CancelAsyncin TestKit contract testsS5034(1) — ValueTask consumed twice in fuzz testS2699(2) — tests missing assertionsS6608(1) — indexer vsLast()S4487(1) — unused private field in timer testS1994(2) —forloop stop conditionS3267(1) — LINQWhererefactor opportunityRCS1194(1) — exception constructor implementationInvalidXmlDocComment(2) — stale crefsThese become the actionable follow-up list once the noise clears.
Fleet consistency
Same shape as
Extensions-Logging-Data.slnx.DotSettings(proving-ground for repo-template#431), extended for this repo's specific tail (example-project patterns and finalizer/throw test doubles). Documented per-category in the file.Test plan
ReSharper InspectCodejob passes (green — no error-severity findings, warnings drop from Code Scanning per the DO_NOT_SHOW entries)gh api "repos/Chris-Wolfgang/ETL-Abstractions/code-scanning/alerts?state=open&tool_name=InspectCode" --paginate --jq '.[].number' | wc -lreturns < 25gh api "repos/Chris-Wolfgang/ETL-Abstractions/code-scanning/alerts?state=open" --paginate --jq '.[].number' | wc -lreturns < 50 totalRefs: #361, repo-template#431.
🤖 Generated with Claude Code