Translating Goto statements which go to the final statement in a block into return statements

Author: SteveWilkes

NextVersion.txt

@@ -1,11 +0,0 @@
-Goto return label:
-
-if (products != Enumerable<Product>.Empty)
-                {
-                    goto Return;
-                }
-
-Select on Constant DbSet<Person>:
-
-Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Phoneta.Domain.People.Person]
-    .Select(

ReadableExpressions.UnitTests/WhenTranslatingGotos.cs

@@ -92,6 +92,37 @@ public void ShouldUnindentGotoTargetLabels()
             Assert.AreEqual(EXPECTED.TrimStart(), translated);
         }
 
+        [TestMethod]
+        public void ShouldTranslateAGotoReturnStatement()
+        {
+            var returnTarget = Expression.Label(typeof(int), "Return");
+
+            var numberParameter = Expression.Parameter(typeof(string), "i");
+            var numberEqualsOne = Expression.Equal(numberParameter, Expression.Constant("One"));
+            var returnOne = Expression.Goto(returnTarget, Expression.Constant(1));
+            var ifOneReturnOne = Expression.IfThen(numberEqualsOne, returnOne);
+
+            var returnLabel = Expression.Label(returnTarget, Expression.Constant(0));
+            var gotoBlock = Expression.Block(ifOneReturnOne, returnLabel);
+
+            var gotoLambda = Expression.Lambda<Func<string, int>>(gotoBlock, numberParameter);
+            gotoLambda.Compile();
+
+            var translated = gotoLambda.ToReadableString();
+
+            const string EXPECTED = @"
+i =>
+{
+    if (i == ""One"")
+    {
+        return 1;
+    }
+
+    return 0;
+}";
+            Assert.AreEqual(EXPECTED.TrimStart(), translated);
+        }
+
         [TestMethod]
         public void ShouldTranslateAReturnStatementWithAValue()
         {
@@ -228,7 +259,6 @@ public void ShouldTranslateALabelWithABlockDefaultValue()
 
     return i;
 };";
-
             var translated = returnBlock.ToReadableString();
 
             Assert.AreEqual(EXPECTED.TrimStart(), translated);

ReadableExpressions/TranslationContext.cs

@@ -71,10 +71,7 @@ private static TranslationSettings GetTranslationSettings(
         /// </summary>
         /// <param name="expression">The <see cref="Expression"/> to translate.</param>
         /// <returns>A source code translation of the given <paramref name="expression"/>.</returns>
-        public string Translate(Expression expression)
-        {
-            return _globalTranslator.Invoke(expression, this);
-        }
+        public string Translate(Expression expression) => _globalTranslator.Invoke(expression, this);
 
         internal string TranslateAsCodeBlock(Expression expression)
         {
@@ -130,7 +127,7 @@ internal ParameterSet TranslateParameters(
         public bool IsNotJoinedAssignment(Expression expression)
         {
             return (expression.NodeType != ExpressionType.Assign) ||
-                !_analyzer.JoinedAssignments.Contains(expression);
+                  !_analyzer.JoinedAssignments.Contains(expression);
         }
 
         /// <summary>
@@ -143,9 +140,19 @@ public bool IsNotJoinedAssignment(Expression expression)
         /// otherwise false.
         /// </returns>
         public bool IsReferencedByGoto(LabelTarget labelTarget)
-        {
-            return _analyzer.NamedLabelTargets.Contains(labelTarget);
-        }
+            => _analyzer.NamedLabelTargets.Contains(labelTarget);
+
+        /// <summary>
+        /// Returns a value indicating whether the given <paramref name="goto"/> goes to the 
+        /// final statement in a block, and so should be rendered as a return statement.
+        /// </summary>
+        /// <param name="goto">The GotoExpression for which to make the determination.</param>
+        /// <returns>
+        /// True if the given <paramref name="goto"/> goes to the final statement in a block,
+        /// otherwise false.
+        /// </returns>
+        public bool GoesToReturnLabel(GotoExpression @goto)
+            => _analyzer.GotoReturnGotos.Contains(@goto);
 
         /// <summary>
         /// Returns a value indicating whether the given <paramref name="methodCall"/> is part of a chain
@@ -161,11 +168,14 @@ public bool IsPartOfMethodCallChain(Expression methodCall)
             return _analyzer.ChainedMethodCalls.Contains(methodCall);
         }
 
+        #region Helper Class
+
         private class ExpressionAnalysisVisitor : ExpressionVisitor
         {
             private readonly Dictionary<BinaryExpression, object> _constructsByAssignment;
             private readonly List<ParameterExpression> _accessedVariables;
             private readonly List<Expression> _assignedAssignments;
+            private readonly Stack<BlockExpression> _blocks;
             private readonly Stack<object> _constructs;
 
             private ExpressionAnalysisVisitor()
@@ -176,7 +186,9 @@ private ExpressionAnalysisVisitor()
                 JoinedAssignments = new List<BinaryExpression>();
                 _assignedAssignments = new List<Expression>();
                 NamedLabelTargets = new List<LabelTarget>();
+                GotoReturnGotos = new List<GotoExpression>();
                 ChainedMethodCalls = new List<MethodCallExpression>();
+                _blocks = new Stack<BlockExpression>();
                 _constructs = new Stack<object>();
             }
 
@@ -226,6 +238,8 @@ private static Expression GetCoreExpression(Expression expression)
 
             public ICollection<LabelTarget> NamedLabelTargets { get; }
 
+            public ICollection<GotoExpression> GotoReturnGotos { get; }
+
             public List<MethodCallExpression> ChainedMethodCalls { get; }
 
             protected override Expression VisitParameter(ParameterExpression variable)
@@ -263,6 +277,17 @@ protected override Expression VisitParameter(ParameterExpression variable)
                 return base.VisitParameter(variable);
             }
 
+            protected override Expression VisitBlock(BlockExpression block)
+            {
+                _blocks.Push(block);
+
+                var result = base.VisitBlock(block);
+
+                _blocks.Pop();
+
+                return result;
+            }
+
             protected override Expression VisitBinary(BinaryExpression binary)
             {
                 if ((binary.NodeType == ExpressionType.Assign) &&
@@ -320,11 +345,27 @@ private void AddAssignmentIfAppropriate(Expression assignedValue)
 
             protected override Expression VisitGoto(GotoExpression @goto)
             {
-                if (@goto.Kind == GotoExpressionKind.Goto)
+                if (@goto.Kind != GotoExpressionKind.Goto)
+                {
+                    return base.VisitGoto(@goto);
+                }
+
+                var currentBlockFinalExpression = _blocks.Peek()?.Expressions.Last();
+
+                if (currentBlockFinalExpression?.NodeType == ExpressionType.Label)
                 {
-                    NamedLabelTargets.Add(@goto.Target);
+                    var returnLabel = (LabelExpression)currentBlockFinalExpression;
+
+                    if (@goto.Target == returnLabel.Target)
+                    {
+                        GotoReturnGotos.Add(@goto);
+
+                        return base.VisitGoto(@goto);
+                    }
                 }
 
+                NamedLabelTargets.Add(@goto.Target);
+
                 return base.VisitGoto(@goto);
             }
 
@@ -398,5 +439,7 @@ private TResult VisitConstruct<TArg, TResult>(TArg expression, Func<TArg, TResul
 
             #endregion
         }
+
+        #endregion
     }
 }

ReadableExpressions/Translators/GotoExpressionTranslator.cs

@@ -33,8 +33,15 @@ private static string TranslateBreak(GotoExpression breakExpression, Translation
         private static string TranslateContinue(GotoExpression continueExpression, TranslationContext context)
             => "continue;";
 
-        private static string TranslateGoto(GotoExpression gotoExpression, TranslationContext context)
-            => $"goto {gotoExpression.Target.Name};";
+        private static string TranslateGoto(GotoExpression @goto, TranslationContext context)
+        {
+            if (context.GoesToReturnLabel(@goto))
+            {
+                return TranslateReturn(@goto, context);
+            }
+
+            return $"goto {@goto.Target.Name};";
+        }
 
         private static string TranslateReturn(GotoExpression returnExpression, TranslationContext context)
         {