Skip to content

Commit cc37cfb

Browse files
SteveWilkesSteveWilkes
committed
Improved translation of ternaries with multiple-line branches / Analyzing assignment statements before translation / Renaming CodeBlock methods as appropriate
1 parent c110e95 commit cc37cfb

12 files changed

+137
-67
lines changed

‎ReadableExpressions.UnitTests/WhenTranslatingAssignments.cs‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,5 +318,73 @@ public void ShouldAssignAVariableInAMethodCallArgument()
318318

319319
Assert.AreEqual(EXPECTED.TrimStart(), translated);
320320
}
321+
322+
[TestMethod]
323+
public void ShouldTranslateAMultipleLineTernaryAssignment()
324+
{
325+
Expression<Action> consoleRead = () => Console.Read();
326+
327+
var variableOne = Expression.Variable(typeof(int), "one");
328+
var variableTwo = Expression.Variable(typeof(int), "two");
329+
var resultVariableOne = Expression.Variable(typeof(int), "resultOne");
330+
331+
var variableOneAssignment = Expression.Assign(variableOne, consoleRead.Body);
332+
var variableTwoAssignment = Expression.Assign(variableTwo, consoleRead.Body);
333+
334+
var variableOneTimesTwo = Expression.Multiply(variableOne, variableTwo);
335+
var resultOneAssignment = Expression.Assign(resultVariableOne, variableOneTimesTwo);
336+
337+
var ifTrueBlock = Expression.Block(
338+
new[] { variableOne, variableTwo, resultVariableOne },
339+
variableOneAssignment,
340+
variableTwoAssignment,
341+
resultOneAssignment,
342+
resultVariableOne);
343+
344+
var variableThree = Expression.Variable(typeof(int), "three");
345+
var variableFour = Expression.Variable(typeof(int), "four");
346+
var resultVariableTwo = Expression.Variable(typeof(int), "resultTwo");
347+
348+
var variableThreeAssignment = Expression.Assign(variableThree, consoleRead.Body);
349+
var variableFourAssignment = Expression.Assign(variableFour, consoleRead.Body);
350+
351+
var variableThreeDivideFour = Expression.Divide(variableThree, variableFour);
352+
var resultTwoAssignment = Expression.Assign(resultVariableTwo, variableThreeDivideFour);
353+
354+
var ifFalseBlock = Expression.Block(
355+
new[] { variableThree, variableFour, resultVariableTwo },
356+
variableThreeAssignment,
357+
variableFourAssignment,
358+
resultTwoAssignment,
359+
resultVariableTwo);
360+
361+
var dateTimeNow = Expression.Property(null, typeof(DateTime), "Now");
362+
var nowHour = Expression.Property(dateTimeNow, "Hour");
363+
var nowHourModuloTwo = Expression.Modulo(nowHour, Expression.Constant(2));
364+
var nowHourIsEven = Expression.Equal(nowHourModuloTwo, Expression.Constant(0));
365+
366+
var conditional = Expression.Condition(nowHourIsEven, ifTrueBlock, ifFalseBlock);
367+
368+
var resultVariable = Expression.Variable(typeof(int), "result");
369+
var resultAssignment = Expression.Assign(resultVariable, conditional);
370+
371+
var translated = resultAssignment.ToReadableString();
372+
373+
const string EXPECTED = @"
374+
result = ((DateTime.Now.Hour % 2) == 0)
375+
? {
376+
var one = Console.Read();
377+
var two = Console.Read();
378+
var resultOne = one * two;
379+
return resultOne;
380+
}
381+
: {
382+
var three = Console.Read();
383+
var four = Console.Read();
384+
var resultTwo = three / four;
385+
return resultTwo;
386+
}";
387+
Assert.AreEqual(EXPECTED.TrimStart(), translated);
388+
}
321389
}
322390
}

‎ReadableExpressions.UnitTests/WhenTranslatingConditionals.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public void ShouldTranslateAnIfElseIfStatement()
145145
}
146146

147147
[TestMethod]
148-
public void ShouldTranslateMultipleLineIfElseStatements()
148+
public void ShouldTranslateMultipleLineVoidIfElseStatements()
149149
{
150150
var intVariable = Expression.Variable(typeof(int), "i");
151151
var zero = Expression.Constant(0);
@@ -173,7 +173,7 @@ public void ShouldTranslateMultipleLineIfElseStatements()
173173
}
174174

175175
[TestMethod]
176-
public void ShouldTranslateAMultipleLineConditional()
176+
public void ShouldTranslateAMultipleLineNonVoidIfElseStatements()
177177
{
178178
var intVariable = Expression.Variable(typeof(int), "i");
179179
var zero = Expression.Constant(0);
@@ -182,7 +182,7 @@ public void ShouldTranslateAMultipleLineConditional()
182182
Expression<Action> writeGoodbye = () => Console.WriteLine("Goodbye");
183183
var helloThenGoodbye = Expression.Block(writeHello.Body, writeGoodbye.Body, intVariable);
184184
var goodbyeThenHello = Expression.Block(writeGoodbye.Body, writeHello.Body, intVariable);
185-
var writeHelloAndGoodbye = Expression.Condition(intVariableEqualsZero, helloThenGoodbye, goodbyeThenHello);
185+
var writeHelloAndGoodbye = Expression.IfThenElse(intVariableEqualsZero, helloThenGoodbye, goodbyeThenHello);
186186

187187
var translated = writeHelloAndGoodbye.ToReadableString();
188188

@@ -321,7 +321,7 @@ public void ShouldIncludeReturnKeywordsForConstantsAndCasts()
321321
var oneCastToLong = Expression.Convert(Expression.Constant(1), typeof(long?));
322322
var elseBlock = Expression.Block(writeOne.Body, writeOne.Body, oneCastToLong);
323323

324-
var nullOrOne = Expression.Condition(Expression.Constant(true), nullLong, elseBlock);
324+
var nullOrOne = Expression.IfThenElse(Expression.Constant(true), nullLong, elseBlock);
325325

326326
var translated = nullOrOne.ToReadableString();
327327

‎ReadableExpressions/TranslationContext.cs‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ public string GetTranslation(Expression expression)
2929
return _globalTranslator.Invoke(expression, this);
3030
}
3131

32-
public bool IsAssignedValue(Expression expression) => _analyzer.AssignedValues.Contains(expression);
33-
3432
public bool IsNotJoinedAssignment(Expression expression)
3533
{
3634
return (expression.NodeType != ExpressionType.Assign) ||
@@ -51,7 +49,6 @@ private class ExpressionAnalysisVisitor : ExpressionVisitor
5149
{
5250
private readonly List<ParameterExpression> _accessedVariables;
5351
private readonly List<ParameterExpression> _assignedVariables;
54-
private readonly List<Expression> _assignedValues;
5552
private readonly List<Expression> _assignedAssignments;
5653
private readonly List<BinaryExpression> _joinedAssignments;
5754
private readonly List<LabelTarget> _namedLabelTargets;
@@ -62,7 +59,6 @@ private ExpressionAnalysisVisitor()
6259
{
6360
_accessedVariables = new List<ParameterExpression>();
6461
_assignedVariables = new List<ParameterExpression>();
65-
_assignedValues = new List<Expression>();
6662
_assignedAssignments = new List<Expression>();
6763
_joinedAssignments = new List<BinaryExpression>();
6864
_namedLabelTargets = new List<LabelTarget>();
@@ -77,6 +73,12 @@ public static ExpressionAnalysisVisitor Analyse(Expression expression)
7773

7874
var coreExpression = GetCoreExpression(expression);
7975

76+
if (coreExpression.IsAssignment())
77+
{
78+
analyzer.Visit(coreExpression);
79+
return analyzer;
80+
}
81+
8082
switch (coreExpression.NodeType)
8183
{
8284
case ExpressionType.Block:
@@ -118,8 +120,6 @@ private static Expression GetCoreExpression(Expression expression)
118120

119121
public IEnumerable<ParameterExpression> AssignedVariables => _assignedVariables;
120122

121-
public IEnumerable<Expression> AssignedValues => _assignedValues;
122-
123123
public IEnumerable<BinaryExpression> JoinedAssignments => _joinedAssignments;
124124

125125
public IEnumerable<LabelTarget> NamedLabelTargets => _namedLabelTargets;
@@ -145,26 +145,22 @@ protected override Expression VisitParameter(ParameterExpression variable)
145145

146146
protected override Expression VisitBinary(BinaryExpression binaryExpression)
147147
{
148-
if (binaryExpression.NodeType == ExpressionType.Assign)
148+
if ((binaryExpression.NodeType == ExpressionType.Assign) &&
149+
(binaryExpression.Left.NodeType == ExpressionType.Parameter) &&
150+
((_currentBlock == null) || _currentBlock.Expressions.Contains(binaryExpression)) &&
151+
!_assignedVariables.Contains(binaryExpression.Left) &&
152+
!_assignedAssignments.Contains(binaryExpression))
149153
{
150-
_assignedValues.Add(binaryExpression.Right);
154+
var variable = (ParameterExpression)binaryExpression.Left;
151155

152-
if ((binaryExpression.Left.NodeType == ExpressionType.Parameter) &&
153-
((_currentBlock == null) || _currentBlock.Expressions.Contains(binaryExpression)) &&
154-
!_assignedVariables.Contains(binaryExpression.Left) &&
155-
!_assignedAssignments.Contains(binaryExpression))
156+
if (VariableHasNotYetBeenAccessed(variable))
156157
{
157-
var variable = (ParameterExpression)binaryExpression.Left;
158-
159-
if (VariableHasNotYetBeenAccessed(variable))
160-
{
161-
_joinedAssignments.Add(binaryExpression);
162-
_accessedVariables.Add(variable);
163-
_assignedVariables.Add(variable);
164-
}
165-
166-
AddAssignmentIfAppropriate(binaryExpression.Right);
158+
_joinedAssignments.Add(binaryExpression);
159+
_accessedVariables.Add(variable);
160+
_assignedVariables.Add(variable);
167161
}
162+
163+
AddAssignmentIfAppropriate(binaryExpression.Right);
168164
}
169165

170166
return base.VisitBinary(binaryExpression);

‎ReadableExpressions/Translators/AssignmentExpressionTranslator.cs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,16 @@ private string GetValueTranslation(Expression value, TranslationContext context)
7171

7272
if (valueBlock.IsASingleStatement)
7373
{
74-
return valueBlock.WithoutParentheses().Unterminated();
74+
return valueBlock.WithoutCurlyBraces().Unterminated();
7575
}
7676

77-
if (value.NodeType == ExpressionType.Lambda)
77+
if ((value.NodeType == ExpressionType.Conditional) ||
78+
(value.NodeType == ExpressionType.Lambda))
7879
{
79-
return valueBlock.WithoutParentheses();
80+
return valueBlock.WithoutCurlyBraces();
8081
}
8182

82-
return valueBlock.WithReturn().WithParentheses();
83+
return valueBlock.WithCurlyBraces();
8384
}
8485

8586
private static string AdjustForCheckedAssignmentIfAppropriate(

‎ReadableExpressions/Translators/ConditionalExpressionTranslator.cs‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ public override string Translate(Expression expression, TranslationContext conte
2222

2323
if (hasNoElseCondition)
2424
{
25-
return IfStatement(test, ifTrueBlock.WithReturn().WithParentheses());
25+
return IfStatement(test, ifTrueBlock.WithCurlyBraces());
2626
}
2727

2828
var ifFalseBlock = GetTranslatedExpressionBody(conditional.IfFalse, context);
2929

30-
if (IsSuitableForTernary(conditional, ifTrueBlock, ifFalseBlock))
30+
if (IsTernary(conditional))
3131
{
3232
return new TernaryExpression(test, ifTrueBlock, ifFalseBlock);
3333
}
@@ -56,19 +56,17 @@ private static string IfStatement(string test, string body)
5656
return $"if {test}{body}";
5757
}
5858

59-
private static bool IsSuitableForTernary(Expression conditional, CodeBlock ifTrue, CodeBlock ifFalse)
59+
private static bool IsTernary(Expression conditional)
6060
{
61-
return (conditional.Type != typeof(void)) &&
62-
ifTrue.IsASingleStatement &&
63-
ifFalse.IsASingleStatement;
61+
return conditional.Type != typeof(void);
6462
}
6563

6664
private static string ShortCircuitingIf(string test, CodeBlock ifTrue, CodeBlock ifFalse)
6765
{
6866
var ifElseBlock = $@"
69-
if {test}{ifTrue.WithReturn().WithParentheses()}
67+
if {test}{ifTrue.WithCurlyBraces()}
7068
71-
{ifFalse.WithoutParentheses()}";
69+
{ifFalse.WithoutCurlyBraces()}";
7270

7371
return ifElseBlock.TrimStart();
7472
}
@@ -85,11 +83,11 @@ private static string IfElse(
8583
bool isElseIf)
8684
{
8785
var ifFalseBlock = isElseIf
88-
? " " + ifFalse.WithoutParentheses()
89-
: ifFalse.WithParentheses();
86+
? " " + ifFalse.WithoutCurlyBraces()
87+
: ifFalse.WithCurlyBraces();
9088

9189
string ifElseBlock = $@"
92-
if {test}{ifTrue.WithParentheses()}
90+
if {test}{ifTrue.WithCurlyBraces()}
9391
else{ifFalseBlock}";
9492

9593

‎ReadableExpressions/Translators/Formatting/CodeBlock.cs‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public CodeBlock Append(params string[] lines)
5555
return new CodeBlock(_expression, _blockLines.Concat(lines).ToArray());
5656
}
5757

58-
public string WithoutParentheses()
58+
public string WithoutCurlyBraces()
5959
{
6060
return GetCodeBlock();
6161
}
@@ -81,22 +81,7 @@ private static bool ExpressionHasReturn(Expression expression)
8181
}
8282
}
8383

84-
public CodeBlock WithReturn()
85-
{
86-
if (!_expression.IsReturnable() || HasReturn())
87-
{
88-
return this;
89-
}
90-
91-
return new CodeBlock(
92-
_expression,
93-
_blockLines
94-
.Take(_blockLines.Length - 1)
95-
.Concat(new[] { "return " + _blockLines.Last() })
96-
.ToArray());
97-
}
98-
99-
public string WithParentheses()
84+
public string WithCurlyBraces()
10085
{
10186
if (_blockLines.Length == 0)
10287
{
@@ -118,6 +103,21 @@ public string WithParentheses()
118103
}}";
119104
}
120105

106+
private CodeBlock WithReturn()
107+
{
108+
if (!_expression.IsReturnable() || HasReturn())
109+
{
110+
return this;
111+
}
112+
113+
return new CodeBlock(
114+
_expression,
115+
_blockLines
116+
.Take(_blockLines.Length - 1)
117+
.Concat(new[] { "return " + _blockLines.Last() })
118+
.ToArray());
119+
}
120+
121121
private string GetCodeBlock()
122122
{
123123
AddSemiColonIfRequired();

‎ReadableExpressions/Translators/Formatting/TernaryExpression.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ internal class TernaryExpression : FormattableExpressionBase
66
{
77
public TernaryExpression(string test, CodeBlock ifTrue, CodeBlock ifFalse)
88
{
9-
var ifTrueString = ifTrue.AsExpressionBody();
10-
var ifFalseString = ifFalse.AsExpressionBody();
9+
var ifTrueString = GetBranch(ifTrue);
10+
var ifFalseString = GetBranch(ifFalse);
1111

1212
SingleLineTranslationFactory = () => $"{test} ?{ifTrueString} :{ifFalseString}";
1313

@@ -19,6 +19,13 @@ public TernaryExpression(string test, CodeBlock ifTrue, CodeBlock ifFalse)
1919
(":" + ifFalseString).Indent();
2020
}
2121

22+
private static string GetBranch(CodeBlock codeBlock)
23+
{
24+
return codeBlock.IsASingleStatement
25+
? codeBlock.AsExpressionBody()
26+
: " " + codeBlock.WithCurlyBraces().TrimStart();
27+
}
28+
2229
protected override Func<string> SingleLineTranslationFactory { get; }
2330

2431
protected override Func<string> MultipleLineTranslationFactory { get; }

‎ReadableExpressions/Translators/LambdaExpressionTranslator.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public override string Translate(Expression expression, TranslationContext conte
1919

2020
var body = bodyBlock.IsASingleStatement
2121
? bodyBlock.AsExpressionBody()
22-
: bodyBlock.WithParentheses();
22+
: bodyBlock.WithCurlyBraces();
2323

2424
return parameters + " =>" + body;
2525
}

‎ReadableExpressions/Translators/LoopExpressionTranslator.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public override string Translate(Expression expression, TranslationContext conte
1515

1616
var loopBodyBlock = GetTranslatedExpressionBody(loop.Body, context);
1717

18-
return $"while (true){loopBodyBlock.WithParentheses()}";
18+
return $"while (true){loopBodyBlock.WithCurlyBraces()}";
1919
}
2020
}
2121
}

‎ReadableExpressions/Translators/QuotedLambdaExpressionTranslator.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public override string Translate(Expression expression, TranslationContext conte
1818
var quotedLambdaBlock = Expression.Block(comment, quote.Operand);
1919
var translatedLambda = GetTranslatedExpressionBody(quotedLambdaBlock, context);
2020

21-
return Environment.NewLine + translatedLambda.Indented().WithoutParentheses();
21+
return Environment.NewLine + translatedLambda.Indented().WithoutCurlyBraces();
2222
}
2323
}
2424
}

0 commit comments

Comments
 (0)