diff --git a/CLAUDE.md b/CLAUDE.md index c12fdb4..693daff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,9 +127,11 @@ side of it. means malformed metadata silently generates something wrong. "Malformed" is wider than bad JSON: `JsonSerializer` reports a shape it cannot construct as `NotSupportedException` and some converter-configuration failures as `InvalidOperationException`, neither of which derives from - `JsonException`. `Deserialize` catches all three, because one escaping to the Roslyn driver - becomes a generic CS8785 that names neither the file nor the reason — and abandons every other - file in the same invocation. + `JsonException`, and the model's own constructor or setters throw whatever ordinary code throws + (`ArgumentNullException`, `NullReferenceException`, `FormatException`, ...). `Deserialize` + reports every exception except `OperationCanceledException` and `OutOfMemoryException`, because + one escaping to the Roslyn driver becomes a generic CS8785 that names neither the file nor the + reason — and abandons every other file in the same invocation. - **The harness is a separate package, not a separate test project.** A consumer testing their own generator needs it, so it ships. It is separate from the analyzer package because it reads files, which RS1035 bans for code that runs in an analyzer host — and `EnforceExtendedAnalyzerRules` diff --git a/SourceGeneratorToolkit.Test/GeneratorBaseTests.cs b/SourceGeneratorToolkit.Test/GeneratorBaseTests.cs index 7ff495f..f0ec5f8 100644 --- a/SourceGeneratorToolkit.Test/GeneratorBaseTests.cs +++ b/SourceGeneratorToolkit.Test/GeneratorBaseTests.cs @@ -222,6 +222,25 @@ public void AModelConstructorThatThrowsIsReportedAndTheOtherFilesStillProcessed( StringAssert.Contains(result.GeneratedSources[0].SourceText.ToString(), "1 others, things unread"); } + [TestMethod] + [DataRow("""{"things":[{"size":"1"}]}""", "NullReferenceException", DisplayName = "NullReferenceException")] + [DataRow("""{"things":[{"name":"a","size":"big"}]}""", "FormatException", DisplayName = "FormatException")] + [DataRow("""{"things":[{"name":"a","size":"99999999999"}]}""", "OverflowException", DisplayName = "OverflowException")] + public void AnyExceptionFromModelCodeIsReportedRatherThanCrashingTheGenerator(string things, string exceptionType) + { + GeneratorRunResult result = Harness.Run( + new ParsedPairGenerator(), + new Dictionary { ["things.json"] = things }); + + Assert.IsNull(result.Exception, $"The generator threw instead of reporting a diagnostic: {result.Exception}"); + Assert.AreEqual(1, result.Diagnostics.Length); + Assert.AreEqual(TestDiagnostics.MetadataParseFailed.Id, result.Diagnostics[0].Id); + StringAssert.Contains(result.Diagnostics[0].GetMessage(), "things.json"); + StringAssert.Contains(result.Diagnostics[0].GetMessage(), exceptionType); + Assert.AreEqual(1, result.GeneratedSources.Length); + StringAssert.Contains(result.GeneratedSources[0].SourceText.ToString(), "1 others, things unread"); + } + [TestMethod] public void AJsonNullDocumentIsReportedRatherThanTreatedAsEmpty() { diff --git a/SourceGeneratorToolkit.Test/TestGenerators.cs b/SourceGeneratorToolkit.Test/TestGenerators.cs index 568b33e..4886d26 100644 --- a/SourceGeneratorToolkit.Test/TestGenerators.cs +++ b/SourceGeneratorToolkit.Test/TestGenerators.cs @@ -290,6 +290,58 @@ protected override void Generate(SourceProductionContext context, MetadataSet me } } +/// +/// An entry whose constructor does what ordinary model code does with its input, and so throws +/// , or +/// rather than an argument exception. +/// +[System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "The missing guard is the point: a missing name has to throw NullReferenceException.")] +public sealed class ParsedThing(string name, string size) +{ + public string Name { get; } = name.Trim(); + + public string Size { get; } = size; + + public int SizeValue { get; } = int.Parse(size, System.Globalization.CultureInfo.InvariantCulture); +} + +/// A metadata shape whose entries are . +public sealed class ParsedThingsMetadata +{ + public List Things { get; set; } = []; +} + +/// +/// Reads both metadata files, the first into , and emits from +/// whichever one deserialized. +/// +internal sealed class ParsedPairGenerator : GeneratorBase +{ + protected override IReadOnlyList MetadataFileNames => ["things.json", "others.json"]; + + protected override DiagnosticCatalog Diagnostics => TestDiagnostics.Catalog; + + protected override DiagnosticDescriptor MetadataFileMissing => TestDiagnostics.MetadataFileMissing; + + protected override DiagnosticDescriptor MetadataParseFailed => TestDiagnostics.MetadataParseFailed; + + protected override void Generate(SourceProductionContext context, MetadataSet metadata) + { + ParsedThingsMetadata? things = metadata["things.json"]?.Deserialize(context, MetadataParseFailed); + OthersMetadata? others = metadata["others.json"]?.Deserialize(context, MetadataParseFailed); + + if (others is null) + { + return; + } + + using CodeBlocker codeBlocker = CreateCodeBlocker(); + WriteFileHeader(codeBlocker, TestDiagnostics.Copyright); + codeBlocker.WriteLine($"// {others.Others.Count} others, things {(things is null ? "unread" : "read")}"); + context.AddSource("Parsed.g.cs", codeBlocker.ToString()); + } +} + /// /// Declares a file nothing supplies, so the missing-file path has a generator to run. /// diff --git a/SourceGeneratorToolkit/MetadataFile.cs b/SourceGeneratorToolkit/MetadataFile.cs index 02ccee2..ff5de31 100644 --- a/SourceGeneratorToolkit/MetadataFile.cs +++ b/SourceGeneratorToolkit/MetadataFile.cs @@ -150,8 +150,12 @@ public Location FindLocation(string anchor, string needle) /// /// also lets the model's own constructor exceptions through /// unwrapped, so an ordinary guard clause meeting a missing property throws - /// . and its derivations are - /// reported the same way. + /// , and ordinary model code throws whatever it throws: a + /// from name.Trim() on a missing property, a + /// or from int.Parse. Any of + /// those escaping would be the same CS8785, so every exception is reported the same way, with its + /// type name in the reason. Only , which the driver uses + /// to stop a run, and are let through. /// /// public T? Deserialize(SourceProductionContext context, DiagnosticDescriptor parseFailed) @@ -167,17 +171,10 @@ public Location FindLocation(string anchor, string needle) context.ReportAt(parseFailed, LocationOf(ex), FileName, ex.Message); return null; } - catch (NotSupportedException ex) + catch (Exception ex) when (ex is not OperationCanceledException and not OutOfMemoryException) { - return ParseFailed(ex); - } - catch (InvalidOperationException ex) - { - return ParseFailed(ex); - } - catch (ArgumentException ex) - { - return ParseFailed(ex); + context.Report(parseFailed, FileName, $"{ex.GetType().Name}: {ex.Message}"); + return null; } if (metadata is not null) @@ -187,12 +184,6 @@ public Location FindLocation(string anchor, string needle) context.Report(parseFailed, FileName, "the document deserialized to null"); return null; - - T? ParseFailed(Exception ex) - { - context.Report(parseFailed, FileName, ex.Message); - return null; - } } ///