Fixing use of blocks in figuring out if variable declarations and assignments can be joined

Author: SteveWilkes

ReadableExpressions.UnitTests/WhenFormattingCode.cs

@@ -2,6 +2,7 @@
 {
     using System;
     using System.Collections.Generic;
+    using System.IO;
     using System.Linq;
     using System.Linq.Expressions;
     using System.Reflection;
@@ -350,6 +351,106 @@ public void ShouldNotVarAssignAVariableAssignedInATryButUsedInACatch()
             Assert.AreEqual(EXPECTED.TrimStart(), translated);
         }
 
+        [TestMethod]
+        public void ShouldVarAssignAVariableUsedInNestedConstructs()
+        {
+            var returnLabel = Expression.Label(typeof(long), "Return");
+            var streamVariable = Expression.Variable(typeof(Stream), "stream");
+
+            var memoryStreamVariable = Expression.Variable(typeof(MemoryStream), "memoryStream");
+            var streamAsMemoryStream = Expression.TypeAs(streamVariable, typeof(MemoryStream));
+            var memoryStreamAssignment = Expression.Assign(memoryStreamVariable, streamAsMemoryStream);
+            var nullMemoryStream = Expression.Default(memoryStreamVariable.Type);
+            var memoryStreamNotNull = Expression.NotEqual(memoryStreamVariable, nullMemoryStream);
+            var msLengthVariable = Expression.Variable(typeof(long), "msLength");
+            var memoryStreamLength = Expression.Property(memoryStreamVariable, "Length");
+            var msLengthAssignment = Expression.Assign(msLengthVariable, memoryStreamLength);
+
+            var msTryBlock = Expression.Block(new[] { msLengthVariable }, msLengthAssignment, msLengthVariable);
+            var newNotSupportedException = Expression.New(typeof(NotSupportedException));
+            var throwMsException = Expression.Throw(newNotSupportedException, typeof(long));
+            var msCatchBlock = Expression.Catch(typeof(Exception), throwMsException);
+            var memoryStreamTryCatch = Expression.TryCatch(msTryBlock, msCatchBlock);
+            var returnMemoryStreamResult = Expression.Return(returnLabel, memoryStreamTryCatch);
+            var ifMemoryStreamTryCatch = Expression.IfThen(memoryStreamNotNull, returnMemoryStreamResult);
+
+            var fileStreamVariable = Expression.Variable(typeof(FileStream), "fileStream");
+            var streamAsFileStream = Expression.TypeAs(streamVariable, typeof(FileStream));
+            var fileStreamAssignment = Expression.Assign(fileStreamVariable, streamAsFileStream);
+            var nullFileStream = Expression.Default(fileStreamVariable.Type);
+            var fileStreamNotNull = Expression.NotEqual(fileStreamVariable, nullFileStream);
+            var fsLengthVariable = Expression.Variable(typeof(long), "fsLength");
+            var fileStreamLength = Expression.Property(fileStreamVariable, "Length");
+            var fsLengthAssignment = Expression.Assign(fsLengthVariable, fileStreamLength);
+
+            var fsTryBlock = Expression.Block(new[] { fsLengthVariable }, fsLengthAssignment, fsLengthVariable);
+            var newIoException = Expression.New(typeof(IOException));
+            var throwIoException = Expression.Throw(newIoException, typeof(long));
+            var fsCatchBlock = Expression.Catch(typeof(Exception), throwIoException);
+            var fileStreamTryCatch = Expression.TryCatch(fsTryBlock, fsCatchBlock);
+            var returnFileStreamResult = Expression.Return(returnLabel, fileStreamTryCatch);
+            var ifFileStreamTryCatch = Expression.IfThen(fileStreamNotNull, returnFileStreamResult);
+
+            var overallBlock = Expression.Block(
+                new[] { memoryStreamVariable, fileStreamVariable },
+                memoryStreamAssignment,
+                ifMemoryStreamTryCatch,
+                fileStreamAssignment,
+                ifFileStreamTryCatch,
+                Expression.Label(returnLabel, Expression.Constant(0L)));
+
+            var overallCatchBlock = Expression.Catch(typeof(Exception), Expression.Constant(-1L));
+            var overallTryCatch = Expression.TryCatch(overallBlock, overallCatchBlock);
+
+            const string EXPECTED = @"
+try
+{
+    var memoryStream = stream as MemoryStream;
+    if (memoryStream != null)
+    {
+        return 
+        {
+            try
+            {
+                var msLength = memoryStream.Length;
+                return msLength;
+            }
+            catch
+            {
+                throw new NotSupportedException();
+            }
+        }
+    }
+
+    var fileStream = stream as FileStream;
+    if (fileStream != null)
+    {
+        return 
+        {
+            try
+            {
+                var fsLength = fileStream.Length;
+                return fsLength;
+            }
+            catch
+            {
+                throw new IOException();
+            }
+        }
+    }
+
+    return 0L;
+}
+catch
+{
+    return -1L;
+}";
+
+            var translated = overallTryCatch.ToReadableString();
+
+            Assert.AreEqual(EXPECTED.TrimStart(), translated);
+        }
+
         [TestMethod]
         public void ShouldTranslateAnExtensionExpressionType()
         {

ReadableExpressions/TranslationContext.cs

@@ -145,7 +145,6 @@ private class ExpressionAnalysisVisitor : ExpressionVisitor
             private readonly List<ParameterExpression> _accessedVariables;
             private readonly List<Expression> _assignedAssignments;
             private readonly Stack<object> _constructs;
-            private BlockExpression _currentBlock;
 
             private ExpressionAnalysisVisitor()
             {
@@ -157,7 +156,6 @@ private ExpressionAnalysisVisitor()
                 NamedLabelTargets = new List<LabelTarget>();
                 ChainedMethodCalls = new List<MethodCallExpression>();
                 _constructs = new Stack<object>();
-                _currentBlock = null;
             }
 
             #region Factory Method
@@ -208,48 +206,45 @@ private static Expression GetCoreExpression(Expression expression)
 
             public List<MethodCallExpression> ChainedMethodCalls { get; }
 
-            protected override Expression VisitBlock(BlockExpression block)
-            {
-                _currentBlock = block;
-
-                return base.VisitBlock(block);
-            }
-
             protected override Expression VisitParameter(ParameterExpression variable)
             {
                 if (VariableHasNotYetBeenAccessed(variable))
                 {
                     _accessedVariables.Add(variable);
                 }
-                else if (JoinedAssignedVariables.Contains(variable))
+
+                if (!JoinedAssignedVariables.Contains(variable))
                 {
-                    var joinedAssignmentData = _constructsByAssignment
-                        .Where(kvp => kvp.Key.Left == variable)
-                        .Select(kvp => new
-                        {
-                            Assignment = kvp.Key,
-                            Construct = kvp.Value
-                        })
-                        .FirstOrDefault();
+                    return base.VisitParameter(variable);
+                }
 
-                    if ((joinedAssignmentData != null) && !_constructs.Contains(joinedAssignmentData.Construct))
+                var joinedAssignmentData = _constructsByAssignment
+                    .Where(kvp => kvp.Key.Left == variable)
+                    .Select(kvp => new
                     {
-                        // This variable was assigned within a construct but is being accessed 
-                        // outside of that scope, so the assignment shouldn't be joined:
-                        JoinedAssignedVariables.Remove(variable);
-                        JoinedAssignments.Remove(joinedAssignmentData.Assignment);
-                        _constructsByAssignment.Remove(joinedAssignmentData.Assignment);
-                    }
+                        Assignment = kvp.Key,
+                        Construct = kvp.Value
+                    })
+                    .FirstOrDefault();
+
+                if ((joinedAssignmentData == null) || _constructs.Contains(joinedAssignmentData.Construct))
+                {
+                    return base.VisitParameter(variable);
                 }
 
+                // This variable was assigned within a construct but is being accessed 
+                // outside of that scope, so the assignment shouldn't be joined:
+                JoinedAssignedVariables.Remove(variable);
+                JoinedAssignments.Remove(joinedAssignmentData.Assignment);
+                _constructsByAssignment.Remove(joinedAssignmentData.Assignment);
+
                 return base.VisitParameter(variable);
             }
 
             protected override Expression VisitBinary(BinaryExpression binary)
             {
                 if ((binary.NodeType == ExpressionType.Assign) &&
                     (binary.Left.NodeType == ExpressionType.Parameter) &&
-                    ((_currentBlock == null) || _currentBlock.Expressions.Contains(binary)) &&
                     !JoinedAssignedVariables.Contains(binary.Left) &&
                     !_assignedAssignments.Contains(binary))
                 {