From 91827522e18380101c5f786cc781644f51b622de Mon Sep 17 00:00:00 2001 From: Bela VanderVoort Date: Sat, 28 Oct 2023 14:00:11 -0500 Subject: [PATCH] Fixing compare issue --- Src/CSharpier.Cli/CommandLineFormatter.cs | 2 +- .../SyntaxNodeComparerTests.cs | 24 ++++++++----------- Src/CSharpier/CSharpFormatter.cs | 6 ++--- Src/CSharpier/CodeFormatterResult.cs | 2 +- Src/CSharpier/SyntaxNodeComparer.cs | 20 ++++++++++++++++ .../SyntaxPrinter/FormattingContext.cs | 4 +++- Src/CSharpier/SyntaxPrinter/Modifiers.cs | 2 +- .../SyntaxPrinter/UsingDirectives.cs | 13 ++++++++++ 8 files changed, 52 insertions(+), 21 deletions(-) diff --git a/Src/CSharpier.Cli/CommandLineFormatter.cs b/Src/CSharpier.Cli/CommandLineFormatter.cs index b8d509c07..ef3409be9 100644 --- a/Src/CSharpier.Cli/CommandLineFormatter.cs +++ b/Src/CSharpier.Cli/CommandLineFormatter.cs @@ -397,7 +397,7 @@ CancellationToken cancellationToken var syntaxNodeComparer = new SyntaxNodeComparer( fileToFormatInfo.FileContents, codeFormattingResult.Code, - codeFormattingResult.ReorderedModifiers, + codeFormattingResult.IgnoreDisabledText, cancellationToken ); diff --git a/Src/CSharpier.Tests/SyntaxNodeComparerTests.cs b/Src/CSharpier.Tests/SyntaxNodeComparerTests.cs index 602a07e0d..a43b1e68e 100644 --- a/Src/CSharpier.Tests/SyntaxNodeComparerTests.cs +++ b/Src/CSharpier.Tests/SyntaxNodeComparerTests.cs @@ -465,9 +465,9 @@ 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 @@ -475,28 +475,24 @@ public void Usings_With_Directives_Pass_Validation() // 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(); } @@ -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(); diff --git a/Src/CSharpier/CSharpFormatter.cs b/Src/CSharpier/CSharpFormatter.cs index 408e3e7b7..f7c534d12 100644 --- a/Src/CSharpier/CSharpFormatter.cs +++ b/Src/CSharpier/CSharpFormatter.cs @@ -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)) { @@ -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 @@ -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) diff --git a/Src/CSharpier/CodeFormatterResult.cs b/Src/CSharpier/CodeFormatterResult.cs index bd74098b9..3eca17be0 100644 --- a/Src/CSharpier/CodeFormatterResult.cs +++ b/Src/CSharpier/CodeFormatterResult.cs @@ -14,5 +14,5 @@ internal CodeFormatterResult() { } internal static readonly CodeFormatterResult Null = new(); - internal bool ReorderedModifiers { get; init; } + internal bool IgnoreDisabledText { get; init; } } diff --git a/Src/CSharpier/SyntaxNodeComparer.cs b/Src/CSharpier/SyntaxNodeComparer.cs index f0a51f87c..2b3cb437f 100644 --- a/Src/CSharpier/SyntaxNodeComparer.cs +++ b/Src/CSharpier/SyntaxNodeComparer.cs @@ -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 diff --git a/Src/CSharpier/SyntaxPrinter/FormattingContext.cs b/Src/CSharpier/SyntaxPrinter/FormattingContext.cs index 6c7aad0de..a103d29c5 100644 --- a/Src/CSharpier/SyntaxPrinter/FormattingContext.cs +++ b/Src/CSharpier/SyntaxPrinter/FormattingContext.cs @@ -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; } } diff --git a/Src/CSharpier/SyntaxPrinter/Modifiers.cs b/Src/CSharpier/SyntaxPrinter/Modifiers.cs index 290ae595e..79b2ef219 100644 --- a/Src/CSharpier/SyntaxPrinter/Modifiers.cs +++ b/Src/CSharpier/SyntaxPrinter/Modifiers.cs @@ -101,7 +101,7 @@ Func, Doc> print && sortedModifiers.Zip(modifiers, (original, sorted) => original != sorted).Any() ) { - context.ReorderedModifiers = true; + context.IgnoreDisabledText = true; } return print(sortedModifiers.ToArray()); diff --git a/Src/CSharpier/SyntaxPrinter/UsingDirectives.cs b/Src/CSharpier/SyntaxPrinter/UsingDirectives.cs index 1d0c4d4a1..18109c67a 100644 --- a/Src/CSharpier/SyntaxPrinter/UsingDirectives.cs +++ b/Src/CSharpier/SyntaxPrinter/UsingDirectives.cs @@ -84,6 +84,8 @@ bool printExtraLines } var isFirst = true; + var index = 0; + var reorderedDirectives = false; foreach ( var groupOfUsingData in GroupUsings( usingList, @@ -105,6 +107,12 @@ 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)); } @@ -112,6 +120,11 @@ var groupOfUsingData in GroupUsings( } } + if (reorderedDirectives) + { + context.IgnoreDisabledText = true; + } + return Doc.Concat(docs); }