Skip to content

YAPCSSF: Automatically add projects if they don't exist when updating #10335

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
davidwengier merged 3 commits into from
May 2, 2024
Merged

YAPCSSF: Automatically add projects if they don't exist when updating #10335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dea497e
Automatically add projects if they don't exist when updating
davidwengier Apr 24, 2024
fec62b6
Ensure we regenerate C# code when documents move from misc files to r…
davidwengier May 2, 2024
809ebcd
Fix tests
davidwengier May 2, 2024
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
30 changes: 12 additions & 18 deletions src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Microsoft.CodeAnalysis.Razor.Utilities;
using Microsoft.CodeAnalysis.Razor.Workspaces;
Expand All @@ -29,14 +30,16 @@ internal partial class OpenDocumentGenerator : IRazorStartupService, IDisposable
private readonly ImmutableArray<IDocumentProcessedListener> _listeners;
private readonly IProjectSnapshotManager _projectManager;
private readonly LanguageServerFeatureOptions _options;
private readonly ILogger _logger;

private readonly AsyncBatchingWorkQueue<IDocumentSnapshot> _workQueue;
private readonly CancellationTokenSource _disposeTokenSource;

public OpenDocumentGenerator(
IEnumerable<IDocumentProcessedListener> listeners,
IProjectSnapshotManager projectManager,
LanguageServerFeatureOptions options)
LanguageServerFeatureOptions options,
ILoggerFactory loggerFactory)
{
_listeners = listeners.ToImmutableArray();
_projectManager = projectManager;
Expand All @@ -49,6 +52,7 @@ public OpenDocumentGenerator(
_disposeTokenSource.Token);

_projectManager.Changed += ProjectManager_Changed;
_logger = loggerFactory.GetOrCreateLogger<OpenDocumentGenerator>();
}

public void Dispose()
Expand Down Expand Up @@ -88,6 +92,8 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args)
return;
}

_logger.LogDebug($"Got a project change of type {args.Kind} for {args.ProjectKey.Id}");

switch (args.Kind)
{
case ProjectChangeKind.ProjectChanged:
Expand All @@ -106,25 +112,11 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args)
}

case ProjectChangeKind.DocumentAdded:
{
var newProject = args.Newer.AssumeNotNull();
var documentFilePath = args.DocumentFilePath.AssumeNotNull();

if (newProject.TryGetDocument(documentFilePath, out var document))
{
// We don't enqueue the current document because added documents are initially closed.
Copy link
Member Author

@davidwengier davidwengier May 2, 2024

Choose a reason for hiding this comment

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

The change is because we have too many moving pieces at the moment. When I changed things so that newly added documents stopped guessing which was the right project to be added to, it meant we didn't start generating when a document moved from the misc files project to the right one. Since the EnqueueIfNecessary already had a check for only generating open documents, this explicit lack of work made no sense to me, and seems to me like it was only valid because the "guess which project to go in" was guessing correctly most of the time.

Copy link
Member Author

@davidwengier davidwengier May 2, 2024

Choose a reason for hiding this comment

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

"Why isn't it broken in main then?" I hear you ask. Good question! It works in main for precisely the inefficiency this PR (and all of the others 😁) is trying to solve! In main, rather than migrating one document, we fully reset the whole project, which means going to 0 tag helpers, then in a future update back to the real number of tag helpers. That change in tag helpers (among other changes) triggers a ProjectChanged change type, which is handled elsewhere in this switch, and triggers the generation of the document.


foreach (var relatedDocument in newProject.GetRelatedDocuments(document))
{
EnqueueIfNecessary(relatedDocument);
}
}

break;
}

case ProjectChangeKind.DocumentChanged:
{
// Most of the time Add will be called on closed files, but when migrating files to/from the misc files
// project they could already be open, but with a different generated C# path

var newProject = args.Newer.AssumeNotNull();
var documentFilePath = args.DocumentFilePath.AssumeNotNull();

Expand Down Expand Up @@ -178,6 +170,8 @@ void EnqueueIfNecessary(IDocumentSnapshot document)
return;
}

_logger.LogDebug($"Enqueuing generation of {document.FilePath} in {document.Project.Key.Id}");

_workQueue.AddWork(document);
}
}
Expand Down
7 changes: 7 additions & 0 deletions .../src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectConfigurationFileChangeEventArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.AspNetCore.Razor.ProjectSystem;
using Microsoft.AspNetCore.Razor.Utilities;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Microsoft.CodeAnalysis.Razor.Workspaces;

namespace Microsoft.AspNetCore.Razor.LanguageServer;
Expand Down Expand Up @@ -77,4 +78,10 @@ public bool TryDeserialize(LanguageServerFeatureOptions options, [NotNullWhen(tr

return projectInfo is not null;
}

internal ProjectKey GetProjectKey()
{
var intermediateOutputPath = FilePathNormalizer.GetNormalizedDirectoryName(ConfigurationFilePath);
return ProjectKey.FromString(intermediateOutputPath);
}
}
42 changes: 8 additions & 34 deletions ...crosoft.AspNetCore.Razor.LanguageServer/ProjectConfigurationStateSynchronizer.Comparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.Extensions.Internal;

namespace Microsoft.AspNetCore.Razor.LanguageServer;

Expand All @@ -17,42 +18,15 @@ private Comparer()
}

public bool Equals(Work? x, Work? y)
{
if (x is null)
{
return y is null;
}
else if (y is null)
{
return x is null;
}

// For purposes of removing duplicates from batches, two Work instances
// are equal only if their identifying properties are equal. So, only
// configuration file paths and project keys.

if (!FilePathComparer.Instance.Equals(x.ConfigurationFilePath, y.ConfigurationFilePath))
{
return false;
}

return (x, y) switch
{
(AddProject, AddProject) => true,
=> (x, y) switch
{
(Work(var keyX), Work(var keyY)) => keyX == keyY,
(null, null) => true,

(ResetProject { ProjectKey: var keyX },
ResetProject { ProjectKey: var keyY })
=> keyX == keyY,

(UpdateProject { ProjectKey: var keyX },
UpdateProject { ProjectKey: var keyY })
=> keyX == keyY,

_ => false,
};
}
_ => false
};

public int GetHashCode(Work obj)
=> obj.GetHashCode();
=> obj.ProjectKey.GetHashCode();
}
}
132 changes: 35 additions & 97 deletions ...or/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectConfigurationStateSynchronizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,26 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Diagnostics;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.LanguageServer.Common;
using Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.AspNetCore.Razor.ProjectSystem;
using Microsoft.AspNetCore.Razor.Utilities;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Microsoft.CodeAnalysis.Razor.Utilities;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.VisualStudio.Threading;

namespace Microsoft.AspNetCore.Razor.LanguageServer;

internal partial class ProjectConfigurationStateSynchronizer : IProjectConfigurationFileChangeListener, IDisposable
{
private abstract record Work(string ConfigurationFilePath);
private sealed record AddProject(string ConfigurationFilePath, RazorProjectInfo ProjectInfo) : Work(ConfigurationFilePath);
private sealed record ResetProject(string ConfigurationFilePath, ProjectKey ProjectKey) : Work(ConfigurationFilePath);
private sealed record UpdateProject(string ConfigurationFilePath, ProjectKey ProjectKey, RazorProjectInfo ProjectInfo) : Work(ConfigurationFilePath);
private abstract record Work(ProjectKey ProjectKey);
private sealed record ResetProject(ProjectKey ProjectKey) : Work(ProjectKey);
private sealed record UpdateProject(ProjectKey ProjectKey, RazorProjectInfo ProjectInfo) : Work(ProjectKey);

private static readonly TimeSpan s_delay = TimeSpan.FromMilliseconds(250);

Expand All @@ -37,8 +34,7 @@ private sealed record UpdateProject(string ConfigurationFilePath, ProjectKey Pro
private readonly CancellationTokenSource _disposeTokenSource;
private readonly AsyncBatchingWorkQueue<Work> _workQueue;

private ImmutableDictionary<string, ProjectKey> _filePathToProjectKeyMap =
ImmutableDictionary<string, ProjectKey>.Empty.WithComparers(keyComparer: FilePathComparer.Instance);
private readonly Dictionary<ProjectKey, ResetProject> _resetProjectMap = new();

public ProjectConfigurationStateSynchronizer(
IRazorProjectService projectService,
Expand Down Expand Up @@ -67,44 +63,26 @@ public void Dispose()
_disposeTokenSource.Cancel();
_disposeTokenSource.Dispose();
}

private async ValueTask ProcessBatchAsync(ImmutableArray<Work> items, CancellationToken token)
{
foreach (var item in items.GetMostRecentUniqueItems(Comparer.Instance))
{
if (token.IsCancellationRequested)
{
return;
}

var itemTask = item switch
{
AddProject(var configurationFilePath, var projectInfo) => AddProjectAsync(configurationFilePath, projectInfo, token),
ResetProject(_, var projectKey) => ResetProjectAsync(projectKey, token),
UpdateProject(_, var projectKey, var projectInfo) => UpdateProjectAsync(projectKey, projectInfo, token),
ResetProject(var projectKey) => ResetProjectAsync(projectKey, token),
UpdateProject(var projectKey, var projectInfo) => UpdateProjectAsync(projectKey, projectInfo, token),
_ => Assumed.Unreachable<Task>()
};

await itemTask.ConfigureAwait(false);
}

async Task AddProjectAsync(string configurationFilePath, RazorProjectInfo projectInfo, CancellationToken token)
{
var projectFilePath = FilePathNormalizer.Normalize(projectInfo.FilePath);
var intermediateOutputPath = FilePathNormalizer.GetNormalizedDirectoryName(configurationFilePath);

var projectKey = await _projectService
.AddProjectAsync(
projectFilePath,
intermediateOutputPath,
projectInfo.Configuration,
projectInfo.RootNamespace,
projectInfo.DisplayName,
token)
.ConfigureAwait(false);

_logger.LogInformation($"Added {projectKey.Id}.");

ImmutableInterlocked.AddOrUpdate(ref _filePathToProjectKeyMap, configurationFilePath, projectKey, static (k, v) => v);
_logger.LogInformation($"Project configuration file added for project '{projectFilePath}': '{configurationFilePath}'");

await UpdateProjectAsync(projectKey, projectInfo, token).ConfigureAwait(false);
}

Task ResetProjectAsync(ProjectKey projectKey, CancellationToken token)
{
_logger.LogInformation($"Resetting {projectKey.Id}.");
Expand All @@ -125,8 +103,9 @@ Task UpdateProjectAsync(ProjectKey projectKey, RazorProjectInfo projectInfo, Can
_logger.LogInformation($"Updating {projectKey.Id}.");

return _projectService
.UpdateProjectAsync(
.AddOrUpdateProjectAsync(
projectKey,
projectInfo.FilePath,
projectInfo.Configuration,
projectInfo.RootNamespace,
projectInfo.DisplayName,
Expand All @@ -138,55 +117,25 @@ Task UpdateProjectAsync(ProjectKey projectKey, RazorProjectInfo projectInfo, Can

public void ProjectConfigurationFileChanged(ProjectConfigurationFileChangeEventArgs args)
{
var configurationFilePath = FilePathNormalizer.Normalize(args.ConfigurationFilePath);

switch (args.Kind)
{
case RazorFileChangeKind.Changed:
{
if (args.TryDeserialize(_options, out var projectInfo))
{
if (_filePathToProjectKeyMap.TryGetValue(configurationFilePath, out var projectKey))
{
_logger.LogInformation($"""
Configuration file changed for project '{projectKey.Id}'.
Configuration file path: '{configurationFilePath}'
""");

_workQueue.AddWork(new UpdateProject(configurationFilePath, projectKey, projectInfo));
}
else
{
_logger.LogWarning($"""
Adding project for previously unseen configuration file.
Configuration file path: '{configurationFilePath}'
""");

_workQueue.AddWork(new AddProject(configurationFilePath, projectInfo));
}
var projectKey = ProjectKey.From(projectInfo);
_logger.LogInformation($"Configuration file changed for project '{projectKey.Id}'.");

_workQueue.AddWork(new UpdateProject(projectKey, projectInfo));
}
else
{
if (_filePathToProjectKeyMap.TryGetValue(configurationFilePath, out var projectKey))
{
_logger.LogWarning($"""
Failed to deserialize after change to configuration file for project '{projectKey.Id}'.
Configuration file path: '{configurationFilePath}'
""");

// We found the last associated project file for the configuration file. Reset the project since we can't
// accurately determine its configurations.

_workQueue.AddWork(new ResetProject(configurationFilePath, projectKey));
}
else
{
// Could not resolve an associated project file.
_logger.LogWarning($"""
Failed to deserialize after change to previously unseen configuration file.
Configuration file path: '{configurationFilePath}'
""");
}
var projectKey = args.GetProjectKey();
_logger.LogWarning($"Failed to deserialize after change to configuration file for project '{projectKey.Id}'.");

// We found the last associated project file for the configuration file. Reset the project since we can't
// accurately determine its configurations.
_workQueue.AddWork(new ResetProject(projectKey));
}
}

Expand All @@ -196,39 +145,28 @@ Failed to deserialize after change to previously unseen configuration file.
{
if (args.TryDeserialize(_options, out var projectInfo))
{
_workQueue.AddWork(new AddProject(configurationFilePath, projectInfo));
var projectKey = ProjectKey.From(projectInfo);
_logger.LogInformation($"Configuration file added for project '{projectKey.Id}'.");

// Update will add the project if it doesn't exist
_workQueue.AddWork(new UpdateProject(projectKey, projectInfo));
}
else
{
// This is the first time we've seen this configuration file, but we can't deserialize it.
// The only thing we can really do is issue a warning.
_logger.LogWarning($"""
Failed to deserialize previously unseen configuration file.
Configuration file path: '{configurationFilePath}'
""");
_logger.LogWarning($"Failed to deserialize previously unseen configuration file '{args.ConfigurationFilePath}'");
}
}

break;

case RazorFileChangeKind.Removed:
{
if (ImmutableInterlocked.TryRemove(ref _filePathToProjectKeyMap, configurationFilePath, out var projectKey))
{
_logger.LogInformation($"""
Configuration file removed for project '{projectKey}'.
Configuration file path: '{configurationFilePath}'
""");
var projectKey = args.GetProjectKey();
_logger.LogInformation($"Configuration file removed for project '{projectKey}'.");

_workQueue.AddWork(new ResetProject(configurationFilePath, projectKey));
}
else
{
_logger.LogWarning($"""
Failed to resolve associated project on configuration removed event.
Configuration file path: '{configurationFilePath}'
""");
}
_workQueue.AddWork(new ResetProject(projectKey));
}

break;
Expand Down
10 changes: 10 additions & 0 deletions ...Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectSystem/IRazorProjectService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,14 @@ Task UpdateProjectAsync(
ProjectWorkspaceState projectWorkspaceState,
ImmutableArray<DocumentSnapshotHandle> documents,
CancellationToken cancellationToken);

Task AddOrUpdateProjectAsync(
ProjectKey projectKey,
string filePath,
RazorConfiguration? configuration,
string? rootNamespace,
string displayName,
ProjectWorkspaceState projectWorkspaceState,
ImmutableArray<DocumentSnapshotHandle> documents,
CancellationToken cancellationToken);
}
Loading