Skip to content

Commit

Permalink
Merge pull request #981 from JoeRobich/report-fixable-diagnostics
Browse files Browse the repository at this point in the history 
Only report fixable compiler diagnostics.
  • Loading branch information
@JoeRobich
JoeRobich committed Feb 16, 2021
2 parents 742a537 + 5a80629 commit ab1d826
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 33 deletions.
18 changes: 9 additions & 9 deletions src/Analyzers/AnalyzerFormatter.cs
View file
Expand Up @@ -68,9 +68,9 @@ internal class AnalyzerFormatter : ICodeFormatter
var allFixers = projectAnalyzersAndFixers.Values.SelectMany(analyzersAndFixers => analyzersAndFixers.Fixers).ToImmutableArray();

// Only include compiler diagnostics if we have a fixer that can fix them.
var includeCompilerDiagnostics = allFixers.Any(
codefix => codefix.FixableDiagnosticIds.Any(
id => id.StartsWith("CS") || id.StartsWith("BC")));
var fixableCompilerDiagnostics = allFixers
.SelectMany(codefix => codefix.FixableDiagnosticIds.Where(id => id.StartsWith("CS") || id.StartsWith("BC")))
.ToImmutableHashSet();

var analysisStopwatch = Stopwatch.StartNew();
logger.LogTrace(Resources.Running_0_analysis, _name);
Expand All @@ -86,7 +86,7 @@ internal class AnalyzerFormatter : ICodeFormatter
var projectAnalyzers = await FilterBySeverityAsync(solution, projectAnalyzersAndFixers, formattablePaths, severity, cancellationToken).ConfigureAwait(false);

// Determine which diagnostics are being reported for each project.
var projectDiagnostics = await GetProjectDiagnosticsAsync(solution, projectAnalyzers, formattablePaths, formatOptions, severity, includeCompilerDiagnostics, logger, formattedFiles, cancellationToken).ConfigureAwait(false);
var projectDiagnostics = await GetProjectDiagnosticsAsync(solution, projectAnalyzers, formattablePaths, formatOptions, severity, fixableCompilerDiagnostics, logger, formattedFiles, cancellationToken).ConfigureAwait(false);

var projectDiagnosticsMS = analysisStopwatch.ElapsedMilliseconds;
logger.LogTrace(Resources.Complete_in_0_ms, projectDiagnosticsMS);
Expand All @@ -97,7 +97,7 @@ internal class AnalyzerFormatter : ICodeFormatter
logger.LogTrace(Resources.Fixing_diagnostics);

// Run each analyzer individually and apply fixes if possible.
solution = await FixDiagnosticsAsync(solution, projectAnalyzers, allFixers, projectDiagnostics, formattablePaths, severity, includeCompilerDiagnostics, logger, cancellationToken).ConfigureAwait(false);
solution = await FixDiagnosticsAsync(solution, projectAnalyzers, allFixers, projectDiagnostics, formattablePaths, severity, fixableCompilerDiagnostics, logger, cancellationToken).ConfigureAwait(false);

var fixDiagnosticsMS = analysisStopwatch.ElapsedMilliseconds - projectDiagnosticsMS;
logger.LogTrace(Resources.Complete_in_0_ms, fixDiagnosticsMS);
Expand All @@ -114,7 +114,7 @@ internal class AnalyzerFormatter : ICodeFormatter
ImmutableHashSet<string> formattablePaths,
FormatOptions options,
DiagnosticSeverity severity,
bool includeCompilerDiagnostics,
ImmutableHashSet<string> fixableCompilerDiagnostics,
ILogger logger,
List<FormattedFile> formattedFiles,
CancellationToken cancellationToken)
Expand All @@ -132,7 +132,7 @@ internal class AnalyzerFormatter : ICodeFormatter
}

// Run all the filtered analyzers to determine which are reporting diagnostic.
await _runner.RunCodeAnalysisAsync(result, analyzers, project, formattablePaths, severity, includeCompilerDiagnostics, logger, cancellationToken).ConfigureAwait(false);
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);
Expand Down Expand Up @@ -179,7 +179,7 @@ static void LogDiagnosticLocations(Solution solution, IEnumerable<Diagnostic> di
ImmutableDictionary<ProjectId, ImmutableHashSet<string>> projectDiagnostics,
ImmutableHashSet<string> formattablePaths,
DiagnosticSeverity severity,
bool includeCompilerDiagnostics,
ImmutableHashSet<string> fixableCompilerDiagnostics,
ILogger logger,
CancellationToken cancellationToken)
{
Expand Down Expand Up @@ -217,7 +217,7 @@ static void LogDiagnosticLocations(Solution solution, IEnumerable<Diagnostic> di
var analyzers = projectAnalyzers[project.Id]
.Where(analyzer => analyzer.SupportedDiagnostics.Any(descriptor => descriptor.Id == diagnosticId))
.ToImmutableArray();
await _runner.RunCodeAnalysisAsync(result, analyzers, project, formattablePaths, severity, includeCompilerDiagnostics, logger, cancellationToken).ConfigureAwait(false);
await _runner.RunCodeAnalysisAsync(result, analyzers, project, formattablePaths, severity, fixableCompilerDiagnostics, logger, cancellationToken).ConfigureAwait(false);
}

var hasDiagnostics = result.Diagnostics.Any(kvp => kvp.Value.Count > 0);
Expand Down
22 changes: 14 additions & 8 deletions src/Analyzers/AnalyzerRunner.cs
View file
Expand Up @@ -17,24 +17,24 @@ internal partial class AnalyzerRunner : IAnalyzerRunner
Project project,
ImmutableHashSet<string> formattableDocumentPaths,
DiagnosticSeverity severity,
bool includeCompilerDiagnostics,
ImmutableHashSet<string> fixableCompilerDiagnostics,
ILogger logger,
CancellationToken cancellationToken)
=> RunCodeAnalysisAsync(result, ImmutableArray.Create(analyzers), project, formattableDocumentPaths, severity, includeCompilerDiagnostics, logger, cancellationToken);
=> RunCodeAnalysisAsync(result, ImmutableArray.Create(analyzers), project, formattableDocumentPaths, severity, fixableCompilerDiagnostics, logger, cancellationToken);

public async Task RunCodeAnalysisAsync(
CodeAnalysisResult result,
ImmutableArray<DiagnosticAnalyzer> analyzers,
Project project,
ImmutableHashSet<string> formattableDocumentPaths,
DiagnosticSeverity severity,
bool includeCompilerDiagnostics,
ImmutableHashSet<string> fixableCompilerDiagnostics,
ILogger logger,
CancellationToken cancellationToken)
{
// If are not running any analyzers and are not reporting compiler diagnostics, then there is
// nothing to report.
if (analyzers.IsEmpty && includeCompilerDiagnostics)
if (analyzers.IsEmpty && fixableCompilerDiagnostics.IsEmpty)
{
return;
}
Expand All @@ -53,10 +53,16 @@ internal partial class AnalyzerRunner : IAnalyzerRunner
return;
}

var compilerDiagnostics = !fixableCompilerDiagnostics.IsEmpty
? compilation.GetDiagnostics(cancellationToken)
.Where(diagnostic => fixableCompilerDiagnostics.Contains(diagnostic.Id))
.ToImmutableArray()
: ImmutableArray<Diagnostic>.Empty;

ImmutableArray<Diagnostic> diagnostics;
if (analyzers.IsEmpty)
{
diagnostics = compilation.GetDiagnostics(cancellationToken);
diagnostics = compilerDiagnostics;
}
else
{
Expand All @@ -69,9 +75,9 @@ internal partial class AnalyzerRunner : IAnalyzerRunner
logAnalyzerExecutionTime: false,
reportSuppressedDiagnostics: false);
var analyzerCompilation = compilation.WithAnalyzers(analyzers, analyzerOptions);
diagnostics = includeCompilerDiagnostics
? await analyzerCompilation.GetAllDiagnosticsAsync(cancellationToken).ConfigureAwait(false)
: await analyzerCompilation.GetAnalyzerDiagnosticsAsync(cancellationToken).ConfigureAwait(false);

diagnostics = await analyzerCompilation.GetAnalyzerDiagnosticsAsync(cancellationToken).ConfigureAwait(false);
diagnostics = diagnostics.AddRange(compilerDiagnostics);
}

// filter diagnostics
Expand Down
4 changes: 2 additions & 2 deletions src/Analyzers/Interfaces/IAnalyzerRunner.cs
View file
Expand Up @@ -16,7 +16,7 @@ internal interface IAnalyzerRunner
Project project,
ImmutableHashSet<string> formattableDocumentPaths,
DiagnosticSeverity severity,
bool includeCompilerDiagnostics,
ImmutableHashSet<string> fixableCompilerDiagnostics,
ILogger logger,
CancellationToken cancellationToken);

Expand All @@ -26,7 +26,7 @@ internal interface IAnalyzerRunner
Project project,
ImmutableHashSet<string> formattableDocumentPaths,
DiagnosticSeverity severity,
bool includeCompilerDiagnostics,
ImmutableHashSet<string> fixableCompilerDiagnostics,
ILogger logger,
CancellationToken cancellationToken);
}
Expand Down
15 changes: 15 additions & 0 deletions tests/Analyzers/CodeStyleAnalyzerFormatterTests.cs
View file
Expand Up @@ -59,5 +59,20 @@ void M()

await AssertCodeChangedAsync(testCode, expectedCode, editorConfig, fixCategory: FixCategory.CodeStyle);
}

[Fact]
public async Task TestNonFixableCompilerDiagnostics_AreNotReported()
{
var testCode = @"
class C
{
public int M()
{
return null; // Cannot convert null to 'int' because it is a non-nullable value type (CS0037)
}
}";

await AssertNoReportedFileChangesAsync(testCode, "root = true", fixCategory: FixCategory.CodeStyle, codeStyleSeverity: DiagnosticSeverity.Warning);
}
}
}
108 changes: 94 additions & 14 deletions tests/Formatters/AbstractFormatterTests.cs
View file
Expand Up @@ -86,6 +86,46 @@ protected AbstractFormatterTest()
{ string.Join(Environment.NewLine, editorConfig.Select(kvp => $"{kvp.Key} = {kvp.Value}")) }
";

private protected Task<SourceText> AssertNoReportedFileChangesAsync(
string code,
IReadOnlyDictionary<string, string> editorConfig,
Encoding? encoding = null,
FixCategory fixCategory = FixCategory.Whitespace,
IEnumerable<AnalyzerReference>? analyzerReferences = null,
DiagnosticSeverity codeStyleSeverity = DiagnosticSeverity.Error,
DiagnosticSeverity analyzerSeverity = DiagnosticSeverity.Error)
{
return AssertNoReportedFileChangesAsync(code, ToEditorConfig(editorConfig), encoding, fixCategory, analyzerReferences, codeStyleSeverity, analyzerSeverity);
}

private protected async Task<SourceText> AssertNoReportedFileChangesAsync(
string code,
string editorConfig,
Encoding? encoding = null,
FixCategory fixCategory = FixCategory.Whitespace,
IEnumerable<AnalyzerReference>? analyzerReferences = null,
DiagnosticSeverity codeStyleSeverity = DiagnosticSeverity.Error,
DiagnosticSeverity analyzerSeverity = DiagnosticSeverity.Error)
{
var (formattedText, formattedFiles, logger) = await ApplyFormatterAsync(code, editorConfig, encoding, fixCategory, analyzerReferences, codeStyleSeverity, analyzerSeverity);

try
{
// Ensure the code is unchanged
Assert.Equal(code, formattedText.ToString());

// Ensure no non-fixable diagnostics were reported
Assert.Empty(formattedFiles);
}
catch
{
TestOutputHelper?.WriteLine(logger.GetLog());
throw;
}

return formattedText;
}

private protected Task<SourceText> AssertCodeUnchangedAsync(
string code,
IReadOnlyDictionary<string, string> editorConfig,
Expand All @@ -95,7 +135,32 @@ protected AbstractFormatterTest()
DiagnosticSeverity codeStyleSeverity = DiagnosticSeverity.Error,
DiagnosticSeverity analyzerSeverity = DiagnosticSeverity.Error)
{
return AssertCodeChangedAsync(code, code, ToEditorConfig(editorConfig), encoding, fixCategory, analyzerReferences, codeStyleSeverity, analyzerSeverity);
return AssertCodeUnchangedAsync(code, ToEditorConfig(editorConfig), encoding, fixCategory, analyzerReferences, codeStyleSeverity, analyzerSeverity);
}

private protected async Task<SourceText> AssertCodeUnchangedAsync(
string code,
string editorConfig,
Encoding? encoding = null,
FixCategory fixCategory = FixCategory.Whitespace,
IEnumerable<AnalyzerReference>? analyzerReferences = null,
DiagnosticSeverity codeStyleSeverity = DiagnosticSeverity.Error,
DiagnosticSeverity analyzerSeverity = DiagnosticSeverity.Error)
{
var (formattedText, _, logger) = await ApplyFormatterAsync(code, editorConfig, encoding, fixCategory, analyzerReferences, codeStyleSeverity, analyzerSeverity);

try
{
// Ensure the code is unchanged
Assert.Equal(code, formattedText.ToString());
}
catch
{
TestOutputHelper?.WriteLine(logger.GetLog());
throw;
}

return formattedText;
}

private protected Task<SourceText> AssertCodeChangedAsync(
Expand All @@ -121,7 +186,31 @@ protected AbstractFormatterTest()
DiagnosticSeverity codeStyleSeverity = DiagnosticSeverity.Error,
DiagnosticSeverity analyzerSeverity = DiagnosticSeverity.Error)
{
var text = SourceText.From(testCode, encoding ?? Encoding.UTF8);
var (formattedText, _, logger) = await ApplyFormatterAsync(testCode, editorConfig, encoding, fixCategory, analyzerReferences, codeStyleSeverity, analyzerSeverity);

try
{
Assert.Equal(expectedCode, formattedText.ToString());
}
catch
{
TestOutputHelper?.WriteLine(logger.GetLog());
throw;
}

return formattedText;
}

private protected async Task<(SourceText FormattedText, List<FormattedFile> FormattedFiles, TestLogger Logger)> ApplyFormatterAsync(
string code,
string editorConfig,
Encoding? encoding = null,
FixCategory fixCategory = FixCategory.Whitespace,
IEnumerable<AnalyzerReference>? analyzerReferences = null,
DiagnosticSeverity codeStyleSeverity = DiagnosticSeverity.Error,
DiagnosticSeverity analyzerSeverity = DiagnosticSeverity.Error)
{
var text = SourceText.From(code, encoding ?? Encoding.UTF8);
TestState.Sources.Add(text);

var solution = await GetSolutionAsync(TestState.Sources.ToArray(), TestState.AdditionalFiles.ToArray(), TestState.AdditionalReferences.ToArray(), editorConfig, analyzerReferences);
Expand All @@ -145,22 +234,13 @@ protected AbstractFormatterTest()
var pathsToFormat = GetOnlyFileToFormat(solution);

var logger = new TestLogger();
var formattedFiles = new List<FormattedFile>();

var formattedSolution = await Formatter.FormatAsync(solution, pathsToFormat, formatOptions, logger, new List<FormattedFile>(), default);
var formattedSolution = await Formatter.FormatAsync(solution, pathsToFormat, formatOptions, logger, formattedFiles, default);
var formattedDocument = GetOnlyDocument(formattedSolution);
var formattedText = await formattedDocument.GetTextAsync();

try
{
Assert.Equal(expectedCode, formattedText.ToString());
}
catch
{
TestOutputHelper?.WriteLine(logger.GetLog());
throw;
}

return formattedText;
return (formattedText, formattedFiles, logger);
}

/// <summary>
Expand Down

0 comments on commit ab1d826

Please sign in to comment.