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.

Sign up for GitHub

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

Fixing some constructor edge cases #373

Merged
merged 6 commits into from
Jul 27, 2021
Merged

Fixing some constructor edge cases #373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9962e60
Fixing some edge cases for ConstructorDeclaration
belav Jul 26, 2021
cccdcf1
Handling an edge case found while reviewing code
belav Jul 26, 2021
c00df3e
Another edge case
belav Jul 26, 2021
5bee197
Merge branch 'master' into constructor-edge-cases
belav Jul 26, 2021
411a27d
Merge branch 'master' into constructor-edge-cases
belav Jul 27, 2021
e87b2c7
Undoing some changes that weren't actually needed
belav Jul 27, 2021
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
73 changes: 63 additions & 10 deletions Src/CSharpier.Tests/TestFiles/ConstructorDeclaration/ConstructorDeclarations.cst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,41 +27,94 @@ public class Initializers : BasicClass
public Initializers(string value) : base(value) { }

public Initializers(
string reallyLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongParameter
string longParameter_______________________________________________________________________________
) : base(
reallyLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongParameterIMeanIt
longParameter_______________________________________________________________________________
) { }

public Initializers(
string reallyLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongParameter
string longParameter_______________________________________________________________________________
) : base(
longParameter_______________________________________________________________________________
) {
WithBody();
}

public Initializers(
string longParameter____________________________________________________________
) : base(false) { }

public ReallyLoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooogMethodName()
public Initializers(
string longParameter____________________________________________________________
) : base(false)
{
WithBody();
}

public LongMethodName___________________________________________________________________________()
: base(false) { }

public ReallyLoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooogMethodName()
public WithParameters(string parameter)
: base(
longParameter____________________________________________________________________________
) { }

// this may change, see #372
public WithParameters(string parameter)
: base(
longParameter____________________________________________________________________________
) {
WithBody();
}

public LongMethodName___________________________________________________________________________()
: base(
reallyLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongParameterIMeanIt
longParameter____________________________________________________________________________
) { }

public Initializers(
string value
) /*Comment*/
: base(value) { }

public ReallyLoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooogMethodName()
public LongMethodName___________________________________________________________()
/*Comment*/: base(false) { }

public LongInitializerBreaksAtEdgeCase()
: this(someParameter, someOtherParameter___________________________________________________)
{
WithBody();
}

public LongParametersWithEmptyBody(string parameter, bool anotherParameter_________________) { }

public LongParametersWithEmptyBody(string parameter, bool anotherParameter__________________)
{ }

public LongInitializerWithEmptyBody() : base(parameter, anotherParameter___________________) { }

public LongInitializerWithEmptyBody() : base(parameter, anotherParameter__________________) { }

public LongInitializerWithEmptyBody()
: base(parameter______________, anotherParameter___________________________________________)
{ }
}

public class Exactly80
public class Exactly100
{
public Exactly80(int lajsfaskdlfklasdfklskadlfklsdfkasdkfjaskdfkdfdddd)
public Exactly100()
: this(someParameter, someOtherParameter___________________________________________________)
{
WithBody();
}

public Exactly100(int longParameter____________________________________________________________)
{
return;
}

// leading comments don't affect this
public Exactly80(string lajsfaskdlfklasdfklskadlfklsdfkasdkfjaskdfkdfd)
public Exactly100(int longParameter____________________________________________________________)
{
return;
}
Expand Down
2 changes: 0 additions & 2 deletions Src/CSharpier/SyntaxPrinter/Node.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ public static Doc Print(SyntaxNode syntaxNode)
return ConstantPattern.Print(constantPatternSyntax);
case ConstructorConstraintSyntax constructorConstraintSyntax:
return ConstructorConstraint.Print(constructorConstraintSyntax);
case ConstructorInitializerSyntax constructorInitializerSyntax:
return ConstructorInitializer.Print(constructorInitializerSyntax);
case ContinueStatementSyntax continueStatementSyntax:
return ContinueStatement.Print(continueStatementSyntax);
case DeclarationExpressionSyntax declarationExpressionSyntax:
Expand Down
69 changes: 54 additions & 15 deletions Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseMethodDeclaration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public static Doc Print(CSharpSyntaxNode node)
BlockSyntax? body = null;
ArrowExpressionClauseSyntax? expressionBody = null;
SyntaxToken? semicolonToken = null;
string? groupId = null;
string? parameterGroupId = null;
string? constructorInitializerGroupId = null;

if (node is BaseMethodDeclarationSyntax baseMethodDeclarationSyntax)
{
Expand Down Expand Up @@ -141,22 +142,47 @@ public static Doc Print(CSharpSyntaxNode node)
// will cause SpaceBrace when it isn't wanted.
if (parameterList.Parameters.Count > 0)
{
groupId = Guid.NewGuid().ToString();
parameterGroupId = Guid.NewGuid().ToString();
}
declarationGroup.Add(ParameterList.Print(parameterList, groupId));
declarationGroup.Add(ParameterList.Print(parameterList, parameterGroupId));
declarationGroup.Add(Doc.IfBreak(Doc.Null, Doc.SoftLine));
}

if (constructorInitializer != null)
{
declarationGroup.Add(
groupId != null
? ConstructorInitializer.PrintWithConditionalSpace(
constructorInitializer,
groupId
)
: ConstructorInitializer.Print(constructorInitializer)
constructorInitializerGroupId = Guid.NewGuid().ToString();
var colonToken = Token.PrintWithSuffix(constructorInitializer.ColonToken, " ");
var argumentList = Doc.GroupWithId(
constructorInitializerGroupId,
ArgumentList.Print(constructorInitializer.ArgumentList)
);

if (parameterGroupId != null)
{
declarationGroup.Add(
Doc.Group(
Doc.Indent(Doc.IfBreak(" ", Doc.Line, parameterGroupId)),
Doc.IfBreak(
Doc.Align(2, colonToken),
Doc.Indent(colonToken),
parameterGroupId
),
Token.Print(constructorInitializer.ThisOrBaseKeyword),
Doc.IfBreak(argumentList, Doc.Indent(argumentList), parameterGroupId)
)
);
}
else
{
declarationGroup.Add(
Doc.Group(
Doc.Indent(Doc.Line),
Doc.Indent(colonToken),
Token.Print(constructorInitializer.ThisOrBaseKeyword),
Doc.Indent(argumentList)
)
);
}
}

if (modifiers is { Count: > 0 })
Expand All @@ -173,18 +199,31 @@ public static Doc Print(CSharpSyntaxNode node)
if (constraintClauses != null)
{
docs.Add(
groupId != null
? ConstraintClauses.PrintWithConditionalSpace(constraintClauses, groupId)
parameterGroupId != null
? ConstraintClauses.PrintWithConditionalSpace(
constraintClauses,
parameterGroupId
)
Comment on lines +204 to +206
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should half-indent ternary operators?
That way, the closing parenthesis will be aligned with the start of the method.

And things that aren't method calls (e.g. binary operators) usually indent themselves anyway, so they should look fine.

: ConstraintClauses.Print(constraintClauses)
);
}

if (body != null)
{
docs.Add(
groupId != null
string? conditionalSpaceGroupId = null;
if (constructorInitializerGroupId != null)
{
conditionalSpaceGroupId = constructorInitializerGroupId;
}
else if (
parameterGroupId != null
&& (constraintClauses == null || constraintClauses.Value.Count == 0)
? Block.PrintWithConditionalSpace(body, groupId)
) {
conditionalSpaceGroupId = parameterGroupId;
}
docs.Add(
conditionalSpaceGroupId != null
? Block.PrintWithConditionalSpace(body, conditionalSpaceGroupId)
: Block.Print(body)
);
}
Expand Down
35 changes: 0 additions & 35 deletions Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/ConstructorInitializer.cs
View file
Open in desktop

This file was deleted.