Skip to content

Commit

Permalink
Merge pull request #2028 from OmniSharp/bugfix/editorconfig
Browse files Browse the repository at this point in the history 
Editorconfig improvements - do not lose state, trigger re-analysis on change
  • Loading branch information
@JoeRobich
JoeRobich authored Dec 1, 2020
2 parents 2d54d05 + fe970ff commit 495bcc2
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 26 deletions.
20 changes: 11 additions & 9 deletions src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfoExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,22 @@ internal static class ProjectFileInfoExtensions
public static CSharpCompilationOptions CreateCompilationOptions(this ProjectFileInfo projectFileInfo)
{
var compilationOptions = new CSharpCompilationOptions(projectFileInfo.OutputKind);
return projectFileInfo.CreateCompilationOptions(compilationOptions);
}

compilationOptions = compilationOptions.WithAssemblyIdentityComparer(DesktopAssemblyIdentityComparer.Default)
.WithSpecificDiagnosticOptions(projectFileInfo.GetDiagnosticOptions())
.WithOverflowChecks(projectFileInfo.CheckForOverflowUnderflow);
public static CSharpCompilationOptions CreateCompilationOptions(this ProjectFileInfo projectFileInfo, CSharpCompilationOptions existingCompilationOptions)
{
var compilationOptions = existingCompilationOptions.WithAssemblyIdentityComparer(DesktopAssemblyIdentityComparer.Default)
.WithSpecificDiagnosticOptions(projectFileInfo.GetDiagnosticOptions())
.WithOverflowChecks(projectFileInfo.CheckForOverflowUnderflow);

if (projectFileInfo.AllowUnsafeCode)
if (projectFileInfo.AllowUnsafeCode != compilationOptions.AllowUnsafe)
{
compilationOptions = compilationOptions.WithAllowUnsafe(true);
compilationOptions = compilationOptions.WithAllowUnsafe(projectFileInfo.AllowUnsafeCode);
}

if (projectFileInfo.TreatWarningsAsErrors)
{
compilationOptions = compilationOptions.WithGeneralDiagnosticOption(ReportDiagnostic.Error);
}
compilationOptions = projectFileInfo.TreatWarningsAsErrors ?
compilationOptions.WithGeneralDiagnosticOption(ReportDiagnostic.Error) : compilationOptions.WithGeneralDiagnosticOption(ReportDiagnostic.Default);

if (projectFileInfo.NullableContextOptions != compilationOptions.NullableContextOptions)
{
Expand Down
89 changes: 74 additions & 15 deletions src/OmniSharp.MSBuild/ProjectManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,14 @@ private class ProjectToUpdate
public ProjectIdInfo ProjectIdInfo;
public string FilePath { get; }
public bool AllowAutoRestore { get; set; }
public string ChangeTriggerPath { get; }
public ProjectLoadedEventArgs LoadedEventArgs { get; set; }

public ProjectToUpdate(string filePath, bool allowAutoRestore, ProjectIdInfo projectIdInfo)
public ProjectToUpdate(string filePath, bool allowAutoRestore, ProjectIdInfo projectIdInfo, string changeTriggerPath)
{
ProjectIdInfo = projectIdInfo ?? throw new ArgumentNullException(nameof(projectIdInfo));
FilePath = filePath ?? throw new ArgumentNullException(nameof(filePath));
ChangeTriggerPath = changeTriggerPath;
AllowAutoRestore = allowAutoRestore;
}
}
Expand Down Expand Up @@ -158,10 +160,10 @@ protected override void DisposeCore(bool disposing)
public IEnumerable<ProjectFileInfo> GetAllProjects() => _projectFiles.GetItems();
public bool TryGetProject(string projectFilePath, out ProjectFileInfo projectFileInfo) => _projectFiles.TryGetValue(projectFilePath, out projectFileInfo);

public void QueueProjectUpdate(string projectFilePath, bool allowAutoRestore, ProjectIdInfo projectId)
public void QueueProjectUpdate(string projectFilePath, bool allowAutoRestore, ProjectIdInfo projectId, string changeTriggerFilePath = null)
{
_logger.LogInformation($"Queue project update for '{projectFilePath}'");
_queue.Post(new ProjectToUpdate(projectFilePath, allowAutoRestore, projectId));
_queue.Post(new ProjectToUpdate(projectFilePath, allowAutoRestore, projectId, changeTriggerFilePath));
}

public async Task WaitForQueueEmptyAsync()
Expand Down Expand Up @@ -262,7 +264,7 @@ private void ProcessQueue(CancellationToken cancellationToken)
{
foreach (var project in projectList)
{
UpdateProject(project.FilePath);
UpdateProject(project.FilePath, project.ChangeTriggerPath);

// Fire loaded events
if (project.LoadedEventArgs != null)
Expand Down Expand Up @@ -382,15 +384,15 @@ private void WatchProjectFiles(ProjectFileInfo projectFileInfo)
// as "updates". We should properly remove projects that are deleted.
_fileSystemWatcher.Watch(projectFileInfo.FilePath, (file, changeType) =>
{
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: true, projectFileInfo.ProjectIdInfo);
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: true, projectFileInfo.ProjectIdInfo, file);
});

if (_workspace.EditorConfigEnabled)
{
// Watch beneath the Project folder for changes to .editorconfig files.
_fileSystemWatcher.Watch(".editorconfig", (file, changeType) =>
{
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo, file);
});

// Watch in folders above the Project folder for changes to .editorconfig files.
Expand All @@ -404,7 +406,7 @@ private void WatchProjectFiles(ProjectFileInfo projectFileInfo)

_fileSystemWatcher.Watch(Path.Combine(parentPath, ".editorconfig"), (file, changeType) =>
{
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo, file);
});
}
}
Expand All @@ -413,15 +415,15 @@ private void WatchProjectFiles(ProjectFileInfo projectFileInfo)
{
_fileSystemWatcher.Watch(projectFileInfo.RuleSet.FilePath, (file, changeType) =>
{
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo, file);
});
}

if (!string.IsNullOrEmpty(projectFileInfo.ProjectAssetsFile))
{
_fileSystemWatcher.Watch(projectFileInfo.ProjectAssetsFile, (file, changeType) =>
{
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo, file);
});

var restoreDirectory = Path.GetDirectoryName(projectFileInfo.ProjectAssetsFile);
Expand All @@ -432,22 +434,22 @@ private void WatchProjectFiles(ProjectFileInfo projectFileInfo)

_fileSystemWatcher.Watch(nugetCacheFile, (file, changeType) =>
{
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo, file);
});

_fileSystemWatcher.Watch(nugetPropsFile, (file, changeType) =>
{
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo, file);
});

_fileSystemWatcher.Watch(nugetTargetsFile, (file, changeType) =>
{
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo);
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo, file);
});
}
}

private void UpdateProject(string projectFilePath)
private void UpdateProject(string projectFilePath, string changeTriggerFilePath)
{
if (!_projectFiles.TryGetValue(projectFilePath, out var projectFileInfo))
{
Expand All @@ -462,18 +464,43 @@ private void UpdateProject(string projectFilePath)
return;
}

// if the update was triggered by a change to an editorconfig file, only reload that analyzer config file
// this will propagata a reanalysis of the project
if (changeTriggerFilePath != null && changeTriggerFilePath.ToLowerInvariant().EndsWith(".editorconfig"))
{
UpdateAnalyzerConfigFile(project, changeTriggerFilePath);
return;
}

// for other update triggers, perform a full check of all options
UpdateSourceFiles(project, projectFileInfo.SourceFiles);
UpdateParseOptions(project, projectFileInfo.LanguageVersion, projectFileInfo.PreprocessorSymbolNames, !string.IsNullOrWhiteSpace(projectFileInfo.DocumentationFile));
UpdateProjectReferences(project, projectFileInfo.ProjectReferences);
UpdateAnalyzerConfigFiles(project, projectFileInfo.AnalyzerConfigFiles);
UpdateReferences(project, projectFileInfo.ProjectReferences, projectFileInfo.References);
UpdateAnalyzerReferences(project, projectFileInfo);
UpdateAdditionalFiles(project, projectFileInfo.AdditionalFiles);
UpdateAnalyzerConfigFiles(project, projectFileInfo.AnalyzerConfigFiles);
UpdateProjectProperties(project, projectFileInfo);

_workspace.AddDocumentInclusionRuleForProject(project.Id, (path) => projectFileInfo.IsFileIncluded(path));
_workspace.TryPromoteMiscellaneousDocumentsToProject(project);
_workspace.UpdateCompilationOptionsForProject(project.Id, projectFileInfo.CreateCompilationOptions());

UpdateCompilationOptions(project, projectFileInfo);
}

private void UpdateCompilationOptions(Project project, ProjectFileInfo projectFileInfo)
{
// if project already has compilation options, then we shall use that to compute new compilation options based on the project file
// and then only set those if it's really necessary
if (project.CompilationOptions != null && project.CompilationOptions is CSharpCompilationOptions existingCompilationOptions)
{
var newCompilationOptions = projectFileInfo.CreateCompilationOptions(existingCompilationOptions);
if (newCompilationOptions != existingCompilationOptions)
{
_workspace.UpdateCompilationOptionsForProject(project.Id, newCompilationOptions);
_logger.LogDebug($"Updated project compilation options on project {project.Name}.");
}
}
}

private void UpdateAnalyzerReferences(Project project, ProjectFileInfo projectFileInfo)
Expand Down Expand Up @@ -529,16 +556,43 @@ private void UpdateAdditionalFiles(Project project, IList<string> additionalFile
}
}

private void UpdateAnalyzerConfigFile(Project project, string analyzerConfigFile)
{
if (!_workspace.EditorConfigEnabled)
{
_logger.LogDebug($".editorconfig files were configured by the project {project.Name} but will not be respected because the feature is switched off in OmniSharp. Enable .editorconfig support in OmniSharp to take advantage of them.");
return;
}

var currentAnalyzerConfigDocument = project.AnalyzerConfigDocuments.FirstOrDefault(x => x.FilePath.Equals(analyzerConfigFile));
if (currentAnalyzerConfigDocument == null)
{
_logger.LogDebug($"The change was reported in {analyzerConfigFile} but it doesn't belong to any project.");
return;
}

if (!File.Exists(analyzerConfigFile))
{
_logger.LogWarning($"The change was reported in {analyzerConfigFile} but it doesn't exist on disk.");
return;
}

_workspace.ReloadAnalyzerConfigDocument(currentAnalyzerConfigDocument.Id, analyzerConfigFile);
_logger.LogDebug($"Reloaded {currentAnalyzerConfigDocument.Id} from {analyzerConfigFile} in project {project.Name}.");
}

private void UpdateAnalyzerConfigFiles(Project project, IList<string> analyzerConfigFiles)
{
if (!_workspace.EditorConfigEnabled)
{
_logger.LogDebug($".editorconfig files were configured by the project {project.Name} but will not be respected because the feature is switched off in OmniSharp. Enable .editorconfig support in OmniSharp to take advantage of them.");
return;
}

var currentAnalyzerConfigDocuments = project.AnalyzerConfigDocuments;
foreach (var document in currentAnalyzerConfigDocuments)
{
_logger.LogDebug($".editorconfig file '{document.Name}' removed from project {project.Name}");
_workspace.RemoveAnalyzerConfigDocument(document.Id);
}

Expand All @@ -547,6 +601,11 @@ private void UpdateAnalyzerConfigFiles(Project project, IList<string> analyzerCo
if (File.Exists(file))
{
_workspace.AddAnalyzerConfigDocument(project.Id, file);
_logger.LogDebug($".editorconfig file '{file}' added for project {project.Name}");
}
else
{
_logger.LogWarning($".editorconfig file '{file}' for project {project.Name} was expected but not found on disk.");
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,15 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs changeEv
_logger.LogDebug($"Tried to remove non existent document from analysis, document: {changeEvent.DocumentId}");
}
break;
case WorkspaceChangeKind.AnalyzerConfigDocumentChanged:
_logger.LogDebug($"Analyzer config document {changeEvent.DocumentId} changed, which triggered re-analysis of project {changeEvent.ProjectId}.");
QueueForAnalysis(_workspace.CurrentSolution.GetProject(changeEvent.ProjectId).Documents.Select(x => x.Id).ToImmutableArray(), AnalyzerWorkType.Background);
break;
case WorkspaceChangeKind.ProjectAdded:
case WorkspaceChangeKind.ProjectChanged:
case WorkspaceChangeKind.ProjectReloaded:
_logger.LogDebug($"Project {changeEvent.ProjectId} updated, reanalyzing its diagnostics.");
var projectDocumentIds = _workspace.CurrentSolution.GetProject(changeEvent.ProjectId).Documents.Select(x => x.Id).ToImmutableArray();
QueueForAnalysis(projectDocumentIds, AnalyzerWorkType.Background);
QueueForAnalysis(_workspace.CurrentSolution.GetProject(changeEvent.ProjectId).Documents.Select(x => x.Id).ToImmutableArray(), AnalyzerWorkType.Background);
break;
case WorkspaceChangeKind.SolutionAdded:
case WorkspaceChangeKind.SolutionChanged:
Expand Down
7 changes: 7 additions & 0 deletions src/OmniSharp.Roslyn/OmniSharpWorkspace.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ public DocumentId TryAddMiscellaneousDocument(string filePath, TextLoader loader
var newAnalyzerConfigFiles = EditorConfigFinder
.GetEditorConfigPaths(filePath)
.Except(analyzerConfigFiles);

foreach (var analyzerConfigFile in newAnalyzerConfigFiles)
{
AddAnalyzerConfigDocument(projectInfo.Id, analyzerConfigFile);
Expand Down Expand Up @@ -555,6 +556,12 @@ public void AddAnalyzerConfigDocument(ProjectId projectId, string filePath)
OnAnalyzerConfigDocumentAdded(documentInfo);
}

public void ReloadAnalyzerConfigDocument(DocumentId documentId, string filePath)
{
var loader = new OmniSharpTextLoader(filePath);
OnAnalyzerConfigDocumentTextLoaderChanged(documentId, loader);
}

public void RemoveAdditionalDocument(DocumentId documentId)
{
OnAdditionalDocumentRemoved(documentId);
Expand Down
26 changes: 26 additions & 0 deletions tests/OmniSharp.MSBuild.Tests/ProjectWithAnalyzersTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,32 @@ public async Task WhenProjectEditorConfigIsChangedThenAnalyzerConfigurationUpdat
}
}

[Fact]
public async Task WhenProjectChangesAnalyzerConfigurationIsPreserved()
{
var emitter = new ProjectLoadTestEventEmitter();

using (var testProject = await TestAssets.Instance.GetTestProjectAsync("ProjectWithAnalyzersAndEditorConfig"))
using (var host = CreateMSBuildTestHost(
testProject.Directory,
emitter.AsExportDescriptionProvider(LoggerFactory),
TestHelpers.GetConfigurationDataWithAnalyzerConfig(roslynAnalyzersEnabled: true, editorConfigEnabled: true)))
{
var initialProject = host.Workspace.CurrentSolution.Projects.Single();
var firstDiagnosticsSet = await host.RequestCodeCheckAsync(Path.Combine(testProject.Directory, "Program.cs"));
Assert.NotEmpty(firstDiagnosticsSet.QuickFixes);
Assert.Contains(firstDiagnosticsSet.QuickFixes.OfType<DiagnosticLocation>(), x => x.Id == "IDE0005" && x.LogLevel == "Error");

// report reloading of a project
await NotifyFileChanged(host, initialProject.FilePath);
emitter.WaitForProjectUpdate();

var secondDiagnosticsSet = await host.RequestCodeCheckAsync(Path.Combine(testProject.Directory, "Program.cs"));
Assert.NotEmpty(secondDiagnosticsSet.QuickFixes);
Assert.Contains(secondDiagnosticsSet.QuickFixes.OfType<DiagnosticLocation>(), x => x.Id == "IDE0005" && x.LogLevel == "Error");
}
}

[Fact]
public async Task WhenProjectIsLoadedThenItContainsAnalyzerConfigurationFromParentFolder()
{
Expand Down

0 comments on commit 495bcc2

Please sign in to comment.