diff --git a/Src/CSharpier.Cli/FormattingCache.cs b/Src/CSharpier.Cli/FormattingCache.cs index 83290f5e4..0ae16fccf 100644 --- a/Src/CSharpier.Cli/FormattingCache.cs +++ b/Src/CSharpier.Cli/FormattingCache.cs @@ -1,7 +1,7 @@ using System.Collections.Concurrent; using System.IO.Abstractions; using System.IO.Hashing; -using System.Text; +using System.Runtime.InteropServices; using System.Text.Json; using CSharpier.Cli.Options; using CSharpier.Core.Utilities; @@ -99,10 +99,9 @@ IFileSystem fileSystem public bool CanSkipFormatting(FileToFormatInfo fileToFormatInfo) { - var currentHash = Hash(fileToFormatInfo.FileContents) + this.optionsHash; if (cacheDictionary.TryGetValue(fileToFormatInfo.Path, out var cachedHash)) { - if (currentHash == cachedHash) + if (this.HashMatches(cachedHash, fileToFormatInfo.FileContents)) { return true; } @@ -113,6 +112,15 @@ public bool CanSkipFormatting(FileToFormatInfo fileToFormatInfo) return false; } + private bool HashMatches(string cachedHash, string fileContents) + { + var contentHash = Hash(fileContents); + + return cachedHash.Length == contentHash.Length + this.optionsHash.Length + && cachedHash.AsSpan(0, contentHash.Length).SequenceEqual(contentHash.AsSpan()) + && cachedHash.AsSpan(contentHash.Length).SequenceEqual(this.optionsHash.AsSpan()); + } + public void CacheResult(string code, FileToFormatInfo fileToFormatInfo) { cacheDictionary[fileToFormatInfo.Path] = Hash(code) + this.optionsHash; @@ -124,10 +132,13 @@ private static string GetOptionsHash(OptionsProvider optionsProvider) return Hash($"{csharpierVersion}_${optionsProvider.Serialize()}"); } + // hashes the utf-16 payload in place - transcoding to ascii first would both copy the whole + // file and collapse every non-ascii character to '?', letting two different files collide private static string Hash(string input) { - var result = XxHash32.Hash(Encoding.ASCII.GetBytes(input)); - return Convert.ToHexString(result); + Span destination = stackalloc byte[sizeof(uint)]; + XxHash32.Hash(MemoryMarshal.AsBytes(input.AsSpan()), destination); + return Convert.ToHexString(destination); } public async Task ResolveAsync(CancellationToken cancellationToken) diff --git a/Src/CSharpier.Cli/Options/ConfigurationFileOptions.cs b/Src/CSharpier.Cli/Options/ConfigurationFileOptions.cs index 73b4c10d3..ca3d64280 100644 --- a/Src/CSharpier.Cli/Options/ConfigurationFileOptions.cs +++ b/Src/CSharpier.Cli/Options/ConfigurationFileOptions.cs @@ -41,7 +41,8 @@ out var parsedFormatter ?? PrinterOptions.GetXmlWhitespaceSensitivity(filePath) ) { - IndentSize = matchingOverride.IndentSize, + IndentSize = + matchingOverride.IndentSize ?? (parsedFormatter == Formatter.XML ? 2 : 4), UseTabs = matchingOverride.UseTabs, Width = matchingOverride.PrintWidth, EndOfLine = matchingOverride.EndOfLine, @@ -81,7 +82,7 @@ internal class Override private GlobMatcher? matcher; public int PrintWidth { get; init; } = 100; - public int IndentSize { get; init; } = 4; + public int? IndentSize { get; init; } public bool UseTabs { get; init; } [JsonConverter(typeof(CaseInsensitiveEnumConverter))] diff --git a/Src/CSharpier.Cli/Server/CSharpierServiceImplementation.cs b/Src/CSharpier.Cli/Server/CSharpierServiceImplementation.cs index 513f97d8d..bc1c34641 100644 --- a/Src/CSharpier.Cli/Server/CSharpierServiceImplementation.cs +++ b/Src/CSharpier.Cli/Server/CSharpierServiceImplementation.cs @@ -60,7 +60,7 @@ CancellationToken cancellationToken } var printerOptions = await optionsProvider.GetPrinterOptionsForAsync( - formatFileParameter.fileName, + fileName, cancellationToken ); if (printerOptions == null || printerOptions.Formatter is Formatter.Unknown) @@ -68,7 +68,6 @@ CancellationToken cancellationToken return new FormatFileResult(Status.UnsupportedFile); } - // TODO #819 if there are compilation errors we need to do something here var result = await CodeFormatter.FormatAsync( formatFileParameter.fileContents, printerOptions, @@ -83,6 +82,25 @@ CancellationToken cancellationToken }; } + if (string.IsNullOrEmpty(result.Code)) + { + if (!string.IsNullOrEmpty(result.WarningMessage)) + { + return new FormatFileResult(Status.Failed) + { + errorMessage = result.WarningMessage, + }; + } + + if (!string.IsNullOrEmpty(result.FailureMessage)) + { + return new FormatFileResult(Status.Failed) + { + errorMessage = result.FailureMessage, + }; + } + } + return new FormatFileResult(Status.Formatted) { formattedFile = result.Code }; } catch (Exception ex) diff --git a/Src/CSharpier.Core/CSharp/SyntaxPrinter/SeparatedSyntaxList.cs b/Src/CSharpier.Core/CSharp/SyntaxPrinter/SeparatedSyntaxList.cs index f9028b086..1eec97549 100644 --- a/Src/CSharpier.Core/CSharp/SyntaxPrinter/SeparatedSyntaxList.cs +++ b/Src/CSharpier.Core/CSharp/SyntaxPrinter/SeparatedSyntaxList.cs @@ -74,7 +74,7 @@ private static Doc Print( if (x < list.SeparatorCount) { unFormattedCode.Append(list.GetSeparator(x).ToFullString().Trim()); - unFormattedCode.Append(Environment.NewLine); + unFormattedCode.Append(context.LineEnding); } continue; diff --git a/Src/CSharpier.Core/DocTypes/Doc.cs b/Src/CSharpier.Core/DocTypes/Doc.cs index a66874763..31d6aa728 100644 --- a/Src/CSharpier.Core/DocTypes/Doc.cs +++ b/Src/CSharpier.Core/DocTypes/Doc.cs @@ -104,12 +104,12 @@ public static Doc Join(Doc separator, IEnumerable enumerable) public static ForceFlat ForceFlat(List contents) { - return new ForceFlat { Contents = contents.Count == 0 ? contents[0] : Concat(contents) }; + return new ForceFlat { Contents = contents.Count == 1 ? contents[0] : Concat(contents) }; } public static ForceFlat ForceFlat(params Doc[] contents) { - return new ForceFlat { Contents = contents.Length == 0 ? contents[0] : Concat(contents) }; + return new ForceFlat { Contents = contents.Length == 1 ? contents[0] : Concat(contents) }; } public static Group Group(List contents) diff --git a/Src/CSharpier.Core/Xml/XNodePrinters/ElementChildren.cs b/Src/CSharpier.Core/Xml/XNodePrinters/ElementChildren.cs index 02bfcbcd2..f0b34d3a5 100644 --- a/Src/CSharpier.Core/Xml/XNodePrinters/ElementChildren.cs +++ b/Src/CSharpier.Core/Xml/XNodePrinters/ElementChildren.cs @@ -21,7 +21,6 @@ public static Doc Print(RawNode node, XmlPrintingContext context) if (childNode.CSharpierIgnoreType is CSharpierIgnoreType.IgnoreEnd) { printIgnored = false; - x++; } if (printIgnored) diff --git a/Src/CSharpier.Core/Xml/XNodePrinters/Node.cs b/Src/CSharpier.Core/Xml/XNodePrinters/Node.cs index 7cd908462..6e2bb1c20 100644 --- a/Src/CSharpier.Core/Xml/XNodePrinters/Node.cs +++ b/Src/CSharpier.Core/Xml/XNodePrinters/Node.cs @@ -90,11 +90,6 @@ private static Doc GetTextValue(RawNode rawNode, XmlPrintingContext context) { var textValue = rawNode.Value; - if (string.IsNullOrEmpty(textValue)) - { - return Doc.Null; - } - if (rawNode.XmlWhitespaceSensitivity is XmlWhitespaceSensitivity.Ignore) { if (rawNode.PreviousNode is null) @@ -118,6 +113,11 @@ private static Doc GetTextValue(RawNode rawNode, XmlPrintingContext context) } } + if (string.IsNullOrEmpty(textValue)) + { + return Doc.Null; + } + if (rawNode.Parent?.Nodes.First() == rawNode) { if (textValue[0] is '\r') @@ -125,7 +125,7 @@ private static Doc GetTextValue(RawNode rawNode, XmlPrintingContext context) textValue = textValue[1..]; } - if (textValue[0] is '\n') + if (textValue.Length > 0 && textValue[0] is '\n') { textValue = textValue[1..]; } diff --git a/Src/CSharpier.Tests/Cli/CSharpierServiceImplementationTests.cs b/Src/CSharpier.Tests/Cli/CSharpierServiceImplementationTests.cs new file mode 100644 index 000000000..a954df291 --- /dev/null +++ b/Src/CSharpier.Tests/Cli/CSharpierServiceImplementationTests.cs @@ -0,0 +1,49 @@ +using AwesomeAssertions; +using CSharpier.Cli.Server; +using Microsoft.Extensions.Logging.Abstractions; + +namespace CSharpier.Tests.Cli; + +internal sealed class CSharpierServiceImplementationTests +{ + [Test] + public async Task Should_Report_Failure_When_Formatting_Produces_A_FailureMessage() + { + var service = new CSharpierServiceImplementation(NullLogger.Instance); + + var result = await service.FormatFile( + new FormatFileParameter + { + fileName = Path.Combine(Path.GetTempPath(), "DeepRecursion.cs"), + fileContents = DeeplyConcatenatedString, + }, + CancellationToken.None + ); + + result.status.Should().Be(Status.Failed); + result.errorMessage.Should().Contain("deep of recursion"); + } + + [Test] + public async Task Should_Not_Return_Empty_Content_When_Formatting_Fails() + { + var service = new CSharpierServiceImplementation(NullLogger.Instance); + + var result = await service.FormatFile( + new FormatFileParameter + { + fileName = Path.Combine(Path.GetTempPath(), "DeepRecursion.cs"), + fileContents = DeeplyConcatenatedString, + }, + CancellationToken.None + ); + + result.formattedFile.Should().BeNullOrEmpty(); + result.status.Should().NotBe(Status.Formatted); + } + + private static readonly string DeeplyConcatenatedString = + "public class ClassName\n{\n private string field = " + + string.Join(" + ", Enumerable.Repeat("\"1\"", 200)) + + ";\n}\n"; +} diff --git a/Src/CSharpier.Tests/Cli/FormattingCacheTests.cs b/Src/CSharpier.Tests/Cli/FormattingCacheTests.cs new file mode 100644 index 000000000..91503faca --- /dev/null +++ b/Src/CSharpier.Tests/Cli/FormattingCacheTests.cs @@ -0,0 +1,64 @@ +using System.IO.Abstractions.TestingHelpers; +using System.Text; +using AwesomeAssertions; +using CSharpier.Cli; +using CSharpier.Cli.Options; +using Microsoft.Extensions.Logging.Abstractions; + +namespace CSharpier.Tests.Cli; + +internal sealed class FormattingCacheTests +{ + [Test] + public async Task Should_Not_Skip_File_That_Differs_Only_By_Non_Ascii_Character() + { + var cache = await CreateCacheAsync(); + var path = OperatingSystem.IsWindows() ? "c:/test/Class.cs" : "/test/Class.cs"; + + cache.CacheResult("public class Café { }\n", FileAt(path, "public class Café { }\n")); + + var otherFile = FileAt(path, "public class Cafè { }\n"); + + cache.CanSkipFormatting(otherFile).Should().BeFalse(); + } + + [Test] + public async Task Should_Skip_File_With_Unchanged_Non_Ascii_Content() + { + var cache = await CreateCacheAsync(); + var path = OperatingSystem.IsWindows() ? "c:/test/Class.cs" : "/test/Class.cs"; + var contents = "public class Café { }\n"; + + cache.CacheResult(contents, FileAt(path, contents)); + + cache.CanSkipFormatting(FileAt(path, contents)).Should().BeTrue(); + } + + private static FileToFormatInfo FileAt(string path, string contents) + { + return FileToFormatInfo.Create(path, contents, Encoding.UTF8); + } + + private static async Task CreateCacheAsync() + { + var directory = OperatingSystem.IsWindows() ? "c:/test" : "/test"; + var fileSystem = new MockFileSystem(); + fileSystem.AddDirectory(directory); + + var optionsProvider = await OptionsProvider.Create( + directory, + null, + null, + fileSystem, + NullLogger.Instance, + CancellationToken.None + ); + + return await FormattingCacheFactory.InitializeAsync( + new CommandLineOptions(), + optionsProvider, + fileSystem, + CancellationToken.None + ); + } +} diff --git a/Src/CSharpier.Tests/LineEndingTests.cs b/Src/CSharpier.Tests/LineEndingTests.cs index 445b90bae..6053191a6 100644 --- a/Src/CSharpier.Tests/LineEndingTests.cs +++ b/Src/CSharpier.Tests/LineEndingTests.cs @@ -82,6 +82,31 @@ EndOfLine endOfLine result.Code.Should().NotContain($"one{newLine}two"); } + [Test] + [Arguments(EndOfLine.LF)] + [Arguments(EndOfLine.CRLF)] + public async Task Ignored_Range_In_Separated_List_Should_Respect_LineEnding(EndOfLine endOfLine) + { + var code = """ + var value = new() + { + // csharpier-ignore-start + First = 1, + Second = 2 + // csharpier-ignore-end + }; + + """; + + var printerOptions = new PrinterOptions(Formatter.CSharp, XmlWhitespaceSensitivity.Strict) + { + EndOfLine = endOfLine, + }; + var result = await CSharpFormatter.FormatAsync(code, printerOptions); + + result.Code.Should().Be(code.ReplaceLineEndings(endOfLine == EndOfLine.LF ? "\n" : "\r\n")); + } + [Test] [Arguments("\\r\\n", EndOfLine.LF)] [Arguments("\\n", EndOfLine.CRLF)] diff --git a/Src/CSharpier.Tests/OptionsProviderTests.cs b/Src/CSharpier.Tests/OptionsProviderTests.cs index f3f1e1ffe..27e060349 100644 --- a/Src/CSharpier.Tests/OptionsProviderTests.cs +++ b/Src/CSharpier.Tests/OptionsProviderTests.cs @@ -309,6 +309,32 @@ public async Task Should_Return_IndentSize_For_Override_With_Json() result.XmlWhitespaceSensitivity.Should().Be(XmlWhitespaceSensitivity.Ignore); } + [Test] + [Arguments("xml", 2)] + [Arguments("csharp", 4)] + public async Task Should_Return_Formatter_Default_IndentSize_For_Override_Without_IndentSize( + string formatter, + int expectedIndentSize + ) + { + var context = new TestContext(); + context.WhenAFileExists( + "c:/test/.csharpierrc", + $""" +overrides: + - files: "*.override" + formatter: "{formatter}" +""" + ); + + var result = await context.CreateProviderAndGetOptionsFor( + "c:/test", + "c:/test/test.override" + ); + + result.IndentSize.Should().Be(expectedIndentSize); + } + [Test] [Arguments("cs")] [Arguments("csx")] diff --git a/Src/CSharpier.Tests/Xml/XmlNodePrinterTests.cs b/Src/CSharpier.Tests/Xml/XmlNodePrinterTests.cs new file mode 100644 index 000000000..de1a699ff --- /dev/null +++ b/Src/CSharpier.Tests/Xml/XmlNodePrinterTests.cs @@ -0,0 +1,45 @@ +using System.Xml; +using AwesomeAssertions; +using CSharpier.Core; +using CSharpier.Core.Xml; +using Node = CSharpier.Core.Xml.XNodePrinters.Node; + +namespace CSharpier.Tests.Xml; + +internal sealed class XmlNodePrinterTests +{ + [Test] + public void Should_Format_Element_Ending_With_CSharpierIgnoreEnd() + { + var result = () => XmlFormatter.Format(""); + + result.Should().NotThrow(); + } + + [Test] + [Arguments(" ")] + [Arguments("\n")] + [Arguments("\r\n")] + public void Should_Print_Whitespace_Only_Text_Node(string value) + { + var context = new XmlPrintingContext { NormalizedXml = value, LineEnding = "\n" }; + var textNode = new RawNode + { + NodeType = XmlNodeType.Text, + Value = value, + XmlWhitespaceSensitivity = XmlWhitespaceSensitivity.Ignore, + }; + var parent = new RawNode + { + Name = "Root", + NodeType = XmlNodeType.Element, + XmlWhitespaceSensitivity = XmlWhitespaceSensitivity.Ignore, + Nodes = [textNode], + }; + textNode.Parent = parent; + + var result = () => Node.Print(textNode, context); + + result.Should().NotThrow(); + } +}