Skip to content

Guard the public API surface with Verify snapshots (alternative to #530)#540

Draft
ChrisonSimtian wants to merge 2 commits into
Fallout-build:mainfrom
ChrisonSimtian:build/public-api-verify-guard
Draft

Guard the public API surface with Verify snapshots (alternative to #530)#540
ChrisonSimtian wants to merge 2 commits into
Fallout-build:mainfrom
ChrisonSimtian:build/public-api-verify-guard

Conversation

@ChrisonSimtian

Copy link
Copy Markdown
Collaborator

Alternative to #530 for public-API break detection — the leaner, Verify-snapshot approach (the path taken in the TvdbApi modernisation). Raised so the two can be compared side by side. Pick one — not for merge alongside #530. Non-breaking / dev-infra only.

Approach

vs #530 (Roslyn PublicApiAnalyzers)

#530 (analyzer) this PR (snapshot)
New dependency Microsoft.CodeAnalysis.PublicApiAnalyzers PublicApiGenerator
Contract lives in PublicAPI.*.txt in src/ (×11) .verified.txt in tests/
Config surface .editorconfig + Directory.Build.props + WarningsAsErrors one PackageVersion + one test file
Fits mandated test stack orthogonal to it is the stack
Review gesture read .txt diff read .verified.txt diff

Generated tool wrappers

The ~60k generated tool symbols are excluded via a [GeneratedCode] marker now stamped by Fallout.Tooling.Generator (all 63 *.Generated.cs regenerated). PublicApiGenerator reads compiled metadata, so it can only exclude whole types#530 excludes by file (*.Generated.cs).

⚠️ Residual precision gap — the main axis for comparison. The 20 <Tool>Tasks classes are split partials (generated half + hand-written half merged into one type). The merged type carries [GeneratedCode], so it is dropped whole — ~25 hand-written members that #530 keeps go unguarded here. Member-level file precision is not reachable by a metadata-based tool. This is the cost of "leaner".

Cost of the generated-code stamp

The [GeneratedCode] attribute is the bulk of the diff: 63 regenerated wrappers (~1.2k lines of pure churn) + the 4 one-line generator edits. Without needing generated exclusion this approach would be ~150 lines; #530's file-level exclusion avoids touching the generator at all.

Verified

  • dotnet build green; specs 11/11 pass (baseline round-trips clean).
  • Break detection: adding/removing a public member fails exactly the affected case with a reviewable .verified.txt diff.
  • Generated surface excluded; hand-written standalone Tools types (AzureKeyVault*, MSBuildProject, DotNetRuntimeIdentifier, …) kept.
  • [GeneratedCode] stamp round-trips through GenerateTools (the VerifyGeneratedTools git-clean gate stays satisfied).

Decision

Two viable guards for the same goal. #530 = file-precise, zero gap, more config + a heavier analyzer dep. This = leaner, reuses the existing test stack, but a ~25-member precision gap on split-partial tool tasks and a generator change to stamp [GeneratedCode].

ChrisonSimtian and others added 2 commits July 24, 2026 15:41
The tooling generator now emits [System.CodeDom.Compiler.GeneratedCode] alongside the
existing [ExcludeFromCodeCoverage] on every wrapper class, settings/data class, extension
class and enumeration it produces. This gives a reflection-visible marker that a
metadata-based public-API snapshot can filter on, so the ~60k generated tool symbols can be
excluded without the file-level knowledge a Roslyn analyzer has. All 63 *.Generated.cs
wrappers regenerated via `./build.sh GenerateTools`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds tests/Fallout.PublicApi.Specs: one Verify snapshot of each consumer-facing
assembly's public surface, produced by PublicApiGenerator. A surface change fails the
matching case until its .verified.txt is re-accepted, so an accidental break fails CI
and an intentional one lands as a reviewable snapshot diff.

This is the leaner counterpart to the Roslyn PublicApiAnalyzers guard (Fallout-build#530): one test
file plus snapshots under tests/, reusing the mandated xUnit + Verify stack, in place of
per-assembly PublicAPI.*.txt in src/ with editorconfig/WarningsAsErrors plumbing. Covers
the same 11 assemblies Fallout-build#530 opted in.

Generated tool wrappers are excluded by their [GeneratedCode] marker. A metadata-based
tool can only exclude whole types, so a class split across a generated and a hand-written
partial (the 20 <Tool>Tasks classes) is dropped entirely — ~25 hand-written members that
Fallout-build#530's file-level exclusion keeps. That residual gap is the precision ceiling of the
approach and the main axis for the two PRs' comparison.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ChrisonSimtian ChrisonSimtian added enhancement New feature or request target/vCurrent Targets the current version labels Jul 24, 2026
@ChrisonSimtian

Copy link
Copy Markdown
Collaborator Author

@dennisdoomen thats another approach...
still lots of ceremony, this one also requires us to annotate the code generated classes...

@dennisdoomen

Copy link
Copy Markdown
Collaborator

still lots of ceremony, this one also requires us to annotate the code generated classes...

Oh, that's quite a disappointment indeed. Although I'm not sure how the approach in FluentAssertions deals with that.

@ITaluone

ITaluone commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Puh.. either one is a suprisingly "high" effort...

For me: either one is fine...

Edit: I don't get, though, why the tool wrappers do not count as "public API" here? Since this also ships to the consumer (and at worst break things - I remember times where "suddenly" tool extension were gone or renamed).

@ChrisonSimtian

Copy link
Copy Markdown
Collaborator Author

Although I'm not sure how the approach in FluentAssertions deals with that.
do you wanna spin up a PR with that approach?
I mean we do need SOMETHING, ideally something with low friction and maintenance.

I don't get, though, why the tool wrappers do not count as "public API" here?
I've excluded them on purpose because its not handwritten code but generated code.
So the flow here would be a little bit different, I'd rather check the source files that lead to code generation and diff those.
Besides those are not per-se something we maintain and break as part of "hey lets do feature XYZ" but more along the lines of the CLI surface has changed.
If thats the case then we HAVE to do a breaking change, you know? Its not as much a design decision as it is a "the underlying tooling has changed".

TLDR; I'd like to have something that keeps an eye on our public facing methods and tells us when we break them to:

  1. help us with the breaking-change label
  2. help us understand if and when we change a public facing contract (and maybe rethink what we do there)
  3. spin up a task for the migrator cli to have a step for this breaking change

@ChrisonSimtian ChrisonSimtian changed the title Guard the public API surface with Verify snapshots (leaner alternative to #530) Guard the public API surface with Verify snapshots (alternative to #530) Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request skip-changelog target/vCurrent Targets the current version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants