Skip to content

Commit

Permalink
feat: unmapped source property diagnostic (#77)
Browse files Browse the repository at this point in the history
New diagnostic RMG020 Source property is not mapped to any target property which allows to configure a strict mapping mode
  • Loading branch information
latonz committed May 16, 2022
1 parent e8351f1 commit 68236ba
Show file tree
Hide file tree
Showing 35 changed files with 369 additions and 48 deletions.
8 changes: 8 additions & 0 deletions README.md
Expand Up @@ -99,6 +99,14 @@ Available strategies:

The `IgnoreCase` property allows to opt in for case insensitive mappings.

### Strict mappings

To enforce strict mappings (all source properties have to be mapped to a target property and all target properties have to be mapped from a source property) set the following two editorconfig settings:
```editorconfig
dotnet_diagnostic.RMG012.severity = error # Unmapped target property
dotnet_diagnostic.RMG020.severity = error # Unmapped source property
```

## Static mappers and extension methods

Mapperly supports static mappers and extension methods:
Expand Down
1 change: 1 addition & 0 deletions src/Riok.Mapperly/AnalyzerReleases.Shipped.md
Expand Up @@ -30,3 +30,4 @@ Rule ID | Category | Severity | Notes
--------|----------|----------|--------------------
RMG018 | Mapper | Error | Partial static mapping method in an instance mapper
RMG019 | Mapper | Error | Partial instance mapping method in a static mapper
RMG020 | Mapper | Info | Source property is not mapped to any target property
@@ -0,0 +1,23 @@
using Riok.Mapperly.Descriptors.Mappings;
using Riok.Mapperly.Descriptors.Mappings.PropertyMappings;

namespace Riok.Mapperly.Descriptors.MappingBuilder;

public class NewInstanceMappingBuilderContext : ObjectPropertyMappingBuilderContext<NewInstanceObjectPropertyMapping>
{
public NewInstanceMappingBuilderContext(MappingBuilderContext builderContext, NewInstanceObjectPropertyMapping mapping) : base(builderContext, mapping)
{
}

public void AddInitPropertyMapping(PropertyAssignmentMapping mapping)
{
SetSourcePropertyMapped(mapping.SourcePath);
Mapping.AddInitPropertyMapping(mapping);
}

public void AddConstructorParameterMapping(ConstructorParameterMapping mapping)
{
SetSourcePropertyMapped(mapping.DelegateMapping.SourcePath);
Mapping.AddConstructorParameterMapping(mapping);
}
}
Expand Up @@ -5,9 +5,6 @@
using Riok.Mapperly.Descriptors.Mappings.PropertyMappings;
using Riok.Mapperly.Diagnostics;
using Riok.Mapperly.Helpers;
using NewInstanceMappingBuilderContext =
Riok.Mapperly.Descriptors.MappingBuilder.ObjectPropertyMappingBuilderContext<
Riok.Mapperly.Descriptors.Mappings.NewInstanceObjectPropertyMapping>;

namespace Riok.Mapperly.Descriptors.MappingBuilder;

Expand Down Expand Up @@ -142,7 +139,7 @@ private static void BuildInitOnlyPropertyMappings(NewInstanceMappingBuilderConte
var propertyAssignmentMapping = new PropertyAssignmentMapping(
targetPath,
propertyMapping);
ctx.Mapping.AddInitPropertyMapping(propertyAssignmentMapping);
ctx.AddInitPropertyMapping(propertyAssignmentMapping);
}

private static bool TryBuildConstructorMapping(
Expand Down Expand Up @@ -186,7 +183,7 @@ private static void BuildInitOnlyPropertyMappings(NewInstanceMappingBuilderConte

foreach (var constructorParameterMapping in constructorParameterMappings)
{
ctx.Mapping.AddConstructorParameterMapping(constructorParameterMapping);
ctx.AddConstructorParameterMapping(constructorParameterMapping);
}

return true;
Expand Down
Expand Up @@ -48,6 +48,7 @@ public static void BuildMappingBody(ObjectPropertyMappingBuilderContext ctx)

ctx.AddUnmatchedIgnoredPropertiesDiagnostics();
ctx.AddUnmatchedTargetPropertiesDiagnostics();
ctx.AddUnmatchedSourcePropertiesDiagnostics();
}

private static void BuildPropertyAssignmentMapping(ObjectPropertyMappingBuilderContext ctx, MapPropertyAttribute config)
Expand Down
Expand Up @@ -17,19 +17,21 @@ public ObjectPropertyMappingBuilderContext(MappingBuilderContext builderContext,
Mapping = mapping;
}

public new T Mapping { get; }
protected new T Mapping { get; }
}

public class ObjectPropertyMappingBuilderContext
{
private readonly Dictionary<PropertyPath, PropertyNullDelegateAssignmentMapping> _nullDelegateMappings = new();
private readonly HashSet<string> _unmappedSourcePropertyNames;

public ObjectPropertyMappingBuilderContext(MappingBuilderContext builderContext, ObjectPropertyMapping mapping)
{
BuilderContext = builderContext;
Mapping = mapping;

IgnoredTargetProperties = GetIgnoredTargetProperties();
IgnoredTargetPropertyNames = GetIgnoredTargetProperties();
_unmappedSourcePropertyNames = GetSourcePropertyNames(IgnoredTargetPropertyNames);
TargetProperties = GetTargetProperties();
PropertyConfigsByRootTargetName = GetPropertyConfigurations();
}
Expand All @@ -38,18 +40,15 @@ public ObjectPropertyMappingBuilderContext(MappingBuilderContext builderContext,

public ObjectPropertyMapping Mapping { get; }

public HashSet<string> IgnoredTargetProperties { get; }
public HashSet<string> IgnoredTargetPropertyNames { get; }

public Dictionary<string, IPropertySymbol> TargetProperties { get; }

public Dictionary<string, List<MapPropertyAttribute>> PropertyConfigsByRootTargetName { get; }

public void AddPropertyAssignmentMapping(PropertyAssignmentMapping propertyMapping)
=> AddPropertyAssignmentMapping(Mapping, propertyMapping);

public void AddUnmatchedIgnoredPropertiesDiagnostics()
{
foreach (var notFoundIgnoredProperty in IgnoredTargetProperties)
foreach (var notFoundIgnoredProperty in IgnoredTargetPropertyNames)
{
BuilderContext.ReportDiagnostic(
DiagnosticDescriptors.IgnoredPropertyNotFound,
Expand All @@ -69,6 +68,21 @@ public void AddUnmatchedTargetPropertiesDiagnostics()
}
}

public void AddUnmatchedSourcePropertiesDiagnostics()
{
foreach (var sourcePropertyName in _unmappedSourcePropertyNames)
{
BuilderContext.ReportDiagnostic(
DiagnosticDescriptors.SourcePropertyNotMapped,
sourcePropertyName,
Mapping.SourceType,
Mapping.TargetType);
}
}

public void AddPropertyAssignmentMapping(PropertyAssignmentMapping propertyMapping)
=> AddPropertyAssignmentMapping(Mapping, propertyMapping);

public void AddNullDelegatePropertyAssignmentMapping(PropertyAssignmentMapping propertyMapping)
{
var nullConditionSourcePath = new PropertyPath(propertyMapping.SourcePath.PathWithoutTrailingNonNullable().ToList());
Expand All @@ -78,10 +92,14 @@ public void AddNullDelegatePropertyAssignmentMapping(PropertyAssignmentMapping p

private void AddPropertyAssignmentMapping(IPropertyAssignmentMappingContainer container, PropertyAssignmentMapping mapping)
{
SetSourcePropertyMapped(mapping.SourcePath);
container.AddPropertyMappings(BuildNullPropertyInitializers(mapping.TargetPath));
container.AddPropertyMapping(mapping);
}

protected void SetSourcePropertyMapped(PropertyPath sourcePath)
=> _unmappedSourcePropertyNames.Remove(sourcePath.Path.First().Name);

private IEnumerable<PropertyNullAssignmentInitializerMapping> BuildNullPropertyInitializers(PropertyPath path)
{
foreach (var nullableTrailPath in path.ObjectPathNullableSubPaths())
Expand Down Expand Up @@ -134,14 +152,20 @@ private HashSet<string> GetIgnoredTargetProperties()
.ToHashSet();
}

private HashSet<string> GetSourcePropertyNames(IReadOnlyCollection<string> ignoredPropertyNames)
{
return Mapping.SourceType
.GetAllAccessibleProperties()
.Where(x => !ignoredPropertyNames.Contains(x.Name))
.Select(x => x.Name)
.ToHashSet(StringComparer.OrdinalIgnoreCase);
}

private Dictionary<string, IPropertySymbol> GetTargetProperties()
{
return Mapping.TargetType
.GetAllMembers()
.OfType<IPropertySymbol>()
.Where(x => x.IsAccessible())
.DistinctBy(x => x.Name)
.Where(x => !IgnoredTargetProperties.Remove(x.Name))
.GetAllAccessibleProperties()
.Where(x => !IgnoredTargetPropertyNames.Remove(x.Name))
.ToDictionary(x => x.Name, StringComparer.OrdinalIgnoreCase);
}

Expand Down
Expand Up @@ -7,22 +7,23 @@ namespace Riok.Mapperly.Descriptors.Mappings.PropertyMappings;
public class ConstructorParameterMapping
{
private readonly IParameterSymbol _parameter;
private readonly NullPropertyMapping _mapping;
private readonly bool _selfOrPreviousIsUnmappedOptional;

public ConstructorParameterMapping(
IParameterSymbol parameter,
NullPropertyMapping mapping,
NullPropertyMapping delegateMapping,
bool selfOrPreviousIsUnmappedOptional)
{
DelegateMapping = delegateMapping;
_parameter = parameter;
_mapping = mapping;
_selfOrPreviousIsUnmappedOptional = selfOrPreviousIsUnmappedOptional;
}

public NullPropertyMapping DelegateMapping { get; }

public ArgumentSyntax BuildArgument(ExpressionSyntax source)
{
var argumentExpression = _mapping.Build(source);
var argumentExpression = DelegateMapping.Build(source);
var arg = Argument(argumentExpression);
return _selfOrPreviousIsUnmappedOptional
? arg.WithNameColon(NameColon(_parameter.Name))
Expand All @@ -31,7 +32,7 @@ public ArgumentSyntax BuildArgument(ExpressionSyntax source)

protected bool Equals(ConstructorParameterMapping other)
=> _parameter.Equals(other._parameter, SymbolEqualityComparer.Default)
&& _mapping.Equals(other._mapping)
&& DelegateMapping.Equals(other.DelegateMapping)
&& _selfOrPreviousIsUnmappedOptional == other._selfOrPreviousIsUnmappedOptional;

public override bool Equals(object? obj)
Expand Down Expand Up @@ -59,7 +60,7 @@ public override int GetHashCode()
unchecked
{
var hashCode = SymbolEqualityComparer.Default.GetHashCode(_parameter);
hashCode = (hashCode * 397) ^ _mapping.GetHashCode();
hashCode = (hashCode * 397) ^ DelegateMapping.GetHashCode();
hashCode = (hashCode * 397) ^ _selfOrPreviousIsUnmappedOptional.GetHashCode();
return hashCode;
}
Expand Down
8 changes: 8 additions & 0 deletions src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs
Expand Up @@ -155,4 +155,12 @@ internal static class DiagnosticDescriptors
DiagnosticCategories.Mapper,
DiagnosticSeverity.Error,
true);

public static readonly DiagnosticDescriptor SourcePropertyNotMapped = new(
"RMG020",
"Source property is not mapped to any target property",
"The property {0} on the mapping source type {1} is not mapped to any property on the mapping target type {2}",
DiagnosticCategories.Mapper,
DiagnosticSeverity.Info,
true);
}
9 changes: 9 additions & 0 deletions src/Riok.Mapperly/Helpers/SymbolExtensions.cs
Expand Up @@ -51,6 +51,15 @@ internal static IEnumerable<ISymbol> GetAllMembers(this ITypeSymbol symbol)
: members.Concat(symbol.BaseType.GetAllMembers());
}

internal static IEnumerable<IPropertySymbol> GetAllAccessibleProperties(this ITypeSymbol symbol)
{
return symbol
.GetAllMembers()
.OfType<IPropertySymbol>()
.Where(x => x.IsAccessible())
.DistinctBy(x => x.Name);
}

internal static bool ImplementsGeneric(
this ITypeSymbol t,
INamedTypeSymbol genericInterfaceSymbol,
Expand Down
Expand Up @@ -19,6 +19,18 @@ public void ClassToClassWithOneMatchingCtor()
return target;".ReplaceLineEndings());
}

[Fact]
public Task ClassToClassWithOneMatchingCtorAndUnmatchedSourcePropertyShouldDiagnostic()
{
var source = TestSourceBuilder.Mapping(
"A",
"B",
"class A { public string StringValue { get; set; } public int IntValue { get; set; } }",
"class B { public B(string stringValue) {} { }");

return TestHelper.VerifyGenerator(source);
}

[Fact]
public void ClassToClassWithOneMatchingCtorWithMatchedOptional()
{
Expand Down Expand Up @@ -100,7 +112,7 @@ public Task ClassToClassUnmatchedCtorShouldDiagnostic()
"A",
"B",
"class A { public string StringValue { get; set; } public int IntValue { get; set; } }",
"class B { public B(string StringValue9){} }");
"class B { public B(string StringValue9){} public int IntValue { get; set; } }");

return TestHelper.VerifyGenerator(source);
}
Expand All @@ -112,7 +124,7 @@ public Task ClassToClassUnmatchedAttributedCtorShouldDiagnosticAndUseAlternative
"A",
"B",
"class A { public string StringValue { get; set; } public int IntValue { get; set; } }",
"class B { [MapperConstructor] public B(string StringValue9){} public B(string StringValue) {} }");
"class B { [MapperConstructor] public B(string StringValue9){} public B(string StringValue) {} public int IntValue { get; set; } }");

return TestHelper.VerifyGenerator(source);
}
Expand Down
Expand Up @@ -130,7 +130,7 @@ public Task InitOnlyPropertyShouldDiagnosticOnVoidMethod()
}

[Fact]
public Task InitOnlyPropertySourceNotFoundShoulDiagnostic()
public Task InitOnlyPropertySourceNotFoundShouldDiagnostic()
{
var source = TestSourceBuilder.Mapping(
"A",
Expand Down
8 changes: 4 additions & 4 deletions test/Riok.Mapperly.Tests/TestHelper.cs
Expand Up @@ -40,10 +40,10 @@ public static string GenerateSingleMapperMethodBody(string source, TestHelperOpt

var result = Generate(source, options).GetRunResult();

if (!options.AllowDiagnostics)
{
result.Diagnostics.Should().HaveCount(0);
}
result.Diagnostics
.FirstOrDefault(d => options.AllowedDiagnostics?.Contains(d.Severity) != true)
.Should()
.BeNull();

var mapperClassImpl = result.GeneratedTrees.Single()
.GetRoot() // compilation
Expand Down
2 changes: 1 addition & 1 deletion test/Riok.Mapperly.Tests/TestHelperOptions.cs
Expand Up @@ -6,7 +6,7 @@ namespace Riok.Mapperly.Tests;
public record TestHelperOptions(
NullableContextOptions NullableOption = NullableContextOptions.Enable,
LanguageVersion LanguageVersion = LanguageVersion.Default,
bool AllowDiagnostics = false)
IReadOnlySet<DiagnosticSeverity>? AllowedDiagnostics = null)
{
public static readonly TestHelperOptions Default = new();
}
Expand Up @@ -5,6 +5,7 @@ public partial class Mapper
private partial B Map(A source)
{
var target = new B(source.StringValue);
target.IntValue = source.IntValue;
return target;
}
}
Expand Up @@ -12,6 +12,19 @@
Message: B has no accessible constructor with mappable arguments,
Category: Mapper,
CustomTags: []
},
{
Id: RMG020,
Title: Source property is not mapped to any target property,
Severity: Info,
WarningLevel: 1,
Location: : (10,4)-(10,28),
Description: ,
HelpLink: ,
MessageFormat: The property {0} on the mapping source type {1} is not mapped to any property on the mapping target type {2},
Message: The property StringValue on the mapping source type A is not mapped to any property on the mapping target type B,
Category: Mapper,
CustomTags: []
}
]
}
Expand Up @@ -5,6 +5,7 @@ public partial class Mapper
private partial B Map(A source)
{
var target = new B();
target.IntValue = source.IntValue;
return target;
}
}
@@ -0,0 +1,17 @@
{
Diagnostics: [
{
Id: RMG020,
Title: Source property is not mapped to any target property,
Severity: Info,
WarningLevel: 1,
Location: : (10,4)-(10,28),
Description: ,
HelpLink: ,
MessageFormat: The property {0} on the mapping source type {1} is not mapped to any property on the mapping target type {2},
Message: The property IntValue on the mapping source type A is not mapped to any property on the mapping target type B,
Category: Mapper,
CustomTags: []
}
]
}

0 comments on commit 68236ba

Please sign in to comment.