Skip to content

Remove build-only diagnostics functionality no longer used in product #72941

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

Merged
CyrusNajmabadi merged 1 commit into from
Apr 8, 2024
Merged

Remove build-only diagnostics functionality no longer used in product #72941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 7 additions & 38 deletions src/Features/Core/Portable/Diagnostics/BuildOnlyDiagnosticsService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,17 @@
namespace Microsoft.CodeAnalysis.Diagnostics;

[ExportWorkspaceServiceFactory(typeof(IBuildOnlyDiagnosticsService), ServiceLayer.Default), Shared]
internal sealed class BuildOnlyDiagnosticsServiceFactory : IWorkspaceServiceFactory
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed class BuildOnlyDiagnosticsServiceFactory() : IWorkspaceServiceFactory
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public BuildOnlyDiagnosticsServiceFactory()
{
}

public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
=> new BuildOnlyDiagnosticsService(workspaceServices.Workspace);

private sealed class BuildOnlyDiagnosticsService : IBuildOnlyDiagnosticsService
{
private readonly object _gate = new();
private readonly Dictionary<DocumentId, ImmutableArray<DiagnosticData>> _documentDiagnostics = [];
private readonly Dictionary<ProjectId, ImmutableArray<DiagnosticData>> _projectDiagnostics = [];
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Apr 8, 2024

Choose a reason for hiding this comment

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

data in this was never read in any product code. only in a single test. remove it, and updated codepaths affected by it.


public BuildOnlyDiagnosticsService(Workspace workspace)
{
Expand Down Expand Up @@ -61,18 +56,12 @@ private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e)
}
}

public void AddBuildOnlyDiagnostics(Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableArray<DiagnosticData> diagnostics)
public void AddBuildOnlyDiagnostics(DocumentId documentId, ImmutableArray<DiagnosticData> diagnostics)
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Apr 8, 2024

Choose a reason for hiding this comment

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

since nothing was reading the project side, we only need to call this for teh document side of things.

{
lock (_gate)
{
if (documentId != null)
{
_documentDiagnostics[documentId] = diagnostics;
}
else if (projectId != null)
{
_projectDiagnostics[projectId] = diagnostics;
}
}
}

Expand All @@ -81,7 +70,6 @@ private void ClearAllDiagnostics()
lock (_gate)
{
_documentDiagnostics.Clear();
_projectDiagnostics.Clear();
}
}

Expand All @@ -103,43 +91,24 @@ private void ClearDiagnostics(Project? project)

lock (_gate)
{
_projectDiagnostics.Remove(project.Id);
foreach (var documentId in project.DocumentIds)
_documentDiagnostics.Remove(documentId);
}
}

public void ClearBuildOnlyDiagnostics(Solution solution, ProjectId? projectId, DocumentId? documentId)
public void ClearBuildOnlyDiagnostics(Project project, DocumentId? documentId)
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Apr 8, 2024

Choose a reason for hiding this comment

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

passing in a project-id here meant 'clear out project diags, and all diags for any document in this project'. So, maintaining that logic here.

{
if (documentId != null)
ClearDiagnostics(documentId);
else
ClearDiagnostics(solution.GetProject(projectId));
ClearDiagnostics(project);
}

public ImmutableArray<DiagnosticData> GetBuildOnlyDiagnostics(DocumentId documentId)
{
lock (_gate)
{
if (_documentDiagnostics.TryGetValue(documentId, out var diagnostics))
{
return diagnostics;
}

return [];
}
}

public ImmutableArray<DiagnosticData> GetBuildOnlyDiagnostics(ProjectId projectId)
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Apr 8, 2024

Choose a reason for hiding this comment

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

only called in a single test method, no other product code called this.

{
lock (_gate)
{
if (_projectDiagnostics.TryGetValue(projectId, out var diagnostics))
{
return diagnostics;
}

return [];
return _documentDiagnostics.TryGetValue(documentId, out var diagnostics) ? diagnostics : [];
}
}
}
Expand Down
6 changes: 2 additions & 4 deletions src/Features/Core/Portable/Diagnostics/IBuildOnlyDiagnosticsService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,9 @@ namespace Microsoft.CodeAnalysis.Diagnostics;
/// </summary>
internal interface IBuildOnlyDiagnosticsService : IWorkspaceService
{
void AddBuildOnlyDiagnostics(Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableArray<DiagnosticData> diagnostics);
void AddBuildOnlyDiagnostics(DocumentId documentId, ImmutableArray<DiagnosticData> diagnostics);

void ClearBuildOnlyDiagnostics(Solution solution, ProjectId? projectId, DocumentId? documentId);
void ClearBuildOnlyDiagnostics(Project project, DocumentId? documentId);

ImmutableArray<DiagnosticData> GetBuildOnlyDiagnostics(DocumentId documentId);

ImmutableArray<DiagnosticData> GetBuildOnlyDiagnostics(ProjectId projectId);
}
15 changes: 7 additions & 8 deletions src/VisualStudio/Core/Def/TaskList/ExternalErrorDiagnosticUpdateSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,8 @@
using Microsoft.CodeAnalysis.SolutionCrawler;
using Roslyn.Utilities;

#pragma warning disable CA1200 // Avoid using cref tags with a prefix

namespace Microsoft.VisualStudio.LanguageServices.Implementation.TaskList;

using ProjectErrorMap = ImmutableDictionary<ProjectId, ImmutableArray<DiagnosticData>>;

/// <summary>
/// Diagnostic source for warnings and errors reported from explicit build command invocations in Visual Studio.
/// VS workspaces calls into us when a build is invoked or completed in Visual Studio.
Expand Down Expand Up @@ -520,13 +516,16 @@ private void ProcessAndRaiseDiagnosticsUpdated(ImmutableArray<DiagnosticsUpdated
{
if (args.Kind == DiagnosticsUpdatedKind.DiagnosticsCreated)
{
RoslynDebug.AssertNotNull(args.Solution);
_buildOnlyDiagnosticsService.AddBuildOnlyDiagnostics(args.Solution, args.ProjectId, args.DocumentId, args.Diagnostics);
Contract.ThrowIfNull(args.Solution);
if (args.DocumentId != null)
_buildOnlyDiagnosticsService.AddBuildOnlyDiagnostics(args.DocumentId, args.Diagnostics);
}
else if (args.Kind == DiagnosticsUpdatedKind.DiagnosticsRemoved)
{
RoslynDebug.AssertNotNull(args.Solution);
_buildOnlyDiagnosticsService.ClearBuildOnlyDiagnostics(args.Solution, args.ProjectId, args.DocumentId);
Contract.ThrowIfNull(args.Solution);
var project = args.Solution.GetProject(args.ProjectId);
if (project != null)
_buildOnlyDiagnosticsService.ClearBuildOnlyDiagnostics(project, args.DocumentId);
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/VisualStudio/Core/Test/Diagnostics/ExternalDiagnosticUpdateSourceTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics

Dim buildOnlyDiagnosticService = workspace.Services.GetRequiredService(Of IBuildOnlyDiagnosticsService)
Assert.Empty(buildOnlyDiagnosticService.GetBuildOnlyDiagnostics(project.DocumentIds.First()))
Assert.Empty(buildOnlyDiagnosticService.GetBuildOnlyDiagnostics(project.Id))

Dim diagnostics = source.GetBuildErrors()
Assert.Equal(2, diagnostics.Length)
Expand Down