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

Assignees

No one assigned

    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