Skip to content

Commit

Permalink
fix: improve location of reported diagnostics
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
latonz committed Nov 22, 2023
1 parent 42d3e26 commit d5e991e
Show file tree
Hide file tree
Showing 34 changed files with 172 additions and 103 deletions.
5 changes: 5 additions & 0 deletions src/Riok.Mapperly/Configuration/AttributeDataAccessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ public IEnumerable<TAttribute> Access<TAttribute>(ISymbol symbol)
syntaxIndex++;
}

if (attr is HasSyntaxReference symbolRefHolder)
{
symbolRefHolder.SyntaxReference = attrData.ApplicationSyntaxReference?.GetSyntax();
}

return attr;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
14 changes: 14 additions & 0 deletions src/Riok.Mapperly/Configuration/HasSyntaxReference.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using Microsoft.CodeAnalysis;

namespace Riok.Mapperly.Configuration;

public record HasSyntaxReference
{
/// <summary>
/// Gets or sets the syntax reference, from where the data of this configuration was read.
/// Initialized by <see cref="AttributeDataAccessor"/>.
/// </summary>
public SyntaxNode? SyntaxReference { get; set; }

public Location? Location => SyntaxReference?.GetLocation();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
8 changes: 4 additions & 4 deletions src/Riok.Mapperly/Descriptors/DescriptorBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ MapperConfiguration defaultMapperConfiguration
_configurationReader,
_symbolAccessor,
attributeAccessor,
_mapperDescriptor,
_unsafeAccessorContext,
_diagnostics,
new MappingBuilder(_mappings),
new ExistingTargetMappingBuilder(_mappings)
new ExistingTargetMappingBuilder(_mappings),
mapperDeclaration.Syntax.GetLocation()
);
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Expand Down Expand Up @@ -82,10 +80,12 @@ public override bool IsConversionEnabled(MappingConversionType conversionType)
/// </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);
Expand All @@ -99,7 +99,7 @@ public override bool IsConversionEnabled(MappingConversionType conversionType)
// 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);
Expand Down Expand Up @@ -136,12 +136,14 @@ public override bool IsConversionEnabled(MappingConversionType conversionType)
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)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ IReadOnlyCollection<PropertyMappingConfiguration> memberConfigs
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)
{
Expand Down Expand Up @@ -267,7 +267,10 @@ private static void BuildConstructorMapping(INewInstanceBuilderContext<IMapping>
// 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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ private static void BuildTupleConstructorMapping(INewValueTupleBuilderContext<IN
// 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ MemberPath targetMemberPath
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)
Expand Down
66 changes: 45 additions & 21 deletions src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;
Expand All @@ -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>
Expand All @@ -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)
Expand All @@ -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>
Expand Down Expand Up @@ -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 (formatProviderName != null && formatProvider == null)
{
ReportDiagnostic(DiagnosticDescriptors.FormatProviderNotFound, formatProviderName);
Expand Down Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ bool duplicatedSourceTypesAllowed
var mapping = ctx.FindOrBuildMapping(
sourceType,
targetType,
MappingBuildingOptions.KeepUserSymbol | MappingBuildingOptions.MarkAsReusable | MappingBuildingOptions.ClearDerivedTypes
MappingBuildingOptions.KeepUserSymbol | MappingBuildingOptions.MarkAsReusable | MappingBuildingOptions.ClearDerivedTypes,
config.Location
);
if (mapping == null)
{
Expand Down

0 comments on commit d5e991e

Please sign in to comment.