Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Project.HasSuccessfullyLoadedAsync logic #66569

Merged
merged 4 commits into from
Feb 2, 2023
Merged

Update Project.HasSuccessfullyLoadedAsync logic #66569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a71b8af
Update Project.HasSuccessfullyLoadedAsync logic
mavasani Jan 27, 2023
4d32d11
Add test + fix existing SolutionTests
mavasani Jan 27, 2023
9cae144
More test fixes
mavasani Jan 27, 2023
b1449e0
Fix another test
mavasani Jan 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Analyzers/VisualBasic/Tests/UnsealClass/UnsealClassTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ end class")
Public Async Function RemovedFromAllPartialClassDeclarationsAcrossFiles() As Task
Await TestInRegularAndScriptAsync(
<Workspace>
<Project Language="Visual Basic">
<Project Language="Visual Basic" CommonReferences="true">
<Document>
partial public notinheritable class C
end class
Expand All @@ -162,7 +162,7 @@ end class
</Project>
</Workspace>.ToString(),
<Workspace>
<Project Language="Visual Basic">
<Project Language="Visual Basic" CommonReferences="true">
<Document>
partial public class C
end class
Expand Down
6 changes: 3 additions & 3 deletions src/EditorFeatures/CSharpTest/Diagnostics/GenerateType/GenerateTypeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5414,7 +5414,7 @@ public async Task TestMethodGroupWithMissingSystemActionAndFunc(string returnTyp
await TestInRegularAndScriptAsync(
$@"
<Workspace>
<Project Language=""C#"" CommonReferences=""false"">
<Project Language=""C#"" CommonReferencesMinCorlib=""true"">
<Document><![CDATA[
class C
{{
Expand Down Expand Up @@ -5446,9 +5446,9 @@ void M()

internal class Class
{{
private global::System.Object method;
private object method;

public Class(global::System.Object method)
public Class(object method)
{{
this.method = method;
}}
Expand Down
46 changes: 23 additions & 23 deletions src/EditorFeatures/CSharpTest/Diagnostics/UpgradeProject/UpgradeProjectTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ public async Task UpgradeAllProjectsToCSharp7()
{
await TestLanguageVersionUpgradedAsync(
@"<Workspace>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
<Document>
class C
{
Expand All @@ -373,9 +373,9 @@ void A()
}
</Document>
</Project>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
</Project>
<Project Language=""C#"" LanguageVersion=""7"">
<Project Language=""C#"" LanguageVersion=""7"" CommonReferences=""true"">
</Project>
<Project Language=""Visual Basic"">
</Project>
Expand All @@ -390,19 +390,19 @@ public async Task UpgradeAllProjectsToCSharp8()
{
await TestLanguageVersionUpgradedAsync(
@"<Workspace>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
<Document>
class C
{
object F = [|null!|];
}
</Document>
</Project>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
</Project>
<Project Language=""C#"" LanguageVersion=""7"">
<Project Language=""C#"" LanguageVersion=""7"" CommonReferences=""true"">
</Project>
<Project Language=""Visual Basic"">
<Project Language=""Visual Basic"" CommonReferences=""true"">
</Project>
</Workspace>",
LanguageVersion.CSharp8,
Expand Down Expand Up @@ -444,18 +444,18 @@ public async Task UpgradeAllProjectsToCSharp9()
{
await TestLanguageVersionUpgradedAsync(
@"<Workspace>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
<Document>
[|System.Console.WriteLine();|]
</Document>
</Project>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
</Project>
<Project Language=""C#"" LanguageVersion=""7"">
<Project Language=""C#"" LanguageVersion=""7"" CommonReferences=""true"">
</Project>
<Project Language=""C#"" LanguageVersion=""8"">
<Project Language=""C#"" LanguageVersion=""8"" CommonReferences=""true"">
</Project>
<Project Language=""Visual Basic"">
<Project Language=""Visual Basic"" CommonReferences=""true"">
</Project>
</Workspace>",
LanguageVersion.CSharp9,
Expand All @@ -469,7 +469,7 @@ public async Task ListAllSuggestions()
await TestExactActionSetOfferedAsync(

@"<Workspace>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
<Document>
class C
{
Expand All @@ -480,9 +480,9 @@ void A()
}
</Document>
</Project>
<Project Language=""C#"" LanguageVersion=""7"">
<Project Language=""C#"" LanguageVersion=""7"" CommonReferences=""true"">
</Project>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
</Project>
</Workspace>",
new[] {
Expand Down Expand Up @@ -527,7 +527,7 @@ public async Task FixAllProjectsNotOffered()
await TestExactActionSetOfferedAsync(

@"<Workspace>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
<Document>
class C
{
Expand All @@ -538,7 +538,7 @@ void A()
}
</Document>
</Project>
<Project Language=""Visual Basic"">
<Project Language=""Visual Basic"" CommonReferences=""true"">
</Project>
</Workspace>",
new[] {
Expand All @@ -552,7 +552,7 @@ public async Task OnlyOfferFixAllProjectsFromCSharp6ToCSharp7WhenApplicable()
await TestExactActionSetOfferedAsync(

@"<Workspace>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
<Document>
class C
{
Expand All @@ -563,9 +563,9 @@ void A()
}
</Document>
</Project>
<Project Language=""C#"" LanguageVersion=""7"">
<Project Language=""C#"" LanguageVersion=""7"" CommonReferences=""true"">
</Project>
<Project Language=""Visual Basic"">
<Project Language=""Visual Basic"" CommonReferences=""true"">
</Project>
</Workspace>",
new[] {
Expand Down Expand Up @@ -608,17 +608,17 @@ public async Task OnlyOfferFixAllProjectsToCSharp8WhenApplicable()
await TestExactActionSetOfferedAsync(

@"<Workspace>
<Project Language=""C#"" LanguageVersion=""6"">
<Project Language=""C#"" LanguageVersion=""6"" CommonReferences=""true"">
<Document>
class C
{
object F = [|null!|];
}
</Document>
</Project>
<Project Language=""C#"" LanguageVersion=""8"">
<Project Language=""C#"" LanguageVersion=""8"" CommonReferences=""true"">
</Project>
<Project Language=""Visual Basic"">
<Project Language=""Visual Basic"" CommonReferences=""true"">
</Project>
</Workspace>",
new[] {
Expand Down
6 changes: 3 additions & 3 deletions src/EditorFeatures/CSharpTest/GenerateConstructor/GenerateConstructorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3639,7 +3639,7 @@ public async Task TestMethodGroupWithMissingSystemActionAndFunc(string returnTyp
await TestInRegularAndScriptAsync(
$@"
<Workspace>
<Project Language=""C#"" CommonReferences=""false"">
<Project Language=""C#"" CommonReferencesMinCorlib=""true"">
<Document><![CDATA[
class C
{{
Expand Down Expand Up @@ -3675,9 +3675,9 @@ void M()

internal class Class
{{
private global::System.Object method;
private object method;

public Class(global::System.Object method)
public Class(object method)
{{
this.method = method;
}}
Expand Down
33 changes: 0 additions & 33 deletions ...ics/AbstractPushOrPullDiagnosticsTaggerProvider.SingleDiagnosticKindPullTaggerProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<Project>.GetInstance(out var seenProjects);
var hasSuccessfullyLoaded = HasSuccessfullyLoaded(document.Project, seenProjects);
if (!hasSuccessfullyLoaded)
return;
}

var requestedSpan = documentSpanToTag.SnapshotSpan;

var diagnostics = await _analyzerService.GetDiagnosticsForSpanAsync(
Expand Down Expand Up @@ -158,25 +144,6 @@ private async Task ProduceTagsAsync(
}
}

private bool HasSuccessfullyLoaded(Project? project, HashSet<Project> 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);
}
Expand Down
1 change: 1 addition & 0 deletions src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_Create.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public partial class TestWorkspace
private const string CommonReferencesNetCoreAppName = "CommonReferencesNetCoreApp";
private const string CommonReferencesNet6Name = "CommonReferencesNet6";
private const string CommonReferencesNetStandard20Name = "CommonReferencesNetStandard20";
private const string CommonReferencesMinCorlibName = "CommonReferencesMinCorlib";
private const string ReferencesOnDiskAttributeName = "ReferencesOnDisk";
private const string FilePathAttributeName = "FilePath";
private const string FoldersAttributeName = "Folders";
Expand Down
8 changes: 8 additions & 0 deletions src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlConsumption.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1117,6 +1117,14 @@ private static IList<MetadataReference> 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<MetadataReference> { TestBase.MinCorlibRef };
}

return references;
}

Expand Down
3 changes: 2 additions & 1 deletion src/EditorFeatures/VisualBasicTest/Diagnostics/GenerateMethod/GenerateMethodTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4408,7 +4408,7 @@ End Namespace")
Public Async Function TestAcrossFiles() As Task
Await TestInRegularAndScriptAsync(
"<Workspace>
<Project Language=""Visual Basic"">
<Project Language=""Visual Basic"" CommonReferences=""true"">
<Document>
Public Class DataContainer
Property PossibleInProcessTests As string
Expand Down Expand Up @@ -4462,6 +4462,7 @@ Public Class DataContainer
End Sub

Friend Sub ArbitraryPositionMethod()
Throw New NotImplementedException()
End Sub

Function Bazz() As Object
Expand Down
9 changes: 7 additions & 2 deletions src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,8 +703,13 @@ private async Task<CompilationInfo> 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
mavasani marked this conversation as resolved.
Show resolved Hide resolved
// 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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't mind this to fix the issue. but i am curious if this indicates a bug with the PS since they're the ones telling us that all info is available, even if no metadata refs are tehre. Should we be telling them there's an issue on their side? @jasonmalinowski ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, if we're seeing HasAllInformation = true and no references, that's technically a project system bug. That all said, this might be a good tactical fix for 17.5 while we chase down the source of that problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing that is more concerning for me is project system is never calling the setter for “IWorkspaceProjectContext.LastDesignTimeBuildSucceeded” (neither true nor false) and we always have the default true value for HasAllInformation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets open a PS bug to track for 17.6.

mavasani marked this conversation as resolved.
Show resolved Hide resolved

var newReferences = new List<MetadataReference>();
var metadataReferenceToProjectId = new Dictionary<MetadataReference, ProjectId>();
Expand Down
Loading