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

Figuring out a way to get syntax tree validation happy when we add a … #1392

Merged
merged 2 commits into from
Nov 22, 2024
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
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
Loading