From 2d26d0f69554ce9b92d9749480112e5b43a07491 Mon Sep 17 00:00:00 2001 From: belav Date: Sun, 15 Aug 2021 18:39:10 -0500 Subject: [PATCH 1/8] Support for basic if/elif directives Adding some basic validation support for disabled text closes #15 --- Src/CSharpier.Benchmarks/Program.cs | 8 +- .../CommandLineFormatterTests.cs | 39 ++++++ .../ConfigurationFileOptionsTests.cs | 27 +++- .../DisabledTextComparerTests.cs | 24 ++++ .../FormattingTests/BaseTest.cs | 3 + Src/CSharpier.Tests/Samples/Samples.cs | 11 ++ Src/CSharpier.Tests/Samples/Scratch.cst | 86 ++++++++++-- .../SyntaxNodeComparerTests.cs | 23 ++++ .../SyntaxPrinter/PreprocessorSymbolsTests.cs | 80 +++++++++++ .../Directives/PreprocessorSymbols.cst | 52 +++++++ .../PreprocessorSymbols.expected.cst | 52 +++++++ Src/CSharpier/CodeFormatter.cs | 48 ++++++- Src/CSharpier/ConfigurationFileOptions.cs | 4 +- Src/CSharpier/DisabledTextComparer.cs | 73 ++++++++++ Src/CSharpier/PrinterOptions.cs | 3 + Src/CSharpier/SyntaxNodeComparer.cs | 19 ++- .../SyntaxPrinter/PreprocessorSymbols.cs | 127 ++++++++++++++++++ Src/CSharpier/SyntaxPrinter/Token.cs | 2 + 18 files changed, 654 insertions(+), 27 deletions(-) create mode 100644 Src/CSharpier.Tests/DisabledTextComparerTests.cs create mode 100644 Src/CSharpier.Tests/SyntaxPrinter/PreprocessorSymbolsTests.cs create mode 100644 Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.cst create mode 100644 Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.expected.cst create mode 100644 Src/CSharpier/DisabledTextComparer.cs create mode 100644 Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs diff --git a/Src/CSharpier.Benchmarks/Program.cs b/Src/CSharpier.Benchmarks/Program.cs index 25943ba07..3c6bfa2ab 100644 --- a/Src/CSharpier.Benchmarks/Program.cs +++ b/Src/CSharpier.Benchmarks/Program.cs @@ -9,7 +9,7 @@ namespace CSharpier.Benchmarks [MemoryDiagnoser] public class Benchmarks { - [Benchmark] + //[Benchmark] public void Default_CodeFormatter() { var codeFormatter = new CodeFormatter(); @@ -23,6 +23,12 @@ public void Default_SyntaxNodeComparer() syntaxNodeComparer.CompareSource(); } + [Benchmark] + public void IsCodeBasicallyEqual_SyntaxNodeComparer() + { + DisabledTextComparer.IsCodeBasicallyEqual(code, code); + } + private string code = @"using System; using System.Collections.Generic; diff --git a/Src/CSharpier.Tests/CommandLineFormatterTests.cs b/Src/CSharpier.Tests/CommandLineFormatterTests.cs index 179343779..4a4db278c 100644 --- a/Src/CSharpier.Tests/CommandLineFormatterTests.cs +++ b/Src/CSharpier.Tests/CommandLineFormatterTests.cs @@ -248,6 +248,45 @@ public void File_With_Mismatched_Line_Endings_In_Verbatim_String_Should_Pass_Val exitCode.Should().Be(0); } + [Test] + public void File_Should_Format_With_Supplied_Symbols() + { + WhenAFileExists(".csharpierrc", @"{ ""preprocessorSymbolSets"": [[""FORMAT""]] }"); + WhenAFileExists( + "file1.cs", + @"public class ClassName +{ +#if FORMAT + public string ShortPropertyName; +#elif NO_FORMAT + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif +} +" + ); + + this.Format(); + + var result = GetFileContent("file1.cs"); + + result.Should() + .Be( + @"public class ClassName +{ +#if FORMAT + public string ShortPropertyName; +#elif NO_FORMAT + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif +} +" + ); + } + private (int exitCode, IList lines) Format( bool skipWrite = false, bool check = false, diff --git a/Src/CSharpier.Tests/ConfigurationFileOptionsTests.cs b/Src/CSharpier.Tests/ConfigurationFileOptionsTests.cs index 5466eeba3..4cccf6eb7 100644 --- a/Src/CSharpier.Tests/ConfigurationFileOptionsTests.cs +++ b/Src/CSharpier.Tests/ConfigurationFileOptionsTests.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using System.IO.Abstractions.TestingHelpers; using FluentAssertions; using NUnit.Framework; @@ -36,22 +37,43 @@ public void Should_Return_Default_Options_With_No_File() [Test] public void Should_Return_Json_Extension_Options() { - WhenAFileExists("c:/test/.csharpierrc.json", "{ \"printWidth\": 10 }"); + WhenAFileExists( + "c:/test/.csharpierrc.json", + @"{ + ""printWidth"": 10, + ""preprocessorSymbolSets"": [[""1"",""2""], [""3""]] +}" + ); var result = CreateConfigurationOptions("c:/test"); result.PrintWidth.Should().Be(10); + result.PreprocessorSymbolSets.Should() + .BeEquivalentTo(new List { new[] { "1", "2" }, new[] { "3" } }); } [TestCase("yaml")] [TestCase("yml")] public void Should_Return_Yaml_Extension_Options(string extension) { - WhenAFileExists($"c:/test/.csharpierrc.{extension}", "printWidth: 10"); + WhenAFileExists( + $"c:/test/.csharpierrc.{extension}", + @" +printWidth: 10 +preprocessorSymbolSets: + - + - 1 + - 2 + - + - 3 +" + ); var result = CreateConfigurationOptions("c:/test"); result.PrintWidth.Should().Be(10); + result.PreprocessorSymbolSets.Should() + .BeEquivalentTo(new List { new[] { "1", "2" }, new[] { "3" } }); } [TestCase("{ \"printWidth\": 10 }")] @@ -115,6 +137,7 @@ public void Should_Return_PrintWidth_With_Yaml() private void ShouldHaveDefaultOptions(ConfigurationFileOptions configurationFileOptions) { configurationFileOptions.PrintWidth.Should().Be(100); + configurationFileOptions.PreprocessorSymbolSets.Should().BeNull(); } private ConfigurationFileOptions CreateConfigurationOptions(string baseDirectoryPath) diff --git a/Src/CSharpier.Tests/DisabledTextComparerTests.cs b/Src/CSharpier.Tests/DisabledTextComparerTests.cs new file mode 100644 index 000000000..f4fbe46ae --- /dev/null +++ b/Src/CSharpier.Tests/DisabledTextComparerTests.cs @@ -0,0 +1,24 @@ +using FluentAssertions; +using NUnit.Framework; + +namespace CSharpier.Tests +{ + [TestFixture] + public class DisabledTextComparerTests + { + [Test] + public void IsCodeBasicallyEqual_Should_Return_True_For_Basic_Case() + { + var before = "public string Tester;"; + + var after = + @"public +string Tester; +""; + + using var stringReader = new StringReader(jsontext);"; + + DisabledTextComparer.IsCodeBasicallyEqual(before, after).Should().BeTrue(); + } + } +} diff --git a/Src/CSharpier.Tests/FormattingTests/BaseTest.cs b/Src/CSharpier.Tests/FormattingTests/BaseTest.cs index d83f34342..4f9c6fb80 100644 --- a/Src/CSharpier.Tests/FormattingTests/BaseTest.cs +++ b/Src/CSharpier.Tests/FormattingTests/BaseTest.cs @@ -3,6 +3,7 @@ using System.IO.Abstractions; using System.Text; using System.Threading; +using CSharpier.SyntaxPrinter; using DiffEngine; using FluentAssertions; using NUnit.Framework; @@ -34,6 +35,8 @@ protected void RunTest(string fileName, bool useTabs = false) var fileReaderResult = FileReader.ReadFile(filePath, new FileSystem(), CancellationToken.None).Result; + PreprocessorSymbols.Reset(); + var formatter = new CodeFormatter(); var result = formatter.Format( fileReaderResult.FileContents, diff --git a/Src/CSharpier.Tests/Samples/Samples.cs b/Src/CSharpier.Tests/Samples/Samples.cs index 66b1b8ac0..a3efeb73d 100644 --- a/Src/CSharpier.Tests/Samples/Samples.cs +++ b/Src/CSharpier.Tests/Samples/Samples.cs @@ -1,5 +1,7 @@ using System.IO; using System.Text; +using System.Threading; +using FluentAssertions; using NUnit.Framework; namespace CSharpier.Tests.Samples @@ -34,6 +36,15 @@ public void RunTest(string fileName) new PrinterOptions { IncludeDocTree = true, IncludeAST = true, } ); + var syntaxNodeComparer = new SyntaxNodeComparer( + code, + result.Code, + CancellationToken.None + ); + + var compareResult = syntaxNodeComparer.CompareSource(); + compareResult.Should().BeEmpty(); + File.WriteAllText(file.Replace(".cst", ".actual.cst"), result.Code, Encoding.UTF8); File.WriteAllText(file.Replace(".cst", ".doctree.txt"), result.DocTree, Encoding.UTF8); } diff --git a/Src/CSharpier.Tests/Samples/Scratch.cst b/Src/CSharpier.Tests/Samples/Scratch.cst index 954e20cb8..a43ffd29e 100644 --- a/Src/CSharpier.Tests/Samples/Scratch.cst +++ b/Src/CSharpier.Tests/Samples/Scratch.cst @@ -1,15 +1,77 @@ - public class FunctionParameter : EntityBase +#region License +// Copyright (c) 2007 James Newton-King +// +// Permission is hereby granted, free of charge, to any person +// obtaining a copy of this software and associated documentation +// files (the "Software"), to deal in the Software without +// restriction, including without limitation the rights to use, +// copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +// OTHER DEALINGS IN THE SOFTWARE. +#endregion + +#if (NET45 || NET50) +#if DNXCORE50 +using Xunit; +using Test = Xunit.FactAttribute; +using Assert = Newtonsoft.Json.Tests.XUnitAssert; +#else +using NUnit.Framework; +#endif +using System.Collections.Generic; +using Newtonsoft.Json.Serialization; +using Newtonsoft.Json.Converters; +using System.Collections; +using System; +using System.IO; +using Newtonsoft.Json.Linq; + +namespace Newtonsoft.Json.Tests.Issues +{ + [TestFixture] + public class Issue2492 { - public Function Function - - [Required] - [UniqueIndex(AdditionalColumns = nameof(Name))] - public Guid FunctionId { get; set; } - - public string Name { get; set; } - - public override string GetDisplayName() + [Test] + public void Test_Object() { - return this.Name; - + string jsontext = @"{ ""ABC"": //DEF +{}}"; + + using var stringReader = new StringReader(jsontext); + using var jsonReader = new JsonTextReader(stringReader); + + JsonSerializer serializer = JsonSerializer.Create(); + var x = serializer.Deserialize(jsonReader); + + Assert.AreEqual(JTokenType.Object, x["ABC"].Type); + } + + [Test] + public void Test_Integer() + { + string jsontext = "{ \"ABC\": /*DEF*/ 1}"; + + using var stringReader = new StringReader(jsontext); + using var jsonReader = new JsonTextReader(stringReader); + + JsonSerializer serializer = JsonSerializer.Create(); + var x = serializer.Deserialize(jsonReader); + + Assert.AreEqual(JTokenType.Integer, x["ABC"].Type); + } } +} +#endif diff --git a/Src/CSharpier.Tests/SyntaxNodeComparerTests.cs b/Src/CSharpier.Tests/SyntaxNodeComparerTests.cs index d251e87b7..a401d57d4 100644 --- a/Src/CSharpier.Tests/SyntaxNodeComparerTests.cs +++ b/Src/CSharpier.Tests/SyntaxNodeComparerTests.cs @@ -292,6 +292,29 @@ public void Mismatched_Line_Endings_In_Verbatim_String_Should_Not_Print_Error(st result.Should().BeEmpty(); } + [Test] + public void Mismatched_Disabled_Text_Should_Not_Print_Error() + { + var left = + @"class ClassName +{ +#if DEBUG + public string Tester; +#endif +}"; + var right = + @"class ClassName +{ +#if DEBUG + public string Tester; +#endif +} +"; + + var result = this.AreEqual(left, right); + result.Should().BeEmpty(); + } + private void ResultShouldBe(string result, string be) { if (Environment.GetEnvironmentVariable("NormalizeLineEndings") != null) diff --git a/Src/CSharpier.Tests/SyntaxPrinter/PreprocessorSymbolsTests.cs b/Src/CSharpier.Tests/SyntaxPrinter/PreprocessorSymbolsTests.cs new file mode 100644 index 000000000..31f6e26b9 --- /dev/null +++ b/Src/CSharpier.Tests/SyntaxPrinter/PreprocessorSymbolsTests.cs @@ -0,0 +1,80 @@ +using System; +using System.Linq; +using CSharpier.SyntaxPrinter; +using FluentAssertions; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using NUnit.Framework; + +namespace CSharpier.Tests.SyntaxPrinter +{ + public class PreprocessorSymbolsTests + { + [TestCase("BASIC_IF", "BASIC_IF")] + [TestCase("!NOT_IF", "NOT_IF")] + [TestCase("EQUALS_TRUE == true", "EQUALS_TRUE")] + [TestCase("EQUALS_FALSE == false", "EQUALS_FALSE")] + [TestCase("true == TRUE_EQUALS", "TRUE_EQUALS")] + [TestCase("false == FALSE_EQUALS", "FALSE_EQUALS")] + [TestCase("LEFT_OR || RIGHT_OR", "LEFT_OR")] + [TestCase("LEFT_AND && RIGHT_AND", "LEFT_AND", "RIGHT_AND")] + [TestCase("(EQUALS_TRUE_IN_PARENS == true)", "EQUALS_TRUE_IN_PARENS")] + public void AddSymbolSet_For_If(string condition, params string[] symbols) + { + var trivia = WhenIfDirectiveHasCondition(condition); + + var result = AddSymbolSet(trivia); + + result.Should().ContainInOrder(symbols); + } + + [Test] + public void AddSymbolSet_For_Basic_Elif_Adds_Symbol() + { + var trivia = WhenElifDirectiveHasCondition("DEBUG"); + + var result = AddSymbolSet(trivia); + + result.Should().ContainInOrder("DEBUG"); + } + + private string[] AddSymbolSet(ConditionalDirectiveTriviaSyntax trivia) + { + TestablePreprocessorSymbols.Reset(); + + TestablePreprocessorSymbols.AddSymbolSet(trivia); + + return TestablePreprocessorSymbols.GetSymbolSets().FirstOrDefault() + ?? Array.Empty(); + } + + private IfDirectiveTriviaSyntax WhenIfDirectiveHasCondition(string condition) + { + return SyntaxFactory.IfDirectiveTrivia( + SyntaxFactory.ParseExpression(condition), + true, + true, + true + ); + } + + private ElifDirectiveTriviaSyntax WhenElifDirectiveHasCondition(string condition) + { + return SyntaxFactory.ElifDirectiveTrivia( + SyntaxFactory.ParseExpression(condition), + true, + true, + true + ); + } + + private class TestablePreprocessorSymbols : PreprocessorSymbols + { + public static void AddSymbolSet( + ConditionalDirectiveTriviaSyntax conditionalDirectiveTriviaSyntax + ) { + AddSymbolSetForConditional(conditionalDirectiveTriviaSyntax); + } + } + } +} diff --git a/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.cst b/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.cst new file mode 100644 index 000000000..e3971c6af --- /dev/null +++ b/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.cst @@ -0,0 +1,52 @@ +public class ClassName +{ +#if BASIC_IF + public string ShortPropertyName; +#elif BASIC_ELIF + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if !NOT_IF + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if EQUALS_TRUE == true + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if true == TRUE_EQUALS + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if NOT_EQUALS_TRUE != true + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if true != TRUE_NOT_EQUALS + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if LEFT_AND && RIGHT_AND + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if (LEFT_PAREN && RIGHT_PAREN) + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif +} \ No newline at end of file diff --git a/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.expected.cst b/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.expected.cst new file mode 100644 index 000000000..1f60bc0b5 --- /dev/null +++ b/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.expected.cst @@ -0,0 +1,52 @@ +public class ClassName +{ +#if BASIC_IF + public string ShortPropertyName; +#elif BASIC_ELIF + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if !NOT_IF + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if EQUALS_TRUE == true + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if true == TRUE_EQUALS + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if NOT_EQUALS_TRUE != true + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if true != TRUE_NOT_EQUALS + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if LEFT_AND && RIGHT_AND + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif + +#if (LEFT_PAREN && RIGHT_PAREN) + public string ShortPropertyName; +#else + public string ShortPropertyName; +#endif +} diff --git a/Src/CSharpier/CodeFormatter.cs b/Src/CSharpier/CodeFormatter.cs index aab9a6862..bd01215f6 100644 --- a/Src/CSharpier/CodeFormatter.cs +++ b/Src/CSharpier/CodeFormatter.cs @@ -24,11 +24,25 @@ public async Task FormatAsync( PrinterOptions printerOptions, CancellationToken cancellationToken ) { - var syntaxTree = CSharpSyntaxTree.ParseText( - code, - new CSharpParseOptions(LanguageVersion.CSharp9, DocumentationMode.Diagnose), - cancellationToken: cancellationToken - ); + SyntaxTree ParseText(string codeToFormat, params string[] preprocessorSymbols) + { + return CSharpSyntaxTree.ParseText( + codeToFormat, + new CSharpParseOptions( + LanguageVersion.CSharp9, + DocumentationMode.Diagnose, + preprocessorSymbols: preprocessorSymbols + ), + cancellationToken: cancellationToken + ); + } + + // if a user supplied symbolSets, then we should start with the first one + var initialSymbolSet = printerOptions.PreprocessorSymbolSets is { Count: > 0 } + ? printerOptions.PreprocessorSymbolSets.First() + : Array.Empty(); + + var syntaxTree = ParseText(code, initialSymbolSet); var syntaxNode = await syntaxTree.GetRootAsync(cancellationToken); if (syntaxNode is not CompilationUnitSyntax rootNode) { @@ -57,6 +71,16 @@ CancellationToken cancellationToken try { + if (printerOptions.PreprocessorSymbolSets is { Count: > 0 }) + { + PreprocessorSymbols.StopCollecting(); + PreprocessorSymbols.SetSymbolSets( + // we already formatted with the first set above + printerOptions.PreprocessorSymbolSets.Skip(1).ToList() + ); + } + + PreprocessorSymbols.Reset(); var document = Node.Print(rootNode); var lineEnding = GetLineEnding(code, printerOptions); var formattedCode = DocPrinter.DocPrinter.Print( @@ -64,6 +88,20 @@ CancellationToken cancellationToken printerOptions, lineEnding ); + + PreprocessorSymbols.StopCollecting(); + foreach (var symbolSet in PreprocessorSymbols.GetSymbolSets()) + { + syntaxTree = ParseText(formattedCode, symbolSet); + + document = Node.Print(await syntaxTree.GetRootAsync(cancellationToken)); + formattedCode = DocPrinter.DocPrinter.Print( + document, + printerOptions, + lineEnding + ); + } + return new CSharpierResult { Code = formattedCode, diff --git a/Src/CSharpier/ConfigurationFileOptions.cs b/Src/CSharpier/ConfigurationFileOptions.cs index 3510bef88..8b6fb4459 100644 --- a/Src/CSharpier/ConfigurationFileOptions.cs +++ b/Src/CSharpier/ConfigurationFileOptions.cs @@ -13,6 +13,7 @@ namespace CSharpier public class ConfigurationFileOptions { public int PrintWidth { get; init; } = 100; + public List? PreprocessorSymbolSets { get; init; } private static string[] validExtensions = { ".csharpierrc", ".json", ".yml", ".yaml" }; @@ -27,7 +28,8 @@ IFileSystem fileSystem TabWidth = 4, UseTabs = false, Width = configurationFileOptions.PrintWidth, - EndOfLine = EndOfLine.Auto + EndOfLine = EndOfLine.Auto, + PreprocessorSymbolSets = configurationFileOptions.PreprocessorSymbolSets ?? new(), }; } diff --git a/Src/CSharpier/DisabledTextComparer.cs b/Src/CSharpier/DisabledTextComparer.cs new file mode 100644 index 000000000..7c9305c5b --- /dev/null +++ b/Src/CSharpier/DisabledTextComparer.cs @@ -0,0 +1,73 @@ +using System.Text; + +namespace CSharpier +{ + public class DisabledTextComparer + { + public static bool IsCodeBasicallyEqual(string code, string formattedCode) + { + var squashCode = Squash(code); + var squashFormattedCode = Squash(formattedCode); + + var result = squashCode == squashFormattedCode; + return result; + } + + private static string Squash(string code) + { + var result = new StringBuilder(); + for (var index = 0; index < code.Length; index++) + { + var nextChar = code[index]; + if (nextChar is ' ' or '\t' or '\r' or '\n') + { + if ( + index != code.Length - 1 + && ( + result.Length == 0 + || result[^1] + is not (' ' + or '(' + or ')' + or '{' + or '}' + or '[' + or ']' + or '<' + or '>' + or ',' + or ':' + or ';') + ) + ) { + result.Append(' '); + } + } + else if ( + nextChar + is '(' + or ')' + or '{' + or '}' + or ']' + or '[' + or '.' + or '<' + or '>' + or ';' + or ':' + && result.Length > 0 + && result[^1] is ' ' + ) { + result[^1] = nextChar; + } + else + { + result.Append(nextChar); + } + } + + return result.ToString(); + } + } +} diff --git a/Src/CSharpier/PrinterOptions.cs b/Src/CSharpier/PrinterOptions.cs index cfc319fad..fb95b926b 100644 --- a/Src/CSharpier/PrinterOptions.cs +++ b/Src/CSharpier/PrinterOptions.cs @@ -1,3 +1,5 @@ +using System.Collections.Generic; + namespace CSharpier { public class PrinterOptions @@ -9,6 +11,7 @@ public class PrinterOptions public int Width { get; init; } = 100; public EndOfLine EndOfLine { get; init; } = EndOfLine.Auto; public bool TrimInitialLines { get; init; } = true; + public List? PreprocessorSymbolSets { get; init; } public const int WidthUsedByTests = 100; } diff --git a/Src/CSharpier/SyntaxNodeComparer.cs b/Src/CSharpier/SyntaxNodeComparer.cs index 1e17f38fd..3eccf9d1a 100644 --- a/Src/CSharpier/SyntaxNodeComparer.cs +++ b/Src/CSharpier/SyntaxNodeComparer.cs @@ -1,13 +1,10 @@ using System; using System.Collections.Generic; using System.IO; -using System.Linq; using System.Threading; using System.Threading.Tasks; -using CSharpier.Utilities; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Text; namespace CSharpier @@ -220,12 +217,22 @@ private CompareResult Compare( private CompareResult Compare(SyntaxTrivia originalTrivia, SyntaxTrivia formattedTrivia) { - if (originalTrivia.ToString().TrimEnd() != formattedTrivia.ToString().TrimEnd()) + // when we encounter disabled text, it is inside an #if when the condition on the #if is false + // we format all that code by doing multiple passes of csharpier + // this comparer is very slow, and running it after every pass on a file would take a long time + // so we do this quick version of validating that the code is basically the same besides + // line breaks and spaces + if (originalTrivia.Kind() is SyntaxKind.DisabledTextTrivia) { - return NotEqual(originalTrivia.Span, formattedTrivia.Span); + return DisabledTextComparer.IsCodeBasicallyEqual( + originalTrivia.ToString(), + formattedTrivia.ToString() + ) ? Equal : NotEqual(originalTrivia.Span, formattedTrivia.Span); } - return Equal; + return originalTrivia.ToString().TrimEnd() == formattedTrivia.ToString().TrimEnd() + ? Equal + : NotEqual(originalTrivia.Span, formattedTrivia.Span); } private CompareResult Compare(SyntaxTriviaList originalList, SyntaxTriviaList formattedList) diff --git a/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs b/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs new file mode 100644 index 000000000..f0ad30f74 --- /dev/null +++ b/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs @@ -0,0 +1,127 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace CSharpier.SyntaxPrinter +{ + // TODO these files fail validation + // \Newtonsoft.Json\Src\Newtonsoft.Json\Serialization\JsonTypeReflector.cs + // \aspnetcore\src\Razor\Microsoft.AspNetCore.Razor.Language\src\Legacy\CSharpLanguageCharacteristics.cs + // \runtime\src\libraries\Common\src\Microsoft\Win32\SafeHandles\SafeX509Handles.Unix.cs + // \runtime\src\libraries\System.Text.RegularExpressions\src\System\Text\RegularExpressions\RegexBoyerMoore.cs + + // plus some files fail to compile, compare that list + + public class PreprocessorSymbols + { + [ThreadStatic] + protected static List? SymbolSets; + + [ThreadStatic] + private static bool doneCollecting; + + public static void StopCollecting() + { + doneCollecting = true; + } + + public static void Reset() + { + SymbolSets?.Clear(); + doneCollecting = false; + } + + public static void SetSymbolSets(List value) + { + SymbolSets = value; + } + + public static void AddSymbolSet(SyntaxTrivia trivia) + { + if (doneCollecting) + { + return; + } + + var kind = trivia.Kind(); + + // TODO == false + // TODO ! + // TODO || + // TODO && + // TODO ( ) + // TODO complex conditions + // TODO nested directives + if (kind is SyntaxKind.IfDirectiveTrivia or SyntaxKind.ElifDirectiveTrivia) + { + var ifDirectiveTriviaSyntax = + trivia.GetStructure() as ConditionalDirectiveTriviaSyntax; + AddSymbolSetForConditional(ifDirectiveTriviaSyntax!); + } + } + + protected static void AddSymbolSetForConditional( + ConditionalDirectiveTriviaSyntax conditionalDirectiveTriviaSyntax + ) { + var stack = new Stack(); + stack.Push(conditionalDirectiveTriviaSyntax.Condition); + + List? symbolSet = null; + + while (stack.Count > 0) + { + var condition = stack.Pop(); + + if (condition is ParenthesizedExpressionSyntax parenthesizedExpressionSyntax) + { + stack.Push(parenthesizedExpressionSyntax.Expression); + } + else if (condition is PrefixUnaryExpressionSyntax prefixUnaryExpressionSyntax) + { + stack.Push(prefixUnaryExpressionSyntax.Operand); + } + else if (condition is BinaryExpressionSyntax binaryExpressionSyntax) + { + stack.Push(binaryExpressionSyntax.Left); + stack.Push(binaryExpressionSyntax.Right); + } + else if (condition is IdentifierNameSyntax identifierNameSyntax) + { + symbolSet ??= new List(); + + if (!symbolSet.Contains(identifierNameSyntax.Identifier.Text)) + { + symbolSet.Add(identifierNameSyntax.Identifier.Text); + } + } + } + + if (symbolSet != null) + { + SymbolSets ??= new(); + var orderedSymbols = symbolSet.OrderBy(o => o).ToArray(); + if (!SymbolSets.Any(o => o.SequenceEqual(orderedSymbols))) + { + SymbolSets.Add(orderedSymbols); + } + } + } + + public static IEnumerable GetSymbolSets() + { + if (SymbolSets == null) + { + yield return Array.Empty(); + yield break; + } + + foreach (var symbol in SymbolSets) + { + yield return symbol; + } + } + } +} diff --git a/Src/CSharpier/SyntaxPrinter/Token.cs b/Src/CSharpier/SyntaxPrinter/Token.cs index 424e9fab0..405120bcb 100644 --- a/Src/CSharpier/SyntaxPrinter/Token.cs +++ b/Src/CSharpier/SyntaxPrinter/Token.cs @@ -182,6 +182,8 @@ private static Doc PrivatePrintLeadingTrivia( Doc.HardLineSkipBreakIfFirstInGroup ); } + + PreprocessorSymbols.AddSymbolSet(trivia); } if (skipLastHardline && docs.Any() && docs.Last() is HardLine) From 7d8013370d0fc4e621ae59c989e4876c349cd279 Mon Sep 17 00:00:00 2001 From: belav Date: Thu, 19 Aug 2021 21:27:55 -0500 Subject: [PATCH 2/8] Fixing unit tests --- Src/CSharpier.Tests/DisabledTextComparerTests.cs | 4 +--- Src/CSharpier/CodeFormatter.cs | 5 ++++- Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs | 7 ------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/Src/CSharpier.Tests/DisabledTextComparerTests.cs b/Src/CSharpier.Tests/DisabledTextComparerTests.cs index f4fbe46ae..3b0b28e40 100644 --- a/Src/CSharpier.Tests/DisabledTextComparerTests.cs +++ b/Src/CSharpier.Tests/DisabledTextComparerTests.cs @@ -14,9 +14,7 @@ public void IsCodeBasicallyEqual_Should_Return_True_For_Basic_Case() var after = @"public string Tester; -""; - - using var stringReader = new StringReader(jsontext);"; +"; DisabledTextComparer.IsCodeBasicallyEqual(before, after).Should().BeTrue(); } diff --git a/Src/CSharpier/CodeFormatter.cs b/Src/CSharpier/CodeFormatter.cs index bd01215f6..3272dcdeb 100644 --- a/Src/CSharpier/CodeFormatter.cs +++ b/Src/CSharpier/CodeFormatter.cs @@ -79,8 +79,11 @@ SyntaxTree ParseText(string codeToFormat, params string[] preprocessorSymbols) printerOptions.PreprocessorSymbolSets.Skip(1).ToList() ); } + else + { + PreprocessorSymbols.Reset(); + } - PreprocessorSymbols.Reset(); var document = Node.Print(rootNode); var lineEnding = GetLineEnding(code, printerOptions); var formattedCode = DocPrinter.DocPrinter.Print( diff --git a/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs b/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs index f0ad30f74..bb90546c0 100644 --- a/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs +++ b/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs @@ -48,13 +48,6 @@ public static void AddSymbolSet(SyntaxTrivia trivia) var kind = trivia.Kind(); - // TODO == false - // TODO ! - // TODO || - // TODO && - // TODO ( ) - // TODO complex conditions - // TODO nested directives if (kind is SyntaxKind.IfDirectiveTrivia or SyntaxKind.ElifDirectiveTrivia) { var ifDirectiveTriviaSyntax = From 2c21b129b3b41d5f142b7e488f4709359ed3b24e Mon Sep 17 00:00:00 2001 From: belav Date: Thu, 19 Aug 2021 23:10:03 -0500 Subject: [PATCH 3/8] Making some progress on edge cases --- .../CommandLineFormatterTests.cs | 27 +++++- .../DisabledTextComparerTests.cs | 93 +++++++++++++++++++ Src/CSharpier/CodeFormatter.cs | 36 +++++-- Src/CSharpier/DisabledTextComparer.cs | 59 ++++++++---- Src/CSharpier/SyntaxNodeComparer.cs | 5 - .../SyntaxPrinter/PreprocessorSymbols.cs | 13 +-- 6 files changed, 191 insertions(+), 42 deletions(-) diff --git a/Src/CSharpier.Tests/CommandLineFormatterTests.cs b/Src/CSharpier.Tests/CommandLineFormatterTests.cs index 4a4db278c..255e10723 100644 --- a/Src/CSharpier.Tests/CommandLineFormatterTests.cs +++ b/Src/CSharpier.Tests/CommandLineFormatterTests.cs @@ -30,9 +30,9 @@ public void Format_Writes_Failed_To_Compile() { WhenAFileExists("Invalid.cs", "asdfasfasdf"); - var result = this.Format(); + var (_, lines) = this.Format(); - result.lines.First() + lines.First() .Should() .Be( $"Warn {Path.DirectorySeparatorChar}Invalid.cs - Failed to compile so was not formatted." @@ -248,6 +248,29 @@ public void File_With_Mismatched_Line_Endings_In_Verbatim_String_Should_Pass_Val exitCode.Should().Be(0); } + [Test] + public void File_With_Compilation_Error_In_If_Should_Not_Lose_Code() + { + var contents = + @"#if DEBUG +?using System; +#endif +"; + WhenAFileExists("Invalid.cs", contents); + + var (_, lines) = this.Format(); + + var result = GetFileContent("Invalid.cs"); + + result.Should().Be(contents); + + lines.First() + .Should() + .Be( + $"Warn {Path.DirectorySeparatorChar}Invalid.cs - Failed to compile so was not formatted." + ); + } + [Test] public void File_Should_Format_With_Supplied_Symbols() { diff --git a/Src/CSharpier.Tests/DisabledTextComparerTests.cs b/Src/CSharpier.Tests/DisabledTextComparerTests.cs index 3b0b28e40..3dcba1bcb 100644 --- a/Src/CSharpier.Tests/DisabledTextComparerTests.cs +++ b/Src/CSharpier.Tests/DisabledTextComparerTests.cs @@ -18,5 +18,98 @@ public void IsCodeBasicallyEqual_Should_Return_True_For_Basic_Case() DisabledTextComparer.IsCodeBasicallyEqual(before, after).Should().BeTrue(); } + + [Test] + public void Squash_Should_Work_With_Pointer_Stuff() + { + var before = + @" [MethodImpl (MethodImplOptions.InternalCall)] + private static unsafe extern void ApplyUpdate_internal (IntPtr base_assm, byte* dmeta_bytes, int dmeta_length, byte *dil_bytes, int dil_length, byte *dpdb_bytes, int dpdb_length);"; + + var after = + @"[MethodImpl(MethodImplOptions.InternalCall)] + private static unsafe extern void ApplyUpdate_internal( + IntPtr base_assm, + byte* dmeta_bytes, + int dmeta_length, + byte* dil_bytes, + int dil_length, + byte* dpdb_bytes, + int dpdb_length + ); +"; + Squash(before).Should().Be(Squash(after)); + } + + [Test] + public void Squash_Should_Work_With_Commas() + { + var before = + @" + TypeBuilder typeBuilder = moduleBuilder.DefineType(assemblyName.FullName + , TypeAttributes.Public | + TypeAttributes.Class | + TypeAttributes.AutoClass | + TypeAttributes.AnsiClass | + TypeAttributes.BeforeFieldInit | + TypeAttributes.AutoLayout + , null); +"; + + var after = + @" +TypeBuilder typeBuilder = moduleBuilder.DefineType( + assemblyName.FullName, + TypeAttributes.Public + | TypeAttributes.Class + | TypeAttributes.AutoClass + | TypeAttributes.AnsiClass + | TypeAttributes.BeforeFieldInit + | TypeAttributes.AutoLayout, + null + ); +"; + Squash(before).Should().Be(Squash(after)); + } + + [Test] + public void Squash_Should_Work_With_Period() + { + var before = + @" + var options2 = (ProxyGenerationOptions)proxy.GetType(). + GetField(""proxyGenerationOptions"", BindingFlags.Static | BindingFlags.NonPublic).GetValue(null); +"; + + var after = + @" + var options2 = (ProxyGenerationOptions)proxy.GetType() + .GetField(""proxyGenerationOptions"", BindingFlags.Static | BindingFlags.NonPublic) + .GetValue(null); +"; + Squash(before).Should().Be(Squash(after)); + } + + [Test] + public void Squash_Should_Work_With_Starting_Indent() + { + var before = @"array = new ulong[] { (ulong)dy.Property_ulong };"; + + var after = @" array = new ulong[] { (ulong)dy.Property_ulong };"; + Squash(before).Should().Be(Squash(after)); + } + + private static string Squash(string value) + { + return TestableDisabledTextComparer.TestSquash(value); + } + + private class TestableDisabledTextComparer : DisabledTextComparer + { + public static string TestSquash(string value) + { + return Squash(value); + } + } } } diff --git a/Src/CSharpier/CodeFormatter.cs b/Src/CSharpier/CodeFormatter.cs index 3272dcdeb..c206c2da6 100644 --- a/Src/CSharpier/CodeFormatter.cs +++ b/Src/CSharpier/CodeFormatter.cs @@ -56,17 +56,30 @@ SyntaxTree ParseText(string codeToFormat, params string[] preprocessorSymbols) return new CSharpierResult { Code = code }; } - var diagnostics = syntaxTree.GetDiagnostics(cancellationToken) - .Where(o => o.Severity == DiagnosticSeverity.Error && o.Id != "CS1029") - .ToList(); - if (diagnostics.Any()) + bool TryGetCompilationFailure(out CSharpierResult? compilationResult) { - return new CSharpierResult + var diagnostics = syntaxTree!.GetDiagnostics(cancellationToken) + .Where(o => o.Severity == DiagnosticSeverity.Error && o.Id != "CS1029") + .ToList(); + if (diagnostics.Any()) { - Code = code, - Errors = diagnostics, - AST = printerOptions.IncludeAST ? this.PrintAST(rootNode) : string.Empty - }; + compilationResult = new CSharpierResult + { + Code = code, + Errors = diagnostics, + AST = printerOptions.IncludeAST ? this.PrintAST(rootNode) : string.Empty + }; + + return true; + } + + compilationResult = null; + return false; + } + + if (TryGetCompilationFailure(out var result)) + { + return result!; } try @@ -97,6 +110,11 @@ SyntaxTree ParseText(string codeToFormat, params string[] preprocessorSymbols) { syntaxTree = ParseText(formattedCode, symbolSet); + if (TryGetCompilationFailure(out result)) + { + return result!; + } + document = Node.Print(await syntaxTree.GetRootAsync(cancellationToken)); formattedCode = DocPrinter.DocPrinter.Print( document, diff --git a/Src/CSharpier/DisabledTextComparer.cs b/Src/CSharpier/DisabledTextComparer.cs index 7c9305c5b..73806549f 100644 --- a/Src/CSharpier/DisabledTextComparer.cs +++ b/Src/CSharpier/DisabledTextComparer.cs @@ -4,6 +4,11 @@ namespace CSharpier { public class DisabledTextComparer { + // when we encounter disabled text, it is inside an #if when the condition on the #if is false + // we format all that code by doing multiple passes of csharpier + // SyntaxNodeComparer is very slow, and running it after every pass on a file would take a long time + // so we do this quick version of validating that the code is basically the same besides + // line breaks and spaces public static bool IsCodeBasicallyEqual(string code, string formattedCode) { var squashCode = Squash(code); @@ -13,7 +18,7 @@ public static bool IsCodeBasicallyEqual(string code, string formattedCode) return result; } - private static string Squash(string code) + protected static string Squash(string code) { var result = new StringBuilder(); for (var index = 0; index < code.Length; index++) @@ -22,23 +27,31 @@ private static string Squash(string code) if (nextChar is ' ' or '\t' or '\r' or '\n') { if ( - index != code.Length - 1 - && ( - result.Length == 0 - || result[^1] - is not (' ' - or '(' - or ')' - or '{' - or '}' - or '[' - or ']' - or '<' - or '>' - or ',' - or ':' - or ';') - ) + result.Length > 0 + && index != code.Length - 1 + && result[^1] + is not (' ' + or '(' + or ')' + or '{' + or '}' + or '[' + or ']' + or '.' + or ',' + or '*' + or '+' + or '-' + or '=' + or '/' + or '%' + or '|' + or '&' + or '!' + or '<' + or '>' + or ':' + or ';') ) { result.Append(' '); } @@ -52,6 +65,16 @@ is not (' ' or ']' or '[' or '.' + or ',' + or '*' + or '+' + or '-' + or '=' + or '/' + or '%' + or '|' + or '&' + or '!' or '<' or '>' or ';' diff --git a/Src/CSharpier/SyntaxNodeComparer.cs b/Src/CSharpier/SyntaxNodeComparer.cs index 3eccf9d1a..6583948b2 100644 --- a/Src/CSharpier/SyntaxNodeComparer.cs +++ b/Src/CSharpier/SyntaxNodeComparer.cs @@ -217,11 +217,6 @@ private CompareResult Compare( private CompareResult Compare(SyntaxTrivia originalTrivia, SyntaxTrivia formattedTrivia) { - // when we encounter disabled text, it is inside an #if when the condition on the #if is false - // we format all that code by doing multiple passes of csharpier - // this comparer is very slow, and running it after every pass on a file would take a long time - // so we do this quick version of validating that the code is basically the same besides - // line breaks and spaces if (originalTrivia.Kind() is SyntaxKind.DisabledTextTrivia) { return DisabledTextComparer.IsCodeBasicallyEqual( diff --git a/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs b/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs index bb90546c0..b01d5df76 100644 --- a/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs +++ b/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs @@ -7,14 +7,11 @@ namespace CSharpier.SyntaxPrinter { - // TODO these files fail validation - // \Newtonsoft.Json\Src\Newtonsoft.Json\Serialization\JsonTypeReflector.cs - // \aspnetcore\src\Razor\Microsoft.AspNetCore.Razor.Language\src\Legacy\CSharpLanguageCharacteristics.cs - // \runtime\src\libraries\Common\src\Microsoft\Win32\SafeHandles\SafeX509Handles.Unix.cs - // \runtime\src\libraries\System.Text.RegularExpressions\src\System\Text\RegularExpressions\RegexBoyerMoore.cs - - // plus some files fail to compile, compare that list - + // the usage or this is a little wonky right now. + // the next iteration of this should find all #if and relates directives + // before we ever format a file + // come up with the symbol sets, and use those in code formatter + // that will get rid of the ugly threadStatic/reset/stopCollecting stuff public class PreprocessorSymbols { [ThreadStatic] From 75f8099c2b6d25e87e930a0b79336c0034341c42 Mon Sep 17 00:00:00 2001 From: belav Date: Sat, 21 Aug 2021 13:41:41 -0500 Subject: [PATCH 4/8] Self code review --- Src/CSharpier.Benchmarks/Program.cs | 2 +- .../FormattingTests/BaseTest.cs | 12 ++++++----- .../Directives/PreprocessorSymbols.cst | 2 +- Src/CSharpier/CodeFormatter.cs | 20 ++++++++++--------- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/Src/CSharpier.Benchmarks/Program.cs b/Src/CSharpier.Benchmarks/Program.cs index 3c6bfa2ab..3c71d1965 100644 --- a/Src/CSharpier.Benchmarks/Program.cs +++ b/Src/CSharpier.Benchmarks/Program.cs @@ -9,7 +9,7 @@ namespace CSharpier.Benchmarks [MemoryDiagnoser] public class Benchmarks { - //[Benchmark] + [Benchmark] public void Default_CodeFormatter() { var codeFormatter = new CodeFormatter(); diff --git a/Src/CSharpier.Tests/FormattingTests/BaseTest.cs b/Src/CSharpier.Tests/FormattingTests/BaseTest.cs index 4f9c6fb80..0cb826d1a 100644 --- a/Src/CSharpier.Tests/FormattingTests/BaseTest.cs +++ b/Src/CSharpier.Tests/FormattingTests/BaseTest.cs @@ -40,7 +40,7 @@ protected void RunTest(string fileName, bool useTabs = false) var formatter = new CodeFormatter(); var result = formatter.Format( fileReaderResult.FileContents, - new PrinterOptions() { Width = PrinterOptions.WidthUsedByTests, UseTabs = useTabs } + new PrinterOptions { Width = PrinterOptions.WidthUsedByTests, UseTabs = useTabs } ); var actualFilePath = filePath.Replace(".cst", ".actual.cst"); @@ -57,26 +57,28 @@ protected void RunTest(string fileName, bool useTabs = false) filePathToChange = expectedFilePath; } + var normalizedCode = result.Code; + if (Environment.GetEnvironmentVariable("NormalizeLineEndings") != null) { expectedCode = expectedCode.Replace("\r\n", "\n"); - result.Code = result.Code.Replace("\r\n", "\n"); + normalizedCode = normalizedCode.Replace("\r\n", "\n"); } var comparer = new SyntaxNodeComparer( expectedCode, - result.Code, + normalizedCode, CancellationToken.None ); result.Errors.Should().BeEmpty(); result.FailureMessage.Should().BeEmpty(); - if (result.Code != expectedCode && !BuildServerDetector.Detected) + if (normalizedCode != expectedCode && !BuildServerDetector.Detected) { DiffRunner.Launch(filePathToChange, actualFilePath); } - result.Code.Should().Be(expectedCode); + normalizedCode.Should().Be(expectedCode); var compareResult = comparer.CompareSource(); compareResult.Should().BeNullOrEmpty(); diff --git a/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.cst b/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.cst index e3971c6af..e23358aab 100644 --- a/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.cst +++ b/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.cst @@ -49,4 +49,4 @@ public class ClassName #else public string ShortPropertyName; #endif -} \ No newline at end of file +} diff --git a/Src/CSharpier/CodeFormatter.cs b/Src/CSharpier/CodeFormatter.cs index c206c2da6..ac2b5c3f9 100644 --- a/Src/CSharpier/CodeFormatter.cs +++ b/Src/CSharpier/CodeFormatter.cs @@ -56,7 +56,7 @@ SyntaxTree ParseText(string codeToFormat, params string[] preprocessorSymbols) return new CSharpierResult { Code = code }; } - bool TryGetCompilationFailure(out CSharpierResult? compilationResult) + bool TryGetCompilationFailure(out CSharpierResult compilationResult) { var diagnostics = syntaxTree!.GetDiagnostics(cancellationToken) .Where(o => o.Severity == DiagnosticSeverity.Error && o.Id != "CS1029") @@ -73,13 +73,13 @@ bool TryGetCompilationFailure(out CSharpierResult? compilationResult) return true; } - compilationResult = null; + compilationResult = CSharpierResult.Null; return false; } if (TryGetCompilationFailure(out var result)) { - return result!; + return result; } try @@ -112,7 +112,7 @@ bool TryGetCompilationFailure(out CSharpierResult? compilationResult) if (TryGetCompilationFailure(out result)) { - return result!; + return result; } document = Node.Print(await syntaxTree.GetRootAsync(cancellationToken)); @@ -174,11 +174,13 @@ private string PrintAST(CompilationUnitSyntax rootNode) public class CSharpierResult { - public string Code { get; set; } = string.Empty; - public string DocTree { get; set; } = string.Empty; - public string AST { get; set; } = string.Empty; - public IEnumerable Errors { get; set; } = Enumerable.Empty(); + public string Code { get; init; } = string.Empty; + public string DocTree { get; init; } = string.Empty; + public string AST { get; init; } = string.Empty; + public IEnumerable Errors { get; init; } = Enumerable.Empty(); - public string FailureMessage { get; set; } = string.Empty; + public string FailureMessage { get; init; } = string.Empty; + + public static readonly CSharpierResult Null = new(); } } From 5499caaa70217d682068bdda089af0485911a2b0 Mon Sep 17 00:00:00 2001 From: belav Date: Sun, 22 Aug 2021 10:27:42 -0500 Subject: [PATCH 5/8] Change format of PreprocessorSymbols --- .../CommandLineFormatterTests.cs | 2 +- .../ConfigurationFileOptionsTests.cs | 15 +++++-------- Src/CSharpier/ConfigurationFileOptions.cs | 22 +++++++++++++++++-- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/Src/CSharpier.Tests/CommandLineFormatterTests.cs b/Src/CSharpier.Tests/CommandLineFormatterTests.cs index 255e10723..296509016 100644 --- a/Src/CSharpier.Tests/CommandLineFormatterTests.cs +++ b/Src/CSharpier.Tests/CommandLineFormatterTests.cs @@ -274,7 +274,7 @@ public void File_With_Compilation_Error_In_If_Should_Not_Lose_Code() [Test] public void File_Should_Format_With_Supplied_Symbols() { - WhenAFileExists(".csharpierrc", @"{ ""preprocessorSymbolSets"": [[""FORMAT""]] }"); + WhenAFileExists(".csharpierrc", @"{ ""preprocessorSymbolSets"": [""FORMAT""] }"); WhenAFileExists( "file1.cs", @"public class ClassName diff --git a/Src/CSharpier.Tests/ConfigurationFileOptionsTests.cs b/Src/CSharpier.Tests/ConfigurationFileOptionsTests.cs index 4cccf6eb7..1606a555c 100644 --- a/Src/CSharpier.Tests/ConfigurationFileOptionsTests.cs +++ b/Src/CSharpier.Tests/ConfigurationFileOptionsTests.cs @@ -41,15 +41,14 @@ public void Should_Return_Json_Extension_Options() "c:/test/.csharpierrc.json", @"{ ""printWidth"": 10, - ""preprocessorSymbolSets"": [[""1"",""2""], [""3""]] + ""preprocessorSymbolSets"": [""1,2"", ""3""] }" ); var result = CreateConfigurationOptions("c:/test"); result.PrintWidth.Should().Be(10); - result.PreprocessorSymbolSets.Should() - .BeEquivalentTo(new List { new[] { "1", "2" }, new[] { "3" } }); + result.PreprocessorSymbolSets.Should().BeEquivalentTo(new List { "1,2", "3" }); } [TestCase("yaml")] @@ -61,19 +60,15 @@ public void Should_Return_Yaml_Extension_Options(string extension) @" printWidth: 10 preprocessorSymbolSets: - - - - 1 - - 2 - - - - 3 + - 1,2 + - 3 " ); var result = CreateConfigurationOptions("c:/test"); result.PrintWidth.Should().Be(10); - result.PreprocessorSymbolSets.Should() - .BeEquivalentTo(new List { new[] { "1", "2" }, new[] { "3" } }); + result.PreprocessorSymbolSets.Should().BeEquivalentTo(new List { "1,2", "3" }); } [TestCase("{ \"printWidth\": 10 }")] diff --git a/Src/CSharpier/ConfigurationFileOptions.cs b/Src/CSharpier/ConfigurationFileOptions.cs index 8b6fb4459..145e8fe2f 100644 --- a/Src/CSharpier/ConfigurationFileOptions.cs +++ b/Src/CSharpier/ConfigurationFileOptions.cs @@ -13,7 +13,7 @@ namespace CSharpier public class ConfigurationFileOptions { public int PrintWidth { get; init; } = 100; - public List? PreprocessorSymbolSets { get; init; } + public List? PreprocessorSymbolSets { get; init; } private static string[] validExtensions = { ".csharpierrc", ".json", ".yml", ".yaml" }; @@ -23,13 +23,31 @@ IFileSystem fileSystem ) { var configurationFileOptions = Create(baseDirectoryPath, fileSystem); + List preprocessorSymbolSets; + if (configurationFileOptions.PreprocessorSymbolSets == null) + { + preprocessorSymbolSets = new(); + } + else + { + preprocessorSymbolSets = configurationFileOptions.PreprocessorSymbolSets.Select( + o => + o.Split( + ",", + StringSplitOptions.RemoveEmptyEntries + | StringSplitOptions.TrimEntries + ) + ) + .ToList(); + } + return new PrinterOptions { TabWidth = 4, UseTabs = false, Width = configurationFileOptions.PrintWidth, EndOfLine = EndOfLine.Auto, - PreprocessorSymbolSets = configurationFileOptions.PreprocessorSymbolSets ?? new(), + PreprocessorSymbolSets = preprocessorSymbolSets, }; } From ab63d3d0c4fe84d97d97f2cd5db681ba11311446 Mon Sep 17 00:00:00 2001 From: belav Date: Sun, 22 Aug 2021 10:30:24 -0500 Subject: [PATCH 6/8] Self code review --- Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs b/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs index b01d5df76..045093bd5 100644 --- a/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs +++ b/Src/CSharpier/SyntaxPrinter/PreprocessorSymbols.cs @@ -8,9 +8,10 @@ namespace CSharpier.SyntaxPrinter { // the usage or this is a little wonky right now. - // the next iteration of this should find all #if and relates directives - // before we ever format a file - // come up with the symbol sets, and use those in code formatter + // the next iteration of this will do the following + // find all #if and related directives (from file contents) + // come up with the symbol sets + // use those in code formatter // that will get rid of the ugly threadStatic/reset/stopCollecting stuff public class PreprocessorSymbols { From d430abcc2972ff3d2ffb51e74c072cb7946366dc Mon Sep 17 00:00:00 2001 From: belav Date: Mon, 23 Aug 2021 09:03:06 -0500 Subject: [PATCH 7/8] Cleanup after rebase --- .../TestFiles}/PreprocessorSymbols.cst | 0 .../PreprocessorSymbols.expected.cst | 0 Src/CSharpier.Tests/Samples/Scratch.cst | 88 +++---------------- 3 files changed, 13 insertions(+), 75 deletions(-) rename Src/CSharpier.Tests/{TestFiles/Directives => FormattingTests/TestFiles}/PreprocessorSymbols.cst (100%) rename Src/CSharpier.Tests/{TestFiles/Directives => FormattingTests/TestFiles}/PreprocessorSymbols.expected.cst (100%) diff --git a/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.cst b/Src/CSharpier.Tests/FormattingTests/TestFiles/PreprocessorSymbols.cst similarity index 100% rename from Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.cst rename to Src/CSharpier.Tests/FormattingTests/TestFiles/PreprocessorSymbols.cst diff --git a/Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.expected.cst b/Src/CSharpier.Tests/FormattingTests/TestFiles/PreprocessorSymbols.expected.cst similarity index 100% rename from Src/CSharpier.Tests/TestFiles/Directives/PreprocessorSymbols.expected.cst rename to Src/CSharpier.Tests/FormattingTests/TestFiles/PreprocessorSymbols.expected.cst diff --git a/Src/CSharpier.Tests/Samples/Scratch.cst b/Src/CSharpier.Tests/Samples/Scratch.cst index a43ffd29e..5c5c0d3dc 100644 --- a/Src/CSharpier.Tests/Samples/Scratch.cst +++ b/Src/CSharpier.Tests/Samples/Scratch.cst @@ -1,77 +1,15 @@ -#region License -// Copyright (c) 2007 James Newton-King -// -// Permission is hereby granted, free of charge, to any person -// obtaining a copy of this software and associated documentation -// files (the "Software"), to deal in the Software without -// restriction, including without limitation the rights to use, -// copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the -// Software is furnished to do so, subject to the following -// conditions: -// -// The above copyright notice and this permission notice shall be -// included in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, -// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES -// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND -// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT -// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, -// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING -// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR -// OTHER DEALINGS IN THE SOFTWARE. -#endregion - -#if (NET45 || NET50) -#if DNXCORE50 -using Xunit; -using Test = Xunit.FactAttribute; -using Assert = Newtonsoft.Json.Tests.XUnitAssert; -#else -using NUnit.Framework; -#endif -using System.Collections.Generic; -using Newtonsoft.Json.Serialization; -using Newtonsoft.Json.Converters; -using System.Collections; -using System; -using System.IO; -using Newtonsoft.Json.Linq; - -namespace Newtonsoft.Json.Tests.Issues -{ - [TestFixture] - public class Issue2492 + public class FunctionParameter : EntityBase { - [Test] - public void Test_Object() + public Function Function + + [Required] + [UniqueIndex(AdditionalColumns = nameof(Name))] + public Guid FunctionId { get; set; } + + public string Name { get; set; } + + public override string GetDisplayName() { - string jsontext = @"{ ""ABC"": //DEF -{}}"; - - using var stringReader = new StringReader(jsontext); - using var jsonReader = new JsonTextReader(stringReader); - - JsonSerializer serializer = JsonSerializer.Create(); - var x = serializer.Deserialize(jsonReader); - - Assert.AreEqual(JTokenType.Object, x["ABC"].Type); - } - - [Test] - public void Test_Integer() - { - string jsontext = "{ \"ABC\": /*DEF*/ 1}"; - - using var stringReader = new StringReader(jsontext); - using var jsonReader = new JsonTextReader(stringReader); - - JsonSerializer serializer = JsonSerializer.Create(); - var x = serializer.Deserialize(jsonReader); - - Assert.AreEqual(JTokenType.Integer, x["ABC"].Type); - } - } -} -#endif + return this.Name; + + } \ No newline at end of file From 4b78480286054fd40f2ea53c289e128e2d99a870 Mon Sep 17 00:00:00 2001 From: belav Date: Mon, 23 Aug 2021 14:32:07 -0500 Subject: [PATCH 8/8] Cleaning up some edge cases around new lines --- .../PreprocessorSymbols_KeepNewIines.cst | 17 +++++++++++++++++ .../SyntaxNodePrinters/BaseTypeDeclaration.cs | 11 ++++++++++- Src/CSharpier/SyntaxPrinter/Token.cs | 2 +- 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 Src/CSharpier.Tests/FormattingTests/TestFiles/PreprocessorSymbols_KeepNewIines.cst diff --git a/Src/CSharpier.Tests/FormattingTests/TestFiles/PreprocessorSymbols_KeepNewIines.cst b/Src/CSharpier.Tests/FormattingTests/TestFiles/PreprocessorSymbols_KeepNewIines.cst new file mode 100644 index 000000000..94ceef2cf --- /dev/null +++ b/Src/CSharpier.Tests/FormattingTests/TestFiles/PreprocessorSymbols_KeepNewIines.cst @@ -0,0 +1,17 @@ +class ClassName1 +{ + public void MethodName1() { } + +#if DEBUG + public void MethodName2() { } +#endif +} + +#if DEBUG + +class ClassName2 +{ + public void MethodName() { } +} + +#endif diff --git a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseTypeDeclaration.cs b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseTypeDeclaration.cs index 1ecec444f..2e329b026 100644 --- a/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseTypeDeclaration.cs +++ b/Src/CSharpier/SyntaxPrinter/SyntaxNodePrinters/BaseTypeDeclaration.cs @@ -134,7 +134,16 @@ public static Doc Print(BaseTypeDeclarationSyntax node) Token.Print(node.OpenBraceToken), members, Doc.HardLine, - Token.Print(node.CloseBraceToken) + node.CloseBraceToken.LeadingTrivia.Any( + o => o.Kind() == SyntaxKind.DisabledTextTrivia + ) + ? Doc.Concat( + Token.PrintLeadingTriviaWithNewLines( + node.CloseBraceToken.LeadingTrivia + ), + Token.PrintWithoutLeadingTrivia(node.CloseBraceToken) + ) + : Token.Print(node.CloseBraceToken) ); } else if (node.OpenBraceToken.Kind() != SyntaxKind.None) diff --git a/Src/CSharpier/SyntaxPrinter/Token.cs b/Src/CSharpier/SyntaxPrinter/Token.cs index 405120bcb..3abfa37fa 100644 --- a/Src/CSharpier/SyntaxPrinter/Token.cs +++ b/Src/CSharpier/SyntaxPrinter/Token.cs @@ -160,7 +160,7 @@ private static Doc PrivatePrintLeadingTrivia( } else if (kind == SyntaxKind.DisabledTextTrivia) { - docs.Add(Doc.Trim, trivia.ToString().TrimEnd('\n', '\r'), Doc.HardLine); + docs.Add(Doc.Trim, trivia.ToString()); } else if (IsDirective(kind) || IsRegion(kind)) {