Update Project.HasSuccessfullyLoadedAsync logic

[mavasani]

Fixes AB#1674516
Fixes AB#1710989
Fixes AB#1665787
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 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.

[mavasani]

We do not need this check in the tagger as this is already checked in the core executor:

roslyn/src/Features/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor.cs

Lines 129 to 141 in 42bd85d

	// if project is not loaded successfully then, we disable semantic errors for compiler analyzers
	var isCompilerAnalyzer = analyzer.IsCompilerAnalyzer();
	if (kind != AnalysisKind.Syntax && isCompilerAnalyzer)
	{
	var isEnabled = await textDocument.Project.HasSuccessfullyLoadedAsync(cancellationToken).ConfigureAwait(false);
	Logger.Log(FunctionId.Diagnostics_SemanticDiagnostic, (a, d, e) => $"{a}, ({d.Id}, {d.Project.Id}), Enabled:{e}", analyzer, textDocument, isEnabled);
	if (!isEnabled)
	{
	return SpecializedCollections.EmptyEnumerable<DiagnosticData>();
	}
	}

[mavasani]

Tagging @arkalyanms - if the code change gets approved, do we want to consider this for 17.5?

[CyrusNajmabadi]

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 ?

[jasonmalinowski]

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.

[mavasani]

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.

[arunchndr]

Lets open a PS bug to track for 17.6.

[jasonmalinowski]

Signing off, at least as a tactical fix for 17.5. @CyrusNajmabadi is correct that if we see this state that's really a project system bug or a bug somewhere higher up. But chasing that down to fix might be too much of trouble for 17.5 at least.

So signing off, but I won't be sad if we want to not take this and instead try to root-cause further.

The base branch was changed.

[mavasani]

Signing off, at least as a tactical fix for 17.5.

Thanks! Rebased the PR to release/dev17.5 @arkalyanms to make a call about whether we want to take this tactical fix for 17.5 or let Jason/ProjectSystem team to figure out a more appropriate fix.

[arunchndr]

Yes let's take this for 17.5. There is a fair bit of feedback around this.

[CyrusNajmabadi]

This needs to flow into Main. What process ensures that?

[sharwell]

@CyrusNajmabadi The branch flows release/dev17.5 to release/dev17.5-vs-deps (latest PR #66766). Once that completes, it will flow to release/dev17.6. Once that completes, it will flow to main.