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

Jump to bottom

Ensure diagnostics are logged before proceeding with further project loading #70467

Merged
merged 3 commits into from
Oct 25, 2023
Merged

Ensure diagnostics are logged before proceeding with further project loading #70467

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a44f928
Ensure diagnostics are logged before proceeding with further project …
dibarbet Oct 19, 2023
70b64e2
Ensure execution diagnostics are logged as well
dibarbet Oct 21, 2023
74f50f2
Merge remote-tracking branch 'upstream/main' into fix_error_logging
dibarbet Oct 23, 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
37 changes: 20 additions & 17 deletions ...Server/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.LanguageServer.HostWorkspace.ProjectTelemetry;
using Microsoft.CodeAnalysis.MSBuild;
using Microsoft.CodeAnalysis.MSBuild.Logging;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.ProjectSystem;
using Microsoft.CodeAnalysis.Shared.TestHooks;
Expand Down Expand Up @@ -205,6 +206,15 @@ private async ValueTask LoadOrReloadProjectsAsync(ImmutableSegmentedList<Project
if (await buildHost.IsProjectFileSupportedAsync(projectPath, cancellationToken))
{
var loadedFile = await buildHost.LoadProjectFileAsync(projectPath, cancellationToken);
var diagnosticLogItems = await loadedFile.GetDiagnosticLogItemsAsync(cancellationToken);
LogDiagnostics(projectPath, diagnosticLogItems);
Copy link
Member

@jasonmalinowski jasonmalinowski Oct 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

One subtle bit: unfortunately how the existing code works is all the diagnostics created for loading get dumped into the same collection, both for evaluation and execution passes. So you probably only want to call LogDiagnostics() if you're in your if check and bailing; otherwise you can have the later LogDiagnostics() call. If both places call it though Warnings might get logged twice.

We should also make LogDiagnostics() just return the most severe type of LSP.MessageType so we can inline that.


if (diagnosticLogItems.Any(item => item.Kind is WorkspaceDiagnosticKind.Failure))
dibarbet marked this conversation as resolved.
Show resolved Hide resolved
{
// If there were any total failures, no point in continuing.
return (LSP.MessageType.Error, preferredBuildHostKind);
}

var loadedProjectInfos = await loadedFile.GetProjectFileInfosAsync(cancellationToken);

// The out-of-proc build host supports more languages than we may actually have Workspace binaries for, so ensure we can actually process that
Expand Down Expand Up @@ -247,23 +257,7 @@ private async ValueTask LoadOrReloadProjectsAsync(ImmutableSegmentedList<Project
}

await _projectLoadTelemetryReporter.ReportProjectLoadTelemetryAsync(projectFileInfos, projectToLoad, cancellationToken);

var diagnosticLogItems = await loadedFile.GetDiagnosticLogItemsAsync(cancellationToken);
if (diagnosticLogItems.Any())
{
foreach (var logItem in diagnosticLogItems)
{
var projectName = Path.GetFileName(projectPath);
_logger.Log(logItem.Kind is WorkspaceDiagnosticKind.Failure ? LogLevel.Error : LogLevel.Warning, $"{logItem.Kind} while loading {logItem.ProjectFilePath}: {logItem.Message}");
}

return (diagnosticLogItems.Any(logItem => logItem.Kind is WorkspaceDiagnosticKind.Failure) ? LSP.MessageType.Error : LSP.MessageType.Warning, preferredBuildHostKind);
}
else
{
_logger.LogInformation($"Successfully completed load of {projectPath}");
return (null, null);
}
_logger.LogInformation($"Successfully completed load of {projectPath}");
}

return (null, null);
Expand All @@ -273,5 +267,14 @@ private async ValueTask LoadOrReloadProjectsAsync(ImmutableSegmentedList<Project
_logger.LogError(e, $"Exception thrown while loading {projectToLoad.Path}");
return (LSP.MessageType.Error, preferredBuildHostKind);
}

void LogDiagnostics(string projectPath, ImmutableArray<DiagnosticLogItem> diagnosticLogItems)
{
foreach (var logItem in diagnosticLogItems)
{
var projectName = Path.GetFileName(projectPath);
_logger.Log(logItem.Kind is WorkspaceDiagnosticKind.Failure ? LogLevel.Error : LogLevel.Warning, $"{logItem.Kind} while loading {logItem.ProjectFilePath}: {logItem.Message}");
}
}
}
}