diff --git a/Src/CSharpier.Cli/CommandLineFormatter.cs b/Src/CSharpier.Cli/CommandLineFormatter.cs index 83c8fbdf5..2e2f9ca40 100644 --- a/Src/CSharpier.Cli/CommandLineFormatter.cs +++ b/Src/CSharpier.Cli/CommandLineFormatter.cs @@ -406,7 +406,7 @@ CommandLineFormatterResult result ) { if ( - (!commandLineOptions.CompilationErrorsAsWarnings && result.FailedCompilation > 0) + (!commandLineOptions.SyntaxErrorsAsWarnings && result.FailedCompilation > 0) || ( commandLineOptions.Check && result.UnformattedFiles > 0 @@ -477,16 +477,16 @@ CancellationToken cancellationToken return; } - if (codeFormattingResult.CompilationErrors.Any()) + if (codeFormattingResult.ErrorDiagnostics.Any()) { var errorMessage = new StringBuilder(); - errorMessage.AppendLine("Failed to compile so was not formatted."); - foreach (var message in codeFormattingResult.CompilationErrors) + errorMessage.AppendLine("Was not formatted due to syntax errors."); + foreach (var message in codeFormattingResult.ErrorDiagnostics) { errorMessage.AppendLine(message.ToString()); } - if (!commandLineOptions.CompilationErrorsAsWarnings) + if (!commandLineOptions.SyntaxErrorsAsWarnings) { fileIssueLogger.WriteError(errorMessage.ToString()); } diff --git a/Src/CSharpier.Cli/CommandLineOptions.cs b/Src/CSharpier.Cli/CommandLineOptions.cs index 38ef4fcc1..204b52d72 100644 --- a/Src/CSharpier.Cli/CommandLineOptions.cs +++ b/Src/CSharpier.Cli/CommandLineOptions.cs @@ -14,7 +14,8 @@ internal class CommandLineOptions public bool WriteStdout { get; init; } public bool NoCache { get; init; } public bool NoMSBuildCheck { get; init; } - public bool CompilationErrorsAsWarnings { get; init; } + + public bool SyntaxErrorsAsWarnings { get; init; } public bool UnformattedAsWarnings { get; init; } public bool IncludeGenerated { get; init; } public string? StandardInFileContents { get; init; } diff --git a/Src/CSharpier.Cli/FormattingCommands.cs b/Src/CSharpier.Cli/FormattingCommands.cs index 5ada501ed..788875e57 100644 --- a/Src/CSharpier.Cli/FormattingCommands.cs +++ b/Src/CSharpier.Cli/FormattingCommands.cs @@ -33,7 +33,7 @@ public static Command CreateFormatCommand() "Write the results of formatting any files to stdout." ); formatCommand.AddOption(writeStdoutOption); - formatCommand.AddOption(CompilationErrorsAsWarningsOption); + formatCommand.AddOption(SyntaxErrorsAsWarningsOption); formatCommand.AddOption(ConfigPathOption); formatCommand.AddOption(IgnorePathOption); formatCommand.AddOption(StdinPathOption); @@ -49,8 +49,8 @@ public static Command CreateFormatCommand() var noCache = context.ParseResult.GetValueForOption(noCacheOption); var noMSBuildCheck = context.ParseResult.GetValueForOption(NoMsBuildCheckOption); var includeGenerated = context.ParseResult.GetValueForOption(IncludeGeneratedOption); - var compilationErrorsAsWarnings = context.ParseResult.GetValueForOption( - CompilationErrorsAsWarningsOption + var syntaxErrorsAsWarnings = context.ParseResult.GetValueForOption( + SyntaxErrorsAsWarningsOption ); var configPath = context.ParseResult.GetValueForOption(ConfigPathOption); var ignorePath = context.ParseResult.GetValueForOption(IgnorePathOption); @@ -68,7 +68,7 @@ public static Command CreateFormatCommand() noCache, noMSBuildCheck, includeGenerated, - compilationErrorsAsWarnings, + syntaxErrorsAsWarnings, false, configPath, ignorePath, @@ -101,7 +101,7 @@ public static Command CreateCheckCommand() checkCommand.AddOption(LogLevelOption); checkCommand.AddOption(IncludeGeneratedOption); checkCommand.AddOption(NoMsBuildCheckOption); - checkCommand.AddOption(CompilationErrorsAsWarningsOption); + checkCommand.AddOption(SyntaxErrorsAsWarningsOption); checkCommand.AddOption(UnformattedAsWarningsOption); checkCommand.SetHandler(async context => @@ -110,8 +110,8 @@ public static Command CreateCheckCommand() var useCache = context.ParseResult.GetValueForOption(useCacheOption); var noMSBuildCheck = context.ParseResult.GetValueForOption(NoMsBuildCheckOption); var includeGenerated = context.ParseResult.GetValueForOption(IncludeGeneratedOption); - var compilationErrorsAsWarnings = context.ParseResult.GetValueForOption( - CompilationErrorsAsWarningsOption + var syntaxErrorsAsWarnings = context.ParseResult.GetValueForOption( + SyntaxErrorsAsWarningsOption ); var unformattedAsWarningsOption = context.ParseResult.GetValueForOption( UnformattedAsWarningsOption @@ -131,7 +131,7 @@ public static Command CreateCheckCommand() noCache: !useCache, noMSBuildCheck, includeGenerated, - compilationErrorsAsWarnings, + syntaxErrorsAsWarnings, unformattedAsWarningsOption, configPath, ignorePath, @@ -154,7 +154,7 @@ private static async Task FormatOrCheck( bool noCache, bool noMSBuildCheck, bool includeGenerated, - bool compilationErrorsAsWarnings, + bool syntaxErrorsAsWarnings, bool unformattedAsWarnings, string? configPath, string? ignorePath, @@ -205,7 +205,7 @@ CancellationToken cancellationToken IncludeGenerated = includeGenerated, ConfigPath = configPath, IgnorePath = ignorePath, - CompilationErrorsAsWarnings = compilationErrorsAsWarnings, + SyntaxErrorsAsWarnings = syntaxErrorsAsWarnings, UnformattedAsWarnings = unformattedAsWarnings, LogFormat = logFormat, }; @@ -281,9 +281,9 @@ Log output format "Bypass the check to determine if a csproj files references a different version of CSharpier.MsBuild." ); - private static readonly Option CompilationErrorsAsWarningsOption = new( - ["--compilation-errors-as-warnings"], - "Treat compilation errors from files as warnings instead of errors." + private static readonly Option SyntaxErrorsAsWarningsOption = new( + ["--syntax-errors-as-warnings"], + "Treat syntax errors from files as warnings instead of errors." ); private static readonly Option UnformattedAsWarningsOption = new( diff --git a/Src/CSharpier.Cli/IgnoreFile.cs b/Src/CSharpier.Cli/IgnoreFile.cs index 3bfe68c4d..43570fbf6 100644 --- a/Src/CSharpier.Cli/IgnoreFile.cs +++ b/Src/CSharpier.Cli/IgnoreFile.cs @@ -7,6 +7,8 @@ namespace CSharpier.Cli; internal class IgnoreFile { + public static IgnoreFile NullIgnore = new([]); + private List Ignores { get; } private IgnoreFile(List ignores) diff --git a/Src/CSharpier.Cli/Options/OptionsProvider.cs b/Src/CSharpier.Cli/Options/OptionsProvider.cs index 132326b79..1a2a7377b 100644 --- a/Src/CSharpier.Cli/Options/OptionsProvider.cs +++ b/Src/CSharpier.Cli/Options/OptionsProvider.cs @@ -66,13 +66,7 @@ CancellationToken cancellationToken cancellationToken ); -#pragma warning disable IDE0270 - if (ignoreFile is null) - { - // should never happen - throw new Exception("Unable to locate an IgnoreFile for " + directoryName); - } -#pragma warning restore IDE0270 + ignoreFile ??= IgnoreFile.NullIgnore; var specifiedEditorConfig = editorConfigPath is not null ? await EditorConfigLocator.FindForDirectoryNameAsync( @@ -218,15 +212,7 @@ CancellationToken cancellationToken ) ); -#pragma warning disable IDE0270 - if (ignoreFile is null) - { - // should never happen - throw new Exception("Unable to locate an IgnoreFile for " + directoryName); - } -#pragma warning restore IDE0270 - - return ignoreFile; + return ignoreFile ?? IgnoreFile.NullIgnore; } /// diff --git a/Src/CSharpier.Cli/Server/CSharpierServiceImplementation.cs b/Src/CSharpier.Cli/Server/CSharpierServiceImplementation.cs index 5c7af8050..513f97d8d 100644 --- a/Src/CSharpier.Cli/Server/CSharpierServiceImplementation.cs +++ b/Src/CSharpier.Cli/Server/CSharpierServiceImplementation.cs @@ -75,7 +75,7 @@ CancellationToken cancellationToken cancellationToken ); - if (result.CompilationErrors.Any()) + if (result.ErrorDiagnostics.Any()) { return new FormatFileResult(Status.Failed) { diff --git a/Src/CSharpier.Core/CSharp/CSharpFormatter.cs b/Src/CSharpier.Core/CSharp/CSharpFormatter.cs index 162d17550..4f71087d6 100644 --- a/Src/CSharpier.Core/CSharp/CSharpFormatter.cs +++ b/Src/CSharpier.Core/CSharp/CSharpFormatter.cs @@ -129,7 +129,7 @@ bool TryGetCompilationFailure(out CodeFormatterResult compilationResult) compilationResult = new CodeFormatterResult { Code = syntaxTree.ToString(), - CompilationErrors = diagnostics, + ErrorDiagnostics = diagnostics, AST = printerOptions.IncludeAST ? PrintAST(rootNode) : string.Empty, }; diff --git a/Src/CSharpier.Core/CodeFormatterResult.cs b/Src/CSharpier.Core/CodeFormatterResult.cs index 7a2b5a68e..f68e44636 100644 --- a/Src/CSharpier.Core/CodeFormatterResult.cs +++ b/Src/CSharpier.Core/CodeFormatterResult.cs @@ -10,7 +10,7 @@ internal CodeFormatterResult() { } internal string DocTree { get; init; } = string.Empty; internal string AST { get; init; } = string.Empty; internal string WarningMessage { get; init; } = string.Empty; - public IEnumerable CompilationErrors { get; internal init; } = []; + public IEnumerable ErrorDiagnostics { get; internal init; } = []; internal string FailureMessage { get; init; } = string.Empty; diff --git a/Src/CSharpier.Core/PublicAPI.Shipped.txt b/Src/CSharpier.Core/PublicAPI.Shipped.txt index 8bf2579ac..d52dff46c 100644 --- a/Src/CSharpier.Core/PublicAPI.Shipped.txt +++ b/Src/CSharpier.Core/PublicAPI.Shipped.txt @@ -13,7 +13,7 @@ CSharpier.Core.CodeFormatterOptions.IndentStyle.init -> void CSharpier.Core.CodeFormatterResult CSharpier.Core.CodeFormatterResult.Code.get -> string! -CSharpier.Core.CodeFormatterResult.CompilationErrors.get -> System.Collections.Generic.IEnumerable! +CSharpier.Core.CodeFormatterResult.ErrorDiagnostics.get -> System.Collections.Generic.IEnumerable! CSharpier.Core.CodeFormatterOptions.IncludeGenerated.get -> bool CSharpier.Core.CodeFormatterOptions.IncludeGenerated.init -> void diff --git a/Src/CSharpier.Core/Xml/XmlFormatter.cs b/Src/CSharpier.Core/Xml/XmlFormatter.cs index 9b0d12629..2ca13d5dc 100644 --- a/Src/CSharpier.Core/Xml/XmlFormatter.cs +++ b/Src/CSharpier.Core/Xml/XmlFormatter.cs @@ -3,6 +3,8 @@ using System.Xml; using CSharpier.Core.CSharp.SyntaxPrinter; using CSharpier.Core.DocTypes; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; using Node = CSharpier.Core.Xml.XNodePrinters.Node; namespace CSharpier.Core.Xml; @@ -56,16 +58,53 @@ PrinterOptions printerOptions AST = RawNodeSyntaxWriter.Write(rootNode), }; } - catch (XmlException) + catch (XmlException ex) { return new CodeFormatterResult { Code = xml, + ErrorDiagnostics = [CreateDiagnosticFromXmlException(xml, ex)], WarningMessage = "Appeared to be invalid xml so was not formatted.", }; } } + private static Diagnostic CreateDiagnosticFromXmlException(string xml, XmlException ex) + { + var sourceText = SourceText.From(xml); + + // XmlException is 1-based; Roslyn is 0-based + var lineIndex = Math.Max(0, ex.LineNumber - 1); + if (lineIndex >= sourceText.Lines.Count) + { + lineIndex = sourceText.Lines.Count - 1; + } + + var line = sourceText.Lines[lineIndex]; + + var charIndexInLine = Math.Max(0, ex.LinePosition - 1); + var position = Math.Min(line.Start + charIndexInLine, line.End); + + var span = new TextSpan(position, 0); + + var location = Location.Create( + filePath: string.Empty, + textSpan: span, + lineSpan: sourceText.Lines.GetLinePositionSpan(span) + ); + + var descriptor = new DiagnosticDescriptor( + id: "XML001", + title: "XML parsing error", + messageFormat: "{0}", + category: "XML", + DiagnosticSeverity.Error, + isEnabledByDefault: true + ); + + return Diagnostic.Create(descriptor, location, ex.Message); + } + // the RawNodeReader is very basic and doesn't care if xml is valid or not and could mangle invalid xml if we allow it to format the invalid xml private static async Task ValidateXmlAsync(string xml) { diff --git a/Src/CSharpier.MsBuild/build/CSharpier.MsBuild.targets b/Src/CSharpier.MsBuild/build/CSharpier.MsBuild.targets index f7283a7ae..508627e16 100644 --- a/Src/CSharpier.MsBuild/build/CSharpier.MsBuild.targets +++ b/Src/CSharpier.MsBuild/build/CSharpier.MsBuild.targets @@ -27,7 +27,7 @@ StdOutEncoding="utf-8" StdErrEncoding="utf-8" IgnoreExitCode="true" - Command=""$(CSharpier_dotnet_Path)" exec "$(CSharpierDllPath)" $(CSharpierCommand) $(CSharpierArgs) --log-format msBuild --no-msbuild-check --compilation-errors-as-warnings "$(MSBuildProjectDirectory)" > $(NullOutput) " + Command=""$(CSharpier_dotnet_Path)" exec "$(CSharpierDllPath)" $(CSharpierCommand) $(CSharpierArgs) --log-format msBuild --no-msbuild-check --syntax-errors-as-warnings "$(MSBuildProjectDirectory)" > $(NullOutput) " /> diff --git a/Src/CSharpier.Playground/Controllers/FormatController.cs b/Src/CSharpier.Playground/Controllers/FormatController.cs index d65f434af..7b1836275 100644 --- a/Src/CSharpier.Playground/Controllers/FormatController.cs +++ b/Src/CSharpier.Playground/Controllers/FormatController.cs @@ -81,7 +81,7 @@ CancellationToken cancellationToken Code = result.Code, Json = result.AST, Doc = result.DocTree, - Errors = result.CompilationErrors.Select(this.ConvertError).ToList(), + Errors = result.ErrorDiagnostics.Select(this.ConvertError).ToList(), SyntaxValidation = await comparer.CompareSourceAsync(CancellationToken.None), }; } diff --git a/Src/CSharpier.Tests/CSharp/CSharpFormatterTests.cs b/Src/CSharpier.Tests/CSharp/CSharpFormatterTests.cs index 820298e08..16271c97e 100644 --- a/Src/CSharpier.Tests/CSharp/CSharpFormatterTests.cs +++ b/Src/CSharpier.Tests/CSharp/CSharpFormatterTests.cs @@ -14,7 +14,7 @@ public void Format_Should_Use_Default_Width() var result = CSharpFormatter.Format(code); result.Code.Should().Be("var someVariable = someValue;\n"); - result.CompilationErrors.Should().BeEmpty(); + result.ErrorDiagnostics.Should().BeEmpty(); } [Test] @@ -24,7 +24,7 @@ public void Format_Should_Return_Compilation_Errors() var result = CSharpFormatter.Format(code); result.Code.Should().Be(code); - result.CompilationErrors.Should().ContainSingle(); + result.ErrorDiagnostics.Should().ContainSingle(); } [Test] diff --git a/Src/CSharpier.Tests/CommandLineFormatterTests.cs b/Src/CSharpier.Tests/CommandLineFormatterTests.cs index 7e743832f..0c26f9b80 100644 --- a/Src/CSharpier.Tests/CommandLineFormatterTests.cs +++ b/Src/CSharpier.Tests/CommandLineFormatterTests.cs @@ -25,7 +25,7 @@ public async Task Format_Writes_Failed_To_Compile() .ErrorOutputLines.First() .Replace('\\', '/') .Should() - .Be("Error ./Invalid.cs - Failed to compile so was not formatted."); + .Be("Error ./Invalid.cs - Was not formatted due to syntax errors."); result.ExitCode.Should().Be(1); } @@ -36,13 +36,13 @@ public async Task Format_Writes_Failed_To_Compile_As_Warning() var context = new TestContext(); context.WhenAFileExists("Invalid.cs", "asdfasfasdf"); - var result = await Format(context, compilationErrorsAsWarnings: true); + var result = await Format(context, syntaxErrorsAsWarnings: true); result .OutputLines.First() .Replace('\\', '/') .Should() - .Be("Warning ./Invalid.cs - Failed to compile so was not formatted."); + .Be("Warning ./Invalid.cs - Was not formatted due to syntax errors."); result.ExitCode.Should().Be(0); } @@ -59,7 +59,7 @@ public async Task Format_Writes_Failed_To_Compile_For_Subdirectory() .ErrorOutputLines.First() .Replace('\\', '/') .Should() - .Be("Error ./Subdirectory/Invalid.cs - Failed to compile so was not formatted."); + .Be("Error ./Subdirectory/Invalid.cs - Was not formatted due to syntax errors."); } [Test] @@ -78,7 +78,7 @@ public async Task Format_Writes_Failed_To_Compile_For_FullPath() .Replace('\\', '/') .Should() .Be( - $"Error {GetRootPath().Replace('\\', '/')}/Subdirectory/Invalid.cs - Failed to compile so was not formatted." + $"Error {GetRootPath().Replace('\\', '/')}/Subdirectory/Invalid.cs - Was not formatted due to syntax errors." ); } @@ -94,7 +94,7 @@ public async Task Format_Writes_Failed_To_Compile_With_Directory() .ErrorOutputLines.First() .Replace('\\', '/') .Should() - .Be("Error ./Directory/Invalid.cs - Failed to compile so was not formatted."); + .Be("Error ./Directory/Invalid.cs - Was not formatted due to syntax errors."); } [Test] @@ -829,7 +829,7 @@ public async Task File_With_Compilation_Error_Should_Not_Lose_Code() .ErrorOutputLines.First() .Replace('\\', '/') .Should() - .Be("Error ./Invalid.cs - Failed to compile so was not formatted."); + .Be("Error ./Invalid.cs - Was not formatted due to syntax errors."); } [Test] @@ -1057,7 +1057,7 @@ private static async Task Format( LogFormat logFormat = LogFormat.Console, bool writeStdout = false, bool includeGenerated = false, - bool compilationErrorsAsWarnings = false, + bool syntaxErrorsAsWarnings = false, string? standardInFileContents = null, string? configPath = null, params string[] directoryOrFilePaths @@ -1090,7 +1090,7 @@ params string[] directoryOrFilePaths WriteStdout = writeStdout || standardInFileContents != null, StandardInFileContents = standardInFileContents, IncludeGenerated = includeGenerated, - CompilationErrorsAsWarnings = compilationErrorsAsWarnings, + SyntaxErrorsAsWarnings = syntaxErrorsAsWarnings, }, context.FileSystem, fakeConsole, diff --git a/Src/CSharpier.Tests/FormattingTests/BaseTest.cs b/Src/CSharpier.Tests/FormattingTests/BaseTest.cs index a77c2cfe4..b1ea3196d 100644 --- a/Src/CSharpier.Tests/FormattingTests/BaseTest.cs +++ b/Src/CSharpier.Tests/FormattingTests/BaseTest.cs @@ -134,7 +134,7 @@ PrinterOptions printerOptions CancellationToken.None ); - result.CompilationErrors.Should().BeEmpty(); + result.ErrorDiagnostics.Should().BeEmpty(); result.FailureMessage.Should().BeEmpty(); result.WarningMessage.Should().BeEmpty(); diff --git a/Src/CSharpierProcess.Tests/CliTests.cs b/Src/CSharpierProcess.Tests/CliTests.cs index d5c2cfc59..203af574d 100644 --- a/Src/CSharpierProcess.Tests/CliTests.cs +++ b/Src/CSharpierProcess.Tests/CliTests.cs @@ -389,6 +389,30 @@ public async Task Format_Should_Not_Format_Piped_File_With_Gitignore_And_Path() result.ExitCode.Should().Be(0); } + [Test] + [Arguments("cs")] + [Arguments("xml")] + public async Task Format_Should_Exit_1_When_Invalid_File(string extension) + { + var unformattedContent1 = "var invalidCode\n"; + + var result = await new CsharpierProcess() + .WithArguments($"format --stdin-path ./Stdin/Test.{extension}") + .WithPipedInput(unformattedContent1) + .ExecuteAsync(); + + using (Assert.Multiple()) + { + await Assert.That(result.ExitCode).IsEqualTo(1); + await Assert + .That(result.ErrorOutput) + .StartsWith( + $"Error ./Stdin/Test.{extension} - Was not formatted due to syntax errors." + ); + await Assert.That(result.Output).IsNullOrEmpty(); + } + } + [Test] public async Task Format_Should_Format_Piped_File_With_EditorConfig() { @@ -466,7 +490,7 @@ public async Task Format_Should_Write_To_StdError_For_Piped_Invalid_File() result.Output.Should().BeEmpty(); result.ExitCode.Should().Be(1); - result.ErrorOutput.Should().Contain("Failed to compile so was not formatted"); + result.ErrorOutput.Should().Contain("Was not formatted due to syntax errors."); } [Test] @@ -481,7 +505,7 @@ public async Task Check_Should_Write_To_StdError_For_Piped_Invalid_File() result.Output.Should().BeEmpty(); result.ExitCode.Should().Be(1); - result.ErrorOutput.Should().Contain("Failed to compile so was not formatted"); + result.ErrorOutput.Should().Contain("Was not formatted due to syntax errors."); } [Test] @@ -564,7 +588,7 @@ string output result .ErrorOutput.Should() .StartWith( - $"Error {output} - Failed to compile so was not formatted.{Environment.NewLine} (1,26): error CS1513: }}" + $"Error {output} - Was not formatted due to syntax errors.{Environment.NewLine} (1,26): error CS1513: }}" ); result.ExitCode.Should().Be(1); } diff --git a/docs/CLI.md b/docs/CLI.md index e7f15c8c2..1506634c6 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -151,9 +151,9 @@ If a list of paths is not supplied, then stdin is read as a file, formatted and Skip writing changes. Generally used for testing to ensure csharpier doesn't throw any errors or cause syntax tree validation failures. -- `--compilation-errors-as-warnings` +- `--syntax-errors-as-warnings` - Treat compilation errors from files as warnings instead of errors. + Treat syntax errors from files as warnings instead of errors. ### Check `dotnet csharpier check []` @@ -169,7 +169,7 @@ Treat unformatted files as a warning instead of an error. If there are unformatt See the `format` command for descriptions of these additional options - `--include-generated` - `--no-msbuild-check` -- `--compilation-errors-as-warnings` +- `--syntax-errors-as-warnings` - `--config-path` - `--log-format` - `--log-level`