From 540a30cc005fd17d3e0fd1f94b246113a673603d Mon Sep 17 00:00:00 2001 From: Thorsten Sommer Date: Tue, 3 Nov 2026 14:56:31 +0100 Subject: [PATCH] Added an analyzer which refuses JSON contracts built by reflection --- .../AnalyzerReleases.Shipped.md | 1 + .../SourceCodeRules/Identifier.cs | 1 + .../JsonSourceGenerationAnalyzer.cs | 146 ++++++++++++++++++ .../JsonSourceGenerationAnalyzerTests.cs | 106 +++++++++++++ 4 files changed, 254 insertions(+) create mode 100644 app/SourceCodeRules/SourceCodeRules/UsageAnalyzers/JsonSourceGenerationAnalyzer.cs create mode 100644 app/Tests/Analyzers/JsonSourceGenerationAnalyzerTests.cs diff --git a/app/SourceCodeRules/SourceCodeRules/AnalyzerReleases.Shipped.md b/app/SourceCodeRules/SourceCodeRules/AnalyzerReleases.Shipped.md index 308d6c626..716a5fb13 100644 --- a/app/SourceCodeRules/SourceCodeRules/AnalyzerReleases.Shipped.md +++ b/app/SourceCodeRules/SourceCodeRules/AnalyzerReleases.Shipped.md @@ -18,3 +18,4 @@ MWAIS0012 | Usage | Error | DirectI18NGetTextAnalyzer MWAIS0013 | Usage | Error | ModelPatternLiteralAnalyzer MWAIS0014 | Usage | Error | MissingInterpolationAnalyzer + MWAIS0015 | Usage | Error | JsonSourceGenerationAnalyzer diff --git a/app/SourceCodeRules/SourceCodeRules/Identifier.cs b/app/SourceCodeRules/SourceCodeRules/Identifier.cs index 007684364..38cf392b3 100644 --- a/app/SourceCodeRules/SourceCodeRules/Identifier.cs +++ b/app/SourceCodeRules/SourceCodeRules/Identifier.cs @@ -16,4 +16,5 @@ public static class Identifier public const string DIRECT_I18N_GET_TEXT_ANALYZER = $"{Tools.ID_PREFIX}0012"; public const string MODEL_PATTERN_LITERAL_ANALYZER = $"{Tools.ID_PREFIX}0013"; public const string MISSING_INTERPOLATION_ANALYZER = $"{Tools.ID_PREFIX}0014"; + public const string JSON_SOURCE_GENERATION_ANALYZER = $"{Tools.ID_PREFIX}0015"; } \ No newline at end of file diff --git a/app/SourceCodeRules/SourceCodeRules/UsageAnalyzers/JsonSourceGenerationAnalyzer.cs b/app/SourceCodeRules/SourceCodeRules/UsageAnalyzers/JsonSourceGenerationAnalyzer.cs new file mode 100644 index 000000000..c0df67f5b --- /dev/null +++ b/app/SourceCodeRules/SourceCodeRules/UsageAnalyzers/JsonSourceGenerationAnalyzer.cs @@ -0,0 +1,146 @@ +using System.Collections.Immutable; +using System.Linq; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace SourceCodeRules.UsageAnalyzers; + +/// +/// Reports JSON which System.Text.Json would read or write with contracts it builds by reflection. +/// +/// +/// The options of AI Studio name a JsonSerializerContext as their TypeInfoResolver, so that the contracts of +/// their types are generated while compiling. A contract which System.Text.Json builds by reflection instead +/// may lack a property or fail altogether in a published build, since the trimmer removes code which only +/// reflection reaches. Neither a debug build nor the tests show that. Switching reflection off for the whole +/// app is no option, because Blazor Server needs it for JSON of its own. Hence, this analyzer refuses every +/// way into reflection within the code of AI Studio: a call without options, the options +/// JsonSerializerOptions.Default and .Web, a DefaultJsonTypeInfoResolver, and new options without a +/// TypeInfoResolver. +/// +#pragma warning disable RS1038 +[DiagnosticAnalyzer(LanguageNames.CSharp)] +#pragma warning restore RS1038 +public sealed class JsonSourceGenerationAnalyzer : DiagnosticAnalyzer +{ + private const string DIAGNOSTIC_ID = Identifier.JSON_SOURCE_GENERATION_ANALYZER; + private const string CATEGORY = "Usage"; + + private const string JSON_NAMESPACE = "System.Text.Json"; + private const string SERIALIZATION_NAMESPACE = "System.Text.Json.Serialization"; + private const string METADATA_NAMESPACE = "System.Text.Json.Serialization.Metadata"; + private const string OPTIONS_TYPE = "JsonSerializerOptions"; + private const string TYPE_INFO_RESOLVER = "TypeInfoResolver"; + + private const string TITLE = "JSON must use the generated contracts of its options"; + + private const string MESSAGE_FORMAT = "{0} A contract which System.Text.Json builds by reflection may break in a published build, since the trimmer removes what reflection needs"; + + private const string DESCRIPTION = "AI Studio reads and writes JSON with options whose TypeInfoResolver is a JsonSerializerContext, so that every contract is generated while compiling. A call without options, the options JsonSerializerOptions.Default and .Web, a DefaultJsonTypeInfoResolver, and new options without a TypeInfoResolver all make System.Text.Json build the contracts by reflection, which only a published build shows to fail."; + + /// + /// The types whose methods read or write JSON, and which fall back to reflection without options. + /// + private static readonly ImmutableHashSet SERIALIZING_TYPES = + [ + "System.Text.Json.JsonSerializer", + "System.Net.Http.Json.HttpClientJsonExtensions", + "System.Net.Http.Json.HttpContentJsonExtensions", + "System.Net.Http.Json.JsonContent", + ]; + + private static readonly DiagnosticDescriptor RULE = new(DIAGNOSTIC_ID, TITLE, MESSAGE_FORMAT, CATEGORY, DiagnosticSeverity.Error, isEnabledByDefault: true, description: DESCRIPTION); + + public override ImmutableArray SupportedDiagnostics => [RULE]; + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterOperationAction(AnalyzeInvocation, OperationKind.Invocation); + context.RegisterOperationAction(AnalyzePropertyReference, OperationKind.PropertyReference); + context.RegisterOperationAction(AnalyzeObjectCreation, OperationKind.ObjectCreation); + } + + /// + /// Reports reading or writing JSON without options, or with options which are null. + /// + private static void AnalyzeInvocation(OperationAnalysisContext context) + { + var invocation = (IInvocationOperation)context.Operation; + var method = invocation.TargetMethod; + if (!SERIALIZING_TYPES.Contains(method.ContainingType.ToDisplayString())) + return; + + // + // A method which gets the contract itself, or the context which holds it, needs no options: + // + if (method.Parameters.Any(parameter => IsContract(parameter.Type))) + return; + + var options = invocation.Arguments.FirstOrDefault(argument => Is(argument.Parameter?.Type, JSON_NAMESPACE, OPTIONS_TYPE)); + if (options is not null && options.ArgumentKind is not ArgumentKind.DefaultValue && !IsNull(options.Value)) + return; + + context.ReportDiagnostic(Diagnostic.Create(RULE, invocation.Syntax.GetLocation(), $"{method.ContainingType.Name}.{method.Name} gets no options, so it builds the contract of its type by reflection: pass the options whose context knows the type.")); + } + + /// + /// Reports the shared options of System.Text.Json, which build every contract by reflection. + /// + private static void AnalyzePropertyReference(OperationAnalysisContext context) + { + var reference = (IPropertyReferenceOperation)context.Operation; + if (reference.Property is not { IsStatic: true, Name: "Default" or "Web" } property || !Is(property.ContainingType, JSON_NAMESPACE, OPTIONS_TYPE)) + return; + + context.ReportDiagnostic(Diagnostic.Create(RULE, reference.Syntax.GetLocation(), $"JsonSerializerOptions.{property.Name} builds every contract by reflection: use the options whose context knows the type.")); + } + + /// + /// Reports a resolver which builds contracts by reflection, and new options which name no resolver. + /// + private static void AnalyzeObjectCreation(OperationAnalysisContext context) + { + var creation = (IObjectCreationOperation)context.Operation; + if (IsReflectionResolver(creation.Type)) + { + context.ReportDiagnostic(Diagnostic.Create(RULE, creation.Syntax.GetLocation(), $"{creation.Type!.Name} builds contracts by reflection: register the type in the JsonSerializerContext of its options instead.")); + return; + } + + if (!Is(creation.Type, JSON_NAMESPACE, OPTIONS_TYPE) || NamesTypeInfoResolver(creation.Initializer)) + return; + + context.ReportDiagnostic(Diagnostic.Create(RULE, creation.Syntax.GetLocation(), "These options name no TypeInfoResolver, so System.Text.Json builds their contracts by reflection: set it to the JsonSerializerContext which knows their types, right in the object initializer.")); + } + + /// + /// Whether a parameter takes a contract, as a JsonTypeInfo, or the context which holds the contracts. + /// + private static bool IsContract(ITypeSymbol type) => + Is(type, METADATA_NAMESPACE, "JsonTypeInfo") || + Is(type, SERIALIZATION_NAMESPACE, "JsonSerializerContext"); + + private static bool IsReflectionResolver(ITypeSymbol? type) + { + for (var current = type; current is not null; current = current.BaseType) + if (Is(current, METADATA_NAMESPACE, "DefaultJsonTypeInfoResolver")) + return true; + + return false; + } + + private static bool NamesTypeInfoResolver(IObjectOrCollectionInitializerOperation? initializer) => + initializer is not null && + initializer.Initializers.Any(item => item is ISimpleAssignmentOperation { Target: IPropertyReferenceOperation { Property.Name: TYPE_INFO_RESOLVER } } assignment && !IsNull(assignment.Value)); + + private static bool IsNull(IOperation value) => value.ConstantValue is { HasValue: true, Value: null }; + + private static bool Is(ITypeSymbol? type, string namespaceName, string typeName) => + type is not null && + type.Name == typeName && + type.ContainingNamespace?.ToDisplayString() == namespaceName; +} diff --git a/app/Tests/Analyzers/JsonSourceGenerationAnalyzerTests.cs b/app/Tests/Analyzers/JsonSourceGenerationAnalyzerTests.cs new file mode 100644 index 000000000..e100c1793 --- /dev/null +++ b/app/Tests/Analyzers/JsonSourceGenerationAnalyzerTests.cs @@ -0,0 +1,106 @@ +using AIStudio.Tests.Models.Generation; + +using Microsoft.CodeAnalysis; + +using SourceCodeRules.UsageAnalyzers; + +namespace AIStudio.Tests.Analyzers; + +/// +/// Checks that every way into JSON contracts built by reflection is refused while compiling. +/// +/// +/// The pre-release v26.10.1 failed because a published build lacked what reflection needed for the JSON of +/// AI Studio. Since then, the options of AI Studio name a JsonSerializerContext, whose contracts are generated +/// while compiling. A single call without options brings reflection back, and only a published build shows +/// it, so MWAIS0015 turns such a call into a build error. These tests say which code it refuses and which it +/// leaves alone. +/// +[TestFixture] +public sealed class JsonSourceGenerationAnalyzerTests +{ + [TestCase("JsonSerializer.Serialize(meeting)", "JsonSerializer.Serialize gets no options", TestName = "Writing without options")] + [TestCase("JsonSerializer.Serialize(meeting, options: null)", "JsonSerializer.Serialize gets no options", TestName = "Writing with options which are null")] + [TestCase("JsonSerializer.Deserialize(json)", "JsonSerializer.Deserialize gets no options", TestName = "Reading without options")] + [TestCase("element.Deserialize()", "JsonSerializer.Deserialize gets no options", TestName = "Reading a JSON element without options")] + [TestCase("client.GetFromJsonAsync(\"/meeting\")", "HttpClientJsonExtensions.GetFromJsonAsync gets no options", TestName = "Reading a web answer without options")] + [TestCase("client.PostAsJsonAsync(\"/meeting\", meeting)", "HttpClientJsonExtensions.PostAsJsonAsync gets no options", TestName = "Sending a web request without options")] + [TestCase("content.ReadFromJsonAsync()", "HttpContentJsonExtensions.ReadFromJsonAsync gets no options", TestName = "Reading web content without options")] + [TestCase("JsonContent.Create(meeting)", "JsonContent.Create gets no options", TestName = "Creating web content without options")] + [TestCase("JsonSerializer.Serialize(meeting, JsonSerializerOptions.Default)", "JsonSerializerOptions.Default builds every contract by reflection", TestName = "The default options")] + [TestCase("JsonSerializerOptions.Web", "JsonSerializerOptions.Web builds every contract by reflection", TestName = "The options for the web")] + [TestCase("new DefaultJsonTypeInfoResolver()", "DefaultJsonTypeInfoResolver builds contracts by reflection", TestName = "The resolver which uses reflection")] + [TestCase("new JsonSerializerOptions { TypeInfoResolver = new DefaultJsonTypeInfoResolver() }", "DefaultJsonTypeInfoResolver builds contracts by reflection", TestName = "Options with the resolver which uses reflection")] + [TestCase("new JsonSerializerOptions { WriteIndented = true }", "These options name no TypeInfoResolver", TestName = "Options without a resolver")] + [TestCase("new JsonSerializerOptions(JsonSerializerDefaults.Web)", "These options name no TypeInfoResolver", TestName = "Options for the web without a resolver")] + [TestCase("new JsonSerializerOptions { TypeInfoResolver = null }", "These options name no TypeInfoResolver", TestName = "Options with a resolver which is null")] + public async Task AWayIntoReflectionIsRefused(string expression, string expectedMessage) + { + var reported = await AnalyzeAsync(expression); + + using (Assert.EnterMultipleScope()) + { + Assert.That(reported, Has.Count.EqualTo(1)); + Assert.That(reported.FirstOrDefault()?.Id, Is.EqualTo("MWAIS0015")); + Assert.That(reported.FirstOrDefault()?.GetMessage(), Does.StartWith(expectedMessage)); + } + } + + [Test] + public async Task OptionsCreatedByATargetTypedNewWithoutAResolverAreRefused() + { + var reported = await AnalyzeAsync("OTHER_OPTIONS", "private static readonly JsonSerializerOptions OTHER_OPTIONS = new() { WriteIndented = true };"); + + using (Assert.EnterMultipleScope()) + { + Assert.That(reported, Has.Count.EqualTo(1)); + Assert.That(reported.FirstOrDefault()?.GetMessage(), Does.StartWith("These options name no TypeInfoResolver")); + } + } + + [TestCase("JsonSerializer.Serialize(meeting, OPTIONS)", TestName = "Writing with options")] + [TestCase("JsonSerializer.Serialize(meeting, contract)", TestName = "Writing with the contract")] + [TestCase("element.Deserialize(contract)", TestName = "Reading a JSON element with the contract")] + [TestCase("client.GetFromJsonAsync(\"/meeting\", OPTIONS)", TestName = "Reading a web answer with options")] + [TestCase("content.ReadFromJsonAsync(contract)", TestName = "Reading web content with the contract")] + [TestCase("JsonContent.Create(meeting, contract)", TestName = "Creating web content with the contract")] + [TestCase("new JsonSerializerOptions { WriteIndented = true, TypeInfoResolver = resolver }", TestName = "Options with a resolver")] + [TestCase("new JsonSerializerOptions(JsonSerializerDefaults.Web) { TypeInfoResolver = resolver }", TestName = "Options for the web with a resolver")] + [TestCase("JsonNode.Parse(json)", TestName = "A tree of JSON nodes")] + public async Task JsonWithGeneratedContractsIsLeftAlone(string expression) + { + var reported = await AnalyzeAsync(expression); + + Assert.That(reported, Is.Empty); + } + + private static async Task> AnalyzeAsync(string expression, string otherMembers = "") + { + var source = + $$""" + using System.Net.Http; + using System.Net.Http.Json; + using System.Text.Json; + using System.Text.Json.Nodes; + using System.Text.Json.Serialization.Metadata; + + namespace Sample; + + public sealed class Meeting + { + public string Location { get; set; } = "Cologne"; + } + + public static class MeetingJson + { + private static readonly JsonSerializerOptions OPTIONS = new() { TypeInfoResolver = JsonTypeInfoResolver.Combine() }; + + {{otherMembers}} + + public static object? Use(Meeting meeting, string json, JsonElement element, HttpClient client, HttpContent content, JsonTypeInfo contract, IJsonTypeInfoResolver resolver) => {{expression}}; + } + """; + + return await CompilationHarness.AnalyzeAsync(source, new JsonSourceGenerationAnalyzer()); + } +} \ No newline at end of file