Editorconfig improvements - do not lose state, trigger re-analysis on change

src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfoExtensions.cs

@@ -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)
             {

src/OmniSharp.MSBuild/ProjectManager.cs

@@ -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;
             }
         }
@@ -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()
@@ -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)
@@ -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.
@@ -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);
                     });
                 }
             }
@@ -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);
@@ -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))
             {
@@ -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)
@@ -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);
             }
 
@@ -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.");
                 }
             }
         }

src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/CSharpDiagnosticWorkerWithAnalyzers.cs

@@ -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:

src/OmniSharp.Roslyn/OmniSharpWorkspace.cs

@@ -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);
@@ -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);

tests/OmniSharp.MSBuild.Tests/ProjectWithAnalyzersTests.cs

@@ -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()
         {