Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling edge cases with conditionals in a return statement. #435

Merged
merged 6 commits into from
Sep 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion Src/CSharpier.Tests/DocPrinterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,23 @@ public void IfBreak_Should_Print_Break_Contents_When_Group_Does_Not_Fit()
[Test]
public void IfBreak_Should_Print_Flat_Contents_When_GroupId_Does_Not_Break()
{
var doc = Doc.Concat(Doc.GroupWithId("1", "1"), Doc.IfBreak("break", "flat", "1"));
var doc = Doc.Concat(Doc.GroupWithId("id", "1"), Doc.IfBreak("break", "flat", "id"));

PrintedDocShouldBe(doc, "1flat");
}

[Test]
public void IfBreak_Should_Throw_Exception_When_It_Targets_Not_Yet_Printed_Group()
{
var doc = Doc.Concat(Doc.IfBreak("break", "flat", "id"), Doc.GroupWithId("id", "1"));

this.Invoking(o => PrintedDocShouldBe(doc, "flat1"))
.Should()
.Throw<Exception>()
.WithMessage("You cannot use an ifBreak before the group it targets.");
;
}

[Test]
public void IfBreak_Should_Print_Break_Contents_When_GroupId_Breaks()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,42 @@ public class ClassName
encoding ? GetEncoding(1252) : encoding,
cancellationToken
);

return someLongCondition___________________________
? trueValue________________________________
: falseValue________________________________;

return
someLongCondition____________________________________
&& someOtherLongCondition____________________________________
? trueValue________________________________
: falseValue_______________________________;

return
someLongCondition____________________________________
is SomeLongType___________________________
? trueValue________________________________
: falseValue_______________________________;

return (
someLongCondition____________________________________
&& someOtherLongCondition____________________________________
)
? trueValue________________________________
: falseValue_______________________________;

return
someLongCondition____________________________________
== someThingElse______________________
&& someOtherLongCondition____________________________________
? trueValue________________________________
: falseValue_______________________________;

return CallSomeMethod(
someLongCondition____________________________________,
someOtherLongCondition____________________________________
)
? trueValue________________________________
: falseValue_______________________________;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,5 @@ public record LotsOfParametersWithBaseWithLots(
theyLookLikeParameters,
"someExtraValueToMakeThisReallyLong"
);

public record SomeRecord : SomeBaseRecord { }
15 changes: 11 additions & 4 deletions Src/CSharpier/DocPrinter/DocPrinter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,17 @@ private void ProcessNextCommand()
break;
case IfBreak ifBreak:
{
var groupMode =
ifBreak.GroupId != null && GroupModeMap.ContainsKey(ifBreak.GroupId)
? GroupModeMap[ifBreak.GroupId]
: mode;
var groupMode = mode;
if (ifBreak.GroupId != null)
{
if (!GroupModeMap.TryGetValue(ifBreak.GroupId, out groupMode))
{
throw new Exception(
"You cannot use an ifBreak before the group it targets."
);
}
}

var contents =
groupMode == PrintMode.Break ? ifBreak.BreakContents : ifBreak.FlatContents;
Push(contents, mode, indent);
Expand Down
2 changes: 1 addition & 1 deletion Src/CSharpier/DocTypes/Doc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public static IndentDoc Indent(params Doc[] contents) =>

public static IndentDoc Indent(List<Doc> contents) => new() { Contents = Concat(contents) };

public static Doc IndentIf(bool condition, Concat contents)
public static Doc IndentIf(bool condition, Doc contents)
{
return condition ? Doc.Indent(contents) : contents;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public static Doc Print(BaseTypeDeclarationSyntax node)

var groupedBaseListDoc = Doc.Group(Doc.Indent(Doc.Line, baseListDoc));

if (node is RecordDeclarationSyntax)
if (node is RecordDeclarationSyntax && parameterList != null)
{
docs.Add(
Doc.IfBreak(Doc.Concat(" ", baseListDoc), groupedBaseListDoc, groupId)
Expand Down
20 changes: 10 additions & 10 deletions Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BinaryExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,21 @@ public static Doc Print(BinaryExpressionSyntax node)

var shouldNotIndent =
node.Parent
is ReturnStatementSyntax
or WhereClauseSyntax
is ArrowExpressionClauseSyntax
or AssignmentExpressionSyntax
or CatchFilterClauseSyntax
or CheckedExpressionSyntax
or DoStatementSyntax
or EqualsValueClauseSyntax
or ArrowExpressionClauseSyntax
or IfStatementSyntax
or ParenthesizedExpressionSyntax
or ParenthesizedLambdaExpressionSyntax
or AssignmentExpressionSyntax
or SimpleLambdaExpressionSyntax
or IfStatementSyntax
or WhileStatementSyntax
or ReturnStatementSyntax
or SwitchExpressionSyntax
or DoStatementSyntax
or CheckedExpressionSyntax
or CatchFilterClauseSyntax
or ParenthesizedExpressionSyntax
or SwitchStatementSyntax
or WhereClauseSyntax
or WhileStatementSyntax
|| node.Parent is ConditionalExpressionSyntax
&& node.Parent.Parent is not ArgumentSyntax;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using CSharpier.DocTypes;
using Microsoft.CodeAnalysis.CSharp.Syntax;

Expand All @@ -17,9 +18,19 @@ public static Doc Print(ConditionalExpressionSyntax node)
Doc.Align(2, Node.Print(node.WhenFalse))
};

var groupId = Guid.NewGuid().ToString();

return Doc.Group(
Node.Print(node.Condition),
node.Parent is ConditionalExpressionSyntax or ArgumentSyntax
node.Parent is ReturnStatementSyntax
&& node.Condition is BinaryExpressionSyntax or IsPatternExpressionSyntax
? Doc.Indent(
Doc.Group(Doc.IfBreak(Doc.SoftLine, Doc.Null), Node.Print(node.Condition))
)
: Node.Print(node.Condition),
node.Parent
is ConditionalExpressionSyntax
or ArgumentSyntax
or ReturnStatementSyntax
? Doc.Align(2, contents)
: Doc.Indent(contents)
);
Expand Down