Merge pull request #1016 from JoeRobich/diagnostic-report-format

Log all formatter error messages in a csc-style
dotnet · Mar 1, 2021 · ff78edb · ff78edb
2 parents 406fd9e + 5ac861d
commit ff78edb
Show file tree

Hide file tree

Showing 12 changed files with 80 additions and 45 deletions.
diff --git a/src/Analyzers/AnalyzerFormatter.cs b/src/Analyzers/AnalyzerFormatter.cs
@@ -3,7 +3,6 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
-using System.IO;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -141,39 +140,29 @@ internal class AnalyzerFormatter : ICodeFormatter
                 await _runner.RunCodeAnalysisAsync(result, analyzers, project, formattablePaths, severity, fixableCompilerDiagnostics, logger, cancellationToken).ConfigureAwait(false);
             }
 
-            LogDiagnosticLocations(solution, result.Diagnostics.SelectMany(kvp => kvp.Value), options.WorkspaceFilePath, options.ChangesAreErrors, logger, formattedFiles);
+            LogDiagnosticLocations(solution, result.Diagnostics.SelectMany(kvp => kvp.Value), options.SaveFormattedFiles, options.ChangesAreErrors, logger, options.LogLevel, formattedFiles);
 
             return result.Diagnostics.ToImmutableDictionary(kvp => kvp.Key.Id, kvp => kvp.Value.Select(diagnostic => diagnostic.Id).ToImmutableHashSet());
 
-            static void LogDiagnosticLocations(Solution solution, IEnumerable<Diagnostic> diagnostics, string workspacePath, bool changesAreErrors, ILogger logger, List<FormattedFile> formattedFiles)
+            static void LogDiagnosticLocations(Solution solution, IEnumerable<Diagnostic> diagnostics, bool saveFormattedFiles, bool changesAreErrors, ILogger logger, LogLevel logLevel, List<FormattedFile> formattedFiles)
             {
-                var workspaceFolder = Path.GetDirectoryName(workspacePath) ?? workspacePath;
-
                 foreach (var diagnostic in diagnostics)
                 {
-                    var message = $"{diagnostic.GetMessage()} ({diagnostic.Id})";
                     var document = solution.GetDocument(diagnostic.Location.SourceTree);
                     if (document is null)
                     {
                         continue;
                     }
 
-                    var filePath = document.FilePath ?? document.Name;
-
                     var mappedLineSpan = diagnostic.Location.GetMappedLineSpan();
-                    var changePosition = mappedLineSpan.StartLinePosition;
-
-                    var formatMessage = $"{Path.GetRelativePath(workspaceFolder, filePath)}({changePosition.Line + 1},{changePosition.Character + 1}): {message}";
-                    formattedFiles.Add(new FormattedFile(document!, new[] { new FileChange(changePosition, message) }));
+                    var diagnosticPosition = mappedLineSpan.StartLinePosition;
 
-                    if (changesAreErrors)
+                    if (!saveFormattedFiles || logLevel == LogLevel.Debug)
                     {
-                        logger.LogError(formatMessage);
-                    }
-                    else
-                    {
-                        logger.LogWarning(formatMessage);
+                        logger.LogDiagnosticIssue(document, diagnosticPosition, diagnostic, changesAreErrors);
                     }
+
+                    formattedFiles.Add(new FormattedFile(document, new[] { new FileChange(diagnosticPosition, $"{diagnostic.Severity.ToString().ToLower()} {diagnostic.Id}: {diagnostic.GetMessage()}") }));
                 }
             }
         }

diff --git a/src/Formatters/CharsetFormatter.cs b/src/Formatters/CharsetFormatter.cs
@@ -20,6 +20,7 @@ internal sealed class CharsetFormatter : DocumentFormatter
         private static Encoding Utf8 => new UTF8Encoding(encoderShouldEmitUTF8Identifier: false);
         private static Encoding Latin1 => Encoding.GetEncoding("iso-8859-1");
 
+        public override string Name => "CHARSET";
         public override FixCategory Category => FixCategory.Whitespace;
 
         internal override Task<SourceText> FormatFileAsync(

diff --git a/src/Formatters/DocumentFormatter.cs b/src/Formatters/DocumentFormatter.cs
@@ -1,9 +1,7 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
 
-using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
-using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Diagnostics;
@@ -20,6 +18,11 @@ internal abstract class DocumentFormatter : ICodeFormatter
     {
         protected abstract string FormatWarningDescription { get; }
 
+        /// <summary>
+        /// Gets the fix name to use when logging.
+        /// </summary>
+        public abstract string Name { get; }
+
         /// <summary>
         /// Gets the fix category this formatter belongs to.
         /// </summary>
@@ -113,7 +116,7 @@ internal abstract class DocumentFormatter : ICodeFormatter
         /// Applies the changed <see cref="SourceText"/> to each formatted <see cref="Document"/>.
         /// </summary>
         private async Task<Solution> ApplyFileChangesAsync(
-           Solution solution,
+            Solution solution,
             ImmutableArray<(Document, Task<(SourceText originalText, SourceText? formattedText)>)> formattedDocuments,
             FormatOptions formatOptions,
             ILogger logger,
@@ -141,7 +144,7 @@ internal abstract class DocumentFormatter : ICodeFormatter
                     continue;
                 }
 
-                var fileChanges = GetFileChanges(formatOptions, formatOptions.WorkspaceFilePath, document.FilePath, originalText, formattedText, formatOptions.ChangesAreErrors, logger);
+                var fileChanges = GetFileChanges(formatOptions, document, originalText, formattedText, formatOptions.ChangesAreErrors, logger);
                 formattedFiles.Add(new FormattedFile(document, fileChanges));
 
                 formattedSolution = formattedSolution.WithDocumentText(document.Id, formattedText, PreservationMode.PreserveIdentity);
@@ -150,14 +153,8 @@ internal abstract class DocumentFormatter : ICodeFormatter
             return formattedSolution;
         }
 
-        private ImmutableArray<FileChange> GetFileChanges(FormatOptions formatOptions, string workspacePath, string filePath, SourceText originalText, SourceText formattedText, bool changesAreErrors, ILogger logger)
+        private ImmutableArray<FileChange> GetFileChanges(FormatOptions formatOptions, Document document, SourceText originalText, SourceText formattedText, bool changesAreErrors, ILogger logger)
         {
-            var workspaceFolder = Path.GetDirectoryName(workspacePath);
-            if (workspaceFolder is null)
-            {
-                throw new Exception($"Unable to find directory name for '{workspacePath}'");
-            }
-
             var fileChanges = ImmutableArray.CreateBuilder<FileChange>();
             var changes = formattedText.GetChangeRanges(originalText);
 
@@ -169,28 +166,15 @@ private ImmutableArray<FileChange> GetFileChanges(FormatOptions formatOptions, s
                 var fileChange = new FileChange(changePosition, FormatWarningDescription);
                 fileChanges.Add(fileChange);
 
-                if (!formatOptions.SaveFormattedFiles || formatOptions.LogLevel == LogLevel.Trace)
+                if (!formatOptions.SaveFormattedFiles || formatOptions.LogLevel == LogLevel.Debug)
                 {
-                    LogFormattingChanges(filePath, changesAreErrors, logger, workspaceFolder, fileChange);
+                    logger.LogFormattingIssue(document, Name, fileChange, changesAreErrors);
                 }
             }
 
             return fileChanges.ToImmutable();
         }
 
-        private static void LogFormattingChanges(string filePath, bool changesAreErrors, ILogger logger, string workspaceFolder, FileChange fileChange)
-        {
-            var formatMessage = $"{Path.GetRelativePath(workspaceFolder, filePath)}({fileChange.LineNumber},{fileChange.CharNumber}): {fileChange.FormatDescription}";
-            if (changesAreErrors)
-            {
-                logger.LogError(formatMessage);
-            }
-            else
-            {
-                logger.LogWarning(formatMessage);
-            }
-        }
-
         protected static async Task<bool> IsSameDocumentAndVersionAsync(Document a, Document b, CancellationToken cancellationToken)
         {
             if (a == b)

diff --git a/src/Formatters/EndOfLineFormatter.cs b/src/Formatters/EndOfLineFormatter.cs
@@ -15,6 +15,7 @@ internal sealed class EndOfLineFormatter : DocumentFormatter
     {
         protected override string FormatWarningDescription => Resources.Fix_end_of_line_marker;
 
+        public override string Name => "ENDOFLINE";
         public override FixCategory Category => FixCategory.Whitespace;
 
         internal override Task<SourceText> FormatFileAsync(

diff --git a/src/Formatters/FinalNewlineFormatter.cs b/src/Formatters/FinalNewlineFormatter.cs
@@ -14,6 +14,7 @@ internal sealed class FinalNewlineFormatter : DocumentFormatter
     {
         protected override string FormatWarningDescription => Resources.Fix_final_newline;
 
+        public override string Name => "FINALNEWLINE";
         public override FixCategory Category => FixCategory.Whitespace;
 
         internal override async Task<SourceText> FormatFileAsync(

diff --git a/src/Formatters/OrganizeImportsFormatter.cs b/src/Formatters/OrganizeImportsFormatter.cs
@@ -20,6 +20,7 @@ internal sealed class OrganizeImportsFormatter : DocumentFormatter
         protected override string FormatWarningDescription => Resources.Fix_imports_ordering;
         private readonly DocumentFormatter _endOfLineFormatter = new EndOfLineFormatter();
 
+        public override string Name => "IMPORTS";
         public override FixCategory Category => FixCategory.Whitespace;
 
         internal override async Task<SourceText> FormatFileAsync(

diff --git a/src/Formatters/UnnecessaryImportsFormatter.cs b/src/Formatters/UnnecessaryImportsFormatter.cs
@@ -20,6 +20,7 @@ internal sealed class UnnecessaryImportsFormatter : DocumentFormatter
 
         protected override string FormatWarningDescription => Resources.Remove_unnecessary_import;
 
+        public override string Name => IDE0005;
         public override FixCategory Category => FixCategory.CodeStyle;
 
         internal override async Task<SourceText> FormatFileAsync(

diff --git a/src/Formatters/WhitespaceFormatter.cs b/src/Formatters/WhitespaceFormatter.cs
@@ -17,6 +17,7 @@ internal sealed class WhitespaceFormatter : DocumentFormatter
     {
         protected override string FormatWarningDescription => Resources.Fix_whitespace_formatting;
 
+        public override string Name => "WHITESPACE";
         public override FixCategory Category => FixCategory.Whitespace;
 
         internal override async Task<SourceText> FormatFileAsync(

diff --git a/src/Logging/IIssueFormatter.cs b/src/Logging/IIssueFormatter.cs
@@ -0,0 +1,9 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+namespace Microsoft.CodeAnalysis.Tools.Logging
+{
+    public interface IIssueFormatter
+    {
+        string FormatIssue(Document document, string severity, string issueId, int lineNumber, int charNumber, string message);
+    }
+}
diff --git a/src/Logging/ILoggerExtensions.cs b/src/Logging/ILoggerExtensions.cs
@@ -0,0 +1,37 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using Microsoft.CodeAnalysis.Text;
+using Microsoft.CodeAnalysis.Tools.Logging;
+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.CodeAnalysis.Tools
+{
+    internal static class ILoggerExtensions
+    {
+        private static readonly string s_errorSeverityString = DiagnosticSeverity.Error.ToString().ToLower();
+
+        public static IIssueFormatter IssueFormatter { get; set; } = new MSBuildIssueFormatter();
+
+        public static string LogFormattingIssue(this ILogger logger, Document document, string formatterName, FileChange fileChange, bool changesAreErrors)
+            => LogIssue(logger, document, s_errorSeverityString, formatterName, fileChange.LineNumber, fileChange.CharNumber, fileChange.FormatDescription, changesAreErrors);
+
+        public static string LogDiagnosticIssue(this ILogger logger, Document document, LinePosition diagnosticPosition, Diagnostic diagnostic, bool changesAreErrors)
+            => LogIssue(logger, document, diagnostic.Severity.ToString().ToLower(), diagnostic.Id, diagnosticPosition.Line + 1, diagnosticPosition.Character + 1, diagnostic.GetMessage(), changesAreErrors);
+
+        private static string LogIssue(ILogger logger, Document document, string severity, string issueId, int lineNumber, int charNumber, string message, bool changesAreErrors)
+        {
+            var formattedMessage = IssueFormatter.FormatIssue(document, severity, issueId, lineNumber, charNumber, message);
+
+            if (changesAreErrors)
+            {
+                logger.LogError(formattedMessage);
+            }
+            else
+            {
+                logger.LogWarning(formattedMessage);
+            }
+
+            return formattedMessage;
+        }
+    }
+}
diff --git a/src/Logging/MSBuildIssueFormatter.cs b/src/Logging/MSBuildIssueFormatter.cs
@@ -0,0 +1,10 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+namespace Microsoft.CodeAnalysis.Tools.Logging
+{
+    internal sealed class MSBuildIssueFormatter : IIssueFormatter
+    {
+        public string FormatIssue(Document document, string severity, string issueId, int lineNumber, int charNumber, string message)
+            => $"{document.FilePath ?? document.Name}({lineNumber},{charNumber}): {severity} {issueId}: {message} [{document.Project.FilePath}]";
+    }
+}
diff --git a/src/Logging/SimpleConsoleLogger.cs b/src/Logging/SimpleConsoleLogger.cs
@@ -83,7 +83,7 @@ private void LogToConsole(IConsole console, string message, bool logToErrorStrea
         {
             if (logToErrorStream)
             {
-                console.Error.Write($"  {message}{Environment.NewLine}");
+                console.Error.Write($"{message}{Environment.NewLine}");
             }
             else
             {