fix: improve location of reported diagnostics

Use more accurate location for reported diagnostics. Fallback to mapper class is used a lot less. For nested mappings the last user defined mapping method is used instead. For attributes the attribute syntax is used.
riok · Nov 21, 2023 · 806852b · 806852b
1 parent f25c178
commit 806852b
Show file tree

Hide file tree

Showing 34 changed files with 170 additions and 104 deletions.
diff --git a/src/Riok.Mapperly/Configuration/AttributeDataAccessor.cs b/src/Riok.Mapperly/Configuration/AttributeDataAccessor.cs
@@ -48,7 +48,8 @@ public IEnumerable<TData> Access<TAttribute, TData>(ISymbol symbol)
         var attrDatas = symbolAccessor.GetAttributes<TAttribute>(symbol);
         foreach (var attrData in attrDatas)
         {
-            yield return Access<TAttribute, TData>(attrData);
+            var data = Access<TAttribute, TData>(attrData);
+            yield return data;
         }
     }
 
@@ -78,6 +79,11 @@ internal static TData Access<TAttribute, TData>(AttributeData attrData)
             syntaxIndex++;
         }
 
+        if (attr is HasSyntaxReference symbolRefHolder)
+        {
+            symbolRefHolder.SyntaxReference = attrData.ApplicationSyntaxReference?.GetSyntax();
+        }
+
         return attr;
     }
 

diff --git a/src/Riok.Mapperly/Configuration/DerivedTypeMappingConfiguration.cs b/src/Riok.Mapperly/Configuration/DerivedTypeMappingConfiguration.cs
@@ -10,4 +10,4 @@ namespace Riok.Mapperly.Configuration;
 /// </summary>
 /// <param name="SourceType">The source type of the derived type mapping.</param>
 /// <param name="TargetType">The target type of the derived type mapping.</param>
-public record DerivedTypeMappingConfiguration(ITypeSymbol SourceType, ITypeSymbol TargetType);
+public record DerivedTypeMappingConfiguration(ITypeSymbol SourceType, ITypeSymbol TargetType) : HasSyntaxReference;
diff --git a/src/Riok.Mapperly/Configuration/HasSyntaxReference.cs b/src/Riok.Mapperly/Configuration/HasSyntaxReference.cs
@@ -0,0 +1,10 @@
+using Microsoft.CodeAnalysis;
+
+namespace Riok.Mapperly.Configuration;
+
+public record HasSyntaxReference
+{
+    public SyntaxNode? SyntaxReference { get; set; } // initialized by AttributeDataAccessor
+
+    public Location? Location => SyntaxReference?.GetLocation();
+}
diff --git a/src/Riok.Mapperly/Configuration/PropertyMappingConfiguration.cs b/src/Riok.Mapperly/Configuration/PropertyMappingConfiguration.cs
@@ -2,7 +2,7 @@
 
 namespace Riok.Mapperly.Configuration;
 
-public record PropertyMappingConfiguration(StringMemberPath Source, StringMemberPath Target)
+public record PropertyMappingConfiguration(StringMemberPath Source, StringMemberPath Target) : HasSyntaxReference
 {
     public string? StringFormat { get; set; }
 

diff --git a/src/Riok.Mapperly/Descriptors/DescriptorBuilder.cs b/src/Riok.Mapperly/Descriptors/DescriptorBuilder.cs
@@ -52,11 +52,11 @@ MapperConfiguration defaultMapperConfiguration
             _configurationReader,
             _symbolAccessor,
             attributeAccessor,
-            _mapperDescriptor,
             _unsafeAccessorContext,
             _diagnostics,
             new MappingBuilder(_mappings),
-            new ExistingTargetMappingBuilder(_mappings)
+            new ExistingTargetMappingBuilder(_mappings),
+            mapperDeclaration.Syntax.GetLocation()
         );
     }
 
@@ -143,7 +143,7 @@ private void ExtractUserMappings()
 
     private ObjectFactoryCollection ExtractObjectFactories()
     {
-        return ObjectFactoryBuilder.ExtractObjectFactories(_builderContext, _mapperDescriptor.Symbol);
+        return ObjectFactoryBuilder.ExtractObjectFactories(_builderContext, _mapperDescriptor.Symbol, _mapperDescriptor.Static);
     }
 
     private void EnqueueUserMappings(ObjectFactoryCollection objectFactories, FormatProviderCollection formatProviders)
@@ -172,7 +172,7 @@ private void ExtractExternalMappings()
 
     private FormatProviderCollection ExtractFormatProviders()
     {
-        return FormatProviderBuilder.ExtractFormatProviders(_builderContext, _mapperDescriptor.Symbol);
+        return FormatProviderBuilder.ExtractFormatProviders(_builderContext, _mapperDescriptor.Symbol, _mapperDescriptor.Static);
     }
 
     private void BuildMappingMethodNames()

diff --git a/src/Riok.Mapperly/Descriptors/FormatProviders/FormatProviderBuilder.cs b/src/Riok.Mapperly/Descriptors/FormatProviders/FormatProviderBuilder.cs
@@ -8,12 +8,12 @@ namespace Riok.Mapperly.Descriptors.FormatProviders;
 
 public static class FormatProviderBuilder
 {
-    public static FormatProviderCollection ExtractFormatProviders(SimpleMappingBuilderContext ctx, ITypeSymbol mapperSymbol)
+    public static FormatProviderCollection ExtractFormatProviders(SimpleMappingBuilderContext ctx, ITypeSymbol mapperSymbol, bool isStatic)
     {
         var formatProviders = mapperSymbol
             .GetMembers()
             .Where(x => ctx.SymbolAccessor.HasAttribute<FormatProviderAttribute>(x))
-            .Select(x => BuildFormatProvider(ctx, x))
+            .Select(x => BuildFormatProvider(ctx, x, isStatic))
             .WhereNotNull()
             .ToList();
 
@@ -27,13 +27,13 @@ public static FormatProviderCollection ExtractFormatProviders(SimpleMappingBuild
         return new FormatProviderCollection(formatProvidersByName, defaultFormatProviderCandidates.FirstOrDefault());
     }
 
-    private static FormatProvider? BuildFormatProvider(SimpleMappingBuilderContext ctx, ISymbol symbol)
+    private static FormatProvider? BuildFormatProvider(SimpleMappingBuilderContext ctx, ISymbol symbol, bool isStatic)
     {
         var memberSymbol = MappableMember.Create(ctx.SymbolAccessor, symbol);
         if (memberSymbol == null)
             return null;
 
-        if (!memberSymbol.CanGet || symbol.IsStatic != ctx.Static || !memberSymbol.Type.Implements(ctx.Types.Get<IFormatProvider>()))
+        if (!memberSymbol.CanGet || symbol.IsStatic != isStatic || !memberSymbol.Type.Implements(ctx.Types.Get<IFormatProvider>()))
         {
             ctx.ReportDiagnostic(DiagnosticDescriptors.InvalidFormatProviderSignature, symbol, symbol.Name);
             return null;

diff --git a/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs b/src/Riok.Mapperly/Descriptors/InlineExpressionMappingBuilderContext.cs
@@ -16,10 +16,7 @@ public class InlineExpressionMappingBuilderContext : MappingBuilderContext
     private readonly MappingBuilderContext _parentContext;
 
     public InlineExpressionMappingBuilderContext(MappingBuilderContext ctx, TypeMappingKey mappingKey)
-        : this(ctx, (ctx.FindMapping(mappingKey) as IUserMapping)?.Method, mappingKey) { }
-
-    private InlineExpressionMappingBuilderContext(MappingBuilderContext ctx, IMethodSymbol? userSymbol, TypeMappingKey mappingKey)
-        : base(ctx, userSymbol, mappingKey, false)
+        : base(ctx, (ctx.FindMapping(mappingKey) as IUserMapping)?.Method, null, mappingKey, false)
     {
         _parentContext = ctx;
         _inlineExpressionMappings = new MappingCollection();
@@ -28,10 +25,11 @@ private InlineExpressionMappingBuilderContext(MappingBuilderContext ctx, IMethod
     private InlineExpressionMappingBuilderContext(
         InlineExpressionMappingBuilderContext ctx,
         IMethodSymbol? userSymbol,
+        Location? diagnosticLocation,
         TypeMappingKey mappingKey,
         bool clearDerivedTypes
     )
-        : base(ctx, userSymbol, mappingKey, clearDerivedTypes)
+        : base(ctx, userSymbol, diagnosticLocation, mappingKey, clearDerivedTypes)
     {
         _parentContext = ctx;
         _inlineExpressionMappings = ctx._inlineExpressionMappings;
@@ -82,10 +80,12 @@ conversionType is not MappingConversionType.EnumToString and not MappingConversi
     /// </summary>
     /// <param name="mappingKey">The mapping key.</param>
     /// <param name="options">The options, <see cref="MappingBuildingOptions.MarkAsReusable"/> is ignored.</param>
-    /// <returns></returns>
+    /// <param name="diagnosticLocation">The updated to location where to report diagnostics if a new mapping is being built.</param>
+    /// <returns>The found or created mapping, or <c>null</c> if no mapping could be created.</returns>
     public override INewInstanceMapping? FindOrBuildMapping(
         TypeMappingKey mappingKey,
-        MappingBuildingOptions options = MappingBuildingOptions.Default
+        MappingBuildingOptions options = MappingBuildingOptions.Default,
+        Location? diagnosticLocation = null
     )
     {
         var mapping = FindMapping(mappingKey);
@@ -99,7 +99,7 @@ conversionType is not MappingConversionType.EnumToString and not MappingConversi
         // unset MarkAsReusable and KeepUserSymbol as they have special handling for inline mappings
         options &= ~(MappingBuildingOptions.MarkAsReusable | MappingBuildingOptions.KeepUserSymbol);
 
-        mapping = BuildMapping(userSymbol, mappingKey, options);
+        mapping = BuildMapping(userSymbol, mappingKey, options, diagnosticLocation);
         if (mapping != null)
         {
             _inlineExpressionMappings.Add(mapping, mappingKey.Configuration);
@@ -136,12 +136,14 @@ protected override NullFallbackValue GetNullFallbackValue(ITypeSymbol targetType
     protected override MappingBuilderContext ContextForMapping(
         IMethodSymbol? userSymbol,
         TypeMappingKey mappingKey,
-        MappingBuildingOptions options
+        MappingBuildingOptions options,
+        Location? diagnosticLocation = null
     )
     {
         return new InlineExpressionMappingBuilderContext(
             this,
             userSymbol,
+            diagnosticLocation,
             mappingKey,
             options.HasFlag(MappingBuildingOptions.ClearDerivedTypes)
         );

diff --git a/...iok.Mapperly/Descriptors/MappingBodyBuilders/NewInstanceObjectMemberMappingBodyBuilder.cs b/...iok.Mapperly/Descriptors/MappingBodyBuilders/NewInstanceObjectMemberMappingBodyBuilder.cs
@@ -147,7 +147,7 @@ private static void BuildInitMemberMapping(
             return;
 
         var mappingKey = new TypeMappingKey(sourcePath.MemberType, targetMember.Type, memberConfig?.ToTypeMappingConfiguration());
-        var delegateMapping = ctx.BuilderContext.FindOrBuildLooseNullableMapping(mappingKey);
+        var delegateMapping = ctx.BuilderContext.FindOrBuildLooseNullableMapping(mappingKey, diagnosticLocation: memberConfig?.Location);
 
         if (delegateMapping == null)
         {
@@ -267,7 +267,10 @@ private static bool TryBuildConstructorMapping(
             // nullability is handled inside the member mapping
             var paramType = parameter.Type.WithNullableAnnotation(parameter.NullableAnnotation);
             var typeMapping = new TypeMappingKey(sourcePath.MemberType, paramType, memberConfig?.ToTypeMappingConfiguration());
-            var delegateMapping = ctx.BuilderContext.FindOrBuildLooseNullableMapping(typeMapping);
+            var delegateMapping = ctx.BuilderContext.FindOrBuildLooseNullableMapping(
+                typeMapping,
+                diagnosticLocation: memberConfig?.Location
+            );
 
             if (delegateMapping == null)
             {

diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewValueTupleMappingBodyBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/NewValueTupleMappingBodyBuilder.cs
@@ -83,7 +83,10 @@ out HashSet<string> mappedTargetMemberNames
             // nullability is handled inside the member expressionMapping
             var paramType = targetMember.Type.WithNullableAnnotation(targetMember.NullableAnnotation);
             var mappingKey = new TypeMappingKey(sourcePath.MemberType, paramType, memberConfig?.ToTypeMappingConfiguration());
-            var delegateMapping = ctx.BuilderContext.FindOrBuildLooseNullableMapping(mappingKey);
+            var delegateMapping = ctx.BuilderContext.FindOrBuildLooseNullableMapping(
+                mappingKey,
+                diagnosticLocation: memberConfig?.Location
+            );
             if (delegateMapping == null)
             {
                 ctx.BuilderContext.ReportDiagnostic(

diff --git a/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs b/src/Riok.Mapperly/Descriptors/MappingBodyBuilders/ObjectMemberMappingBodyBuilder.cs
@@ -282,7 +282,7 @@ private static void BuildMemberAssignmentMapping(
             targetMemberPath.MemberType,
             memberConfig?.ToTypeMappingConfiguration()
         );
-        var delegateMapping = ctx.BuilderContext.FindOrBuildLooseNullableMapping(typeMapping);
+        var delegateMapping = ctx.BuilderContext.FindOrBuildLooseNullableMapping(typeMapping, diagnosticLocation: memberConfig?.Location);
 
         // couldn't build the mapping
         if (delegateMapping == null)

diff --git a/src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs b/src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs
@@ -15,26 +15,34 @@ namespace Riok.Mapperly.Descriptors;
 [DebuggerDisplay("{GetType().Name}({Source.Name} => {Target.Name})")]
 public class MappingBuilderContext : SimpleMappingBuilderContext
 {
+    private readonly FormatProviderCollection _formatProviders;
     private CollectionInfos? _collectionInfos;
 
     public MappingBuilderContext(
         SimpleMappingBuilderContext parentCtx,
         ObjectFactoryCollection objectFactories,
         FormatProviderCollection formatProviders,
         IMethodSymbol? userSymbol,
-        TypeMappingKey mappingKey
+        TypeMappingKey mappingKey,
+        Location? diagnosticLocation = null
     )
-        : base(parentCtx)
+        : base(parentCtx, diagnosticLocation ?? userSymbol?.GetSyntaxLocation())
     {
         ObjectFactories = objectFactories;
-        FormatProviders = formatProviders;
+        _formatProviders = formatProviders;
         UserSymbol = userSymbol;
         MappingKey = mappingKey;
         Configuration = ReadConfiguration(new MappingConfigurationReference(UserSymbol, mappingKey.Source, mappingKey.Target));
     }
 
-    protected MappingBuilderContext(MappingBuilderContext ctx, IMethodSymbol? userSymbol, TypeMappingKey mappingKey, bool clearDerivedTypes)
-        : this(ctx, ctx.ObjectFactories, ctx.FormatProviders, userSymbol, mappingKey)
+    protected MappingBuilderContext(
+        MappingBuilderContext ctx,
+        IMethodSymbol? userSymbol,
+        Location? diagnosticLocation,
+        TypeMappingKey mappingKey,
+        bool clearDerivedTypes
+    )
+        : this(ctx, ctx.ObjectFactories, ctx._formatProviders, userSymbol, mappingKey, diagnosticLocation)
     {
         if (clearDerivedTypes)
         {
@@ -60,7 +68,6 @@ protected MappingBuilderContext(MappingBuilderContext ctx, IMethodSymbol? userSy
     public virtual bool IsExpression => false;
 
     public ObjectFactoryCollection ObjectFactories { get; }
-    public FormatProviderCollection FormatProviders { get; }
 
     /// <inheritdoc cref="MappingBuilders.MappingBuilder.UserMappings"/>
     public IReadOnlyCollection<IUserMapping> UserMappings => MappingBuilder.UserMappings;
@@ -85,14 +92,16 @@ protected MappingBuilderContext(MappingBuilderContext ctx, IMethodSymbol? userSy
     /// <param name="sourceType">The source type.</param>
     /// <param name="targetType">The target type.</param>
     /// <param name="options">The mapping building options.</param>
+    /// <param name="diagnosticLocation">The updated to location where to report diagnostics if a new mapping is being built.</param>
     /// <returns>The found or created mapping, or <c>null</c> if no mapping could be created.</returns>
     public INewInstanceMapping? FindOrBuildMapping(
         ITypeSymbol sourceType,
         ITypeSymbol targetType,
-        MappingBuildingOptions options = MappingBuildingOptions.Default
+        MappingBuildingOptions options = MappingBuildingOptions.Default,
+        Location? diagnosticLocation = null
     )
     {
-        return FindOrBuildMapping(new TypeMappingKey(sourceType, targetType), options);
+        return FindOrBuildMapping(new TypeMappingKey(sourceType, targetType), options, diagnosticLocation);
     }
 
     /// <summary>
@@ -106,17 +115,19 @@ protected MappingBuilderContext(MappingBuilderContext ctx, IMethodSymbol? userSy
     /// </summary>
     /// <param name="mappingKey">The mapping key.</param>
     /// <param name="options">The mapping building options.</param>
+    /// <param name="diagnosticLocation">The updated to location where to report diagnostics if a new mapping is being built.</param>
     /// <returns>The found or created mapping, or <c>null</c> if no mapping could be created.</returns>
     public virtual INewInstanceMapping? FindOrBuildMapping(
         TypeMappingKey mappingKey,
-        MappingBuildingOptions options = MappingBuildingOptions.Default
+        MappingBuildingOptions options = MappingBuildingOptions.Default,
+        Location? diagnosticLocation = null
     )
     {
-        return MappingBuilder.Find(mappingKey) ?? BuildMapping(mappingKey, options);
+        return MappingBuilder.Find(mappingKey) ?? BuildMapping(mappingKey, options, diagnosticLocation);
     }
 
     /// <summary>
-    /// Finds or builds a mapping (<seealso cref="FindOrBuildMapping(Riok.Mapperly.Descriptors.TypeMappingKey,Riok.Mapperly.Descriptors.MappingBuildingOptions)"/>).
+    /// Finds or builds a mapping (<seealso cref="FindOrBuildMapping(Riok.Mapperly.Descriptors.TypeMappingKey,Riok.Mapperly.Descriptors.MappingBuildingOptions,Location)"/>).
     /// Before a new mapping is built existing mappings are tried to be found by the following priorities:
     /// 1. exact match
     /// 2. ignoring the nullability of the source (needs to be handled by the caller of this method)
@@ -126,28 +137,35 @@ protected MappingBuilderContext(MappingBuilderContext ctx, IMethodSymbol? userSy
     /// </summary>
     /// <param name="key">The mapping key.</param>
     /// <param name="options">The options to build a new mapping if no existing mapping is found.</param>
+    /// <param name="diagnosticLocation">The updated to location where to report diagnostics if a new mapping is being built.</param>
     /// <returns>The found or built mapping, or <c>null</c> if none could be found and none could be built.</returns>
     public INewInstanceMapping? FindOrBuildLooseNullableMapping(
         TypeMappingKey key,
-        MappingBuildingOptions options = MappingBuildingOptions.Default
+        MappingBuildingOptions options = MappingBuildingOptions.Default,
+        Location? diagnosticLocation = null
     )
     {
         return FindMapping(key)
             ?? FindMapping(key.NonNullableSource())
             ?? FindMapping(key.NonNullableTarget())
-            ?? FindOrBuildMapping(key.NonNullable(), options);
+            ?? FindOrBuildMapping(key.NonNullable(), options, diagnosticLocation);
     }
 
     /// <summary>
     /// Builds a new mapping for the provided types and config with the given options.
     /// </summary>
     /// <param name="mappingKey">The mapping key.</param>
     /// <param name="options">The options.</param>
+    /// <param name="diagnosticLocation">The updated to location where to report diagnostics if a new mapping is being built.</param>
     /// <returns>The created mapping, or <c>null</c> if no mapping could be created.</returns>
-    public INewInstanceMapping? BuildMapping(TypeMappingKey mappingKey, MappingBuildingOptions options = MappingBuildingOptions.Default)
+    public INewInstanceMapping? BuildMapping(
+        TypeMappingKey mappingKey,
+        MappingBuildingOptions options = MappingBuildingOptions.Default,
+        Location? diagnosticLocation = null
+    )
     {
         var userSymbol = options.HasFlag(MappingBuildingOptions.KeepUserSymbol) ? UserSymbol : null;
-        return BuildMapping(userSymbol, mappingKey, options);
+        return BuildMapping(userSymbol, mappingKey, options, diagnosticLocation);
     }
 
     /// <summary>
@@ -211,14 +229,14 @@ protected MappingBuilderContext(MappingBuilderContext ctx, IMethodSymbol? userSy
     }
 
     public void ReportDiagnostic(DiagnosticDescriptor descriptor, params object[] messageArgs) =>
-        base.ReportDiagnostic(descriptor, UserSymbol, messageArgs);
+        base.ReportDiagnostic(descriptor, null, messageArgs);
 
     public NullFallbackValue GetNullFallbackValue(ITypeSymbol? targetType = null) =>
         GetNullFallbackValue(targetType ?? Target, MapperConfiguration.ThrowOnMappingNullMismatch);
 
     public FormatProvider? GetFormatProvider(string? formatProviderName)
     {
-        var formatProvider = FormatProviders.Get(formatProviderName);
+        var formatProvider = _formatProviders.Get(formatProviderName);
         if (formatProvider == null && formatProviderName != null)
         {
             ReportDiagnostic(DiagnosticDescriptors.FormatProviderNotFound, formatProviderName);
@@ -251,15 +269,21 @@ protected virtual NullFallbackValue GetNullFallbackValue(ITypeSymbol targetType,
     protected virtual MappingBuilderContext ContextForMapping(
         IMethodSymbol? userSymbol,
         TypeMappingKey mappingKey,
-        MappingBuildingOptions options
+        MappingBuildingOptions options,
+        Location? diagnosticLocation = null
     )
     {
-        return new(this, userSymbol, mappingKey, options.HasFlag(MappingBuildingOptions.ClearDerivedTypes));
+        return new(this, userSymbol, diagnosticLocation, mappingKey, options.HasFlag(MappingBuildingOptions.ClearDerivedTypes));
     }
 
-    protected INewInstanceMapping? BuildMapping(IMethodSymbol? userSymbol, TypeMappingKey key, MappingBuildingOptions options)
+    protected INewInstanceMapping? BuildMapping(
+        IMethodSymbol? userSymbol,
+        TypeMappingKey mappingKey,
+        MappingBuildingOptions options,
+        Location? diagnosticLocation
+    )
     {
-        var ctx = ContextForMapping(userSymbol, key, options);
+        var ctx = ContextForMapping(userSymbol, mappingKey, options, diagnosticLocation);
         return MappingBuilder.Build(ctx, options.HasFlag(MappingBuildingOptions.MarkAsReusable));
     }
 }