Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle nullable correctly in nullable disabled contexts for generics and arrays #1029

Merged
merged 1 commit into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ ITypeSymbol enumeratedType
type,
typeInfo,
implementedTypes,
enumeratedType,
symbolAccessor.UpgradeNullable(enumeratedType),
FindCountProperty(symbolAccessor, type, typeInfo),
HasValidAddMethod(wellKnownTypes, type, typeInfo, implementedTypes),
collectionTypeInfo?.Immutable == true
Expand All @@ -112,8 +112,8 @@ ITypeSymbol enumeratedType
{
// if type is array return element type
// otherwise using the IEnumerable element type can erase the null annotation for external types
if (type.IsArrayType())
return ((IArrayTypeSymbol)type).ElementType;
if (type.IsArrayType(out var arraySymbol))
return arraySymbol.ElementType;

if (type.ImplementsGeneric(types.Get(typeof(IEnumerable<>)), out var enumerableIntf))
return enumerableIntf.TypeArguments[0];
Expand All @@ -133,6 +133,7 @@ ITypeSymbol enumeratedType
{
return namedType.TypeArguments[0];
}

// Memory<> or ReadOnlyMemory<> etc, get the type symbol
if (
SymbolEqualityComparer.Default.Equals(type.OriginalDefinition, types.Get(typeof(Memory<>)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ out sourcePath
if (mappableField == default)
return false;

sourcePath = new MemberPath(new[] { new FieldMember(mappableField) });
sourcePath = new MemberPath(new[] { new FieldMember(mappableField, ctx.BuilderContext.SymbolAccessor) });
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ bool duplicatedSourceTypesAllowed
var derivedTypeMappingSourceTypes = new HashSet<ITypeSymbol>(SymbolEqualityComparer.Default);
var derivedTypeMappings = new List<TMapping>(configs.Count);
Func<ITypeSymbol, bool> isAssignableToSource = ctx.Source is ITypeParameterSymbol sourceTypeParameter
? t => ctx.SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(sourceTypeParameter, t, ctx.Source.NullableAnnotation)
? t => ctx.SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(sourceTypeParameter, t)
: t => ctx.SymbolAccessor.HasImplicitConversion(t, ctx.Source);
Func<ITypeSymbol, bool> isAssignableToTarget = ctx.Target is ITypeParameterSymbol targetTypeParameter
? t => ctx.SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(targetTypeParameter, t, ctx.Target.NullableAnnotation)
? t => ctx.SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(targetTypeParameter, t)
: t => ctx.SymbolAccessor.HasImplicitConversion(t, ctx.Target);

foreach (var config in configs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,9 @@ INewInstanceMapping valueMapping

if (fromEnumerableCtor != null)
{
var constructedDictionary = dictionaryType.Construct(keyMapping.TargetType, valueMapping.TargetType);
var constructedDictionary = dictionaryType
.Construct(keyMapping.TargetType, valueMapping.TargetType)
.WithNullableAnnotation(NullableAnnotation.NotAnnotated);
return new CtorMapping(ctx.Source, constructedDictionary);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,13 @@ or CollectionType.IEnumerable
if (elementMapping.IsSynthetic)
return null;

// upgrade nullability of element type
var targetType = ((INamedTypeSymbol)ctx.CollectionInfos!.Target.Type.OriginalDefinition).Construct(elementMapping.TargetType);
var targetTypeToInstantiate = ctx.Types.Get(typeof(List<>)).Construct(elementMapping.TargetType);
var targetTypeToInstantiate = ctx.Types.Get(typeof(List<>))
.Construct(elementMapping.TargetType)
.WithNullableAnnotation(NullableAnnotation.NotAnnotated);

return new ForEachAddEnumerableMapping(
ctx.Source,
targetType,
ctx.CollectionInfos!.Target.Type,
elementMapping,
AddMethodName,
targetTypeToInstantiate,
Expand All @@ -207,12 +207,7 @@ private static NewInstanceMapping BuildArrayToArrayMapping(MappingBuilderContext
// use a for loop mapping otherwise.
if (!elementMapping.IsSynthetic)
{
// upgrade nullability of element type
var targetType =
ctx.CollectionInfos!.Target.CollectionType == CollectionType.Array
? ctx.Types.GetArrayType(elementMapping.TargetType)
: ((INamedTypeSymbol)ctx.Target).ConstructedFrom.Construct(elementMapping.TargetType);
return new ArrayForMapping(ctx.Source, targetType, elementMapping, elementMapping.TargetType);
return new ArrayForMapping(ctx.Source, ctx.Target, elementMapping, elementMapping.TargetType);
}

return ctx.MapperConfiguration.UseDeepCloning
Expand All @@ -226,18 +221,12 @@ private static NewInstanceMapping BuildArrayToArrayMapping(MappingBuilderContext
if (elementMapping.IsSynthetic)
return null;

// upgrade nullability of element type
var targetType =
ctx.CollectionInfos!.Target.CollectionType == CollectionType.Array
? ctx.Types.GetArrayType(elementMapping.TargetType)
: ((INamedTypeSymbol)ctx.Target).ConstructedFrom.Construct(elementMapping.TargetType);

return new ArrayForEachMapping(
ctx.Source,
targetType,
ctx.Target,
elementMapping,
elementMapping.TargetType,
ctx.CollectionInfos.Source.CountPropertyName!
ctx.CollectionInfos!.Source.CountPropertyName!
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
return null;

// otherwise try to find parse method ignoring the nullability
parseMethod = parseMethodCandidates.FirstOrDefault(x => SymbolEqualityComparer.Default.Equals(x.ReturnType, nonNullableTarget));
parseMethod = parseMethodCandidates.Find(x => SymbolEqualityComparer.Default.Equals(x.ReturnType, nonNullableTarget));

Check warning on line 44 in src/Riok.Mapperly/Descriptors/MappingBuilders/ParseMappingBuilder.cs

View check run for this annotation

Codecov / codecov/patch

src/Riok.Mapperly/Descriptors/MappingBuilders/ParseMappingBuilder.cs#L44

Added line #L44 was not covered by tests
return parseMethod == null ? null : new StaticMethodMapping(parseMethod);
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Abstractions;
using Riok.Mapperly.Descriptors.Enumerables;
using Riok.Mapperly.Descriptors.Enumerables.EnsureCapacity;
Expand Down Expand Up @@ -175,7 +176,9 @@ private static NewInstanceMapping BuildToArrayOrMap(MappingBuilderContext ctx, I

private static NewInstanceMapping? BuildSpanToList(MappingBuilderContext ctx, INewInstanceMapping elementMapping)
{
var typedList = ctx.Types.Get(typeof(List<>)).Construct(elementMapping.TargetType);
var typedList = ctx.Types.Get(typeof(List<>))
.Construct(elementMapping.TargetType)
.WithNullableAnnotation(NullableAnnotation.NotAnnotated);
if (ctx.FindOrBuildMapping(ctx.Source, typedList) is not { } listMapping)
return null;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,27 @@
using System.Diagnostics;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Riok.Mapperly.Helpers;

namespace Riok.Mapperly.Descriptors.Mappings.ExistingTarget;

/// <summary>
/// A default implementation of <see cref="IExistingTargetMapping"/>.
/// </summary>
public abstract class ExistingTargetMapping(ITypeSymbol sourceType, ITypeSymbol targetType) : IExistingTargetMapping
public abstract class ExistingTargetMapping : IExistingTargetMapping
{
public ITypeSymbol SourceType { get; } = sourceType;
protected ExistingTargetMapping(ITypeSymbol sourceType, ITypeSymbol targetType)
{
Debug.Assert(sourceType.IsNullableUpgraded());
Debug.Assert(targetType.IsNullableUpgraded());

public ITypeSymbol TargetType { get; } = targetType;
SourceType = sourceType;
TargetType = targetType;
}

public ITypeSymbol SourceType { get; }

public ITypeSymbol TargetType { get; }

public virtual bool CallableByOtherMappings => true;

Expand Down
2 changes: 1 addition & 1 deletion src/Riok.Mapperly/Descriptors/Mappings/MethodMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ ITypeSymbol targetType
ReferenceHandlerParameter = referenceHandlerParameter;
_partialMethodDefinition = method;
_methodName = method.Name;
_returnType = method.ReturnType.UpgradeNullable();
_returnType = method.ReturnsVoid ? method.ReturnType : targetType;
}

protected bool IsExtensionMethod { get; }
Expand Down
16 changes: 13 additions & 3 deletions src/Riok.Mapperly/Descriptors/Mappings/NewInstanceMapping.cs
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
using System.Diagnostics;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Riok.Mapperly.Helpers;

namespace Riok.Mapperly.Descriptors.Mappings;

/// <inheritdoc cref="INewInstanceMapping"/>
[DebuggerDisplay("{GetType().Name}({SourceType} => {TargetType})")]
public abstract class NewInstanceMapping(ITypeSymbol sourceType, ITypeSymbol targetType) : INewInstanceMapping
public abstract class NewInstanceMapping : INewInstanceMapping
{
public ITypeSymbol SourceType { get; } = sourceType;
protected NewInstanceMapping(ITypeSymbol sourceType, ITypeSymbol targetType)
{
Debug.Assert(sourceType.IsNullableUpgraded());
Debug.Assert(targetType.IsNullableUpgraded());

public ITypeSymbol TargetType { get; } = targetType;
SourceType = sourceType;
TargetType = targetType;
}

public ITypeSymbol SourceType { get; }

public ITypeSymbol TargetType { get; }

public virtual MappingBodyBuildingPriority BodyBuildingPriority => MappingBodyBuildingPriority.Default;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Riok.Mapperly.Emit;
using Riok.Mapperly.Helpers;
using Riok.Mapperly.Symbols;
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
using static Riok.Mapperly.Emit.Syntax.SyntaxFactoryHelper;
Expand All @@ -17,6 +18,7 @@ public class UserDefinedNewInstanceGenericTypeMapping(
IMethodSymbol method,
GenericMappingTypeParameters typeParameters,
MappingMethodParameters parameters,
ITypeSymbol targetType,
bool enableReferenceHandling,
NullFallbackValue nullArm,
ITypeSymbol objectType
Expand All @@ -25,6 +27,7 @@ ITypeSymbol objectType
method,
parameters.Source,
parameters.ReferenceHandler,
targetType,
enableReferenceHandling,
nullArm,
objectType
Expand All @@ -39,7 +42,7 @@ protected override ExpressionSyntax BuildTargetType()
{
// typeof(TTarget) or typeof(<ReturnType>)
var targetTypeName = TypeParameters.TargetType ?? TargetType;
return TypeOfExpression(FullyQualifiedIdentifier(targetTypeName));
return TypeOfExpression(FullyQualifiedIdentifier(targetTypeName.NonNullable()));
}

protected override ExpressionSyntax? BuildSwitchArmWhenClause(ExpressionSyntax targetType, RuntimeTargetTypeMapping mapping)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Riok.Mapperly.Abstractions.ReferenceHandling;
using Riok.Mapperly.Helpers;
using Riok.Mapperly.Symbols;

namespace Riok.Mapperly.Descriptors.Mappings.UserMappings;
Expand All @@ -13,8 +12,9 @@ public class UserDefinedNewInstanceMethodMapping(
IMethodSymbol method,
MethodParameter sourceParameter,
MethodParameter? referenceHandlerParameter,
ITypeSymbol targetType,
bool enableReferenceHandling
) : MethodMapping(method, sourceParameter, referenceHandlerParameter, method.ReturnType.UpgradeNullable()), IDelegateUserMapping
) : MethodMapping(method, sourceParameter, referenceHandlerParameter, targetType), IDelegateUserMapping
{
public IMethodSymbol Method { get; } = method;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ public abstract class UserDefinedNewInstanceRuntimeTargetTypeMapping(
IMethodSymbol method,
MethodParameter sourceParameter,
MethodParameter? referenceHandlerParameter,
ITypeSymbol targetType,
bool enableReferenceHandling,
NullFallbackValue nullArm,
ITypeSymbol objectType
) : MethodMapping(method, sourceParameter, referenceHandlerParameter, method.ReturnType), IUserMapping
) : MethodMapping(method, sourceParameter, referenceHandlerParameter, targetType), IUserMapping
{
private const string IsAssignableFromMethodName = nameof(Type.IsAssignableFrom);
private const string GetTypeMethodName = nameof(GetType);
Expand Down Expand Up @@ -54,21 +55,21 @@ public override IEnumerable<StatementSyntax> BuildBody(TypeMappingBuildContext c
ctx = ctx.WithRefHandler(referenceHandlerName);
}

var targetType = BuildTargetType();
var targetTypeExpr = BuildTargetType();

// _ => throw new ArgumentException(msg, nameof(ctx.Source)),
var sourceType = Invocation(MemberAccess(ctx.Source, GetTypeMethodName));
var fallbackArm = SwitchArm(
DiscardPattern(),
ThrowArgumentExpression(
InterpolatedString($"Cannot map {sourceType} to {targetType} as there is no known type mapping"),
InterpolatedString($"Cannot map {sourceType} to {targetTypeExpr} as there is no known type mapping"),
ctx.Source
)
);

// source switch { A x when targetType.IsAssignableFrom(typeof(ADto)) => MapToADto(x), B x when targetType.IsAssignableFrom(typeof(BDto)) => MapToBDto(x) }
var (typeArmContext, typeArmVariableName) = ctx.WithNewScopedSource();
var arms = _mappings.Select(x => BuildSwitchArm(typeArmContext, typeArmVariableName, x, targetType));
var arms = _mappings.Select(x => BuildSwitchArm(typeArmContext, typeArmVariableName, x, targetTypeExpr));

// null => default / throw
arms = arms.Append(SwitchArm(ConstantPattern(NullLiteral()), NullSubstitute(TargetType, ctx.Source, nullArm)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ public class UserDefinedNewInstanceRuntimeTargetTypeParameterMapping(
IMethodSymbol method,
RuntimeTargetTypeMappingMethodParameters parameters,
bool enableReferenceHandling,
ITypeSymbol targetType,
NullFallbackValue nullArm,
ITypeSymbol objectType
)
: UserDefinedNewInstanceRuntimeTargetTypeMapping(
method,
parameters.Source,
parameters.ReferenceHandler,
targetType,
enableReferenceHandling,
nullArm,
objectType
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Riok.Mapperly.Descriptors.Mappings.ExistingTarget;
using Riok.Mapperly.Helpers;
using Riok.Mapperly.Symbols;
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
using static Riok.Mapperly.Emit.Syntax.SyntaxFactoryHelper;
Expand All @@ -17,7 +16,7 @@ public class UserImplementedExistingTargetMethodMapping(
MethodParameter sourceParameter,
MethodParameter targetParameter,
MethodParameter? referenceHandlerParameter
) : ExistingTargetMapping(method.Parameters[0].Type.UpgradeNullable(), targetParameter.Type.UpgradeNullable()), IUserMapping
) : ExistingTargetMapping(method.Parameters[0].Type, targetParameter.Type), IUserMapping
{
public IMethodSymbol Method { get; } = method;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Riok.Mapperly.Helpers;
using Riok.Mapperly.Symbols;
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
using static Riok.Mapperly.Emit.Syntax.SyntaxFactoryHelper;
Expand All @@ -14,8 +13,9 @@ public class UserImplementedMethodMapping(
string? receiver,
IMethodSymbol method,
MethodParameter sourceParameter,
ITypeSymbol targetType,
MethodParameter? referenceHandlerParameter
) : NewInstanceMapping(method.Parameters[0].Type.UpgradeNullable(), method.ReturnType.UpgradeNullable()), IUserMapping
) : NewInstanceMapping(sourceParameter.Type, targetType), IUserMapping
{
public IMethodSymbol Method { get; } = method;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ public class GenericSourceObjectFactory(SymbolAccessor symbolAccessor, IMethodSy
{
public override bool CanCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate) =>
SymbolEqualityComparer.Default.Equals(Method.ReturnType, targetTypeToCreate)
&& SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(
Method.TypeParameters[0],
sourceType,
Method.Parameters[0].Type.NullableAnnotation
);
&& SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(Method.TypeParameters[0], sourceType);

protected override ExpressionSyntax BuildCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate, ExpressionSyntax source) =>
GenericInvocation(Method.Name, new[] { NonNullableIdentifier(sourceType) }, source);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,8 @@ public class GenericSourceTargetObjectFactory(SymbolAccessor symbolAccessor, IMe
private readonly int _targetTypeParameterIndex = (sourceTypeParameterIndex + 1) % 2;

public override bool CanCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate) =>
SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(
Method.TypeParameters[sourceTypeParameterIndex],
sourceType,
Method.Parameters[0].Type.NullableAnnotation
)
&& SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(
Method.TypeParameters[_targetTypeParameterIndex],
targetTypeToCreate,
Method.ReturnType.NullableAnnotation
);
SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(Method.TypeParameters[sourceTypeParameterIndex], sourceType)
&& SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(Method.TypeParameters[_targetTypeParameterIndex], targetTypeToCreate);

protected override ExpressionSyntax BuildCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate, ExpressionSyntax source)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ namespace Riok.Mapperly.Descriptors.ObjectFactories;
public class GenericTargetObjectFactory(SymbolAccessor symbolAccessor, IMethodSymbol method) : ObjectFactory(symbolAccessor, method)
{
public override bool CanCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate) =>
SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(
Method.TypeParameters[0],
targetTypeToCreate,
Method.ReturnType.NullableAnnotation
);
SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(Method.TypeParameters[0], targetTypeToCreate);

protected override ExpressionSyntax BuildCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate, ExpressionSyntax source) =>
GenericInvocation(Method.Name, new[] { NonNullableIdentifier(targetTypeToCreate) });
Expand Down
Loading