Skip to content

Commit

Permalink
Roslyn analyzers and code fixes (#1076)
Browse files Browse the repository at this point in the history
* Moved initial logig to fork.

* Version with 'by project' results.

* Now analyzers run for all projects after startup.

* Small refactorings.

* Small refactoring.

* Refactored analyzers to providers.

* Refactored compilation as single solution bound (review fix).

* Undoed accidental change.

* Before debugging strange type error.

* Simple first working version.

* Cleanup.

* Implemented loading from project analyzer references.

* Cleanup.

* Small lazy invocation tweak.

* Updated cacellation token handling and way how invalid analyzer ref is handled.

* Tweaked assembly discovery algorithm for analyzers.

* Initial tests for analyzers.

* Testfix.

* Improved tests.

* Throtling to avoid certain issues with error reporting.

* Testfix.

* Attempt to fix hanging cake tests on travis

* Refactored diagnostic service.

* Tweaks.

* Refactoring and tweaks.

* Added support for remove unnecessary usings.

* Implemented code fixes support by project analyzer references.

* Made semantic analysis result distinct.

* Update to distinct algorithm.

* Improved version but still has issues with larger projects.

* Fixes for larger project (like omnisharp).

* Testfixes and grouping support on code check service.

* Testfixes.

* Small naming fix.

* Implemented first version of project rulesets.

* Implemented test for testrules.

* Implemented another test for rulesets.

* Tweaks for analyzer responsivity.

* Ooppss...

* Improved analysis threading.

* Cleanup.

* User experience and test tweaks.

* Implemented final tests for rules.

* Undo for accidentally pushed change.

* Fixed test.

* Refactored tests.

* Review fixes expect assembly loader from DI.

* Sorted usings.

* DI fixes, theres currently issue during runtime however.

* Implemented missing method.

* Some testfixes, however theres changes from master that require changes still.

* Fix that is required for cake project system to work with diagnostics.

* Removed obsolete hack.

* More robust initialization routine.

* Small style tweaks.

* Removed empty class.

* Some merge fixes.

* Fixed mistake in wait routine.

* Small readibility update.

* Added todo.

* Fixed support with misc files.

* Restored null safety.

* Fixed mistake.

* Review updates, however theres broken csx diagnostic tests still.

* Fixed custom code actions.

* Testfixes.

* Small review fixes and simplified assembly loader algorithm.

* Mergefix.

* Fixed changed MiscFile project name.

* Renamed service with better name and fixed missing language for analysis.

* Review fixes for code fix provider.

* Small tweaks for linq.

* Immutablity to csharpdiagnostic service.

* Comment to explain non self descriptive initial wait.

* Support for roslyn 'IDE' analyzers.

* Testfixes and added test that IDE analyzers are enabled.

* Better comment on code based on review.

* Testfix for signed project.

* Testfix.

* Small test tweak.

* Buildfix.

* Tweaks for issue where workspace updates comes after request.

* Added assert to protect possibly internal changes of roslyn.

* Increased timeout and added comment about asserting.

* Further tweaked timeouts on event mechanism asserts.

* Increased another 'assert' timeout.

* Fix for threading issue in test utility.

* Trigger to test build stability

* Trigger to test build stability.

* Trigger to test build stability.

* Yet another attempt to get build execute.

* Trigger build

* Added original caller name tracking for OmnisharpTestHostDisposing.

* Attempt to improve robustness of tests.

* Moved tool to test utilities.

* Added missing file.

* Test build robustnes

* Test build robustnes

* Fix for possible null ref issue.

* Fixed mistake on exception handling when using WhenAll.

* Trigger build

* Added fallback code to CodeCheckService, next need to figure out how to configure it.

* Removed duplicate code.

* Added flag to options.

* Use expiremental analysis as default during testing.

* Disabled CsharpDiagnosticService workers if not enabled.

* Buildfix.

* Fix for codefix support when analyzers are disabled.

* Updated flag name.

* Fixed default.

* Renamed flag.

* Initial structure divide for testability.

* Few tests.

* More tests for queue functionality.

* Improved functionality to match exceptations.

* Move towards diagnosis which can be switched by common interface.

* Refactored ugly if switching of diagnostics behind common class.

* Implementation of api in worker with analyzers.

* Small refactoring.

* Added logging when codefix provider cannot be created.

* Moved to more robust time control mechanism.

* Throttling period refactoring.

* Small review fix.

* Small review update.

* Typofix.

* Removed unused member.

* Testfix.

* Testfix.

* Restored sdk version to global.json.

* Added test for multiple thead execution of queue.

* Parametrized tests to test both non roslyn analysis and with analysis.

* Attempt to remove retry from asserts.

* Removed unused code and restored correct global.json.

* File rename.

* Added test to queue facts.

* Conditional action facts with and without analyzers.

* Small refactoring.

* Fix for lsp merge.

* Initial version of single file analysis.

* Fixes and tweaks for analysis, timeouts and error handling.

* Testfixes.

* Testfixes.

* Correct global.json.

* Reduced clutter from message.

* Build stability testing.

* Testfix.

* Test tweak.

* Reattempt with travis builds.

* Threading fix.

* Restored task based version to fix possible deadlock with xunit scheduler.

* Retest builds.

* Added skip to possibly hanging cake tests as test.

* Skipped more cake tests as test.

* Restored some cake tests to find correct hanging one.

* Restored tests to see which one is broken one.

* Restored CodeCheckFacts

* Skipped code checks again because built was aborted.

* CodeCheckFacts from cake tests unignored.

* Restored Can_get_code_actions_from_roslyn to check if it is hanging one.

* Refactored initialization routine for diagnostics and updated test defaults.

* Bugfix.

* Test build stability.

* Tiny tweak for logging and comment.

* Simplify method

* Review fixes.

* Small tweak, startup with larger projects.

* Initial fixes for syntax analysis rulesets.

* Initial fix for rulesets with syntax analysis.

* Moved comment to correct location.

* Logging improvement.

* Test for CS rule apply.

* Removed dummy variable.

* Fix for document loading.

* Fixed mistake on document fetching.

* Attempt to fix duplicate key error.

* Small refactoring.

* Removed unnecessary semantic model get.

* Fix for null ref error on document removal.
  • Loading branch information
savpek authored and david-driscoll committed Mar 19, 2019
1 parent 0fcefe1 commit 62b3b52
Show file tree
Hide file tree
Showing 44 changed files with 1,723 additions and 351 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ public class AutoCompleteResponse
/// </summary>
public string CompletionText { get; set; }
public string Description { get; set; }

/// <summary>
/// The text that should be displayed in the auto-complete UI.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@
namespace OmniSharp.Models.Diagnostics
using System.Collections.Generic;

namespace OmniSharp.Models.Diagnostics
{
public class DiagnosticLocation : QuickFix
{
public string LogLevel { get; set; }
public string Id { get; set; }

public override bool Equals(object obj)
{
var location = obj as DiagnosticLocation;
return location != null &&
base.Equals(obj) &&
LogLevel == location.LogLevel &&
Id == location.Id;
}

public override int GetHashCode()
{
var hashCode = -1670479257;
hashCode = hashCode * -1521134295 + base.GetHashCode();
hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(LogLevel);
hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(Id);
return hashCode;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,10 @@ public class DiagnosticResult
{
public string FileName { get; set; }
public IEnumerable<DiagnosticLocation> QuickFixes { get; set; }

public override string ToString()
{
return $"{FileName} -> {string.Join(", ", QuickFixes)}";
}
}
}
}
2 changes: 1 addition & 1 deletion src/OmniSharp.Abstractions/Models/v1/QuickFix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public override int GetHashCode()
}

public override string ToString()
=> $"({Line}:{Column}) - ({EndLine}:{EndColumn})";
=> $"{Text} ({Line}:{Column}) - ({EndLine}:{EndColumn})";

public bool Contains(int line, int column)
{
Expand Down
4 changes: 4 additions & 0 deletions src/OmniSharp.Host/CompositionHostBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Composition.Hosting.Core;
using System.Linq;
using System.Reflection;
using Microsoft.CodeAnalysis;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -43,6 +44,7 @@ public CompositionHost Build()
var memoryCache = _serviceProvider.GetRequiredService<IMemoryCache>();
var loggerFactory = _serviceProvider.GetRequiredService<ILoggerFactory>();
var assemblyLoader = _serviceProvider.GetRequiredService<IAssemblyLoader>();
var analyzerAssemblyLoader = _serviceProvider.GetRequiredService<IAnalyzerAssemblyLoader>();
var environment = _serviceProvider.GetRequiredService<IOmniSharpEnvironment>();
var eventEmitter = _serviceProvider.GetRequiredService<IEventEmitter>();
var dotNetCliService = _serviceProvider.GetRequiredService<IDotNetCliService>();
Expand Down Expand Up @@ -74,6 +76,7 @@ public CompositionHost Build()
.WithProvider(MefValueProvider.From(options.CurrentValue))
.WithProvider(MefValueProvider.From(options.CurrentValue.FormattingOptions))
.WithProvider(MefValueProvider.From(assemblyLoader))
.WithProvider(MefValueProvider.From(analyzerAssemblyLoader))
.WithProvider(MefValueProvider.From(dotNetCliService))
.WithProvider(MefValueProvider.From(metadataHelper))
.WithProvider(MefValueProvider.From(msbuildLocator))
Expand Down Expand Up @@ -122,6 +125,7 @@ public static IServiceProvider CreateDefaultServiceProvider(
// Caching
services.AddSingleton<IMemoryCache, MemoryCache>();
services.AddSingleton<IAssemblyLoader, AssemblyLoader>();
services.AddSingleton<IAnalyzerAssemblyLoader, AssemblyLoader>();
services.AddOptions();

services.AddSingleton<IDotNetCliService, DotNetCliService>();
Expand Down
13 changes: 12 additions & 1 deletion src/OmniSharp.Host/Services/AssemblyLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
using System.IO;
using System.Reflection;
using Microsoft.Extensions.Logging;
using Microsoft.CodeAnalysis;

namespace OmniSharp.Services
{
internal class AssemblyLoader : IAssemblyLoader
internal class AssemblyLoader : IAssemblyLoader, IAnalyzerAssemblyLoader
{
private static readonly ConcurrentDictionary<string, Assembly> AssemblyCache = new ConcurrentDictionary<string, Assembly>(StringComparer.OrdinalIgnoreCase);
private readonly ILogger _logger;
Expand All @@ -18,6 +19,11 @@ public AssemblyLoader(ILoggerFactory loggerFactory)
_logger = loggerFactory.CreateLogger<AssemblyLoader>();
}

public void AddDependencyLocation(string fullPath)
{
LoadFrom(fullPath);
}

public Assembly Load(AssemblyName name)
{
Assembly result = null;
Expand Down Expand Up @@ -90,5 +96,10 @@ public Assembly LoadFrom(string assemblyPath, bool dontLockAssemblyOnDisk = fals
_logger.LogTrace($"Assembly loaded from path: {assemblyPath}");
return assembly;
}

public Assembly LoadFromPath(string fullPath)
{
return LoadFrom(fullPath);
}
}
}
1 change: 0 additions & 1 deletion src/OmniSharp.Http/HttpCommandLineApplication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ public HttpCommandLineApplication() : base()
_serverInterface = Application.Option("-i | --interface", "Server interface address (defaults to 'localhost').", CommandOptionType.SingleValue);
}


public int Port => _port.GetValueOrDefault(2000);
public string Interface => _serverInterface.GetValueOrDefault("localhost");
}
Expand Down
25 changes: 21 additions & 4 deletions src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.ProjectData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Runtime.Versioning;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
Expand Down Expand Up @@ -46,6 +47,7 @@ private class ProjectData
public ImmutableArray<string> References { get; }
public ImmutableArray<PackageReference> PackageReferences { get; }
public ImmutableArray<string> Analyzers { get; }
public RuleSet RuleSet { get; }
public ImmutableDictionary<string, string> ReferenceAliases { get; }

private ProjectData()
Expand Down Expand Up @@ -78,7 +80,8 @@ private ProjectData(
ImmutableArray<string> preprocessorSymbolNames,
ImmutableArray<string> suppressedDiagnosticIds,
bool signAssembly,
string assemblyOriginatorKeyFile)
string assemblyOriginatorKeyFile,
RuleSet ruleset)
: this()
{
Guid = guid;
Expand All @@ -105,6 +108,7 @@ private ProjectData(

SignAssembly = signAssembly;
AssemblyOriginatorKeyFile = assemblyOriginatorKeyFile;
RuleSet = ruleset;
}

private ProjectData(
Expand All @@ -128,10 +132,11 @@ private ProjectData(
ImmutableArray<string> references,
ImmutableArray<PackageReference> packageReferences,
ImmutableArray<string> analyzers,
RuleSet ruleset,
ImmutableDictionary<string, string> referenceAliases)
: this(guid, name, assemblyName, targetPath, outputPath, intermediateOutputPath, projectAssetsFile,
configuration, platform, targetFramework, targetFrameworks, outputKind, languageVersion, nullableContextOptions, allowUnsafeCode,
documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, signAssembly, assemblyOriginatorKeyFile)
documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, signAssembly, assemblyOriginatorKeyFile, ruleset)
{
SourceFiles = sourceFiles.EmptyIfDefault();
ProjectReferences = projectReferences.EmptyIfDefault();
Expand Down Expand Up @@ -176,7 +181,7 @@ public static ProjectData Create(MSB.Evaluation.Project project)
return new ProjectData(
guid, name, assemblyName, targetPath, outputPath, intermediateOutputPath, projectAssetsFile,
configuration, platform, targetFramework, targetFrameworks, outputKind, languageVersion, nullableContextOptions, allowUnsafeCode,
documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, signAssembly, assemblyOriginatorKeyFile);
documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, signAssembly, assemblyOriginatorKeyFile, ruleset: null);
}

public static ProjectData Create(MSB.Execution.ProjectInstance projectInstance)
Expand Down Expand Up @@ -211,6 +216,8 @@ public static ProjectData Create(MSB.Execution.ProjectInstance projectInstance)
var signAssembly = PropertyConverter.ToBoolean(projectInstance.GetPropertyValue(PropertyNames.SignAssembly), defaultValue: false);
var assemblyOriginatorKeyFile = projectInstance.GetPropertyValue(PropertyNames.AssemblyOriginatorKeyFile);

var ruleset = ResolveRulesetIfAny(projectInstance);

var sourceFiles = GetFullPaths(
projectInstance.GetItems(ItemNames.Compile), filter: FileNameIsNotGenerated);

Expand Down Expand Up @@ -259,7 +266,17 @@ public static ProjectData Create(MSB.Execution.ProjectInstance projectInstance)
configuration, platform, targetFramework, targetFrameworks,
outputKind, languageVersion, nullableContextOptions, allowUnsafeCode, documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds,
signAssembly, assemblyOriginatorKeyFile,
sourceFiles, projectReferences, references.ToImmutable(), packageReferences, analyzers, referenceAliases.ToImmutableDictionary());
sourceFiles, projectReferences, references.ToImmutable(), packageReferences, analyzers, ruleset, referenceAliases.ToImmutableDictionary());
}

private static RuleSet ResolveRulesetIfAny(MSB.Execution.ProjectInstance projectInstance)
{
var rulesetIfAny = projectInstance.Properties.FirstOrDefault(x => x.Name == "ResolvedCodeAnalysisRuleSet");

if (rulesetIfAny != null)
return RuleSet.LoadEffectiveRuleSetFromFile(Path.Combine(projectInstance.Directory, rulesetIfAny.EvaluatedValue));

return null;
}

private static bool IsCSharpProject(string filePath)
Expand Down
1 change: 1 addition & 0 deletions src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ internal partial class ProjectFileInfo

public bool SignAssembly => _data.SignAssembly;
public string AssemblyOriginatorKeyFile => _data.AssemblyOriginatorKeyFile;
public RuleSet RuleSet => _data.RuleSet;

public ImmutableArray<string> SourceFiles => _data.SourceFiles;
public ImmutableArray<string> References => _data.References;
Expand Down
23 changes: 20 additions & 3 deletions src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfoExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
using System.IO;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;
using OmniSharp.Helpers;

namespace OmniSharp.MSBuild.ProjectFile
Expand Down Expand Up @@ -40,8 +44,10 @@ public static CSharpCompilationOptions CreateCompilationOptions(this ProjectFile
return result;
}

public static ProjectInfo CreateProjectInfo(this ProjectFileInfo projectFileInfo)
public static ProjectInfo CreateProjectInfo(this ProjectFileInfo projectFileInfo, IAnalyzerAssemblyLoader analyzerAssemblyLoader)
{
var analyzerReferences = ResolveAnalyzerReferencesForProject(projectFileInfo, analyzerAssemblyLoader);

return ProjectInfo.Create(
id: projectFileInfo.Id,
version: VersionStamp.Create(),
Expand All @@ -50,7 +56,18 @@ public static ProjectInfo CreateProjectInfo(this ProjectFileInfo projectFileInfo
language: LanguageNames.CSharp,
filePath: projectFileInfo.FilePath,
outputFilePath: projectFileInfo.TargetPath,
compilationOptions: projectFileInfo.CreateCompilationOptions());
compilationOptions: projectFileInfo.CreateCompilationOptions(),
analyzerReferences: analyzerReferences);
}

private static IEnumerable<AnalyzerReference> ResolveAnalyzerReferencesForProject(ProjectFileInfo projectFileInfo, IAnalyzerAssemblyLoader analyzerAssemblyLoader)
{
foreach(var analyzerAssemblyPath in projectFileInfo.Analyzers.Distinct())
{
analyzerAssemblyLoader.AddDependencyLocation(analyzerAssemblyPath);
}

return projectFileInfo.Analyzers.Select(analyzerCandicatePath => new AnalyzerFileReference(analyzerCandicatePath, analyzerAssemblyLoader));
}
}
}
25 changes: 20 additions & 5 deletions src/OmniSharp.MSBuild/ProjectManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
using OmniSharp.MSBuild.Models.Events;
using OmniSharp.MSBuild.Notification;
using OmniSharp.MSBuild.ProjectFile;
using OmniSharp.Roslyn.CSharp.Services.Diagnostics;
using OmniSharp.Roslyn.CSharp.Services.Refactoring.V2;
using OmniSharp.Options;
using OmniSharp.Roslyn.Utilities;
using OmniSharp.Services;
Expand Down Expand Up @@ -53,24 +55,30 @@ public ProjectToUpdate(string filePath, bool allowAutoRestore, ProjectIdInfo pro
private readonly ConcurrentDictionary<string, int/*unused*/> _projectsRequestedOnDemand;
private readonly ProjectLoader _projectLoader;
private readonly OmniSharpWorkspace _workspace;
private readonly CachingCodeFixProviderForProjects _codeFixesForProject;
private readonly ImmutableArray<IMSBuildEventSink> _eventSinks;

private const int LoopDelay = 100; // milliseconds
private readonly BufferBlock<ProjectToUpdate> _queue;
private readonly CancellationTokenSource _processLoopCancellation;
private readonly Task _processLoopTask;
private readonly IAnalyzerAssemblyLoader _assemblyLoader;
private bool _processingQueue;

private readonly FileSystemNotificationCallback _onDirectoryFileChanged;
private readonly RulesetsForProjects _rulesetsForProjects;

public ProjectManager(ILoggerFactory loggerFactory,
public ProjectManager(
ILoggerFactory loggerFactory,
MSBuildOptions options,
IEventEmitter eventEmitter,
IFileSystemWatcher fileSystemWatcher,
MetadataFileReferenceCache metadataFileReferenceCache,
PackageDependencyChecker packageDependencyChecker,
ProjectLoader projectLoader,
OmniSharpWorkspace workspace,
CachingCodeFixProviderForProjects codeFixesForProject,
RulesetsForProjects rulesetsForProjects,
IAnalyzerAssemblyLoader assemblyLoader,
ImmutableArray<IMSBuildEventSink> eventSinks)
{
_logger = loggerFactory.CreateLogger<ProjectManager>();
Expand All @@ -84,13 +92,14 @@ public ProjectManager(ILoggerFactory loggerFactory,
_projectsRequestedOnDemand = new ConcurrentDictionary<string, int>(StringComparer.OrdinalIgnoreCase);
_projectLoader = projectLoader;
_workspace = workspace;
_codeFixesForProject = codeFixesForProject;
_eventSinks = eventSinks;

_queue = new BufferBlock<ProjectToUpdate>();
_processLoopCancellation = new CancellationTokenSource();
_processLoopTask = Task.Run(() => ProcessLoopAsync(_processLoopCancellation.Token));

_assemblyLoader = assemblyLoader;
_onDirectoryFileChanged = OnDirectoryFileChanged;
_rulesetsForProjects = rulesetsForProjects;

if (_options.LoadProjectsOnDemand)
{
Expand Down Expand Up @@ -347,7 +356,13 @@ private void AddProject(ProjectFileInfo projectFileInfo)

_projectFiles.Add(projectFileInfo);

var projectInfo = projectFileInfo.CreateProjectInfo();
var projectInfo = projectFileInfo.CreateProjectInfo(_assemblyLoader);

_codeFixesForProject.LoadFrom(projectInfo);

if(projectFileInfo.RuleSet != null)
_rulesetsForProjects.AddOrUpdateRuleset(projectFileInfo.Id, projectFileInfo.RuleSet);

var newSolution = _workspace.CurrentSolution.AddProject(projectInfo);

if (!_workspace.TryApplyChanges(newSolution))
Expand Down
Loading

0 comments on commit 62b3b52

Please sign in to comment.