Skip to content
This repository was archived by the owner on Oct 4, 2021. It is now read-only.

Commit 5ea95dd

Browse files
committed
[Ide] Fix analyzers not retriggered on editorconfig changes
If an editorconfig file was changed externally, or edited in the text editor, the changes would not affect the open solution until it was closed and re-opened again. To support this the Roslyn workspace needs to know when the editor config file has changed externally by calling OnAnalyzerConfigDocumentTextChanged. When the editorconfig file is open in the text editor the Roslyn workspace needs to be told the file has opened via OnAnalyzerConfigDocumentOpened and when it has closed via OnAnalyzerConfigDocumentClosed. Then all changes made in the editor are handled by Roslyn and the analyzers are re-run. Fixes VSTS #706520 - Analyzers not retriggered on editorconfig change
1 parent b372165 commit 5ea95dd

File tree

3 files changed

+172
-6
lines changed

3 files changed

+172
-6
lines changed

main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.TypeSystem/MonoDevelopWorkspace.cs

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -618,15 +618,19 @@ TextDocument InternalInformDocumentOpen (DocumentId documentId, SourceTextContai
618618
var project = this.CurrentSolution.GetProject (documentId.ProjectId);
619619
if (project == null)
620620
return null;
621-
TextDocument document = project.GetDocument (documentId) ?? project.GetAdditionalDocument (documentId);
621+
TextDocument document = project.GetDocument (documentId) ??
622+
project.GetAdditionalDocument (documentId) ??
623+
project.GetAnalyzerConfigDocument (documentId);
622624
if (document == null || OpenDocuments.Contains (documentId)) {
623625
return document;
624626
}
625627
OpenDocuments.Add (documentId, sourceTextContainer);
626628
if (document is Document) {
627629
OnDocumentOpened (documentId, sourceTextContainer, isCurrentContext);
628-
} else {
630+
} else if (document is AdditionalDocument) {
629631
OnAdditionalDocumentOpened (documentId, sourceTextContainer, isCurrentContext);
632+
} else if (document is AnalyzerConfigDocument) {
633+
OnAnalyzerConfigDocumentOpened (documentId, sourceTextContainer, isCurrentContext);
630634
}
631635
return document;
632636
}
@@ -655,8 +659,6 @@ internal void InformDocumentClose (DocumentId analysisDocument, SourceTextContai
655659
//it's job of whatever opened to also call CloseAndemoveDocumentInternal
656660
return;
657661
}
658-
if (!CurrentSolution.ContainsDocument (analysisDocument))
659-
return;
660662

661663
// Using a source text container
662664
var loader = new SourceTextLoader (container, null);
@@ -666,8 +668,15 @@ internal void InformDocumentClose (DocumentId analysisDocument, SourceTextContai
666668
var ad = this.GetAdditionalDocument (analysisDocument);
667669
if (ad != null)
668670
OnAdditionalDocumentClosed (analysisDocument, loader);
671+
var analyzerDoc = this.GetAnalyzerConfigDocument (analysisDocument);
672+
if (analyzerDoc != null)
673+
OnAnalyzerConfigDocumentClosed (analysisDocument, loader);
669674
return;
670-
}
675+
}
676+
677+
if (!CurrentSolution.ContainsDocument (analysisDocument))
678+
return;
679+
671680
OnDocumentClosed (analysisDocument, loader);
672681
foreach (var linkedDoc in document.GetLinkedDocumentIds ()) {
673682
OnDocumentClosed (linkedDoc, loader);
@@ -1404,6 +1413,14 @@ static void FailIfDocumentInfoChangesNotSupported (Document document, DocumentIn
14041413
if (project == null)
14051414
return null;
14061415
return project.GetAdditionalDocument (documentId);
1416+
}
1417+
1418+
internal TextDocument GetAnalyzerConfigDocument (DocumentId documentId, CancellationToken cancellationToken = default (CancellationToken))
1419+
{
1420+
var project = CurrentSolution.GetProject (documentId.ProjectId);
1421+
if (project == null)
1422+
return null;
1423+
return project.GetAnalyzerConfigDocument (documentId);
14071424
}
14081425

14091426
internal async Task UpdateFileContent (string fileName, string text)
@@ -1420,6 +1437,8 @@ internal async Task UpdateFileContent (string fileName, string text)
14201437
base.OnDocumentTextChanged (docId, newText, PreservationMode.PreserveIdentity);
14211438
} else if (this.GetAdditionalDocument (docId) != null) {
14221439
base.OnAdditionalDocumentTextChanged (docId, newText, PreservationMode.PreserveIdentity);
1440+
} else if (this.GetAnalyzerConfigDocument (docId) != null) {
1441+
base.OnAnalyzerConfigDocumentTextChanged (docId, newText, PreservationMode.PreserveIdentity);
14231442
}
14241443
} catch (Exception e) {
14251444
LoggingService.LogWarning ("Roslyn error on text change", e);

main/src/core/MonoDevelop.Ide/MonoDevelop.Ide.TypeSystem/TypeSystemService_WorkspaceHandling.cs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ bool TryOpenDocumentInWorkspace (WorkspaceObject owner, FilePath filePath, IText
333333
{
334334
var project = owner as MonoDevelop.Projects.Project;
335335
if (project == null || !project.IsCompileable (filePath) || project.ParentSolution == null) {
336-
return false;
336+
return TryOpenDocumentInAllWorkspaces (filePath, textBuffer);
337337
}
338338

339339
var workspace = GetWorkspace (project.ParentSolution);
@@ -374,6 +374,27 @@ bool TryOpenDocumentInWorkspace (WorkspaceObject owner, FilePath filePath, IText
374374
return false;
375375
}
376376

377+
bool TryOpenDocumentInAllWorkspaces (FilePath filePath, ITextBuffer textBuffer)
378+
{
379+
bool opened = false;
380+
foreach (var workspace in workspaces) {
381+
var documentIds = workspace.CurrentSolution.GetDocumentIdsWithFilePath (filePath);
382+
foreach (var documentId in documentIds) {
383+
if (workspace.IsDocumentOpen (documentId))
384+
continue;
385+
386+
if (workspace.CurrentSolution.ContainsAdditionalDocument (documentId)) {
387+
workspace.InformDocumentOpen (documentId, textBuffer.AsTextContainer ());
388+
opened = true;
389+
} else if (workspace.CurrentSolution.ContainsAnalyzerConfigDocument (documentId)) {
390+
workspace.InformDocumentOpen (documentId, textBuffer.AsTextContainer ());
391+
opened = true;
392+
}
393+
}
394+
}
395+
return opened;
396+
}
397+
377398
void UnregisterOpenDocument (OpenDocumentReference reference)
378399
{
379400
Runtime.AssertMainThread ();
@@ -394,6 +415,8 @@ void UnregisterOpenDocument (OpenDocumentReference reference)
394415
var solution = (reference.Owner as SolutionItem)?.ParentSolution;
395416
if (solution != null)
396417
TryCloseDocumentInWorkspace (reference.FilePath, reference.TextBuffer.AsTextContainer (), solution);
418+
else
419+
TryCloseDocumentInAllWorkspaces (reference.FilePath, reference.TextBuffer.AsTextContainer ());
397420
}
398421

399422
private void TryCloseDocumentInWorkspace (FilePath filePath, SourceTextContainer container, MonoDevelop.Projects.Solution solution)
@@ -409,6 +432,16 @@ private void TryCloseDocumentInWorkspace (FilePath filePath, SourceTextContainer
409432
}
410433
}
411434

435+
void TryCloseDocumentInAllWorkspaces (FilePath filePath, SourceTextContainer container)
436+
{
437+
foreach (var workspace in workspaces) {
438+
var documentIds = workspace.CurrentSolution.GetDocumentIdsWithFilePath (filePath);
439+
foreach (var documentId in documentIds) {
440+
workspace.InformDocumentClose (documentId, container);
441+
}
442+
}
443+
}
444+
412445
internal void UpdateRegisteredOpenDocuments ()
413446
{
414447
Runtime.AssertMainThread ();

main/tests/Ide.Tests/MonoDevelop.Ide.TypeSystem/TypeSystemServiceTests.cs

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
using System.Linq;
3030
using System.Threading.Tasks;
3131
using MonoDevelop.Core;
32+
using MonoDevelop.Ide.Gui.Documents;
3233
using MonoDevelop.Projects;
3334
using NUnit.Framework;
3435
using UnitTests;
@@ -368,12 +369,125 @@ public async Task AdditionalFiles_EditorConfigFiles ()
368369

369370
Assert.IsTrue (additionalDocs.Any (d => d.FilePath == expectedAdditionalFileName));
370371
Assert.IsTrue (editorConfigDocs.Any (d => d.FilePath == expectedEditorConfigFileName));
372+
371373
} finally {
372374
TypeSystemServiceTestExtensions.UnloadSolution (sol);
373375
}
374376
}
375377
}
376378

379+
[Test]
380+
public async Task EditorConfigFile_ModifiedExternally ()
381+
{
382+
FilePath solFile = Util.GetSampleProject ("additional-files", "additional-files.sln");
383+
384+
using (var sol = (Solution)await Services.ProjectService.ReadWorkspaceItem (Util.GetMonitor (), solFile))
385+
using (var ws = await TypeSystemServiceTestExtensions.LoadSolution (sol)) {
386+
try {
387+
var project = sol.GetAllProjects ().Single ();
388+
389+
FilePath editorConfigFileName = solFile.ParentDirectory.Combine (".editorconfig");
390+
var projectInfo = ws.CurrentSolution.Projects.Single ();
391+
var editorConfigDoc = projectInfo.AnalyzerConfigDocuments.Single (d => d.FilePath == editorConfigFileName);
392+
393+
bool analyzerConfigDocumentChanged = false;
394+
ws.WorkspaceChanged += (sender, e) => {
395+
if (e.DocumentId == editorConfigDoc.Id &&
396+
e.Kind == Microsoft.CodeAnalysis.WorkspaceChangeKind.AnalyzerConfigDocumentChanged) {
397+
analyzerConfigDocumentChanged = true;
398+
}
399+
};
400+
401+
// Add error style to .editorconfig
402+
string contents =
403+
"root = true\r\n" +
404+
"\r\n" +
405+
"[*.cs]\r\n" +
406+
"csharp_style_var_for_built_in_types = true:error\r\n";
407+
File.WriteAllText (editorConfigFileName, contents);
408+
FileService.NotifyFileChanged (editorConfigFileName);
409+
410+
Func<bool> action = () => analyzerConfigDocumentChanged;
411+
await AssertIsTrueWithTimeout (action, "Timed out waiting for analyzer config file changed event", 10000);
412+
413+
} finally {
414+
TypeSystemServiceTestExtensions.UnloadSolution (sol);
415+
}
416+
}
417+
}
418+
419+
[Test]
420+
public async Task EditorConfigFile_ModifiedInTextEditor ()
421+
{
422+
FilePath solFile = Util.GetSampleProject ("additional-files", "additional-files.sln");
423+
424+
using (var sol = (Solution)await Services.ProjectService.ReadWorkspaceItem (Util.GetMonitor (), solFile))
425+
using (var ws = await TypeSystemServiceTestExtensions.LoadSolution (sol)) {
426+
try {
427+
var project = sol.GetAllProjects ().Single ();
428+
429+
FilePath editorConfigFileName = solFile.ParentDirectory.Combine (".editorconfig");
430+
var textFileModel = new TextBufferFileModel ();
431+
var mimeType = MimeTypeCatalog.Instance.FindMimeTypeForFile (editorConfigFileName);
432+
textFileModel.CreateNew (editorConfigFileName, mimeType?.Id);
433+
434+
var projectInfo = ws.CurrentSolution.Projects.Single ();
435+
Microsoft.CodeAnalysis.AnalyzerConfigDocument editorConfigDoc =
436+
projectInfo.AnalyzerConfigDocuments.Single (d => d.FilePath == editorConfigFileName);
437+
438+
int analyzerConfigDocumentChangedCount = 0;
439+
ws.WorkspaceChanged += (sender, e) => {
440+
if (e.DocumentId == editorConfigDoc.Id &&
441+
e.Kind == Microsoft.CodeAnalysis.WorkspaceChangeKind.AnalyzerConfigDocumentChanged) {
442+
analyzerConfigDocumentChangedCount++;
443+
}
444+
};
445+
446+
using (var fileRegistration = IdeApp.TypeSystemService.RegisterOpenDocument (
447+
null, // No owner.
448+
editorConfigFileName,
449+
textFileModel.TextBuffer)) {
450+
451+
Assert.IsTrue (ws.IsDocumentOpen (editorConfigDoc.Id));
452+
453+
Func<bool> action = () => analyzerConfigDocumentChangedCount == 1;
454+
await AssertIsTrueWithTimeout (action, "Timed out waiting for analyzer config file changed event on opening file", 100000);
455+
456+
// Add error style to .editorconfig
457+
string contents =
458+
"root = true\r\n" +
459+
"\r\n" +
460+
"[*.cs]\r\n" +
461+
"csharp_style_var_for_built_in_types = true:error\r\n";
462+
textFileModel.SetText (contents);
463+
await textFileModel.Save ();
464+
465+
action = () => analyzerConfigDocumentChangedCount == 2;
466+
await AssertIsTrueWithTimeout (action, "Timed out waiting for analyzer config file changed event", 100000);
467+
}
468+
// After the file registration is disposed the document should be closed.
469+
Assert.IsFalse (ws.IsDocumentOpen (editorConfigDoc.Id));
470+
} finally {
471+
TypeSystemServiceTestExtensions.UnloadSolution (sol);
472+
}
473+
}
474+
}
475+
476+
async Task AssertIsTrueWithTimeout (Func<bool> action, string message, int timeout)
477+
{
478+
int howLong = 0;
479+
const int interval = 200; // ms
480+
481+
while (!action ()) {
482+
if (howLong >= timeout) {
483+
Assert.Fail (message);
484+
}
485+
486+
await Task.Delay (interval);
487+
howLong += interval;
488+
}
489+
}
490+
377491
/// <summary>
378492
/// Clear all other package sources and just use the main NuGet package source when
379493
/// restoring the packages for the project tests.

0 commit comments

Comments
 (0)