Skip to content

Commit

Permalink
Figuring out a way to get syntax tree validation happy when we add a … (
Browse files Browse the repository at this point in the history
#1392)

…trailing comma before a trailing comment

closes #1388
  • Loading branch information
belav authored Nov 22, 2024
1 parent e48c579 commit 576a75b
Show file tree
Hide file tree
Showing 12 changed files with 67 additions and 2 deletions.
1 change: 1 addition & 0 deletions Src/CSharpier.Benchmarks/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public void Default_SyntaxNodeComparer()
this.code,
false,
false,
false,
SourceCodeKind.Regular,
CancellationToken.None
);
Expand Down
1 change: 1 addition & 0 deletions Src/CSharpier.Cli/CommandLineFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ CancellationToken cancellationToken
codeFormattingResult.Code,
codeFormattingResult.ReorderedModifiers,
codeFormattingResult.ReorderedUsingsWithDisabledText,
codeFormattingResult.MovedTrailingTrivia,
sourceCodeKind,
cancellationToken
);
Expand Down
1 change: 1 addition & 0 deletions Src/CSharpier.Playground/Controllers/FormatController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ CancellationToken cancellationToken
result.Code,
result.ReorderedModifiers,
result.ReorderedUsingsWithDisabledText,
result.MovedTrailingTrivia,
sourceCodeKind,
cancellationToken
);
Expand Down
20 changes: 20 additions & 0 deletions Src/CSharpier.Tests/CommandLineFormatterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,26 @@ public void File_With_Reorder_Modifiers_In_If_Directive_Should_Pass_Validation(s
result.OutputLines.First().Should().StartWith("Formatted 1 files in");
}

[Test]
public void File_With_Added_Trailing_Comma_Before_Comment_Should_Pass_Validation()
{
var context = new TestContext();

var fileContents = """
var someObject = new SomeObject()
{
Property1 = 1,
Property2 = 2 // Trailing Comment
};
""";
context.WhenAFileExists("file1.cs", fileContents);

var result = this.Format(context);

result.ErrorOutputLines.Should().BeEmpty();
result.OutputLines.First().Should().StartWith("Formatted 1 files in");
}

[TestCase(".csharpierrc")]
[TestCase(".csharpierrc.json")]
[TestCase(".csharpierrc.yaml")]
Expand Down
1 change: 1 addition & 0 deletions Src/CSharpier.Tests/FormattingTests/BaseTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ protected async Task RunTest(string fileName, string fileExtension, bool useTabs
normalizedCode,
false,
false,
false,
SourceCodeKind.Regular,
CancellationToken.None
);
Expand Down
1 change: 1 addition & 0 deletions Src/CSharpier.Tests/Samples/Samples.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public static async Task RunTest(string fileName)
result.Code,
false,
false,
false,
SourceCodeKind.Regular,
CancellationToken.None
);
Expand Down
28 changes: 27 additions & 1 deletion Src/CSharpier.Tests/SyntaxNodeComparerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,30 @@ public enum Enum
result.Should().BeEmpty();
}

[Test]
public void CollectionExpression_Works_With_Adding_Comma_Before_Comment()
{
var left = """
var someObject = new SomeObject()
{
Property1 = 1,
Property2 = 2 // Trailing Comment
};
""";

var right = """
var someObject = new SomeObject()
{
Property1 = 1,
Property2 = 2, // Trailing Comment
};
""";

var result = CompareSource(left, right, movedTrailingTrivia: true);

result.Should().BeEmpty();
}

private static void ResultShouldBe(string actual, string expected)
{
actual.ReplaceLineEndings().Should().Be(expected.ReplaceLineEndings());
Expand All @@ -882,14 +906,16 @@ private static void ResultShouldBe(string actual, string expected)
private static string CompareSource(
string left,
string right,
bool reorderedUsingsWithDisabledText = false
bool reorderedUsingsWithDisabledText = false,
bool movedTrailingTrivia = false
)
{
var result = new SyntaxNodeComparer(
left,
right,
false,
reorderedUsingsWithDisabledText,
movedTrailingTrivia,
SourceCodeKind.Regular,
CancellationToken.None
).CompareSource();
Expand Down
3 changes: 3 additions & 0 deletions Src/CSharpier/CSharpFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ bool TryGetCompilationFailure(out CodeFormatterResult compilationResult)
var formattedCode = DocPrinter.DocPrinter.Print(document, printerOptions, lineEnding);
var reorderedModifiers = formattingContext.ReorderedModifiers;
var reorderedUsingsWithDisabledText = formattingContext.ReorderedUsingsWithDisabledText;
var movedTrailingTrivia = formattingContext.MovedTrailingTrivia;

foreach (var symbolSet in PreprocessorSymbols.GetSets(syntaxTree))
{
Expand All @@ -155,6 +156,7 @@ await syntaxTree.GetRootAsync(cancellationToken),
reorderedUsingsWithDisabledText =
reorderedUsingsWithDisabledText
|| formattingContext2.ReorderedUsingsWithDisabledText;
movedTrailingTrivia = movedTrailingTrivia || formattingContext2.MovedTrailingTrivia;
}

return new CodeFormatterResult
Expand All @@ -166,6 +168,7 @@ await syntaxTree.GetRootAsync(cancellationToken),
AST = printerOptions.IncludeAST ? PrintAST(rootNode) : string.Empty,
ReorderedModifiers = reorderedModifiers,
ReorderedUsingsWithDisabledText = reorderedUsingsWithDisabledText,
MovedTrailingTrivia = movedTrailingTrivia,
};
}
catch (InTooDeepException)
Expand Down
1 change: 1 addition & 0 deletions Src/CSharpier/CodeFormatterResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ internal CodeFormatterResult() { }

internal bool ReorderedModifiers { get; init; }
internal bool ReorderedUsingsWithDisabledText { get; init; }
internal bool MovedTrailingTrivia { get; init; }
}
7 changes: 6 additions & 1 deletion Src/CSharpier/SyntaxNodeComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ internal partial class SyntaxNodeComparer
protected SyntaxTree NewSyntaxTree { get; }
protected bool ReorderedModifiers { get; }
protected bool ReorderedUsingsWithDisabledText { get; }
protected bool MovedTrailingTrivia { get; }

private static readonly CompareResult Equal = new();

Expand All @@ -18,6 +19,7 @@ public SyntaxNodeComparer(
string newSourceCode,
bool reorderedModifiers,
bool reorderedUsingsWithDisabledText,
bool movedTrailingTrivia,
SourceCodeKind sourceCodeKind,
CancellationToken cancellationToken
)
Expand All @@ -26,6 +28,7 @@ CancellationToken cancellationToken
this.NewSourceCode = newSourceCode;
this.ReorderedModifiers = reorderedModifiers;
this.ReorderedUsingsWithDisabledText = reorderedUsingsWithDisabledText;
this.MovedTrailingTrivia = movedTrailingTrivia;

var cSharpParseOptions = new CSharpParseOptions(
CSharpFormatter.LanguageVersion,
Expand Down Expand Up @@ -274,7 +277,9 @@ originalToken.Parent is InterpolatedStringExpressionSyntax
return result;
}

var result2 = this.Compare(originalToken.TrailingTrivia, formattedToken.TrailingTrivia);
var result2 = this.MovedTrailingTrivia
? Equal
: this.Compare(originalToken.TrailingTrivia, formattedToken.TrailingTrivia);

return result2.IsInvalid ? result2 : Equal;
}
Expand Down
4 changes: 4 additions & 0 deletions Src/CSharpier/SyntaxPrinter/FormattingContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ internal class FormattingContext
// we also need to keep track if we move around usings with disabledText
public bool ReorderedUsingsWithDisabledText { get; set; }

// when adding a trailing comma in front of a trailing comment it is very hard to determine how to compare
// that trailing comment, so just ignore all trailing trivia
public bool MovedTrailingTrivia { get; set; }

public TrailingCommaContext? TrailingComma { get; set; }

public FormattingContext WithSkipNextLeadingTrivia()
Expand Down
1 change: 1 addition & 0 deletions Src/CSharpier/SyntaxPrinter/Token.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ context.TrailingComma is not null
)
{
docs.Add(context.TrailingComma.PrintedTrailingComma);
context.MovedTrailingTrivia = true;
context.TrailingComma = null;
}

Expand Down

0 comments on commit 576a75b

Please sign in to comment.