Make JsonGenerator be an incremental generator (#57088) (#58278)

* Make JsonGenerator be an incremental generator * Improve incrementalism by doing less work when not applicable * Change SourceGeneration.UnitTests to SourceGeneration.Unit.Tests so it is built and executed in CI * Get unit tests running after IIncrementalGenerator migration * Fix duplicate file name tests by working around dotnet/roslyn#54185. * Fix unit tests now that they are running in CI against non-English languages. * Fix System.Text.Json.SourceGeneration.Unit.Tests on WASM * Disable STJ.SourceGeneration.Unit.Tests on Browser Co-authored-by: Eric Erhardt <[email protected]> Co-authored-by: Chris Sienkiewicz <[email protected]>
dotnet · Aug 27, 2021 · 72f18b5 · 72f18b5
1 parent 262b509
commit 72f18b5
Show file tree

Hide file tree

Showing 11 changed files with 101 additions and 60 deletions.
diff --git a/eng/Versions.props b/eng/Versions.props
@@ -40,9 +40,9 @@
     <ProjectServicingConfiguration Include="Microsoft.NETCore.App.Ref" PatchVersion="0" />
   </ItemGroup>
   <PropertyGroup>
-    <!-- For source generator support we need to target a pinned version in order to be able to run on older versions of Roslyn -->
-    <MicrosoftCodeAnalysisCSharpWorkspacesVersion>3.9.0</MicrosoftCodeAnalysisCSharpWorkspacesVersion>
-    <MicrosoftCodeAnalysisVersion>3.9.0</MicrosoftCodeAnalysisVersion>
+    <!-- For source generator support we are targeting the latest version of Roslyn for now, until we can support multi-targeting -->
+    <MicrosoftCodeAnalysisCSharpWorkspacesVersion>4.0.0-3.final</MicrosoftCodeAnalysisCSharpWorkspacesVersion>
+    <MicrosoftCodeAnalysisVersion>4.0.0-3.final</MicrosoftCodeAnalysisVersion>
   </PropertyGroup>
   <PropertyGroup>
     <!-- Code analysis dependencies -->

diff --git a/src/libraries/System.Text.Json/System.Text.Json.sln b/src/libraries/System.Text.Json/System.Text.Json.sln
@@ -43,7 +43,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.Text.Encodings.Web",
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.Collections.Immutable", "..\System.Collections.Immutable\ref\System.Collections.Immutable.csproj", "{BE27618A-2916-4269-9AD5-6BC5EDC32B30}"
 EndProject
-Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "System.Text.Json.SourceGeneration.UnitTests", "tests\System.Text.Json.SourceGeneration.UnitTests\System.Text.Json.SourceGeneration.UnitTests.csproj", "{F6A18EB5-A8CC-4A39-9E85-5FA226019C3D}"
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "System.Text.Json.SourceGeneration.Unit.Tests", "tests\System.Text.Json.SourceGeneration.Unit.Tests\System.Text.Json.SourceGeneration.Unit.Tests.csproj", "{F6A18EB5-A8CC-4A39-9E85-5FA226019C3D}"
 EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution

diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
@@ -79,15 +79,17 @@ private sealed partial class Emitter
                 defaultSeverity: DiagnosticSeverity.Warning,
                 isEnabledByDefault: true);
 
-            private readonly GeneratorExecutionContext _executionContext;
+            private readonly SourceProductionContext _sourceProductionContext;
 
             private ContextGenerationSpec _currentContext = null!;
 
             private readonly SourceGenerationSpec _generationSpec = null!;
 
-            public Emitter(in GeneratorExecutionContext executionContext, SourceGenerationSpec generationSpec)
+            private readonly HashSet<string> _emittedPropertyFileNames = new();
+
+            public Emitter(in SourceProductionContext sourceProductionContext, SourceGenerationSpec generationSpec)
             {
-                _executionContext = executionContext;
+                _sourceProductionContext = sourceProductionContext;
                 _generationSpec = generationSpec;
             }
 
@@ -166,7 +168,7 @@ namespace {@namespace}
                     sb.AppendLine("}");
                 }
 
-                _executionContext.AddSource(fileName, SourceText.From(sb.ToString(), Encoding.UTF8));
+                _sourceProductionContext.AddSource(fileName, SourceText.From(sb.ToString(), Encoding.UTF8));
             }
 
             private void GenerateTypeInfo(TypeGenerationSpec typeGenerationSpec)
@@ -243,7 +245,7 @@ private void GenerateTypeInfo(TypeGenerationSpec typeGenerationSpec)
                         break;
                     case ClassType.TypeUnsupportedBySourceGen:
                         {
-                            _executionContext.ReportDiagnostic(
+                            _sourceProductionContext.ReportDiagnostic(
                                 Diagnostic.Create(TypeNotSupported, Location.None, new string[] { typeGenerationSpec.TypeRef }));
                             return;
                         }
@@ -253,13 +255,16 @@ private void GenerateTypeInfo(TypeGenerationSpec typeGenerationSpec)
                         }
                 }
 
-                try
+                // Don't add a duplicate file, but instead raise a diagnostic to say the duplicate has been skipped.
+                // Workaround https://github.com/dotnet/roslyn/issues/54185 by keeping track of the file names we've used.
+                string propertyFileName = $"{_currentContext.ContextType.Name}.{typeGenerationSpec.TypeInfoPropertyName}.g.cs";
+                if (_emittedPropertyFileNames.Add(propertyFileName))
                 {
-                    AddSource($"{_currentContext.ContextType.Name}.{typeGenerationSpec.TypeInfoPropertyName}.g.cs", source);
+                    AddSource(propertyFileName, source);
                 }
-                catch (ArgumentException)
+                else
                 {
-                    _executionContext.ReportDiagnostic(Diagnostic.Create(DuplicateTypeName, Location.None, new string[] { typeGenerationSpec.TypeInfoPropertyName }));
+                    _sourceProductionContext.ReportDiagnostic(Diagnostic.Create(DuplicateTypeName, Location.None, new string[] { typeGenerationSpec.TypeInfoPropertyName }));
                 }
             }
 

diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
@@ -4,6 +4,7 @@
 using System.Collections;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
@@ -31,7 +32,8 @@ private sealed class Parser
             private const string JsonPropertyNameAttributeFullName = "System.Text.Json.Serialization.JsonPropertyNameAttribute";
             private const string JsonPropertyOrderAttributeFullName = "System.Text.Json.Serialization.JsonPropertyOrderAttribute";
 
-            private readonly GeneratorExecutionContext _executionContext;
+            private readonly Compilation _compilation;
+            private readonly SourceProductionContext _sourceProductionContext;
             private readonly MetadataLoadContextInternal _metadataLoadContext;
 
             private readonly Type _ilistOfTType;
@@ -43,7 +45,7 @@ private sealed class Parser
             private readonly Type? _dictionaryType;
             private readonly Type? _idictionaryOfTKeyTValueType;
             private readonly Type? _ireadonlyDictionaryType;
-            private readonly Type? _isetType; 
+            private readonly Type? _isetType;
             private readonly Type? _stackOfTType;
             private readonly Type? _queueOfTType;
             private readonly Type? _concurrentStackType;
@@ -96,10 +98,11 @@ private sealed class Parser
                 defaultSeverity: DiagnosticSeverity.Error,
                 isEnabledByDefault: true);
 
-            public Parser(in GeneratorExecutionContext executionContext)
+            public Parser(Compilation compilation, in SourceProductionContext sourceProductionContext)
             {
-                _executionContext = executionContext;
-                _metadataLoadContext = new MetadataLoadContextInternal(executionContext.Compilation);
+                _compilation = compilation;
+                _sourceProductionContext = sourceProductionContext;
+                _metadataLoadContext = new MetadataLoadContextInternal(_compilation);
 
                 _ilistOfTType = _metadataLoadContext.Resolve(SpecialType.System_Collections_Generic_IList_T);
                 _icollectionOfTType = _metadataLoadContext.Resolve(SpecialType.System_Collections_Generic_ICollection_T);
@@ -138,9 +141,9 @@ public Parser(in GeneratorExecutionContext executionContext)
                 PopulateKnownTypes();
             }
 
-            public SourceGenerationSpec? GetGenerationSpec(List<ClassDeclarationSyntax> classDeclarationSyntaxList)
+            public SourceGenerationSpec? GetGenerationSpec(ImmutableArray<ClassDeclarationSyntax> classDeclarationSyntaxList)
             {
-                Compilation compilation = _executionContext.Compilation;
+                Compilation compilation = _compilation;
                 INamedTypeSymbol jsonSerializerContextSymbol = compilation.GetTypeByMetadataName("System.Text.Json.Serialization.JsonSerializerContext");
                 INamedTypeSymbol jsonSerializableAttributeSymbol = compilation.GetTypeByMetadataName("System.Text.Json.Serialization.JsonSerializableAttribute");
                 INamedTypeSymbol jsonSourceGenerationOptionsAttributeSymbol = compilation.GetTypeByMetadataName("System.Text.Json.Serialization.JsonSourceGenerationOptionsAttribute");
@@ -198,7 +201,7 @@ public Parser(in GeneratorExecutionContext executionContext)
                     if (!TryGetClassDeclarationList(contextTypeSymbol, out List<string> classDeclarationList))
                     {
                         // Class or one of its containing types is not partial so we can't add to it.
-                        _executionContext.ReportDiagnostic(Diagnostic.Create(ContextClassesMustBePartial, Location.None, new string[] { contextTypeSymbol.Name }));
+                        _sourceProductionContext.ReportDiagnostic(Diagnostic.Create(ContextClassesMustBePartial, Location.None, new string[] { contextTypeSymbol.Name }));
                         continue;
                     }
 
@@ -400,6 +403,36 @@ private static bool TryGetClassDeclarationList(INamedTypeSymbol typeSymbol, [Not
                 return typeGenerationSpec;
             }
 
+            internal static bool IsSyntaxTargetForGeneration(SyntaxNode node) => node is ClassDeclarationSyntax { AttributeLists: { Count: > 0 }, BaseList: { Types : {Count : > 0 } } };
+
+            internal static ClassDeclarationSyntax? GetSemanticTargetForGeneration(GeneratorSyntaxContext context)
+            {
+                var classDeclarationSyntax = (ClassDeclarationSyntax)context.Node;
+
+                foreach (AttributeListSyntax attributeListSyntax in classDeclarationSyntax.AttributeLists)
+                {
+                    foreach (AttributeSyntax attributeSyntax in attributeListSyntax.Attributes)
+                    {
+                        IMethodSymbol attributeSymbol = context.SemanticModel.GetSymbolInfo(attributeSyntax).Symbol as IMethodSymbol;
+                        if (attributeSymbol == null)
+                        {
+                            continue;
+                        }
+
+                        INamedTypeSymbol attributeContainingTypeSymbol = attributeSymbol.ContainingType;
+                        string fullName = attributeContainingTypeSymbol.ToDisplayString();
+
+                        if (fullName == "System.Text.Json.Serialization.JsonSerializableAttribute")
+                        {
+                            return classDeclarationSyntax;
+                        }
+                    }
+
+                }
+
+                return null;
+            }
+
             private static JsonSourceGenerationMode? GetJsonSourceGenerationModeEnumVal(SyntaxNode propertyValueMode)
             {
                 IEnumerable<string> enumTokens = propertyValueMode
@@ -729,7 +762,7 @@ private TypeGenerationSpec GetOrAddTypeGenerationSpec(Type type, JsonSourceGener
                     if (!type.TryGetDeserializationConstructor(useDefaultCtorInAnnotatedStructs, out ConstructorInfo? constructor))
                     {
                         classType = ClassType.TypeUnsupportedBySourceGen;
-                        _executionContext.ReportDiagnostic(Diagnostic.Create(MultipleJsonConstructorAttribute, Location.None, new string[] { $"{type}" }));
+                        _sourceProductionContext.ReportDiagnostic(Diagnostic.Create(MultipleJsonConstructorAttribute, Location.None, new string[] { $"{type}" }));
                     }
                     else
                     {

diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.cs
@@ -1,7 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+//#define LAUNCH_DEBUGGER
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
@@ -16,60 +18,44 @@ namespace System.Text.Json.SourceGeneration
     /// Generates source code to optimize serialization and deserialization with JsonSerializer.
     /// </summary>
     [Generator]
-    public sealed partial class JsonSourceGenerator : ISourceGenerator
+    public sealed partial class JsonSourceGenerator : IIncrementalGenerator
     {
-        /// <summary>
-        /// Registers a syntax resolver to receive compilation units.
-        /// </summary>
-        /// <param name="context"></param>
-        public void Initialize(GeneratorInitializationContext context)
+        public void Initialize(IncrementalGeneratorInitializationContext context)
         {
-            context.RegisterForSyntaxNotifications(() => new SyntaxReceiver());
+            IncrementalValuesProvider<ClassDeclarationSyntax> classDeclarations = context.SyntaxProvider
+                .CreateSyntaxProvider(static (s, _) => Parser.IsSyntaxTargetForGeneration(s), static (s, _) => Parser.GetSemanticTargetForGeneration(s))
+                .Where(static c => c is not null);
+
+            IncrementalValueProvider<(Compilation, ImmutableArray<ClassDeclarationSyntax>)> compilationAndClasses =
+                context.CompilationProvider.Combine(classDeclarations.Collect());
+
+            context.RegisterSourceOutput(compilationAndClasses, (spc, source) => Execute(source.Item1, source.Item2, spc));
         }
 
-        /// <summary>
-        /// Generates source code to optimize serialization and deserialization with JsonSerializer.
-        /// </summary>
-        /// <param name="executionContext"></param>
-        public void Execute(GeneratorExecutionContext executionContext)
+        private void Execute(Compilation compilation, ImmutableArray<ClassDeclarationSyntax> contextClasses, SourceProductionContext context)
         {
 #if LAUNCH_DEBUGGER
             if (!Diagnostics.Debugger.IsAttached)
             {
                 Diagnostics.Debugger.Launch();
             }
 #endif
-            SyntaxReceiver receiver = (SyntaxReceiver)executionContext.SyntaxReceiver;
-            List<ClassDeclarationSyntax>? contextClasses = receiver.ClassDeclarationSyntaxList;
-            if (contextClasses == null)
+            if (contextClasses.IsDefaultOrEmpty)
             {
                 return;
             }
 
-            Parser parser = new(executionContext);
-            SourceGenerationSpec? spec = parser.GetGenerationSpec(receiver.ClassDeclarationSyntaxList);
+            Parser parser = new(compilation, context);
+            SourceGenerationSpec? spec = parser.GetGenerationSpec(contextClasses);
             if (spec != null)
             {
                 _rootTypes = spec.ContextGenerationSpecList[0].RootSerializableTypes;
 
-                Emitter emitter = new(executionContext, spec);
+                Emitter emitter = new(context, spec);
                 emitter.Emit();
             }
         }
 
-        private sealed class SyntaxReceiver : ISyntaxReceiver
-        {
-            public List<ClassDeclarationSyntax>? ClassDeclarationSyntaxList { get; private set; }
-
-            public void OnVisitSyntaxNode(SyntaxNode syntaxNode)
-            {
-                if (syntaxNode is ClassDeclarationSyntax cds)
-                {
-                    (ClassDeclarationSyntaxList ??= new List<ClassDeclarationSyntax>()).Add(cds);
-                }
-            }
-        }
-
         /// <summary>
         /// Helper for unit tests.
         /// </summary>

diff --git a/...Generation.UnitTests/CompilationHelper.cs → ...eneration.Unit.Tests/CompilationHelper.cs b/...Generation.UnitTests/CompilationHelper.cs → ...eneration.Unit.Tests/CompilationHelper.cs
@@ -3,6 +3,7 @@
 
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Globalization;
 using System.IO;
 using System.Linq;
 using System.Reflection;
@@ -16,6 +17,11 @@ namespace System.Text.Json.SourceGeneration.UnitTests
 {
     public class CompilationHelper
     {
+        private static readonly CSharpParseOptions s_parseOptions =
+            new CSharpParseOptions(kind: SourceCodeKind.Regular, documentationMode: DocumentationMode.Parse)
+            // workaround https://github.com/dotnet/roslyn/pull/55866. We can remove "LangVersion=Preview" when we get a Roslyn build with that change.
+            .WithLanguageVersion(LanguageVersion.Preview);
+
         public static Compilation CreateCompilation(
             string source,
             MetadataReference[] additionalReferences = null,
@@ -55,18 +61,18 @@ public static Compilation CreateCompilation(
 
             return CSharpCompilation.Create(
                 assemblyName,
-                syntaxTrees: new[] { CSharpSyntaxTree.ParseText(source) },
+                syntaxTrees: new[] { CSharpSyntaxTree.ParseText(source, s_parseOptions) },
                 references: references.ToArray(),
                 options: new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)
             );
         }
 
-        private static GeneratorDriver CreateDriver(Compilation compilation, params ISourceGenerator[] generators)
+        private static GeneratorDriver CreateDriver(Compilation compilation, IIncrementalGenerator[] generators)
             => CSharpGeneratorDriver.Create(
-                generators: ImmutableArray.Create(generators),
-                parseOptions: new CSharpParseOptions(kind: SourceCodeKind.Regular, documentationMode: DocumentationMode.Parse));
+                generators: generators.Select(g => g.AsSourceGenerator()),
+                parseOptions: s_parseOptions);
 
-        public static Compilation RunGenerators(Compilation compilation, out ImmutableArray<Diagnostic> diagnostics, params ISourceGenerator[] generators)
+        public static Compilation RunGenerators(Compilation compilation, out ImmutableArray<Diagnostic> diagnostics, params IIncrementalGenerator[] generators)
         {
             CreateDriver(compilation, generators).RunGeneratorsAndUpdateCompilation(compilation, out Compilation outCompilation, out diagnostics);
             return outCompilation;
@@ -267,7 +273,15 @@ internal static void CheckDiagnosticMessages(ImmutableArray<Diagnostic> diagnost
             Array.Sort(actualMessages);
             Array.Sort(expectedMessages);
 
-            Assert.Equal(expectedMessages, actualMessages);
+            if (CultureInfo.CurrentUICulture.Name.StartsWith("en", StringComparison.OrdinalIgnoreCase))
+            {
+                Assert.Equal(expectedMessages, actualMessages);
+            }
+            else
+            {
+                // for non-English runs, just compare the number of messages are the same
+                Assert.Equal(expectedMessages.Length, actualMessages.Length);
+            }
         }
     }
 }
diff --git a/...ts/JsonSourceGeneratorDiagnosticsTests.cs → ...ts/JsonSourceGeneratorDiagnosticsTests.cs b/...ts/JsonSourceGeneratorDiagnosticsTests.cs → ...ts/JsonSourceGeneratorDiagnosticsTests.cs
diff --git a/...ion.UnitTests/JsonSourceGeneratorTests.cs → ...on.Unit.Tests/JsonSourceGeneratorTests.cs b/...ion.UnitTests/JsonSourceGeneratorTests.cs → ...on.Unit.Tests/JsonSourceGeneratorTests.cs
diff --git a/...xt.Json.SourceGeneration.UnitTests.csproj → ...t.Json.SourceGeneration.Unit.Tests.csproj b/...xt.Json.SourceGeneration.UnitTests.csproj → ...t.Json.SourceGeneration.Unit.Tests.csproj
@@ -5,7 +5,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.CodeAnalysis" Version="$(MicrosoftCodeAnalysisVersion)" PrivateAssets="all" />
+    <PackageReference Include="Microsoft.CodeAnalysis" Version="$(MicrosoftCodeAnalysisVersion)" />
 
     <ProjectReference Include="..\..\src\System.Text.Json.csproj" />
     <ProjectReference Include="..\..\gen\System.Text.Json.SourceGeneration.csproj" />

diff --git a/...eGeneration.UnitTests/TypeWrapperTests.cs → ...Generation.Unit.Tests/TypeWrapperTests.cs b/...eGeneration.UnitTests/TypeWrapperTests.cs → ...Generation.Unit.Tests/TypeWrapperTests.cs
diff --git a/src/libraries/tests.proj b/src/libraries/tests.proj
@@ -233,6 +233,9 @@
 
     <!-- This OuterLoop test requires browser UI, but the Helix agents are headless -->
     <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Net.WebSockets.Client\tests\wasm\System.Net.WebSockets.Client.Wasm.Tests.csproj" />
+
+    <!-- https://github.com/dotnet/runtime/issues/58226 -->
+    <ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Text.Json\tests\System.Text.Json.SourceGeneration.Unit.Tests\System.Text.Json.SourceGeneration.Unit.Tests.csproj" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetOS)' == 'Browser' and '$(BuildAOTTestsOnHelix)' == 'true' and '$(RunDisabledWasmTests)' != 'true'">