Provide locations for src-gen diagnostic output

dotnet · Nov 11, 2021 · 4eadba4 · 4eadba4
1 parent 17fa0c8
commit 4eadba4
Show file tree

Hide file tree

Showing 11 changed files with 166 additions and 100 deletions.
diff --git a/src/libraries/System.Text.Json/gen/ContextGenerationSpec.cs b/src/libraries/System.Text.Json/gen/ContextGenerationSpec.cs
@@ -5,6 +5,7 @@
 using System.Text.Json.Serialization;
 using System.Text.Json.Reflection;
 using System.Diagnostics;
+using Microsoft.CodeAnalysis;
 
 namespace System.Text.Json.SourceGeneration
 {
@@ -15,6 +16,8 @@ namespace System.Text.Json.SourceGeneration
     [DebuggerDisplay("ContextTypeRef={ContextTypeRef}")]
     internal sealed class ContextGenerationSpec
     {
+        public Location Location { get; init; }
+
         public JsonSourceGenerationOptionsAttribute GenerationOptions { get; init; }
 
         public Type ContextType { get; init; }

diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
@@ -274,8 +274,9 @@ private void GenerateTypeInfo(TypeGenerationSpec typeGenerationSpec)
                         break;
                     case ClassType.TypeUnsupportedBySourceGen:
                         {
+                            Location location = typeGenerationSpec.Type.GetDiagnosticLocation() ?? typeGenerationSpec.AttributeLocation ?? _currentContext.Location;
                             _sourceGenerationContext.ReportDiagnostic(
-                                Diagnostic.Create(TypeNotSupported, Location.None, new string[] { typeGenerationSpec.TypeRef }));
+                                Diagnostic.Create(TypeNotSupported, location, new string[] { typeGenerationSpec.TypeRef }));
                             return;
                         }
                     default:
@@ -293,7 +294,8 @@ private void GenerateTypeInfo(TypeGenerationSpec typeGenerationSpec)
                 }
                 else
                 {
-                    _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(DuplicateTypeName, Location.None, new string[] { typeGenerationSpec.TypeInfoPropertyName }));
+                    Location location = typeGenerationSpec.AttributeLocation ?? _currentContext.Location;
+                    _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(DuplicateTypeName, location, 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
@@ -116,6 +116,8 @@ private sealed class Parser
 
             private readonly HashSet<TypeGenerationSpec> _implicitlyRegisteredTypes = new();
 
+            private readonly HashSet<ValueTuple<Type, DiagnosticDescriptor, string[]>> _typeLevelDiagnostics = new();
+
             private JsonKnownNamingPolicy _currentContextNamingPolicy;
 
             private static DiagnosticDescriptor ContextClassesMustBePartial { get; } = new DiagnosticDescriptor(
@@ -285,15 +287,18 @@ public Parser(Compilation compilation, in JsonSourceGenerationContext sourceGene
                     INamedTypeSymbol contextTypeSymbol = (INamedTypeSymbol)compilationSemanticModel.GetDeclaredSymbol(classDeclarationSyntax);
                     Debug.Assert(contextTypeSymbol != null);
 
+                    Location contextLocation = contextTypeSymbol.Locations[0];
+
                     if (!TryGetClassDeclarationList(contextTypeSymbol, out List<string> classDeclarationList))
                     {
                         // Class or one of its containing types is not partial so we can't add to it.
-                        _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(ContextClassesMustBePartial, Location.None, new string[] { contextTypeSymbol.Name }));
+                        _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(ContextClassesMustBePartial, contextLocation, new string[] { contextTypeSymbol.Name }));
                         continue;
                     }
 
                     ContextGenerationSpec contextGenSpec = new()
                     {
+                        Location = contextLocation,
                         GenerationOptions = options ?? new JsonSourceGenerationOptionsAttribute(),
                         ContextType = contextTypeSymbol.AsType(_metadataLoadContext),
                         ContextClassDeclarationList = classDeclarationList
@@ -316,6 +321,22 @@ public Parser(Compilation compilation, in JsonSourceGenerationContext sourceGene
                         continue;
                     }
 
+                    // Emit type-level diagnostics
+                    foreach ((Type Type, DiagnosticDescriptor Descriptor, string[] MessageArgs) diagnostic in _typeLevelDiagnostics)
+                    {
+                        Type type = diagnostic.Type;
+                        Location location = type.GetDiagnosticLocation();
+
+                        if (location == null)
+                        {
+                            TypeGenerationSpec spec = _typeGenerationSpecCache[type];
+                            location = spec.AttributeLocation;
+                        }
+
+                        location ??= contextLocation;
+                        _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(diagnostic.Descriptor, location, diagnostic.MessageArgs));
+                    }
+
                     contextGenSpec.ImplicitlyRegisteredTypes.UnionWith(_implicitlyRegisteredTypes);
 
                     contextGenSpecList ??= new List<ContextGenerationSpec>();
@@ -324,6 +345,7 @@ public Parser(Compilation compilation, in JsonSourceGenerationContext sourceGene
                     // Clear the cache of generated metadata between the processing of context classes.
                     _typeGenerationSpecCache.Clear();
                     _implicitlyRegisteredTypes.Clear();
+                    _typeLevelDiagnostics.Clear();
                 }
 
                 if (contextGenSpecList == null)
@@ -487,6 +509,8 @@ private static bool TryGetClassDeclarationList(INamedTypeSymbol typeSymbol, [Not
                     typeGenerationSpec.GenerationMode = generationMode;
                 }
 
+                typeGenerationSpec.AttributeLocation = attributeSyntax.GetLocation();
+
                 return typeGenerationSpec;
             }
 
@@ -877,7 +901,7 @@ private TypeGenerationSpec GetOrAddTypeGenerationSpec(Type type, JsonSourceGener
                     if (!type.TryGetDeserializationConstructor(useDefaultCtorInAnnotatedStructs, out ConstructorInfo? constructor))
                     {
                         classType = ClassType.TypeUnsupportedBySourceGen;
-                        _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(MultipleJsonConstructorAttribute, Location.None, new string[] { $"{type}" }));
+                        _typeLevelDiagnostics.Add((type, MultipleJsonConstructorAttribute, new string[] { $"{type}" }));
                     }
                     else
                     {
@@ -944,7 +968,7 @@ private TypeGenerationSpec GetOrAddTypeGenerationSpec(Type type, JsonSourceGener
                                 }
 
                                 spec = GetPropertyGenerationSpec(propertyInfo, isVirtual, generationMode);
-                                CacheMemberHelper();
+                                CacheMemberHelper(propertyInfo.GetDiagnosticLocation());
                             }
 
                             foreach (FieldInfo fieldInfo in currentType.GetFields(bindingFlags))
@@ -955,10 +979,10 @@ private TypeGenerationSpec GetOrAddTypeGenerationSpec(Type type, JsonSourceGener
                                 }
 
                                 spec = GetPropertyGenerationSpec(fieldInfo, isVirtual: false, generationMode);
-                                CacheMemberHelper();
+                                CacheMemberHelper(fieldInfo.GetDiagnosticLocation());
                             }
 
-                            void CacheMemberHelper()
+                            void CacheMemberHelper(Location memberLocation)
                             {
                                 CacheMember(spec, ref propGenSpecList, ref ignoredMembers);
 
@@ -972,13 +996,13 @@ void CacheMemberHelper()
                                 {
                                     if (dataExtensionPropGenSpec != null)
                                     {
-                                        _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(MultipleJsonExtensionDataAttribute, Location.None, new string[] { type.Name }));
+                                        _typeLevelDiagnostics.Add((type, MultipleJsonExtensionDataAttribute, new string[] { type.Name }));
                                     }
 
                                     Type propType = spec.TypeGenerationSpec.Type;
                                     if (!IsValidDataExtensionPropertyType(propType))
                                     {
-                                        _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(DataExtensionPropertyInvalid, Location.None, new string[] { type.Name, spec.ClrName }));
+                                        _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(DataExtensionPropertyInvalid, memberLocation, new string[] { type.Name, spec.ClrName }));
                                     }
 
                                     dataExtensionPropGenSpec = GetOrAddTypeGenerationSpec(propType, generationMode);
@@ -987,13 +1011,13 @@ void CacheMemberHelper()
 
                                 if (!hasInitOnlyProperties && spec.CanUseSetter && spec.IsInitOnlySetter)
                                 {
-                                    _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(InitOnlyPropertyDeserializationNotSupported, Location.None, new string[] { type.Name }));
+                                    _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(InitOnlyPropertyDeserializationNotSupported, memberLocation, new string[] { type.Name }));
                                     hasInitOnlyProperties = true;
                                 }
 
                                 if (spec.HasJsonInclude && (!spec.CanUseGetter || !spec.CanUseSetter || !spec.IsPublic))
                                 {
-                                    _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(InaccessibleJsonIncludePropertiesNotSupported, Location.None, new string[] { type.Name, spec.ClrName }));
+                                    _sourceGenerationContext.ReportDiagnostic(Diagnostic.Create(InaccessibleJsonIncludePropertiesNotSupported, memberLocation, new string[] { type.Name, spec.ClrName }));
                                 }
                             }
                         }

diff --git a/src/libraries/System.Text.Json/gen/Reflection/FieldInfoWrapper.cs b/src/libraries/System.Text.Json/gen/Reflection/FieldInfoWrapper.cs
@@ -100,5 +100,7 @@ public override void SetValue(object obj, object value, BindingFlags invokeAttr,
         {
             throw new NotImplementedException();
         }
+
+        public Location? Location => _field.Locations.Length > 0 ? _field.Locations[0] : null;
     }
 }
diff --git a/src/libraries/System.Text.Json/gen/Reflection/PropertyInfoWrapper.cs b/src/libraries/System.Text.Json/gen/Reflection/PropertyInfoWrapper.cs
@@ -92,5 +92,7 @@ public override void SetValue(object obj, object value, BindingFlags invokeAttr,
         {
             throw new NotSupportedException();
         }
+
+        public Location? Location => _property.Locations.Length > 0 ? _property.Locations[0] : null;
     }
 }
diff --git a/src/libraries/System.Text.Json/gen/Reflection/ReflectionExtensions.cs b/src/libraries/System.Text.Json/gen/Reflection/ReflectionExtensions.cs
@@ -5,6 +5,7 @@
 using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
+using Microsoft.CodeAnalysis;
 
 namespace System.Text.Json.Reflection
 {
@@ -45,5 +46,26 @@ private static bool HasJsonConstructorAttribute(ConstructorInfo constructorInfo)
 
             return false;
         }
+
+        public static Location? GetDiagnosticLocation(this Type type)
+        {
+            TypeWrapper? typeWrapper = type as TypeWrapper;
+            Debug.Assert(typeWrapper != null);
+            return typeWrapper.Location;
+        }
+
+        public static Location? GetDiagnosticLocation(this PropertyInfo propertyInfo)
+        {
+            PropertyInfoWrapper? propertyInfoWrapper = propertyInfo as PropertyInfoWrapper;
+            Debug.Assert(propertyInfoWrapper != null);
+            return propertyInfoWrapper.Location;
+        }
+
+        public static Location? GetDiagnosticLocation(this FieldInfo fieldInfo)
+        {
+            FieldInfoWrapper? fieldInfoWrapper = fieldInfo as FieldInfoWrapper;
+            Debug.Assert(fieldInfoWrapper != null);
+            return fieldInfoWrapper.Location;
+        }
     }
 }
diff --git a/src/libraries/System.Text.Json/gen/Reflection/TypeWrapper.cs b/src/libraries/System.Text.Json/gen/Reflection/TypeWrapper.cs
@@ -600,5 +600,7 @@ public override bool Equals(Type o)
             }
             return base.Equals(o);
         }
+
+        public Location? Location => _typeSymbol.Locations.Length > 0 ? _typeSymbol.Locations[0] : null;
     }
 }
diff --git a/src/libraries/System.Text.Json/gen/TypeGenerationSpec.cs b/src/libraries/System.Text.Json/gen/TypeGenerationSpec.cs
@@ -7,6 +7,7 @@
 using System.Reflection;
 using System.Text.Json.Reflection;
 using System.Text.Json.Serialization;
+using Microsoft.CodeAnalysis;
 
 namespace System.Text.Json.SourceGeneration
 {
@@ -18,6 +19,11 @@ internal class TypeGenerationSpec
         /// </summary>
         public string TypeRef { get; private set; }
 
+        /// <summary>
+        /// If specified as a root type via <c>JsonSerializableAttribute</c>, specifies the location of the attribute application.
+        /// </summary>
+        public Location? AttributeLocation { get; set; }
+
         /// <summary>
         /// The name of the public <c>JsonTypeInfo&lt;T&gt;</c> property for this type on the generated context class.
         /// For example, if the context class is named MyJsonContext, and the value of this property is JsonMessage;

diff --git a/.../System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/CompilationHelper.cs b/.../System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/CompilationHelper.cs
@@ -11,6 +11,7 @@
 using System.Text.Encodings.Web;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.Text;
 using Xunit;
 
 namespace System.Text.Json.SourceGeneration.UnitTests
@@ -336,22 +337,28 @@ public partial class MyJsonContext : JsonSerializerContext
             return CreateCompilation(source);
         }
 
-        internal static void CheckDiagnosticMessages(ImmutableArray<Diagnostic> diagnostics, DiagnosticSeverity level, string[] expectedMessages)
+        internal static void CheckDiagnosticMessages(
+            DiagnosticSeverity level,
+            ImmutableArray<Diagnostic> diagnostics,
+            (TextSpan Location, string Message)[] expectedDiags)
         {
-            string[] actualMessages = diagnostics.Where(diagnostic => diagnostic.Severity == level).Select(diagnostic => diagnostic.GetMessage()).ToArray();
+            (TextSpan Location, string Message)[] actualDiags = diagnostics
+                .Where(diagnostic => diagnostic.Severity == level)
+                .Select(diagnostic => (diagnostic.Location.SourceSpan, diagnostic.GetMessage()))
+                .ToArray();
 
             // Can't depend on reflection order when generating type metadata.
-            Array.Sort(actualMessages);
-            Array.Sort(expectedMessages);
+            Array.Sort(actualDiags);
+            Array.Sort(expectedDiags);
 
             if (CultureInfo.CurrentUICulture.Name.StartsWith("en", StringComparison.OrdinalIgnoreCase))
             {
-                Assert.Equal(expectedMessages, actualMessages);
+                Assert.Equal(expectedDiags, actualDiags);
             }
             else
             {
                 // for non-English runs, just compare the number of messages are the same
-                Assert.Equal(expectedMessages.Length, actualMessages.Length);
+                Assert.Equal(expectedDiags.Length, actualDiags.Length);
             }
         }
     }