Update Aggregate{,Tree} to use parameter names from the first expression

Updated Aggregate{,Tree} to use the parameter names from the first provided input expression when available, as the provided seed/fallback expression is sometimes auto-generated and has meaningless parameter names.
jcracknell · Nov 14, 2024 · eddd8c1 · eddd8c1
1 parent 2c26f94
commit eddd8c1
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 12 deletions.
diff --git a/src/Arborist/src/ExpressionHelper.Aggregate.cs b/src/Arborist/src/ExpressionHelper.Aggregate.cs
@@ -137,6 +137,7 @@ AggregateOptions options
         if(!enumerator.MoveNext())
             return seed;
 
+        var parameters = enumerator.Current.Parameters;
         var replacements = new Dictionary<Expression, Expression>();
         var replacingVisitor = new ReplacingExpressionVisitor(replacements);
         var body = seed.Body;
@@ -147,7 +148,7 @@ AggregateOptions options
             var current = enumerator.Current;
 
             replacements.Clear();
-            foreach(var (ps, pr) in current.Parameters.Zip(seed.Parameters))
+            foreach(var (ps, pr) in current.Parameters.Zip(parameters))
                 replacements[ps] = pr;
 
             var right = replacingVisitor.Visit(current.Body);
@@ -160,13 +161,13 @@ AggregateOptions options
             }
 
             replacements.Clear();
-            replacements[binaryOperator.Parameters[0]] = body;
-            replacements[binaryOperator.Parameters[1]] = right;
+            replacements[op0] = body;
+            replacements[op1] = right;
 
             body = replacingVisitor.Visit(binaryOperator.Body);
         } while(enumerator.MoveNext());
 
-        return Expression.Lambda(body, seed.Parameters);
+        return Expression.Lambda(body, parameters);
     }
 }
 
diff --git a/src/Arborist/src/ExpressionHelper.AggregateTree.cs b/src/Arborist/src/ExpressionHelper.AggregateTree.cs
@@ -122,12 +122,13 @@ LambdaExpression binaryOperator
             case 1: return expressionList[0];
         }
 
+        var parameters = expressionList[0].Parameters;
         var replacements = new Dictionary<Expression, Expression>();
         var replacementVisitor = new ReplacingExpressionVisitor(replacements);
 
         return Expression.Lambda(
-            AggregateTreeBody(expressionList, binaryOperator, 0, expressionList.Count, fallback.Parameters, replacements, replacementVisitor),
-            fallback.Parameters
+            AggregateTreeBody(expressionList, binaryOperator, 0, expressionList.Count, parameters, replacements, replacementVisitor),
+            parameters
         );
     }
 

diff --git a/src/Arborist/test/ExpressionHelperTests.Aggregate.cs b/src/Arborist/test/ExpressionHelperTests.Aggregate.cs
@@ -35,7 +35,7 @@ public void Aggregate1_should_work_as_expected() {
 
     [Fact]
     public void Aggregate2_should_work_as_expected() {
-        var expected = ExpressionOn<Cat, Owner>.Of((p0, p1) => "" + p0.Name + p1.Name);
+        var expected = ExpressionOn<Cat, Owner>.Of((c, o) => "" + c.Name + o.Name);
         var actual = ExpressionHelper.Aggregate(
             expressions: new[] {
                 ExpressionOn<Cat, Owner>.Of((c, o) => c.Name),
@@ -50,7 +50,7 @@ public void Aggregate2_should_work_as_expected() {
 
     [Fact]
     public void Aggregate3_should_work_as_expected() {
-        var expected = ExpressionOn<Cat, Owner, int>.Of((p0, p1, p2) => "" + p0.Name + p1.Name + p2.ToString());
+        var expected = ExpressionOn<Cat, Owner, int>.Of((c, o, i) => "" + c.Name + o.Name + i.ToString());
         var actual = ExpressionHelper.Aggregate(
             expressions: new[] {
                 ExpressionOn<Cat, Owner, int>.Of((c, o, i) => c.Name),
@@ -66,7 +66,7 @@ public void Aggregate3_should_work_as_expected() {
 
     [Fact]
     public void Aggregate4_should_work_as_expected() {
-        var expected = ExpressionOn<Cat, Owner, int, string>.Of((p0, p1, p2, p3) => "" + p0.Name + p1.Name + p2.ToString() + p3.ToLowerInvariant());
+        var expected = ExpressionOn<Cat, Owner, int, string>.Of((c, o, i, s) => "" + c.Name + o.Name + i.ToString() + s.ToLowerInvariant());
         var actual = ExpressionHelper.Aggregate(
             expressions: new[] {
                 ExpressionOn<Cat, Owner, int, string>.Of((c, o, i, s) => c.Name),

diff --git a/src/Arborist/test/ExpressionHelperTests.AggregateTree.cs b/src/Arborist/test/ExpressionHelperTests.AggregateTree.cs
@@ -35,7 +35,7 @@ public void AggregateTree1_should_work_as_expected() {
 
     [Fact]
     public void AggregateTree2_should_work_as_expected() {
-        var expected = ExpressionOn<Cat, Owner>.Of((p0, p1) => p0.Name + p1.Name);
+        var expected = ExpressionOn<Cat, Owner>.Of((c, o) => c.Name + o.Name);
         var actual = ExpressionHelper.AggregateTree(
             expressions: new[] {
                 ExpressionOn<Cat, Owner>.Of((c, o) => c.Name),
@@ -50,7 +50,7 @@ public void AggregateTree2_should_work_as_expected() {
 
     [Fact]
     public void AggregateTree3_should_work_as_expected() {
-        var expected = ExpressionOn<Cat, Owner, int>.Of((p0, p1, p2) => (p0.Name + p1.Name) + p2.ToString());
+        var expected = ExpressionOn<Cat, Owner, int>.Of((c, o, i) => (c.Name + o.Name) + i.ToString());
         var actual = ExpressionHelper.AggregateTree(
             expressions: new[] {
                 ExpressionOn<Cat, Owner, int>.Of((c, o, i) => c.Name),
@@ -66,7 +66,7 @@ public void AggregateTree3_should_work_as_expected() {
 
     [Fact]
     public void AggregateTree4_should_work_as_expected() {
-        var expected = ExpressionOn<Cat, Owner, int, string>.Of((p0, p1, p2, p3) => (p0.Name + p1.Name) + (p2.ToString() + p3.ToLowerInvariant()));
+        var expected = ExpressionOn<Cat, Owner, int, string>.Of((c, o, i, s) => (c.Name + o.Name) + (i.ToString() + s.ToLowerInvariant()));
         var actual = ExpressionHelper.AggregateTree(
             expressions: new[] {
                 ExpressionOn<Cat, Owner, int, string>.Of((c, o, i, s) => c.Name),