Skip to content

Commit

Permalink
Fixing compare issue
Browse files Browse the repository at this point in the history
  • Loading branch information
belav committed Oct 28, 2023
1 parent abdb40d commit 9182752
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 21 deletions.
2 changes: 1 addition & 1 deletion Src/CSharpier.Cli/CommandLineFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ CancellationToken cancellationToken
var syntaxNodeComparer = new SyntaxNodeComparer(
fileToFormatInfo.FileContents,
codeFormattingResult.Code,
codeFormattingResult.ReorderedModifiers,
codeFormattingResult.IgnoreDisabledText,
cancellationToken
);

Expand Down
24 changes: 10 additions & 14 deletions Src/CSharpier.Tests/SyntaxNodeComparerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -465,38 +465,34 @@ public void Sorted_Usings_With_Header_Pass_Validation()
result.Should().BeEmpty();
}

[Test]
[Ignore("no clue how to solve this")]
public void Usings_With_Directives_Pass_Validation()
[TestCase("namespace Namespace { }")]
[TestCase("namespace Namespace;")]
public void Usings_With_Directives_Pass_Validation(string content)
{
// The problem is that the #endif leading trivia to the ClassDeclaration
// which then fails the compare
// that class could be an interface, enum, top level statement, etc
// so there doesn't seem to be any good way to handle this
// it will only fail the compare the first time that it sorts, so doesn't seem worth fixing
var left =
@"#if DEBUG
@$"#if DEBUG
using System;
#else
using Microsoft;
#endif
using System.IO;
private class Class { }
{content}
";

var right =
@"using System.IO;
@$"using System.IO;
#if DEBUG
using System;
#else
using Microsoft;
#endif
private class Class { }
{content}
";

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

result.Should().BeEmpty();
}
Expand All @@ -511,12 +507,12 @@ private static void ResultShouldBe(string result, string be)
result.Should().Be(be);
}

private static string CompareSource(string left, string right)
private static string CompareSource(string left, string right, bool ignoreDisabledText = false)
{
var result = new SyntaxNodeComparer(
left,
right,
false,
ignoreDisabledText,
CancellationToken.None
).CompareSource();

Expand Down
6 changes: 3 additions & 3 deletions Src/CSharpier/CSharpFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ bool TryGetCompilationFailure(out CodeFormatterResult compilationResult)
var formattingContext = new FormattingContext { LineEnding = lineEnding };
var document = Node.Print(rootNode, formattingContext);
var formattedCode = DocPrinter.DocPrinter.Print(document, printerOptions, lineEnding);
var reorderedModifiers = formattingContext.ReorderedModifiers;
var ignoreDisabledText = formattingContext.IgnoreDisabledText;

foreach (var symbolSet in PreprocessorSymbols.GetSets(syntaxTree))
{
Expand All @@ -124,7 +124,7 @@ await syntaxTree.GetRootAsync(cancellationToken),
formattingContext2
);
formattedCode = DocPrinter.DocPrinter.Print(document, printerOptions, lineEnding);
reorderedModifiers = reorderedModifiers || formattingContext2.ReorderedModifiers;
ignoreDisabledText = ignoreDisabledText || formattingContext2.IgnoreDisabledText;
}

return new CodeFormatterResult
Expand All @@ -134,7 +134,7 @@ await syntaxTree.GetRootAsync(cancellationToken),
? DocSerializer.Serialize(document)
: string.Empty,
AST = printerOptions.IncludeAST ? PrintAST(rootNode) : string.Empty,
ReorderedModifiers = reorderedModifiers
IgnoreDisabledText = ignoreDisabledText
};
}
catch (InTooDeepException)
Expand Down
2 changes: 1 addition & 1 deletion Src/CSharpier/CodeFormatterResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ internal CodeFormatterResult() { }

internal static readonly CodeFormatterResult Null = new();

internal bool ReorderedModifiers { get; init; }
internal bool IgnoreDisabledText { get; init; }
}
20 changes: 20 additions & 0 deletions Src/CSharpier/SyntaxNodeComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,26 @@ private CompareResult Compare(
SyntaxNode? formattedNode
)
{
if (
this.IgnoreDisabledText
&& (
(
formattedNode is NamespaceDeclarationSyntax nd
&& nd.NamespaceKeyword == formattedToken
)
|| (
formattedNode is FileScopedNamespaceDeclarationSyntax fsnd
&& fsnd.NamespaceKeyword == formattedToken
)
)
)
{
if (formattedNode.GetLeadingTrivia().ToFullString().Contains("#endif"))
{
return Equal;
}
}

// when a verbatim string contains mismatched line endings they will become consistent
// this validation will fail unless we also get them consistent here
// adding a semi-complicated if check to determine when to do the string replacement
Expand Down
4 changes: 3 additions & 1 deletion Src/CSharpier/SyntaxPrinter/FormattingContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,7 @@ internal class FormattingContext

// we need to keep track if we reordered modifiers because when modifiers are moved inside
// of an #if, then we can't compare the before and after disabled text in the source file
public bool ReorderedModifiers { get; set; }
// we also need to keep track if we move around usings, because then the disabled text may end up on
// the first node after the usings, like namespace or class declaration
public bool IgnoreDisabledText { get; set; }
}
2 changes: 1 addition & 1 deletion Src/CSharpier/SyntaxPrinter/Modifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ Func<IReadOnlyList<SyntaxToken>, Doc> print
&& sortedModifiers.Zip(modifiers, (original, sorted) => original != sorted).Any()
)
{
context.ReorderedModifiers = true;
context.IgnoreDisabledText = true;
}

return print(sortedModifiers.ToArray());
Expand Down
13 changes: 13 additions & 0 deletions Src/CSharpier/SyntaxPrinter/UsingDirectives.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ bool printExtraLines
}

var isFirst = true;
var index = 0;
var reorderedDirectives = false;
foreach (
var groupOfUsingData in GroupUsings(
usingList,
Expand All @@ -105,13 +107,24 @@ var groupOfUsingData in GroupUsings(
}
if (usingData.Using is not null)
{
if (usingData.Using != usings[index])
{
reorderedDirectives = true;
}

index++;
docs.Add(UsingDirective.Print(usingData.Using, context, printExtraLines));
}

isFirst = false;
}
}

if (reorderedDirectives)
{
context.IgnoreDisabledText = true;
}

return Doc.Concat(docs);
}

Expand Down

0 comments on commit 9182752

Please sign in to comment.