From a71b8af24cd2b78f985dd50a97879123a037fbc9 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Fri, 27 Jan 2023 11:44:15 +0530 Subject: [PATCH 1/4] Update Project.HasSuccessfullyLoadedAsync logic Fixes [AB#1674516](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1674516) Fixes [AB#1710989](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1710989) Fixes spurious missing corlib types errors in the error list on loading any decently large solution, such as Roslyn.sln itself. These errors stay in the error list for few seconds before metadata references get added to the project Our current logic in the CompilationTracker to determine `hasSuccessfullyLoaded` for the project relies on the `ProjectState.HasAllInformation` flag to determine if the project has loaded successfully. Additionally, we try to convert the project references to metadata references, and failing such a conversion, we reset `hasSuccessfullyLoaded` to false. However, there seem to be cases during project load where `ProjectState.HasAllInformation` is true, but metadata references haven't yet been added to the project, leading to hundreds of spurious missing type errors for corlib types such as object, int, string, etc. This PR updates the logic for `hasSuccessfullyLoaded` to also check if the project either has non-zero metadata references or is the corlib itself (defines `System.Object`). I confirmed that this fixes all the 3 repro cases/bugs cited above. I no longer see any spurious errors in the error list or squiggles in the editor for all these scenarios. --- ....SingleDiagnosticKindPullTaggerProvider.cs | 33 ------------------- .../SolutionState.CompilationTracker.cs | 9 +++-- 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/src/EditorFeatures/Core/Diagnostics/AbstractPushOrPullDiagnosticsTaggerProvider.SingleDiagnosticKindPullTaggerProvider.cs b/src/EditorFeatures/Core/Diagnostics/AbstractPushOrPullDiagnosticsTaggerProvider.SingleDiagnosticKindPullTaggerProvider.cs index 2a36f5953f529..899d475ac148e 100644 --- a/src/EditorFeatures/Core/Diagnostics/AbstractPushOrPullDiagnosticsTaggerProvider.SingleDiagnosticKindPullTaggerProvider.cs +++ b/src/EditorFeatures/Core/Diagnostics/AbstractPushOrPullDiagnosticsTaggerProvider.SingleDiagnosticKindPullTaggerProvider.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using System.Threading; @@ -16,7 +15,6 @@ using Microsoft.CodeAnalysis.Editor.Tagging; using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Options; -using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Text; @@ -111,18 +109,6 @@ private async Task ProduceTagsAsync( try { - // If this is not the tagger for compiler-syntax, then suppress diagnostics until the project has fully - // loaded. This prevents the user from seeing spurious diagnostics while the project is in the process - // of loading. We do keep compiler-syntax as that's based purely on the parse tree, and doesn't need - // correct project info to get reasonable results. - if (_diagnosticKind != DiagnosticKind.CompilerSyntax) - { - using var _ = PooledHashSet.GetInstance(out var seenProjects); - var hasSuccessfullyLoaded = HasSuccessfullyLoaded(document.Project, seenProjects); - if (!hasSuccessfullyLoaded) - return; - } - var requestedSpan = documentSpanToTag.SnapshotSpan; var diagnostics = await _analyzerService.GetDiagnosticsForSpanAsync( @@ -158,25 +144,6 @@ private async Task ProduceTagsAsync( } } - private bool HasSuccessfullyLoaded(Project? project, HashSet seenProjects) - { - if (project != null && seenProjects.Add(project)) - { - if (!project.State.HasAllInformation) - return false; - - // Ensure our dependencies have all information as well. That's necessary so we can properly get - // compilations for them. - foreach (var reference in project.ProjectReferences) - { - if (!HasSuccessfullyLoaded(project.Solution.GetProject(reference.ProjectId), seenProjects)) - return false; - } - } - - return true; - } - private static bool IsSuppressed(NormalizedSnapshotSpanCollection? suppressedSpans, SnapshotSpan span) => suppressedSpans != null && suppressedSpans.IntersectsWith(span); } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs index a13bedfe56dce..ba81513ba1bda 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs @@ -703,8 +703,13 @@ private async Task FinalizeCompilationAsync( { try { - // if HasAllInformation is false, then this project is always not completed. - var hasSuccessfullyLoaded = this.ProjectState.HasAllInformation; + // Project is complete only if the following are all true: + // 1. HasAllInformation flag is set for the project + // 2. Either the project has non-zero metadata references OR this is the corlib project. + // For the latter, we use a heuristic if the underlying compilation defines "System.Object" type. + var hasSuccessfullyLoaded = this.ProjectState.HasAllInformation && + (this.ProjectState.MetadataReferences.Count > 0 || + compilationWithoutGenerators.GetTypeByMetadataName("System.Object") != null); var newReferences = new List(); var metadataReferenceToProjectId = new Dictionary(); From 4d32d11364b6d847435b900bc7fdef8e097f05c7 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Fri, 27 Jan 2023 15:50:08 +0530 Subject: [PATCH 2/4] Add test + fix existing SolutionTests --- .../CoreTest/SolutionTests/SolutionTests.cs | 53 ++++++++++++++++--- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs b/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs index 1be56118a8360..9a0a6b502f8ba 100644 --- a/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs +++ b/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs @@ -2925,11 +2925,27 @@ public void TestEncodingRetainedAfterTreeChanged() Assert.Equal(Encoding.UTF32, newDoc.GetTextAsync().Result.Encoding); } + [Fact] + public void TestProjectWithNoMetadataReferencesHasIncompleteReferences() + { + var workspace = new AdhocWorkspace(); + var project = workspace.AddProject("CSharpProject", LanguageNames.CSharp); + Assert.False(project.HasSuccessfullyLoadedAsync().Result); + Assert.Empty(project.GetCompilationAsync().Result.ExternalReferences); + } + [Fact] public void TestProjectWithNoBrokenReferencesHasNoIncompleteReferences() { var workspace = new AdhocWorkspace(); - var project1 = workspace.AddProject("CSharpProject", LanguageNames.CSharp); + var project1 = workspace.AddProject( + ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "CSharpProject", + "CSharpProject", + LanguageNames.CSharp, + metadataReferences: new[] { MscorlibRef })); var project2 = workspace.AddProject( ProjectInfo.Create( ProjectId.CreateNewId(), @@ -2937,19 +2953,27 @@ public void TestProjectWithNoBrokenReferencesHasNoIncompleteReferences() "VisualBasicProject", "VisualBasicProject", LanguageNames.VisualBasic, + metadataReferences: new[] { MscorlibRef }, projectReferences: new[] { new ProjectReference(project1.Id) })); // Nothing should have incomplete references, and everything should build Assert.True(project1.HasSuccessfullyLoadedAsync().Result); Assert.True(project2.HasSuccessfullyLoadedAsync().Result); - Assert.Single(project2.GetCompilationAsync().Result.ExternalReferences); + Assert.Equal(2, project2.GetCompilationAsync().Result.ExternalReferences.Length); } [Fact] public void TestProjectWithBrokenCrossLanguageReferenceHasIncompleteReferences() { var workspace = new AdhocWorkspace(); - var project1 = workspace.AddProject("CSharpProject", LanguageNames.CSharp); + var project1 = workspace.AddProject( + ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "CSharpProject", + "CSharpProject", + LanguageNames.CSharp, + metadataReferences: new[] { MscorlibRef })); workspace.AddDocument(project1.Id, "Broken.cs", SourceText.From("class ")); var project2 = workspace.AddProject( @@ -2959,11 +2983,12 @@ public void TestProjectWithBrokenCrossLanguageReferenceHasIncompleteReferences() "VisualBasicProject", "VisualBasicProject", LanguageNames.VisualBasic, + metadataReferences: new[] { MscorlibRef }, projectReferences: new[] { new ProjectReference(project1.Id) })); Assert.True(project1.HasSuccessfullyLoadedAsync().Result); Assert.False(project2.HasSuccessfullyLoadedAsync().Result); - Assert.Empty(project2.GetCompilationAsync().Result.ExternalReferences); + Assert.Single(project2.GetCompilationAsync().Result.ExternalReferences); } [Fact] @@ -3086,7 +3111,14 @@ await documentToFreezeChanged.Project.GetSemanticVersionAsync(), public void TestFrozenPartialProjectAlwaysIsIncomplete() { var workspace = new AdhocWorkspace(); - var project1 = workspace.AddProject("CSharpProject", LanguageNames.CSharp); + var project1 = workspace.AddProject( + ProjectInfo.Create( + ProjectId.CreateNewId(), + VersionStamp.Create(), + "CSharpProject", + "CSharpProject", + LanguageNames.CSharp, + metadataReferences: new[] { MscorlibRef })); var project2 = workspace.AddProject( ProjectInfo.Create( @@ -3095,6 +3127,7 @@ public void TestFrozenPartialProjectAlwaysIsIncomplete() "VisualBasicProject", "VisualBasicProject", LanguageNames.VisualBasic, + metadataReferences: new[] { MscorlibRef }, projectReferences: new[] { new ProjectReference(project1.Id) })); var document = workspace.AddDocument(project2.Id, "Test.cs", SourceText.From("")); @@ -3727,7 +3760,8 @@ private static void GetMultipleProjects( VersionStamp.Create(), "CSharpProject", "CSharpProject", - LanguageNames.CSharp).WithHasAllInformation(hasAllInformation: false)); + LanguageNames.CSharp, + metadataReferences: new[] { MscorlibRef }).WithHasAllInformation(hasAllInformation: false)); vbNormalProject = workspace.AddProject( ProjectInfo.Create( @@ -3735,7 +3769,8 @@ private static void GetMultipleProjects( VersionStamp.Create(), "VisualBasicProject", "VisualBasicProject", - LanguageNames.VisualBasic)); + LanguageNames.VisualBasic, + metadataReferences: new[] { MscorlibRef })); dependsOnBrokenProject = workspace.AddProject( ProjectInfo.Create( @@ -3744,6 +3779,7 @@ private static void GetMultipleProjects( "VisualBasicProject", "VisualBasicProject", LanguageNames.VisualBasic, + metadataReferences: new[] { MscorlibRef }, projectReferences: new[] { new ProjectReference(csBrokenProject.Id), new ProjectReference(vbNormalProject.Id) })); dependsOnVbNormalProject = workspace.AddProject( @@ -3753,6 +3789,7 @@ private static void GetMultipleProjects( "CSharpProject", "CSharpProject", LanguageNames.CSharp, + metadataReferences: new[] { MscorlibRef }, projectReferences: new[] { new ProjectReference(vbNormalProject.Id) })); transitivelyDependsOnBrokenProjects = workspace.AddProject( @@ -3762,6 +3799,7 @@ private static void GetMultipleProjects( "CSharpProject", "CSharpProject", LanguageNames.CSharp, + metadataReferences: new[] { MscorlibRef }, projectReferences: new[] { new ProjectReference(dependsOnBrokenProject.Id) })); transitivelyDependsOnNormalProjects = workspace.AddProject( @@ -3771,6 +3809,7 @@ private static void GetMultipleProjects( "VisualBasicProject", "VisualBasicProject", LanguageNames.VisualBasic, + metadataReferences: new[] { MscorlibRef }, projectReferences: new[] { new ProjectReference(dependsOnVbNormalProject.Id) })); } From 9cae144754f94209fdadcee10fc413be08ac4092 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Fri, 27 Jan 2023 17:11:40 +0530 Subject: [PATCH 3/4] More test fixes --- .../Tests/UnsealClass/UnsealClassTests.vb | 4 +- .../GenerateType/GenerateTypeTests.cs | 6 +-- .../UpgradeProject/UpgradeProjectTests.cs | 46 +++++++++---------- .../GenerateConstructorTests.cs | 6 +-- .../Workspaces/TestWorkspace_Create.cs | 1 + .../TestWorkspace_XmlConsumption.cs | 8 ++++ 6 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/Analyzers/VisualBasic/Tests/UnsealClass/UnsealClassTests.vb b/src/Analyzers/VisualBasic/Tests/UnsealClass/UnsealClassTests.vb index 4bd7192f45067..54600285e8e6d 100644 --- a/src/Analyzers/VisualBasic/Tests/UnsealClass/UnsealClassTests.vb +++ b/src/Analyzers/VisualBasic/Tests/UnsealClass/UnsealClassTests.vb @@ -143,7 +143,7 @@ end class") Public Async Function RemovedFromAllPartialClassDeclarationsAcrossFiles() As Task Await TestInRegularAndScriptAsync( - + partial public notinheritable class C end class @@ -162,7 +162,7 @@ end class .ToString(), - + partial public class C end class diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/GenerateType/GenerateTypeTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/GenerateType/GenerateTypeTests.cs index 2dc921a868b44..91ce8cb99c154 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/GenerateType/GenerateTypeTests.cs +++ b/src/EditorFeatures/CSharpTest/Diagnostics/GenerateType/GenerateTypeTests.cs @@ -5414,7 +5414,7 @@ public async Task TestMethodGroupWithMissingSystemActionAndFunc(string returnTyp await TestInRegularAndScriptAsync( $@" - + - + class C { @@ -373,9 +373,9 @@ void A() } - + - + @@ -390,7 +390,7 @@ public async Task UpgradeAllProjectsToCSharp8() { await TestLanguageVersionUpgradedAsync( @" - + class C { @@ -398,11 +398,11 @@ class C } - + - + - + ", LanguageVersion.CSharp8, @@ -444,18 +444,18 @@ public async Task UpgradeAllProjectsToCSharp9() { await TestLanguageVersionUpgradedAsync( @" - + [|System.Console.WriteLine();|] - + - + - + - + ", LanguageVersion.CSharp9, @@ -469,7 +469,7 @@ public async Task ListAllSuggestions() await TestExactActionSetOfferedAsync( @" - + class C { @@ -480,9 +480,9 @@ void A() } - + - + ", new[] { @@ -527,7 +527,7 @@ public async Task FixAllProjectsNotOffered() await TestExactActionSetOfferedAsync( @" - + class C { @@ -538,7 +538,7 @@ void A() } - + ", new[] { @@ -552,7 +552,7 @@ public async Task OnlyOfferFixAllProjectsFromCSharp6ToCSharp7WhenApplicable() await TestExactActionSetOfferedAsync( @" - + class C { @@ -563,9 +563,9 @@ void A() } - + - + ", new[] { @@ -608,7 +608,7 @@ public async Task OnlyOfferFixAllProjectsToCSharp8WhenApplicable() await TestExactActionSetOfferedAsync( @" - + class C { @@ -616,9 +616,9 @@ class C } - + - + ", new[] { diff --git a/src/EditorFeatures/CSharpTest/GenerateConstructor/GenerateConstructorTests.cs b/src/EditorFeatures/CSharpTest/GenerateConstructor/GenerateConstructorTests.cs index c6a1dbcc1ab59..e0afa5c6c7623 100644 --- a/src/EditorFeatures/CSharpTest/GenerateConstructor/GenerateConstructorTests.cs +++ b/src/EditorFeatures/CSharpTest/GenerateConstructor/GenerateConstructorTests.cs @@ -3639,7 +3639,7 @@ public async Task TestMethodGroupWithMissingSystemActionAndFunc(string returnTyp await TestInRegularAndScriptAsync( $@" - + CreateCommonReferences(TestWorkspace wor references = TargetFrameworkUtil.GetReferences(TargetFramework.Net60).ToList(); } + var mincorlib = element.Attribute(CommonReferencesMinCorlibName); + if (mincorlib != null && + ((bool?)mincorlib).HasValue && + ((bool?)mincorlib).Value) + { + references = new List { TestBase.MinCorlibRef }; + } + return references; } From b1449e00bad7876e4c5a4f1d7adce56b23dc482e Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Sat, 28 Jan 2023 11:42:25 +0530 Subject: [PATCH 4/4] Fix another test --- .../Diagnostics/GenerateMethod/GenerateMethodTests.vb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/VisualBasicTest/Diagnostics/GenerateMethod/GenerateMethodTests.vb b/src/EditorFeatures/VisualBasicTest/Diagnostics/GenerateMethod/GenerateMethodTests.vb index 775c3b9c97e59..6ca10ce231cd5 100644 --- a/src/EditorFeatures/VisualBasicTest/Diagnostics/GenerateMethod/GenerateMethodTests.vb +++ b/src/EditorFeatures/VisualBasicTest/Diagnostics/GenerateMethod/GenerateMethodTests.vb @@ -4408,7 +4408,7 @@ End Namespace") Public Async Function TestAcrossFiles() As Task Await TestInRegularAndScriptAsync( " - + Public Class DataContainer Property PossibleInProcessTests As string @@ -4462,6 +4462,7 @@ Public Class DataContainer End Sub Friend Sub ArbitraryPositionMethod() + Throw New NotImplementedException() End Sub Function Bazz() As Object