Skip to content

Enforce full analyzer set with warnings as errors - #17

Merged
ptr727 merged 0 commit into
developfrom
conformance/analyzer-enforcement
Jul 23, 2026
Merged

Enforce full analyzer set with warnings as errors#17
ptr727 merged 0 commit into
developfrom
conformance/analyzer-enforcement

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Summary

ProjectTemplate#353 downstream - the dedicated analyzer PR of the issue #14 convergence sequence (item 2 of the curated addendum).

  • Remove the dotnet_analyzer_diagnostic.severity = suggestion downgrade from .editorconfig.
  • Add Directory.Build.props with the fleet-standard analyzer block (TreatWarningsAsErrors=true, AnalysisMode=All, AnalysisLevel=latest-all, EnableNETAnalyzers), and dedupe the hoisted common properties from the three csproj files.
  • xUnit1051 (180 sites): every async call in the test suite now passes TestContext.Current.CancellationToken, so xUnit v3 can cancel a stalled test. Passed as a named argument where calls carry other arguments (several target methods have intermediate optional parameters). Two sites that already passed default positionally were converted rather than appended.
  • xUnit1030 (1 site): stray .ConfigureAwait(false) removed from a test.
  • CS8625 (2 sites): benchmarks passed null for the non-nullable sha1 column - fixed with real hash values (computed for the seed record, synthesized for the insert loop), not a null-forgiving operator.
  • Surfaced by full enforcement, fixed at the source:MediaBenchmarks implements IAsyncDisposable (CA1001/CA1816) and stays unsealed (BDN1002); the unreadable-key-file test uses a platform guard CA1416's flow analysis recognizes (Assert.SkipWhen is invisible to it), behavior unchanged.
  • Structural can't-fix rules relaxed per the established per-project pattern: new PhotoCleanerBenchmarks/.editorconfig for CA2007 (harness code, no sync context - same rationale as the tests project) and CA1515 (BenchmarkDotNet requires public, unsealed classes).
  • SQLite/NU1903 needed no change here - resolved on develop by Add --apikey-file for trash and update all dependencies #15; the zero-warning build confirms.

Testing

  • Build is zero-warning / zero-error across all three projects under full enforcement.
  • dotnet format style --verify-no-changes: clean. editorconfig-checker: exit 0.
  • Full test suite: 324/324 pass.

🤖 Generated with Claude Code

CopilotAI review requested due to automatic review settings July 23, 2026 16:31
@codecov

codecovBot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.96%. Comparing base (50a524e) to head (adb224b).

Additional details and impacted files
@@ Coverage Diff @@## develop #17 +/- ##
========================================
Coverage 44.96% 44.96% ========================================
Files 25 25 Lines 3398 3398 Branches 259 259 ========================================
Hits 1528 1528 Misses 1824 1824 Partials 46 46 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens build-time code quality enforcement across the solution by enabling the full .NET analyzer set and treating warnings as errors, then updating projects/tests/benchmarks to comply (notably xUnit v3 cancellation support and benchmark analyzer relaxations).

Changes:

  • Add Directory.Build.props to centralize common project properties and enable full analyzers with warnings-as-errors; remove the .editorconfig analyzer downgrade.
  • Update the xUnit test suite to pass TestContext.Current.CancellationToken through async call chains and remove a stray .ConfigureAwait(false) in a test.
  • Fix benchmark code to satisfy analyzer requirements (non-null sha1, implement IAsyncDisposable) and add a benchmarks-specific .editorconfig to relax structurally incompatible rules.

Reviewed changes

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

Show a summary per file
FileDescription
Directory.Build.propsCentralizes analyzer + common MSBuild properties (TFM, nullable, WAE, analysis mode/level).
.editorconfigRemoves global analyzer severity downgrade to allow full enforcement.
PhotoCleaner/PhotoCleaner.csprojRemoves duplicated analyzer/common properties now provided by Directory.Build.props.
PhotoCleanerTests/PhotoCleanerTests.csprojRemoves duplicated common properties now provided by Directory.Build.props.
PhotoCleanerBenchmarks/PhotoCleanerBenchmarks.csprojRemoves duplicated common properties now provided by Directory.Build.props.
PhotoCleanerBenchmarks/.editorconfigPer-project relaxations for benchmark harness constraints (CA1515/CA2007).
PhotoCleanerBenchmarks/MediaBenchmarks.csAvoids null sha1 inserts and adds async disposal to satisfy analyzer requirements.
PhotoCleanerTests/UndoTaskTests.csPasses xUnit cancellation token into UndoTask.Execute(...) calls.
PhotoCleanerTests/TrashDatabaseTests.csThreads xUnit cancellation token through Trash DB async calls.
PhotoCleanerTests/TrashCommandTests.csThreads xUnit cancellation token through Trash DB verification calls.
PhotoCleanerTests/ProcessTaskTests.csAdds xUnit cancellation token to async I/O + DB/trash DB calls in tests.
PhotoCleanerTests/IndexTaskTests.csThreads xUnit cancellation token through index-task and DB calls.
PhotoCleanerTests/ImportTaskTests.csThreads xUnit cancellation token through hashing/DB/exiftool calls.
PhotoCleanerTests/DatabaseTests.csThreads xUnit cancellation token through DB and file I/O calls.
PhotoCleanerTests/CommandLineTests.csReworks the unreadable-key-file test to use a platform guard analyzers can understand.

Comment threadPhotoCleanerBenchmarks/MediaBenchmarks.cs
Comment threadPhotoCleanerBenchmarks/MediaBenchmarks.cs
Comment threadPhotoCleanerTests/TrashDatabaseTests.cs Outdated
Comment threadPhotoCleanerTests/DatabaseTests.cs Outdated
CopilotAI review requested due to automatic review settings July 23, 2026 16:37

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Comment threadPhotoCleanerTests/TrashDatabaseTests.cs Outdated
Comment threadPhotoCleanerTests/DatabaseTests.cs Outdated
CopilotAI review requested due to automatic review settings July 23, 2026 16:43

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged this pull request into developJul 23, 2026
6 checks passed
@ptr727
ptr727 deleted the conformance/analyzer-enforcement branch July 23, 2026 16:49
ptr727 added a commit that referenced this pull request Aug 3, 2026
* Enforce full analyzer set with warnings as errors
ProjectTemplate#353 downstream (issue #14 item 2):
- Remove the dotnet_analyzer_diagnostic.severity = suggestion
downgrade from .editorconfig.
- Add Directory.Build.props with the fleet-standard analyzer block
(TreatWarningsAsErrors, AnalysisMode All, AnalysisLevel latest-all)
and hoist the common properties out of the three csproj files.
- Pass TestContext.Current.CancellationToken at all 180 xUnit1051
call sites (named argument where calls have other arguments);
remove a stray ConfigureAwait in a test (xUnit1030).
- Fix CS8625 in benchmarks by using real sha1 values instead of null.
- MediaBenchmarks implements IAsyncDisposable (CA1001/CA1816);
BenchmarkDotNet requires the class stay unsealed (BDN1002).
- Restructure the unreadable-key-file test to a platform guard the
CA1416 flow analysis recognizes; behavior unchanged.
- Add PhotoCleanerBenchmarks/.editorconfig relaxing CA2007/CA1515,
mirroring the tests project pattern (harness code, BenchmarkDotNet
public-class requirement).
Build is zero-warning/zero-error under full enforcement; 324 tests
pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Address Copilot review: guarded cleanup, idempotent dispose, lambda tokens
- MediaBenchmarks.Cleanup guards the temp-dir delete (setup may fail
before the directory exists) and DisposeAsync is idempotent (null
check, field cleared after dispose).
- Pass TestContext.Current.CancellationToken to the two
InitializeAsync idempotency assertions wrapped in Func<Task>
lambdas, which the xUnit1051 analyzer does not traverse; swept the
suite for sibling lambda call sites (none).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Pass cancellation token in remaining lambda-wrapped async test calls
Sweep of every lambda-wrapped async call missing the token: the two
duplicate-insert assertions Copilot flagged plus the two concurrent
Select(...) read sites of the same class. The suite grep is now clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ptr727 added a commit that referenced this pull request Aug 3, 2026
* Adopt central package management and LangVersion
Issue #14 round-2 step 2 residual (the analyzer enforcement itself
landed in #17):
- Directory.Packages.props: every dependency version declared once as
a PackageVersion item (18 packages, no cross-project skew).
- Directory.Build.props: add ManagePackageVersionsCentrally and
LangVersion.
- All csproj PackageReference items are versionless; PrivateAssets
and IncludeAssets metadata unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Move ManagePackageVersionsCentrally into Directory.Packages.props
CODESTYLE names Directory.Packages.props as the file that enables
central package management; keep the whole CPM configuration in one
file.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Update SQLite bundle comments for central package management
The direct reference overrides the vulnerable transitive; the version
itself lives in Directory.Packages.props.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727