Skip to content

Commit bbd4238

Browse files
SteveWilkesSteveWilkes
committed
Fixing single lambda parameter argument layout
1 parent c6592a1 commit bbd4238

File tree

6 files changed

+114
-26
lines changed

6 files changed

+114
-26
lines changed

‎ReadableExpressions.UnitTests/WhenFormattingCode.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,36 @@ public void ShouldNotRemoveParenthesesFromALambdaInvokeResultAssignment()
603603
translated.ShouldBe(EXPECTED);
604604
}
605605

606+
[Fact]
607+
public void ShouldPlaceSingleArgumentLambdaParametersOnMethodNameLine()
608+
{
609+
var stringParam1 = Expression.Parameter(typeof(string), "string1");
610+
var stringParam2 = Expression.Parameter(typeof(string), "string2");
611+
612+
var writeString = CreateLambda(() => Console.WriteLine("String!")).Body;
613+
var writeStringsBlock = Expression.Block(writeString, writeString, writeString);
614+
615+
var stringLambda = Expression.Lambda<Action<string, string>>(
616+
writeStringsBlock,
617+
stringParam1,
618+
stringParam2);
619+
620+
var lambdaMethod = typeof(HelperClass).GetPublicStaticMethod("GiveMeALambda");
621+
var lambdaMethodCall = Expression.Call(lambdaMethod, stringLambda);
622+
623+
const string EXPECTED = @"
624+
HelperClass.GiveMeALambda((string1, string2) =>
625+
{
626+
Console.WriteLine(""String!"");
627+
Console.WriteLine(""String!"");
628+
Console.WriteLine(""String!"");
629+
})";
630+
631+
var translated = ToReadableString(lambdaMethodCall);
632+
633+
translated.ShouldBe(EXPECTED.TrimStart());
634+
}
635+
606636
[Fact]
607637
public void ShouldUseMethodGroupsForStaticMethods()
608638
{
@@ -967,6 +997,10 @@ public void GiveMeSomeInts(int intOne, int intTwo, int intThree)
967997
public void GiveMeFourInts(int intOne, int intTwo, int intThree, int intFour)
968998
{
969999
}
1000+
1001+
public static void GiveMeALambda(Action<string, string> lambda)
1002+
{
1003+
}
9701004
}
9711005
// ReSharper restore UnusedParameter.Local
9721006

‎ReadableExpressions/Translations/AssignmentTranslation.cs‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ internal class AssignmentTranslation :
3939
private readonly ITranslation _targetTranslation;
4040
private readonly string _operator;
4141
private readonly ITranslation _valueTranslation;
42+
private bool _suppressSpaceBeforeValue;
4243

4344
public AssignmentTranslation(BinaryExpression assignment, ITranslationContext context)
4445
: this(
@@ -87,16 +88,23 @@ private ITranslation GetNonDefaultValueTranslation(Expression assignedValue, ITr
8788
{
8889
var valueBlock = context.GetCodeBlockTranslationFor(assignedValue);
8990

90-
if (valueBlock.IsMultiStatement)
91+
if (valueBlock.IsMultiStatement == false)
9192
{
92-
return (valueBlock.NodeType == Conditional) || (valueBlock.NodeType == Lambda)
93-
? valueBlock.WithoutBraces()
94-
: valueBlock.WithBraces();
93+
return IsCheckedOperation
94+
? valueBlock.WithoutBraces().WithTermination()
95+
: valueBlock.WithoutBraces().WithoutTermination();
9596
}
9697

97-
return IsCheckedOperation
98-
? valueBlock.WithoutBraces().WithTermination()
99-
: valueBlock.WithoutBraces().WithoutTermination();
98+
if (valueBlock.IsMultiStatementLambda(context))
99+
{
100+
return valueBlock.WithoutBraces();
101+
}
102+
103+
_suppressSpaceBeforeValue = true;
104+
105+
return (valueBlock.NodeType == Conditional)
106+
? valueBlock.WithoutBraces()
107+
: valueBlock.WithBraces();
100108
}
101109

102110
private int GetEstimatedSize()
@@ -133,7 +141,7 @@ public void WriteTo(TranslationBuffer buffer)
133141
_targetTranslation.WriteTo(buffer);
134142
buffer.WriteToTranslation(_operator);
135143

136-
if (_valueTranslation.IsMultiStatement() == false)
144+
if (_suppressSpaceBeforeValue == false)
137145
{
138146
buffer.WriteSpaceToTranslation();
139147
}

‎ReadableExpressions/Translations/CodeBlockTranslation.cs‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,16 @@ internal class CodeBlockTranslation :
1616
private readonly ITranslation _translation;
1717
private bool _ensureTerminated;
1818
private bool _ensureReturnKeyword;
19-
private bool _startOnSameLine;
20-
private bool _formatAsSingleLambdaParameter;
19+
private bool _startOnNewLine;
20+
private bool _indentContents;
2121
private bool _writeBraces;
2222

2323
public CodeBlockTranslation(ITranslation translation)
2424
{
2525
NodeType = translation.NodeType;
2626
_translation = translation;
27-
_writeBraces = translation.IsMultiStatement();
27+
_startOnNewLine = true;
28+
_writeBraces = IsMultiStatement = translation.IsMultiStatement();
2829
CalculateEstimatedSize();
2930
}
3031

@@ -49,12 +50,24 @@ private void CalculateEstimatedSize()
4950

5051
public int EstimatedSize { get; private set; }
5152

52-
public bool IsMultiStatement => _translation.IsMultiStatement();
53+
public bool IsMultiStatement { get; }
5354

5455
public bool IsTerminated => _ensureTerminated || _translation.IsTerminated();
5556

5657
public bool HasBraces => _writeBraces;
5758

59+
public bool IsMultiStatementLambda(ITranslationContext context)
60+
{
61+
switch (NodeType)
62+
{
63+
case ExpressionType.Lambda:
64+
case ExpressionType.Quote when context.Settings.DoNotCommentQuotedLambdas:
65+
return IsMultiStatement;
66+
}
67+
68+
return false;
69+
}
70+
5871
public CodeBlockTranslation WithTermination()
5972
{
6073
_ensureTerminated = true;
@@ -73,12 +86,19 @@ public CodeBlockTranslation WithoutTermination()
7386
return this;
7487
}
7588

76-
public CodeBlockTranslation WithSingleLamdaParameterFormatting()
89+
public CodeBlockTranslation WithSingleCodeBlockParameterFormatting()
7790
{
78-
_formatAsSingleLambdaParameter = true;
91+
_startOnNewLine = false;
92+
_indentContents = true;
7993
return this;
8094
}
8195

96+
public CodeBlockTranslation WithSingleLamdaParameterFormatting()
97+
{
98+
_startOnNewLine = false;
99+
return WithoutBraces();
100+
}
101+
82102
public CodeBlockTranslation WithReturnKeyword()
83103
{
84104
_ensureReturnKeyword = true;
@@ -112,24 +132,23 @@ public CodeBlockTranslation WithoutBraces()
112132

113133
public CodeBlockTranslation WithoutStartingNewLine()
114134
{
115-
_startOnSameLine = true;
135+
_startOnNewLine = false;
116136
return this;
117137
}
118138

119139
public void WriteTo(TranslationBuffer buffer)
120140
{
121141
if (_writeBraces)
122142
{
123-
buffer.WriteOpeningBraceToTranslation(
124-
startOnNewLine: _startOnSameLine == false && _formatAsSingleLambdaParameter == false);
143+
buffer.WriteOpeningBraceToTranslation(_startOnNewLine);
125144

126145
if (WriteEmptyCodeBlock(buffer))
127146
{
128147
return;
129148
}
130149
}
131150

132-
if (_formatAsSingleLambdaParameter)
151+
if (_indentContents)
133152
{
134153
buffer.Indent();
135154
}
@@ -151,7 +170,7 @@ public void WriteTo(TranslationBuffer buffer)
151170
buffer.WriteClosingBraceToTranslation();
152171
}
153172

154-
if (_formatAsSingleLambdaParameter)
173+
if (_indentContents)
155174
{
156175
buffer.Unindent();
157176
}

‎ReadableExpressions/Translations/ConditionTranslation.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public static ITranslation For(Expression condition, ITranslationContext context
2222
var conditionCodeBlockTranslation = new CodeBlockTranslation(conditionTranslation);
2323

2424
return conditionTranslation.IsMultiStatement()
25-
? conditionCodeBlockTranslation.WithSingleLamdaParameterFormatting()
25+
? conditionCodeBlockTranslation.WithSingleCodeBlockParameterFormatting()
2626
: conditionCodeBlockTranslation;
2727
}
2828

‎ReadableExpressions/Translations/LambdaTranslation.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#endif
99
using Interfaces;
1010

11-
internal class LambdaTranslation : ITranslation
11+
internal class LambdaTranslation : ITranslation, IPotentialMultiStatementTranslatable
1212
{
1313
private const string _fatArrow = " => ";
1414

@@ -37,6 +37,8 @@ private int GetEstimatedSize()
3737

3838
public int EstimatedSize { get; }
3939

40+
public bool IsMultiStatement => _bodyTranslation.IsMultiStatement;
41+
4042
public void WriteTo(TranslationBuffer buffer)
4143
{
4244
_parameters.WriteTo(buffer);

‎ReadableExpressions/Translations/ParameterSetTranslation.cs‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
using System.Reflection;
66
#if NET35
77
using Microsoft.Scripting.Ast;
8+
using static Microsoft.Scripting.Ast.ExpressionType;
89
#else
910
using System.Linq.Expressions;
11+
using static System.Linq.Expressions.ExpressionType;
1012
#endif
1113
using Extensions;
1214
using Interfaces;
@@ -18,6 +20,7 @@ internal class ParameterSetTranslation : ITranslatable
1820
private const string _openAndCloseParentheses = "()";
1921

2022
private readonly IList<CodeBlockTranslation> _parameterTranslations;
23+
private readonly bool _hasSingleMultiStatementLambdaParameter;
2124
private ParenthesesMode _parenthesesMode;
2225

2326
public ParameterSetTranslation(ITranslation parameter)
@@ -95,6 +98,8 @@ private ParameterSetTranslation(
9598
methodParameters = null;
9699
}
97100

101+
var hasSingleParameter = ParameterCount == 1;
102+
var singleParameterIsMultiLineLambda = false;
98103
var estimatedSize = 0;
99104

100105
_parameterTranslations = parameters
@@ -105,7 +110,7 @@ private ParameterSetTranslation(
105110
if (CanBeConvertedToMethodGroup(p, out var lambdaBodyMethodCall))
106111
{
107112
translation = new MethodGroupTranslation(
108-
ExpressionType.Lambda,
113+
Lambda,
109114
MethodCallTranslation.GetSubjectTranslation(lambdaBodyMethodCall, context),
110115
lambdaBodyMethodCall.Method);
111116

@@ -131,10 +136,20 @@ private ParameterSetTranslation(
131136
CreateCodeBlock:
132137
estimatedSize += translation.EstimatedSize;
133138

134-
return new CodeBlockTranslation(translation).WithoutTermination();
139+
// TODO: Only use code blocks where useful:
140+
var parameterCodeBlock = new CodeBlockTranslation(translation).WithoutTermination();
141+
142+
if (hasSingleParameter && parameterCodeBlock.IsMultiStatementLambda(context))
143+
{
144+
singleParameterIsMultiLineLambda = true;
145+
parameterCodeBlock.WithSingleLamdaParameterFormatting();
146+
}
147+
148+
return parameterCodeBlock;
135149
})
136150
.ToArray();
137151

152+
_hasSingleMultiStatementLambdaParameter = singleParameterIsMultiLineLambda;
138153
EstimatedSize = estimatedSize + (ParameterCount * 2) + 4;
139154
}
140155

@@ -192,15 +207,15 @@ private static ITranslation GetParameterTranslation(
192207

193208
private static bool CanBeConvertedToMethodGroup(Expression argument, out MethodCallExpression lambdaBodyMethodCall)
194209
{
195-
if (argument.NodeType != ExpressionType.Lambda)
210+
if (argument.NodeType != Lambda)
196211
{
197212
lambdaBodyMethodCall = null;
198213
return false;
199214
}
200215

201216
var argumentLambda = (LambdaExpression)argument;
202217

203-
if (argumentLambda.Body.NodeType != ExpressionType.Call)
218+
if (argumentLambda.Body.NodeType != Call)
204219
{
205220
lambdaBodyMethodCall = null;
206221
return false;
@@ -267,7 +282,7 @@ public void WriteTo(TranslationBuffer buffer)
267282
buffer.WriteToTranslation('(');
268283
}
269284

270-
var writeParametersOnNewLines = (ParameterCount > _splitArgumentsThreshold) || this.ExceedsLengthThreshold();
285+
var writeParametersOnNewLines = WriteParametersOnNewLines();
271286

272287
if (writeParametersOnNewLines)
273288
{
@@ -317,6 +332,16 @@ public void WriteTo(TranslationBuffer buffer)
317332
}
318333
}
319334

335+
private bool WriteParametersOnNewLines()
336+
{
337+
if (_hasSingleMultiStatementLambdaParameter)
338+
{
339+
return false;
340+
}
341+
342+
return (ParameterCount > _splitArgumentsThreshold) || this.ExceedsLengthThreshold();
343+
}
344+
320345
private enum ParenthesesMode
321346
{
322347
Auto,

0 commit comments

Comments
 (0)