Skip to content

Commit

Permalink
Code cleanup and refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
@JoeRobich
JoeRobich committed Jun 3, 2020
1 parent 07ac26d commit 73ce6c1
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 78 deletions.
112 changes: 65 additions & 47 deletions src/Analyzers/AnalyzerFormatter.cs
View file
Expand Up @@ -2,6 +2,7 @@

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Threading;
Expand Down Expand Up @@ -40,76 +41,93 @@ internal class AnalyzerFormatter : ICodeFormatter
var analysisStopwatch = Stopwatch.StartNew();
logger.LogTrace($"Analyzing code style.");

var paths = formattableDocuments.Select(id => solution.GetDocument(id)?.FilePath)
.OfType<string>().ToImmutableArray();

var pairs = _finder.GetAnalyzersAndFixers();

if (!options.SaveFormattedFiles)
{
// no need to run codefixes as we won't persist the changes
var analyzers = pairs.Select(x => x.Analyzer).ToImmutableArray();
var result = new CodeAnalysisResult();
await solution.Projects.ForEachAsync(async (project, token) =>
{
await _runner.RunCodeAnalysisAsync(result, analyzers, project, paths, logger, token);
}, cancellationToken);

LogDiagnosticLocations(result.Diagnostics.SelectMany(kvp => kvp.Value), options.WorkspaceFilePath, options.ChangesAreErrors, logger);
await LogDiagnosticsAsync(solution, formattableDocuments, options, logger, cancellationToken);
}
else
{
// we need to run each codefix iteratively so ensure that all diagnostics are found and fixed
foreach (var (analyzer, codefix) in pairs)
{
var result = new CodeAnalysisResult();
await solution.Projects.ForEachAsync(async (project, token) =>
{
await _runner.RunCodeAnalysisAsync(result, analyzer, project, paths, logger, token);
}, cancellationToken);

var hasDiagnostics = result.Diagnostics.Any(kvp => kvp.Value.Length > 0);
if (hasDiagnostics && codefix is object)
{
logger.LogTrace($"Applying fixes for {codefix.GetType().Name}");
solution = await _applier.ApplyCodeFixesAsync(solution, result, codefix, logger, cancellationToken);
var changedSolution = await _applier.ApplyCodeFixesAsync(solution, result, codefix, logger, cancellationToken);
if (changedSolution.GetChanges(solution).Any())
{
solution = changedSolution;
}
}
}
solution = await FixDiagnosticsAsync(solution, formattableDocuments, logger, cancellationToken);
}

logger.LogTrace("Analysis complete in {0}ms.", analysisStopwatch.ElapsedMilliseconds);

return solution;
}

private void LogDiagnosticLocations(IEnumerable<Diagnostic> diagnostics, string workspacePath, bool changesAreErrors, ILogger logger)
private async Task LogDiagnosticsAsync(Solution solution, ImmutableArray<DocumentId> formattableDocuments, FormatOptions options, ILogger logger, CancellationToken cancellationToken)
{
var workspaceFolder = Path.GetDirectoryName(workspacePath);
var pairs = _finder.GetAnalyzersAndFixers();
var paths = formattableDocuments.Select(id => solution.GetDocument(id)?.FilePath)
.OfType<string>().ToImmutableArray();

foreach (var diagnostic in diagnostics)
// no need to run codefixes as we won't persist the changes
var analyzers = pairs.Select(x => x.Analyzer).ToImmutableArray();
var result = new CodeAnalysisResult();
await solution.Projects.ForEachAsync(async (project, token) =>
{
var message = diagnostic.GetMessage();
var filePath = diagnostic.Location.SourceTree?.FilePath;
await _runner.RunCodeAnalysisAsync(result, analyzers, project, paths, logger, token);
}, cancellationToken);

LogDiagnosticLocations(result.Diagnostics.SelectMany(kvp => kvp.Value), options.WorkspaceFilePath, options.ChangesAreErrors, logger);

var mappedLineSpan = diagnostic.Location.GetMappedLineSpan();
var changePosition = mappedLineSpan.StartLinePosition;
return;

var formatMessage = $"{Path.GetRelativePath(workspaceFolder, filePath)}({changePosition.Line + 1},{changePosition.Character + 1}): {message}";
static void LogDiagnosticLocations(IEnumerable<Diagnostic> diagnostics, string workspacePath, bool changesAreErrors, ILogger logger)
{
var workspaceFolder = Path.GetDirectoryName(workspacePath);

if (changesAreErrors)
foreach (var diagnostic in diagnostics)
{
logger.LogError(formatMessage);
var message = diagnostic.GetMessage();
var filePath = diagnostic.Location.SourceTree?.FilePath;

var mappedLineSpan = diagnostic.Location.GetMappedLineSpan();
var changePosition = mappedLineSpan.StartLinePosition;

var formatMessage = $"{Path.GetRelativePath(workspaceFolder, filePath)}({changePosition.Line + 1},{changePosition.Character + 1}): {message}";

if (changesAreErrors)
{
logger.LogError(formatMessage);
}
else
{
logger.LogWarning(formatMessage);
}
}
else
}
}

private async Task<Solution> FixDiagnosticsAsync(Solution solution, ImmutableArray<DocumentId> formattableDocuments, ILogger logger, CancellationToken cancellationToken)
{
var pairs = _finder.GetAnalyzersAndFixers();
var paths = formattableDocuments.Select(id => solution.GetDocument(id)?.FilePath)
.OfType<string>().ToImmutableArray();

// we need to run each codefix iteratively so ensure that all diagnostics are found and fixed
foreach (var (analyzer, codefix) in pairs)
{
var result = new CodeAnalysisResult();
await solution.Projects.ForEachAsync(async (project, token) =>
{
logger.LogWarning(formatMessage);
await _runner.RunCodeAnalysisAsync(result, analyzer, project, paths, logger, token);
}, cancellationToken);

var hasDiagnostics = result.Diagnostics.Any(kvp => kvp.Value.Count > 0);
if (hasDiagnostics && codefix is object)
{
logger.LogTrace($"Applying fixes for {codefix.GetType().Name}");
solution = await _applier.ApplyCodeFixesAsync(solution, result, codefix, logger, cancellationToken);
var changedSolution = await _applier.ApplyCodeFixesAsync(solution, result, codefix, logger, cancellationToken);
if (changedSolution.GetChanges(solution).Any())
{
solution = changedSolution;
}
}
}

return solution;
}
}
}
18 changes: 11 additions & 7 deletions src/Analyzers/AnalyzerRunner.cs
View file
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
// 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.Immutable;
Expand Down Expand Up @@ -41,12 +41,16 @@ internal partial class AnalyzerRunner : IAnalyzerRunner
cancellationToken);
var diagnostics = await analyzerCompilation.GetAnalyzerDiagnosticsAsync(cancellationToken);
// filter diagnostics
var filteredDiagnostics = diagnostics.Where(
x => !x.IsSuppressed &&
x.Severity >= DiagnosticSeverity.Warning &&
x.Location.IsInSource &&
formattableDocumentPaths.Contains(x.Location.SourceTree?.FilePath, StringComparer.OrdinalIgnoreCase));
result.AddDiagnostic(project, filteredDiagnostics);
foreach (var diagnostic in diagnostics)
{
if (!diagnostic.IsSuppressed &&
diagnostic.Severity >= DiagnosticSeverity.Warning &&
diagnostic.Location.IsInSource &&
formattableDocumentPaths.Contains(diagnostic.Location.SourceTree?.FilePath, StringComparer.OrdinalIgnoreCase))
{
result.AddDiagnostic(project, diagnostic);
}
}
}
}
}
17 changes: 5 additions & 12 deletions src/Analyzers/CodeAnalysisResult.cs
View file
@@ -1,8 +1,6 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

using NonBlocking;

Expand All @@ -13,23 +11,18 @@ internal class CodeAnalysisResult
private readonly ConcurrentDictionary<Project, List<Diagnostic>> _dictionary
= new ConcurrentDictionary<Project, List<Diagnostic>>();

internal void AddDiagnostic(Project project, IEnumerable<Diagnostic> diagnostics)
internal void AddDiagnostic(Project project, Diagnostic diagnostic)
{
_ = _dictionary.AddOrUpdate(project,
addValueFactory: (key) => diagnostics.ToList(),
addValueFactory: (key) => new List<Diagnostic>() { diagnostic },
updateValueFactory: (key, list) =>
{
list.AddRange(diagnostics);
list.Add(diagnostic);
return list;
});
}

public IReadOnlyDictionary<Project, ImmutableArray<Diagnostic>> Diagnostics
=> new Dictionary<Project, ImmutableArray<Diagnostic>>(
_dictionary.Select(
x => KeyValuePair.Create(
x.Key,
x.Value.ToImmutableArray())));

public IReadOnlyDictionary<Project, List<Diagnostic>> Diagnostics
=> _dictionary;
}
}
4 changes: 3 additions & 1 deletion src/Analyzers/RoslynCodeStyleAnalyzerFinder.cs
View file
Expand Up @@ -14,12 +14,13 @@ internal class RoslynCodeStyleAnalyzerFinder : IAnalyzerFinder
private readonly static string s_executingPath = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);

private readonly string _featuresCSharpPath = Path.Combine(s_executingPath, "Microsoft.CodeAnalysis.CSharp.Features.dll");
private readonly string _featuresVisualBasicPath = Path.Combine(s_executingPath, "Microsoft.CodeAnalysis.CSharp.VisualBasic.dll");
private readonly string _featuresVisualBasicPath = Path.Combine(s_executingPath, "Microsoft.CodeAnalysis.VisualBasic.Features.dll");

public ImmutableArray<(DiagnosticAnalyzer Analyzer, CodeFixProvider? Fixer)> GetAnalyzersAndFixers()
{
var analyzers = FindAllAnalyzers();

// TODO: Match CodeFixes to the analyzers that produce the diagnostic ids they fix.
return analyzers.Select(analyzer => (analyzer, (CodeFixProvider?)null)).ToImmutableArray();
}

Expand All @@ -37,6 +38,7 @@ private ImmutableArray<DiagnosticAnalyzer> FindAllAnalyzers()

private ImmutableArray<CodeFixProvider> FindAllCodeFixesAsync()
{
// TODO: Discover CodeFixes
return ImmutableArray<CodeFixProvider>.Empty;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Analyzers/SolutionCodeFixApplier.cs
View file
Expand Up @@ -51,7 +51,7 @@ internal class SolutionCodeFixApplier : ICodeFixApplier
private class DiagnosticProvider : FixAllContext.DiagnosticProvider
{
private static readonly Task<IEnumerable<Diagnostic>> EmptyDignosticResult = Task.FromResult(Enumerable.Empty<Diagnostic>());
private readonly IReadOnlyDictionary<Project, ImmutableArray<Diagnostic>> _diagnosticsByProject;
private readonly IReadOnlyDictionary<Project, List<Diagnostic>> _diagnosticsByProject;

internal DiagnosticProvider(CodeAnalysisResult analysisResult)
{
Expand Down
25 changes: 20 additions & 5 deletions tests/Extensions/ExportProviderExtensions.cs
View file
Expand Up @@ -29,7 +29,7 @@ public CompositionContextShim(ExportProvider exportProvider)
public override bool TryGetExport(CompositionContract contract, out object export)
{
var importMany = contract.MetadataConstraints.Contains(new KeyValuePair<string, object>("IsImportMany", true));
var (contractType, metadataType) = GetContractType(contract.ContractType, importMany);
var (contractType, metadataType, isArray) = GetContractType(contract.ContractType, importMany);

if (metadataType != null)
{
Expand All @@ -41,7 +41,7 @@ where method.GetParameters().Length == 1 && method.GetParameters()[0].ParameterT
var parameterizedMethod = methodInfo.MakeGenericMethod(contractType, metadataType);
export = parameterizedMethod.Invoke(_exportProvider, new[] { contract.ContractName });
}
else
else if (!isArray)
{
var methodInfo = (from method in _exportProvider.GetType().GetTypeInfo().GetMethods()
where method.Name == nameof(ExportProvider.GetExports)
Expand All @@ -51,12 +51,27 @@ where method.GetParameters().Length == 1 && method.GetParameters()[0].ParameterT
var parameterizedMethod = methodInfo.MakeGenericMethod(contractType);
export = parameterizedMethod.Invoke(_exportProvider, new[] { contract.ContractName });
}
else
{
var methodInfo = (from method in _exportProvider.GetType().GetTypeInfo().GetMethods()
where method.Name == nameof(ExportProvider.GetExportedValues)
where method.IsGenericMethod && method.GetGenericArguments().Length == 1
where method.GetParameters().Length == 0
select method).Single();
var parameterizedMethod = methodInfo.MakeGenericMethod(contractType);
export = parameterizedMethod.Invoke(_exportProvider, null);
}

return true;
}

private (Type exportType, Type metadataType) GetContractType(Type contractType, bool importMany)
private (Type exportType, Type metadataType, bool isArray) GetContractType(Type contractType, bool importMany)
{
if (importMany && contractType.BaseType == typeof(Array))
{
return (contractType.GetElementType(), null, true);
}

if (importMany && contractType.IsConstructedGenericType)
{
if (contractType.GetGenericTypeDefinition() == typeof(IList<>)
Expand All @@ -71,11 +86,11 @@ where method.GetParameters().Length == 1 && method.GetParameters()[0].ParameterT
{
if (contractType.GetGenericTypeDefinition() == typeof(Lazy<>))
{
return (contractType.GenericTypeArguments[0], null);
return (contractType.GenericTypeArguments[0], null, false);
}
else if (contractType.GetGenericTypeDefinition() == typeof(Lazy<,>))
{
return (contractType.GenericTypeArguments[0], contractType.GenericTypeArguments[1]);
return (contractType.GenericTypeArguments[0], contractType.GenericTypeArguments[1], false);
}
else
{
Expand Down
7 changes: 2 additions & 5 deletions tests/Formatters/AbstractFormatterTests.cs
View file
Expand Up @@ -69,15 +69,12 @@ static AbstractFormatterTest()

protected AbstractFormatterTest()
{
TestState = new SolutionState(DefaultFolderPath + DefaultFilePathPrefix, DefaultFileExt);
TestState = new SolutionState(DefaultFilePathPrefix, DefaultFileExt);
}

/// <summary>
/// Gets the language name used for the test.
/// </summary>
/// <value>
/// The language name used for the test.
/// </value>
public abstract string Language { get; }

private static ILogger Logger => new TestLogger();
Expand Down Expand Up @@ -238,7 +235,7 @@ protected virtual Project CreateProjectImpl((string filename, SourceText content
{
(var newFileName, var source) = sources[i];
var documentId = DocumentId.CreateNewId(projectId, debugName: newFileName);
solution = solution.AddDocument(documentId, newFileName, source, filePath: newFileName);
solution = solution.AddDocument(documentId, newFileName, source, filePath: Path.Combine(DefaultTestProjectPath, newFileName));
}

for (var i = 0; i < additionalFiles.Length; i++)
Expand Down
10 changes: 10 additions & 0 deletions tests/dotnet-format.UnitTests.csproj
View file
Expand Up @@ -17,6 +17,16 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.VisualBasic" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.Workspaces.Common" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.VisualBasic.Workspaces" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.Features" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Features" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.VisualBasic.Features" Version="$(MicrosoftCodeAnalysisVersion)" />

<PackageReference Include="Microsoft.Build.Locator" Version="$(MicrosoftBuildLocatorVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="$(MicrosoftCodeAnalysisAnalyzersVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.Analyzer.Testing" Version="$(MicrosoftCodeAnalysisAnalyzerTestingVersion)" />
Expand Down

0 comments on commit 73ce6c1

Please sign in to comment.