Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Only report fixable compiler diagnostics. #981

Merged
merged 1 commit into from Feb 16, 2021
Merged

Only report fixable compiler diagnostics. #981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/Analyzers/AnalyzerFormatter.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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