From af57e004da739589c2ff79576ce50e8e4b7d07b0 Mon Sep 17 00:00:00 2001 From: Blaise Taylor Date: Fri, 20 Feb 2026 16:43:53 -0500 Subject: [PATCH] Code QL Notes --- .../TypeMappingsManager.cs | 14 +++------- .../XpressionMapperVisitor.cs | 8 +++--- .../ExpressionMapping.cs | 10 +++---- .../Impl/SourceInjectedQuery.cs | 1 + .../TypeExtensionsTest.cs | 4 +-- .../TypeMappingsManagerTest.cs | 27 +++++++------------ .../XpressionMapper.Structs.Tests.cs | 8 +++--- .../XpressionMapperTests.cs | 8 +++--- 8 files changed, 32 insertions(+), 48 deletions(-) diff --git a/src/AutoMapper.Extensions.ExpressionMapping/TypeMappingsManager.cs b/src/AutoMapper.Extensions.ExpressionMapping/TypeMappingsManager.cs index ca2759b..4db7d4e 100644 --- a/src/AutoMapper.Extensions.ExpressionMapping/TypeMappingsManager.cs +++ b/src/AutoMapper.Extensions.ExpressionMapping/TypeMappingsManager.cs @@ -89,17 +89,14 @@ public Type ReplaceType(Type sourceType) } else if (sourceType.IsGenericType) { - if (TypeMappings.TryGetValue(sourceType, out Type destType)) - return destType; - else - { - return sourceType.GetGenericTypeDefinition().MakeGenericType + return TypeMappings.TryGetValue(sourceType, out Type destType) + ? destType + : sourceType.GetGenericTypeDefinition().MakeGenericType ( [.. sourceType .GetGenericArguments() .Select(ReplaceType)] ); - } } else { @@ -182,11 +179,8 @@ private void FindChildPropertyTypeMaps(Type source, Type dest) FindMaps([.. typeMap.PropertyMaps]); void FindMaps(List maps) { - foreach (PropertyMap pm in maps) + foreach (PropertyMap pm in maps.Where(p => p.SourceMembers.Length > 0 || p.CustomMapExpression != null)) { - if (pm.SourceMembers.Length < 1 && pm.CustomMapExpression == null) - continue; - AddChildMappings ( source.GetFieldOrProperty(pm.DestinationMember.Name).GetMemberType(), diff --git a/src/AutoMapper.Extensions.ExpressionMapping/XpressionMapperVisitor.cs b/src/AutoMapper.Extensions.ExpressionMapping/XpressionMapperVisitor.cs index 09bbc65..672f60f 100644 --- a/src/AutoMapper.Extensions.ExpressionMapping/XpressionMapperVisitor.cs +++ b/src/AutoMapper.Extensions.ExpressionMapping/XpressionMapperVisitor.cs @@ -3,7 +3,6 @@ using AutoMapper.Extensions.ExpressionMapping.Structures; using AutoMapper.Internal; using System; -using System.Collections; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -440,10 +439,9 @@ Expression DoVisitBinary(Expression newLeft, Expression newRight, Expression con { if (newLeft != node.Left || newRight != node.Right || conversion != node.Conversion) { - if (node.NodeType == ExpressionType.Coalesce && node.Conversion != null) - return Expression.Coalesce(newLeft, newRight, conversion as LambdaExpression); - else - return Expression.MakeBinary + return node.NodeType == ExpressionType.Coalesce && node.Conversion != null + ? Expression.Coalesce(newLeft, newRight, conversion as LambdaExpression) + : Expression.MakeBinary ( node.NodeType, newLeft, diff --git a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/ExpressionMapping.cs b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/ExpressionMapping.cs index 6a562d9..d72b7df 100644 --- a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/ExpressionMapping.cs +++ b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/ExpressionMapping.cs @@ -140,11 +140,11 @@ private void Should_Validate() [Fact] public void GrandParent_Mapping_To_Sub_Sub_Property_Condition() { - Expression> _predicateExpression = gp => gp.Parent.Children.Any(c => c.ID2 == 3); - var expression = Mapper.Map>>(_predicateExpression); + Expression> predicateExpression = gp => gp.Parent.Children.Any(c => c.ID2 == 3); + var expression = Mapper.Map>>(predicateExpression); var items = new[] {new GrandParent(){Parent = new Parent(){Children = [new Child(){ID2 = 3}], Child = new Child(){ID2 = 3}}}}.AsQueryable(); items.Where(expression).ShouldContain(items.First()); - var items2 = items.UseAsDataSource(Mapper).For().Where(_predicateExpression); + var items2 = items.UseAsDataSource(Mapper).For().Where(predicateExpression); items2.Count().ShouldBe(1); When_Use_Outside_Class_Method_Call(); } @@ -152,9 +152,9 @@ public void GrandParent_Mapping_To_Sub_Sub_Property_Condition() [Fact] public void GrandParent_Mapping_To_Sub_Sub_Property_Condition2() { - Expression, bool>> _predicateExpression = gps => gps.Any(gp => gp.Parent.Children.Any(c => c.ID_ == 3)); + Expression, bool>> predicateExpression = gps => gps.Any(gp => gp.Parent.Children.Any(c => c.ID_ == 3)); Expression, IQueryable>> _predicateExpression2 = gps => gps.Where(gp => gp.Parent.Children.Any(c => c.ID_ == 3)); - var expression = Mapper.Map, bool>>>(_predicateExpression); + var expression = Mapper.Map, bool>>>(predicateExpression); var expression1 = Mapper.Map, IQueryable>>>(_predicateExpression2); expression.ShouldNotBeNull(); expression1.ShouldNotBeNull(); diff --git a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/Impl/SourceInjectedQuery.cs b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/Impl/SourceInjectedQuery.cs index 94aae4f..be46d4a 100644 --- a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/Impl/SourceInjectedQuery.cs +++ b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/Impl/SourceInjectedQuery.cs @@ -82,6 +82,7 @@ public void Shoud_support_IEnumerable_result() .Where(s => s.DestValue > 6); List list = [.. result]; + Assert.NotEmpty(list); } [Fact] diff --git a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/TypeExtensionsTest.cs b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/TypeExtensionsTest.cs index 35c0c55..8fbed8e 100644 --- a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/TypeExtensionsTest.cs +++ b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/TypeExtensionsTest.cs @@ -207,8 +207,8 @@ public void GetDeclaredProperties_ReturnsAllDeclaredProperties() private class ClassWithStaticMembers { - public static int StaticField = 0; - public int InstanceField = 0; + public static readonly int StaticField = 0; + public readonly int InstanceField = 0; public static int StaticProperty { get; set; } public int InstanceProperty { get; set; } public static void StaticMethod() { } diff --git a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/TypeMappingsManagerTest.cs b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/TypeMappingsManagerTest.cs index 48b4b61..bcf842e 100644 --- a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/TypeMappingsManagerTest.cs +++ b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/TypeMappingsManagerTest.cs @@ -1,8 +1,6 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Linq.Expressions; -using AutoMapper; using Xunit; namespace AutoMapper.Extensions.ExpressionMapping.UnitTests @@ -118,8 +116,7 @@ public void Constructor_AddsMappingsFromDelegateTypes() typeof(Func)); // Assert - Assert.True(manager.TypeMappings.ContainsKey(typeof(SourceModel))); - Assert.Equal(typeof(DestModel), manager.TypeMappings[typeof(SourceModel)]); + Assert.Contains(new KeyValuePair(typeof(SourceModel), typeof(DestModel)), manager.TypeMappings); } #endregion @@ -143,8 +140,7 @@ public void AddTypeMapping_SimpleTypes_AddsMapping() manager.AddTypeMapping(typeof(SourceChild), typeof(DestChild)); // Assert - Assert.True(manager.TypeMappings.ContainsKey(typeof(SourceChild))); - Assert.Equal(typeof(DestChild), manager.TypeMappings[typeof(SourceChild)]); + Assert.Contains(new KeyValuePair(typeof(SourceChild), typeof(DestChild)), manager.TypeMappings); } [Fact] @@ -187,8 +183,7 @@ public void AddTypeMapping_ExpressionTypes_UnwrapsAndAddsMapping() typeof(Expression>)); // Assert - Assert.True(manager.TypeMappings.ContainsKey(typeof(Func))); - Assert.Equal(typeof(Func), manager.TypeMappings[typeof(Func)]); + Assert.Contains(new KeyValuePair(typeof(Func), typeof(Func)), manager.TypeMappings); } [Fact] @@ -231,9 +226,8 @@ public void AddTypeMapping_ListTypes_AddsUnderlyingTypeMappings() manager.AddTypeMapping(typeof(List), typeof(List)); // Assert - Assert.True(manager.TypeMappings.ContainsKey(typeof(List))); - Assert.True(manager.TypeMappings.ContainsKey(typeof(SourceItem))); - Assert.Equal(typeof(DestItem), manager.TypeMappings[typeof(SourceItem)]); + Assert.Contains(new KeyValuePair(typeof(SourceItem), typeof(DestItem)), manager.TypeMappings); + Assert.Contains(new KeyValuePair(typeof(List), typeof(List)), manager.TypeMappings); } [Fact] @@ -254,9 +248,8 @@ public void AddTypeMapping_ArrayTypes_AddsUnderlyingTypeMappings() manager.AddTypeMapping(typeof(SourceItem[]), typeof(DestItem[])); // Assert - Assert.True(manager.TypeMappings.ContainsKey(typeof(SourceItem[]))); - Assert.True(manager.TypeMappings.ContainsKey(typeof(SourceItem))); - Assert.Equal(typeof(DestItem), manager.TypeMappings[typeof(SourceItem)]); + Assert.Contains(new KeyValuePair(typeof(SourceItem), typeof(DestItem)), manager.TypeMappings); + Assert.Contains(new KeyValuePair(typeof(SourceItem[]), typeof(DestItem[])), manager.TypeMappings); } #endregion @@ -589,8 +582,7 @@ public void AddTypeMappingsFromDelegates_MatchingArgumentCounts_AddsMappings() typeof(Func)); // Assert - Assert.True(manager.TypeMappings.ContainsKey(typeof(SourceChild))); - Assert.Equal(typeof(DestChild), manager.TypeMappings[typeof(SourceChild)]); + Assert.Contains(new KeyValuePair(typeof(SourceChild), typeof(DestChild)), manager.TypeMappings); } [Fact] @@ -633,8 +625,7 @@ public void AddTypeMappingsFromDelegates_ActionTypes_AddsMappings() typeof(Action)); // Assert - Assert.True(manager.TypeMappings.ContainsKey(typeof(SourceChild))); - Assert.Equal(typeof(DestChild), manager.TypeMappings[typeof(SourceChild)]); + Assert.Contains(new KeyValuePair(typeof(SourceChild), typeof(DestChild)), manager.TypeMappings); } #endregion diff --git a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/XpressionMapper.Structs.Tests.cs b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/XpressionMapper.Structs.Tests.cs index 3787667..8e6a743 100644 --- a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/XpressionMapper.Structs.Tests.cs +++ b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/XpressionMapper.Structs.Tests.cs @@ -301,7 +301,7 @@ public static implicit operator Source(int i) public override readonly int GetHashCode() { - return this.val.GetHashCode(); + return this.val; } public override readonly bool Equals(object obj) @@ -370,7 +370,7 @@ public static implicit operator Dest(int i) public override readonly int GetHashCode() { - return this.val.GetHashCode(); + return this.val; } public override readonly bool Equals(object obj) @@ -417,7 +417,7 @@ public override readonly bool Equals(object obj) } public override readonly int GetHashCode() { - return Year.GetHashCode(); + return Year; } } @@ -444,7 +444,7 @@ public override readonly bool Equals(object obj) } public override readonly int GetHashCode() { - return Year.GetHashCode(); + return Year; } } diff --git a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/XpressionMapperTests.cs b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/XpressionMapperTests.cs index 63230e9..ec62519 100644 --- a/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/XpressionMapperTests.cs +++ b/tests/AutoMapper.Extensions.ExpressionMapping.UnitTests/XpressionMapperTests.cs @@ -416,10 +416,10 @@ public void Map_where_multiple_arguments() //Act Expression> selectionMapped = mapper.Map>>(selection); - object val = selectionMapped.Compile().Invoke(Users.ToList()[0], Users.ToList()[0].Account); + object valLocal = selectionMapped.Compile().Invoke(Users.ToList()[0], Users.ToList()[0].Account); //Assert - Assert.False((bool)val); + Assert.False((bool)valLocal); } [Fact] @@ -838,9 +838,9 @@ public void Can_map_expressions_with_action_independent_of_expression_param() } object val; - void CallSomeAction(T val) + void CallSomeAction(T valParam) { - this.val = val; + this.val = valParam; } [Fact]