fix: discover underlying mappings for nullable types

[mjameson-se]

No description provided.

[BlaiseD]

Not so sure about this. DateTimeOffset should be one of the literal types. Once it is included the test no longer works.

[mjameson-se]

Not so sure about this. DateTimeOffset should be one of the literal types. Once it is included the test no longer works.

Funny you should mention that... Why does that restriction exist? Obviously, making that change would break my newly-working scenario... This is not just an arbitrary test case, the test pretty accurately depicts what I'm trying to do, with Instant actually coming from NodaTime

[BlaiseD]

Not a fan of adding literal types to typeMappings. Is there a chance of unwanted conversions?

[mjameson-se]

Since we're expressly checking below for a user-defined mapping between the two... It seems unlikely

[BlaiseD]

Probably need more tests. I suspect if you're mapping a LINQ select for a class like this one say you want some integers converted but not others then there may be problems.

[BlaiseD]

Not so sure about this. DateTimeOffset should be one of the literal types. Once it is included the test no longer works.

Funny you should mention that... Why does that restriction exist? Obviously, making that change would break my newly-working scenario... This is not just an arbitrary test case, the test pretty accurately depicts what I'm trying to do, with Instant actually coming from NodaTime

For the reason I mentioned in the other comment.

[mjameson-se]

Not so sure about this. DateTimeOffset should be one of the literal types. Once it is included the test no longer works.

Funny you should mention that... Why does that restriction exist? Obviously, making that change would break my newly-working scenario... This is not just an arbitrary test case, the test pretty accurately depicts what I'm trying to do, with Instant actually coming from NodaTime

For the reason I mentioned in the other comment.

Automatically refusing to attempt a mapping between two types on the basis that "your mileage may vary" doesn't really help anyone. It would be helpful if there was a configuration surface I could add to that would customize the behaviour rather than trying to create something that works in all cases.

That said, I am concerned that I'm going to run into further trouble with binary expressions - for example if I'm mapping one property from string to IPAddress (which will likely be rejected today), other strings might get incorrectly translated. It feels like we need to delay translation of the constant value on the RHS until we know the target type on the LHS (or vice versa, as either side could be a constant) instead of relying on an inferred general rule for the mapping... Which of course is going to be much more complicated!

[BlaiseD]

IMappingExpression.ForMember() is the configuration surface - no?

I think this should have been an issue or discussion about "how do I map this expression?" . For your test:

        [Fact]
        public void Can_map_struct_with_asymmetric_nullability()
        {
            // Arrange
            var config = new MapperConfiguration(c =>
            {
                c.CreateMap<ItemWithInstant, ItemWithDateTimeOffset>()
                    .ForMember(d => d.Date, o => o.MapFrom(s => (DateTimeOffset?)DateTimeOffset.FromUnixTimeSeconds(s.Date.SecondsSinceUnixEpoch)));
            });

            config.AssertConfigurationIsValid();
            var mapper = config.CreateMapper();
            DateTimeOffset firstReleaseDate = DateTimeOffset.FromUnixTimeSeconds(0);
            DateTimeOffset lastReleaseDate = DateTimeOffset.FromUnixTimeSeconds(10);
            Expression<Func<ItemWithDateTimeOffset, bool>> exp = x => x.Date.HasValue && (x.Date >= firstReleaseDate && x.Date <= lastReleaseDate);

            //Act
            Expression<Func<ItemWithInstant, bool>> expMapped = mapper.MapExpression<Expression<Func<ItemWithInstant, bool>>>(exp);

            //Assert
            Assert.NotNull(expMapped);
            Assert.True(expMapped.Compile()(new ItemWithInstant { Date = new Instant { SecondsSinceUnixEpoch = 7 } }));
            Assert.False(expMapped.Compile()(new ItemWithInstant { Date = new Instant { SecondsSinceUnixEpoch = 12 } }));
        }

ConvertUsing is not supported for expression mapping - only ForMember, ForPath and the property maps are supported.

[mjameson-se]

IMappingExpression.ForMember() is the configuration surface - no?

I think this should have been an issue or discussion about "how do I map this expression?" . For your test:

        [Fact]
        public void Can_map_struct_with_asymmetric_nullability()
        {
            // Arrange
            var config = new MapperConfiguration(c =>
            {
                c.CreateMap<ItemWithInstant, ItemWithDateTimeOffset>()
                    .ForMember(d => d.Date, o => o.MapFrom(s => (DateTimeOffset?)DateTimeOffset.FromUnixTimeSeconds(s.Date.SecondsSinceUnixEpoch)));
            });

            config.AssertConfigurationIsValid();
            var mapper = config.CreateMapper();
            DateTimeOffset firstReleaseDate = DateTimeOffset.FromUnixTimeSeconds(0);
            DateTimeOffset lastReleaseDate = DateTimeOffset.FromUnixTimeSeconds(10);
            Expression<Func<ItemWithDateTimeOffset, bool>> exp = x => x.Date.HasValue && (x.Date >= firstReleaseDate && x.Date <= lastReleaseDate);

            //Act
            Expression<Func<ItemWithInstant, bool>> expMapped = mapper.MapExpression<Expression<Func<ItemWithInstant, bool>>>(exp);

            //Assert
            Assert.NotNull(expMapped);
            Assert.True(expMapped.Compile()(new ItemWithInstant { Date = new Instant { SecondsSinceUnixEpoch = 7 } }));
            Assert.False(expMapped.Compile()(new ItemWithInstant { Date = new Instant { SecondsSinceUnixEpoch = 12 } }));
        }

ConvertUsing is not supported for expression mapping - only ForMember, ForPath and the property maps are supported.

ConvertUsing does work correctly if there isn't this issue with nullability. Explicitly mapping each member is not ideal when my API requires hundreds of these conversions

[BlaiseD]

That's incorrect on ConvertUsing. The expression mapper visitor calls IMapper.Map when there are constants in the expression e.g. here in which case all types of configurations apply. In general do NOT expect ConvertUsing to work for mapping expressions.

Every library can be improved but we've discussed the problems with this approach.