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 internal visibility correctly #597

Merged
merged 1 commit into from
Jul 27, 2023
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 @@ -163,13 +163,14 @@
{
return true;
}

// has valid add if type implements ISet and has implicit Add method
if (
implementedTypes.HasFlag(CollectionType.ISet)
&& t.HasImplicitGenericImplementation(types.Get(typeof(ISet<>)), nameof(ISet<object>.Add))
)
{
return true;

Check warning on line 173 in src/Riok.Mapperly/Descriptors/Enumerables/CollectionInfoBuilder.cs

View check run for this annotation

Codecov / codecov/patch

src/Riok.Mapperly/Descriptors/Enumerables/CollectionInfoBuilder.cs#L172-L173

Added lines #L172 - L173 were not covered by tests
}

return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Riok.Mapperly.Descriptors.Mappings.MemberMappings;
using Riok.Mapperly.Diagnostics;
using Riok.Mapperly.Helpers;
using Riok.Mapperly.Symbols;

namespace Riok.Mapperly.Descriptors.MappingBodyBuilders.BuilderContext;
Expand Down Expand Up @@ -39,7 +38,7 @@ private void AddNullMemberInitializers(IMemberAssignmentMappingContainer contain
{
var nullablePath = new MemberPath(nullableTrailPath);
var type = nullablePath.Member.Type;
if (!type.HasAccessibleParameterlessConstructor())
if (!BuilderContext.SymbolAccessor.HasAccessibleParameterlessConstructor(type))
{
BuilderContext.ReportDiagnostic(DiagnosticDescriptors.NoParameterlessConstructorFound, type);
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ private static void BuildConstructorMapping(INewInstanceBuilderContext<IMapping>
// then by descending parameter count
// ctors annotated with [Obsolete] are considered last unless they have a MapperConstructor attribute set
var ctorCandidates = namedTargetType.InstanceConstructors
.Where(ctor => ctor.IsAccessible())
.Where(ctor => ctx.BuilderContext.SymbolAccessor.IsAccessible(ctor))
.OrderByDescending(x => ctx.BuilderContext.SymbolAccessor.HasAttribute<MapperConstructorAttribute>(x))
.ThenBy(x => ctx.BuilderContext.SymbolAccessor.HasAttribute<MapperConstructorAttribute>(x))
.ThenByDescending(x => x.Parameters.Length == 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,23 +196,17 @@ out sourcePath
if (!ctx.Mapping.SourceType.IsTupleType || ctx.Mapping.SourceType is not INamedTypeSymbol namedType)
return false;

var mappableMember = namedType.TupleElements
.Where(
x =>
x.CorrespondingTupleField != default
&& !ctx.IgnoredSourceMemberNames.Contains(x.Name)
&& string.Equals(field.CorrespondingTupleField!.Name, x.CorrespondingTupleField!.Name)
)
.Select(MappableMember.Create)
.WhereNotNull()
.FirstOrDefault();
var mappableField = namedType.TupleElements.FirstOrDefault(
x =>
x.CorrespondingTupleField != default
&& !ctx.IgnoredSourceMemberNames.Contains(x.Name)
&& string.Equals(field.CorrespondingTupleField!.Name, x.CorrespondingTupleField!.Name)
);

if (mappableMember != default)
{
sourcePath = new MemberPath(new[] { mappableMember });
return true;
}
if (mappableField == default)
return false;

return false;
sourcePath = new MemberPath(new[] { new FieldMember(mappableField) });
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,14 @@ public static void BuildMappingBody(MappingBuilderContext ctx, UserDefinedNewIns
// therefore set source type always to nun-nullable
// as non-nullables are also assignable to nullables.
var mappings = GetUserMappingCandidates(ctx)
.Where(x => mapping.TypeParameters.CanConsumeTypes(ctx.Compilation, x.SourceType.NonNullable(), x.TargetType));
.Where(
x =>
mapping.TypeParameters.DoesTypesSatisfyTypeParameterConstraints(
ctx.SymbolAccessor,
x.SourceType.NonNullable(),
x.TargetType
)
);

BuildMappingBody(ctx, mapping, mappings);
}
Expand All @@ -27,8 +34,8 @@ public static void BuildMappingBody(MappingBuilderContext ctx, UserDefinedNewIns
var mappings = GetUserMappingCandidates(ctx)
.Where(
x =>
x.SourceType.NonNullable().IsAssignableTo(ctx.Compilation, mapping.SourceType)
&& x.TargetType.IsAssignableTo(ctx.Compilation, mapping.TargetType)
ctx.SymbolAccessor.HasImplicitConversion(x.SourceType.NonNullable(), mapping.SourceType)
&& ctx.SymbolAccessor.HasImplicitConversion(x.TargetType, mapping.TargetType)
);

BuildMappingBody(ctx, mapping, mappings);
Expand Down Expand Up @@ -74,7 +81,7 @@ IEnumerable<ITypeMapping> childMappings
.ThenBy(x => x.TargetType.IsNullable())
.GroupBy(x => new TypeMappingKey(x, false))
.Select(x => x.First())
.Select(x => new RuntimeTargetTypeMapping(x, x.TargetType.IsAssignableTo(ctx.Compilation, ctx.Target)));
.Select(x => new RuntimeTargetTypeMapping(x, ctx.Compilation.HasImplicitConversion(x.TargetType, ctx.Target)));
mapping.AddMappings(runtimeTargetTypeMappings);
}
}
2 changes: 1 addition & 1 deletion src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ protected virtual NullFallbackValue GetNullFallbackValue(ITypeSymbol targetType,
if (targetType.SpecialType == SpecialType.System_String)
return NullFallbackValue.EmptyString;

if (targetType.HasAccessibleParameterlessConstructor())
if (SymbolAccessor.HasAccessibleParameterlessConstructor(targetType))
return NullFallbackValue.CreateInstance;

ReportDiagnostic(DiagnosticDescriptors.NoParameterlessConstructorFound, targetType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public static class CtorMappingBuilder

// resolve ctors which have the source as single argument
var ctorMethod = namedTarget.InstanceConstructors
.Where(x => x.IsAccessible())
.Where(ctx.SymbolAccessor.IsAccessible)
.FirstOrDefault(
m =>
m.Parameters.Length == 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ bool duplicatedSourceTypesAllowed
var derivedTypeMappingSourceTypes = new HashSet<ITypeSymbol>(SymbolEqualityComparer.Default);
var derivedTypeMappings = new List<ITypeMapping>(configs.Count);
Func<ITypeSymbol, bool> isAssignableToSource = ctx.Source is ITypeParameterSymbol sourceTypeParameter
? t => sourceTypeParameter.CanConsumeType(ctx.Compilation, ctx.Source.NullableAnnotation, t)
: t => t.IsAssignableTo(ctx.Compilation, ctx.Source);
? t => ctx.SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(sourceTypeParameter, t, ctx.Source.NullableAnnotation)
: t => ctx.SymbolAccessor.HasImplicitConversion(t, ctx.Source);
Func<ITypeSymbol, bool> isAssignableToTarget = ctx.Target is ITypeParameterSymbol targetTypeParameter
? t => targetTypeParameter.CanConsumeType(ctx.Compilation, ctx.Target.NullableAnnotation, t)
: t => t.IsAssignableTo(ctx.Compilation, ctx.Target);
? t => ctx.SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(targetTypeParameter, t, ctx.Target.NullableAnnotation)
: 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 @@ -64,7 +64,7 @@ or CollectionType.IReadOnlyDictionary
// it should have a an object factory or a parameterless public ctor
if (
!ctx.ObjectFactories.TryFindObjectFactory(ctx.Source, ctx.Target, out var objectFactory)
&& !ctx.Target.HasAccessibleParameterlessConstructor()
&& !ctx.SymbolAccessor.HasAccessibleParameterlessConstructor(ctx.Target)
)
{
ctx.ReportDiagnostic(DiagnosticDescriptors.NoParameterlessConstructorFound, ctx.Target);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ ITypeMapping elementMapping
{
if (
!ctx.ObjectFactories.TryFindObjectFactory(ctx.Source, ctx.Target, out var objectFactory)
&& !ctx.Target.HasAccessibleParameterlessConstructor()
&& !ctx.SymbolAccessor.HasAccessibleParameterlessConstructor(ctx.Target)
)
{
ctx.ReportDiagnostic(DiagnosticDescriptors.NoParameterlessConstructorFound, ctx.Target);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static class NewInstanceObjectPropertyMappingBuilder
ctx.MapperConfiguration.UseReferenceHandling
);

if (ctx.Target is not INamedTypeSymbol namedTarget || namedTarget.Constructors.All(x => !x.IsAccessible()))
if (ctx.Target is not INamedTypeSymbol namedTarget || namedTarget.Constructors.All(x => !ctx.SymbolAccessor.IsAccessible(x)))
return null;

if (ctx.Source.IsEnum() || ctx.Target.IsEnum())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using Riok.Mapperly.Descriptors.Mappings.ExistingTarget;
using Riok.Mapperly.Descriptors.ObjectFactories;
using Riok.Mapperly.Diagnostics;
using Riok.Mapperly.Helpers;

namespace Riok.Mapperly.Descriptors.MappingBuilders;

Expand Down Expand Up @@ -130,7 +129,7 @@ ForEachAddEnumerableExistingTargetMapping CreateForEach(string methodName)

if (
!ctx.ObjectFactories.TryFindObjectFactory(ctx.Source, ctx.Target, out var objectFactory)
&& !ctx.Target.HasAccessibleParameterlessConstructor()
&& !ctx.SymbolAccessor.HasAccessibleParameterlessConstructor(ctx.Target)
)
{
return MapSpanArrayToEnumerableMethod(ctx);
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 static Riok.Mapperly.Emit.SyntaxFactoryHelper;

namespace Riok.Mapperly.Descriptors.ObjectFactories;
Expand All @@ -12,17 +11,16 @@ namespace Riok.Mapperly.Descriptors.ObjectFactories;
/// </summary>
public class GenericSourceObjectFactory : ObjectFactory
{
private readonly Compilation _compilation;

public GenericSourceObjectFactory(IMethodSymbol method, Compilation compilation)
: base(method)
{
_compilation = compilation;
}
public GenericSourceObjectFactory(SymbolAccessor symbolAccessor, IMethodSymbol method)
: base(symbolAccessor, method) { }

public override bool CanCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate) =>
SymbolEqualityComparer.Default.Equals(Method.ReturnType, targetTypeToCreate)
&& Method.TypeParameters[0].CanConsumeType(_compilation, Method.Parameters[0].Type.NullableAnnotation, sourceType);
&& SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(
Method.TypeParameters[0],
sourceType,
Method.Parameters[0].Type.NullableAnnotation
);

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
@@ -1,34 +1,31 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Riok.Mapperly.Helpers;
using static Riok.Mapperly.Emit.SyntaxFactoryHelper;

namespace Riok.Mapperly.Descriptors.ObjectFactories;

public class GenericSourceTargetObjectFactory : ObjectFactory
{
private readonly Compilation _compilation;
private readonly int _sourceTypeParameterIndex;
private readonly int _targetTypeParameterIndex;

public GenericSourceTargetObjectFactory(IMethodSymbol method, Compilation compilation, int sourceTypeParameterIndex)
: base(method)
public GenericSourceTargetObjectFactory(SymbolAccessor symbolAccessor, IMethodSymbol method, int sourceTypeParameterIndex)
: base(symbolAccessor, method)
{
_compilation = compilation;
_sourceTypeParameterIndex = sourceTypeParameterIndex;
_targetTypeParameterIndex = (sourceTypeParameterIndex + 1) % 2;
}

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

protected override ExpressionSyntax BuildCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate, ExpressionSyntax source)
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 static Riok.Mapperly.Emit.SyntaxFactoryHelper;

namespace Riok.Mapperly.Descriptors.ObjectFactories;
Expand All @@ -12,16 +11,15 @@ namespace Riok.Mapperly.Descriptors.ObjectFactories;
/// </summary>
public class GenericTargetObjectFactory : ObjectFactory
{
private readonly Compilation _compilation;

public GenericTargetObjectFactory(IMethodSymbol method, Compilation compilation)
: base(method)
{
_compilation = compilation;
}
public GenericTargetObjectFactory(SymbolAccessor symbolAccessor, IMethodSymbol method)
: base(symbolAccessor, method) { }

public override bool CanCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate) =>
Method.TypeParameters[0].CanConsumeType(_compilation, Method.ReturnType.NullableAnnotation, targetTypeToCreate);
SymbolAccessor.DoesTypeSatisfyTypeParameterConstraints(
Method.TypeParameters[0],
targetTypeToCreate,
Method.ReturnType.NullableAnnotation
);

protected override ExpressionSyntax BuildCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate, ExpressionSyntax source) =>
GenericInvocation(Method.Name, new[] { NonNullableIdentifier(targetTypeToCreate) });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ namespace Riok.Mapperly.Descriptors.ObjectFactories;
/// </summary>
public class GenericTargetObjectFactoryWithSource : GenericTargetObjectFactory
{
public GenericTargetObjectFactoryWithSource(IMethodSymbol method, Compilation compilation)
: base(method, compilation) { }
public GenericTargetObjectFactoryWithSource(SymbolAccessor symbolAccessor, IMethodSymbol method)
: base(symbolAccessor, method) { }

public override bool CanCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate) =>
base.CanCreateType(sourceType, targetTypeToCreate) && SymbolEqualityComparer.Default.Equals(Method.Parameters[0].Type, sourceType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@ namespace Riok.Mapperly.Descriptors.ObjectFactories;
/// </summary>
public abstract class ObjectFactory
{
protected ObjectFactory(IMethodSymbol method)
protected ObjectFactory(SymbolAccessor symbolAccessor, IMethodSymbol method)
{
Method = method;
SymbolAccessor = symbolAccessor;
}

protected SymbolAccessor SymbolAccessor { get; }

protected IMethodSymbol Method { get; }

public ExpressionSyntax CreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate, ExpressionSyntax source) =>
Expand All @@ -38,7 +41,7 @@ private ExpressionSyntax HandleNull(ExpressionSyntax expression, ITypeSymbol typ
if (!Method.ReturnType.UpgradeNullable().IsNullable())
return expression;

ExpressionSyntax nullFallback = typeToCreate.HasAccessibleParameterlessConstructor()
ExpressionSyntax nullFallback = SymbolAccessor.HasAccessibleParameterlessConstructor(typeToCreate)
? CreateInstance(typeToCreate)
: ThrowNullReferenceException($"The object factory {Method.Name} returned null");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ public static ObjectFactoryCollection ExtractObjectFactories(SimpleMappingBuilde
if (!methodSymbol.IsGenericMethod)
{
return methodSymbol.Parameters.Length == 1
? new SimpleObjectFactoryWithSource(methodSymbol)
: new SimpleObjectFactory(methodSymbol);
? new SimpleObjectFactoryWithSource(ctx.SymbolAccessor, methodSymbol)
: new SimpleObjectFactory(ctx.SymbolAccessor, methodSymbol);
}

switch (methodSymbol.TypeParameters.Length)
Expand Down Expand Up @@ -76,12 +76,12 @@ public static ObjectFactoryCollection ExtractObjectFactories(SimpleMappingBuilde
if (returnTypeIsGeneric)
{
return hasSourceParameter
? new GenericTargetObjectFactoryWithSource(methodSymbol, ctx.Compilation)
: new GenericTargetObjectFactory(methodSymbol, ctx.Compilation);
? new GenericTargetObjectFactoryWithSource(ctx.SymbolAccessor, methodSymbol)
: new GenericTargetObjectFactory(ctx.SymbolAccessor, methodSymbol);
}

if (hasSourceParameter)
return new GenericSourceObjectFactory(methodSymbol, ctx.Compilation);
return new GenericSourceObjectFactory(ctx.SymbolAccessor, methodSymbol);

ctx.ReportDiagnostic(DiagnosticDescriptors.InvalidObjectFactorySignature, methodSymbol, methodSymbol.Name);
return null;
Expand Down Expand Up @@ -109,6 +109,6 @@ public static ObjectFactoryCollection ExtractObjectFactories(SimpleMappingBuilde
return null;
}

return new GenericSourceTargetObjectFactory(methodSymbol, ctx.Compilation, sourceParameterIndex);
return new GenericSourceTargetObjectFactory(ctx.SymbolAccessor, methodSymbol, sourceParameterIndex);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ namespace Riok.Mapperly.Descriptors.ObjectFactories;
/// </summary>
public class SimpleObjectFactory : ObjectFactory
{
public SimpleObjectFactory(IMethodSymbol method)
: base(method) { }
public SimpleObjectFactory(SymbolAccessor symbolAccessor, IMethodSymbol method)
: base(symbolAccessor, method) { }

public override bool CanCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate) =>
SymbolEqualityComparer.Default.Equals(Method.ReturnType, targetTypeToCreate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ namespace Riok.Mapperly.Descriptors.ObjectFactories;
/// </summary>
public class SimpleObjectFactoryWithSource : SimpleObjectFactory
{
public SimpleObjectFactoryWithSource(IMethodSymbol method)
: base(method) { }
public SimpleObjectFactoryWithSource(SymbolAccessor symbolAccessor, IMethodSymbol method)
: base(symbolAccessor, method) { }

public override bool CanCreateType(ITypeSymbol sourceType, ITypeSymbol targetTypeToCreate) =>
base.CanCreateType(sourceType, targetTypeToCreate) && SymbolEqualityComparer.Default.Equals(sourceType, Method.Parameters[0].Type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ protected SimpleMappingBuilderContext(SimpleMappingBuilderContext ctx)
public MapperAttribute MapperConfiguration => _configuration.Mapper;

public WellKnownTypes Types { get; }

public SymbolAccessor SymbolAccessor { get; }

protected MappingBuilder MappingBuilder { get; }
Expand Down
Loading