Skip to content

GeneratorBase.Diagnostics must be overridden but is never read, so a null or wrong-prefix catalogue is silently accepted #58

Open

Description

@matt-edmondson

What's wrong

GeneratorBase declares protected abstract DiagnosticCatalog Diagnostics { get; } (SourceGeneratorToolkit/GeneratorBase.cs:36-39) and documents it as "the catalogue this generator's diagnostics are allocated from". Every consumer has to override it, and the README examples (README.md:119, :164) and the seam example in CLAUDE.md (:102) all do.

Nothing in SourceGeneratorToolkit or SourceGeneratorToolkit.Testing ever reads it. The declaration is the only reference. Initialize reports the MetadataFileMissing descriptor directly, GeneratorBase<T>.Generate reports MetadataParseFailed directly, and neither descriptor is checked against the catalogue.

Repro (HEAD 8a5bbf1)

static class Old  { public static DiagnosticCatalog C = new("OLD","old"); /* M = C.Warning(1,..), P = C.Error(2,..) */ }
static class Mine { public static DiagnosticCatalog C = new("MYG","mine"); }

sealed class NullCatalogGen() : GeneratorBase<Meta>("things.json") {
  protected override DiagnosticCatalog Diagnostics => null!;
  protected override DiagnosticDescriptor MetadataFileMissing => Old.M;
  protected override DiagnosticDescriptor MetadataParseFailed => Old.P; ... }
sealed class MismatchGen() : GeneratorBase<Meta>("things.json") {
  protected override DiagnosticCatalog Diagnostics => Mine.C;          // MYG
  protected override DiagnosticDescriptor MetadataFileMissing => Old.M; // OLD001
  protected override DiagnosticDescriptor MetadataParseFailed => Old.P; // OLD002 ... }

Each was run through GeneratorHarness with { not json:

Diagnostics => null!                               : exception=none diags=OLD002
Diagnostics => MYG catalogue, descriptors from OLD : exception=none diags=OLD002

Why it matters

The catalogue exists to prevent identifier drift. The DiagnosticCatalog remarks name this exact failure as the typical one: "a base class reporting parse failures under its own prefix while everything derived from it uses another". The base class asks every consumer for the information it would need to catch that drift, then never uses it. A required member looks enforced. In practice a generator can report its two built-in diagnostics under a prefix that differs from the one its tests enumerate for AnalyzerReleases.Unshipped.md, and nothing notices.

Suggested fix / acceptance criteria

Pick one:

  • (a) Use it. In Initialize, check that MetadataFileMissing and MetadataParseFailed are both in Diagnostics.Descriptors (by reference or ID). If either is missing, fail loudly with a message naming the descriptor and the catalogue prefix. Add a test with a mismatched catalogue that asserts the failure, and one with a null catalogue.
  • (b) Remove it. Drop the abstract member ([major]) and update the README and CLAUDE.md examples.

Either way, a consumer can no longer supply a catalogue that has no effect.

Activity

  1. matt-edmondson commented on Nov 1, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions