Communicate with OOP process to determine if an AnalyzerReference has analyzers or source generators. #74810
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
CyrusNajmabadi
changed the title
| @@ -2,19 +2,18 @@ | |||
| // The .NET Foundation licenses this file to you under the MIT license. | |||
There was a problem hiding this comment.
View with whitespace off.
| @@ -2,237 +2,174 @@ | |||
| // The .NET Foundation licenses this file to you under the MIT license. | |||
There was a problem hiding this comment.
this is effectively a rewrite. I recommend SxS diff view.
|
|
||
| public bool HasItems => true; | ||
|
|
||
| public IEnumerable Items => _folderItems; | ||
|
|
||
| public object SourceItem => _projectHierarchyItem; | ||
|
|
||
| internal void Update() |
There was a problem hiding this comment.
this was only called in the constructor. so it could be inlined. once inlined, it meant that almost all state held int his type went away.
src/Workspaces/Core/Portable/Shared/Extensions/AnalyzerReferenceExtensions.cs
Outdated
Show resolved
Hide resolved
| /// equal to <paramref name="analyzerReferenceFullPath"/> has any analyzers or source generators. | ||
| /// </summary> | ||
| ValueTask<bool> HasAnalyzersOrSourceGeneratorsAsync( | ||
| Checksum solutionChecksum, ProjectId projectId, string analyzerReferenceFullPath, CancellationToken cancellationToken); |
There was a problem hiding this comment.
this was the core goal of hte PR. The VS SolutionExplroer side should defer to it to answer this question, instead of loading analyzers/generators in proc (which doesn't work with reloading with .Net Framework).
| @@ -143,10 +143,25 @@ public ValueTask<ImmutableArray<SourceGeneratorIdentity>> GetSourceGeneratorIden | |||
| { | |||
| var project = solution.GetRequiredProject(projectId); | |||
| var analyzerReference = project.AnalyzerReferences | |||
| .OfType<AnalyzerFileReference>() | |||
There was a problem hiding this comment.
this was a mistake. And will not work once #74780 goes in. In that PR we no longer have AnalyzerFileReference as the outermost type in the OOP side. Instead, we have a wrapper AnalyzerReference type that ensures that we track if analyzers/generators are asked for.
| { | ||
| // The set of AnalyzerItems hasn't been realized yet. Just signal that HasItems | ||
| // may have changed. | ||
| private readonly BulkObservableCollection<AnalyzerItem> _items = []; |
There was a problem hiding this comment.
instead of a collection we overwrite, we now just use a BOC as that can be mutated in place while handling notification to appropriate listeners.
…mabadi/roslyn into remoteAnalyzerReference
| { | ||
| _analyzerReferences = project.AnalyzerReferences; | ||
| private readonly CancellationTokenSource _cancellationTokenSource = new(); | ||
| private readonly AsyncBatchingWorkQueue _workQueue; |
There was a problem hiding this comment.
switched to a work queue model. We listen to workspace vents, which then just pulse the queue.
| } | ||
| */ | ||
| } | ||
| var client = await RemoteHostClient.TryGetClientAsync(this.Workspace, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
we now call to oop if possible to figure out this information.
| using Microsoft.CodeAnalysis.Diagnostics; | ||
| using Microsoft.Internal.VisualStudio.PlatformUI; | ||
| using Microsoft.VisualStudio.Imaging; | ||
| using Microsoft.VisualStudio.Imaging.Interop; | ||
|
|
||
| namespace Microsoft.VisualStudio.LanguageServices.Implementation.SolutionExplorer; | ||
|
|
||
| internal partial class AnalyzerItem( | ||
| internal sealed partial class AnalyzerItem( |
There was a problem hiding this comment.
just cleanup. No other changes.
| } | ||
| } | ||
| => analyzerReference is UnresolvedAnalyzerReference unresolvedAnalyzerReference | ||
| ? unresolvedAnalyzerReference.FullPath |
There was a problem hiding this comment.
this prevents an NRT warning by accessing FullPath through teh derived type.
| protected override IAttachedCollectionSource? CreateCollectionSource(AnalyzersFolderItem analyzersFolder, string relationshipName) | ||
| => relationshipName == KnownRelationships.Contains | ||
| ? new AnalyzerItemSource(analyzersFolder, commandHandler, listenerProvider) | ||
| : null; | ||
| } |
There was a problem hiding this comment.
just cleanup. No other changes.
| VisualStudioWorkspace workspace, | ||
| [Import(typeof(AnalyzersCommandHandler))] IAnalyzersCommandHandler commandHandler) | ||
| : AttachedCollectionSourceProvider<IVsHierarchyItem> | ||
| { | ||
| private readonly IThreadingContext _threadingContext = threadingContext; | ||
| private readonly Workspace _workspace = workspace; |
There was a problem hiding this comment.
passing along IThreadingContext so we update the UI on the UI thread.
| .First(r => r.FullPath == analyzerReferenceFullPath); | ||
|
|
||
| return ValueTaskFactory.FromResult(analyzerReference.HasAnalyzersOrSourceGenerators(project.Language)); | ||
| }, cancellationToken); |
There was a problem hiding this comment.
same pattern as the above method.
|
@ToddGrun @jasonmalinowski ptal. |
...sualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/BaseDiagnosticAndGeneratorItemSource.cs
Outdated
Show resolved
Hide resolved
| var referencesToAdd = GetFilteredAnalyzers(_analyzerReferences, project) | ||
| .Where(r => !_analyzerItems.Any(item => item.AnalyzerReference == r)) | ||
| .ToArray(); | ||
| _workQueue = new AsyncBatchingWorkQueue( |
There was a problem hiding this comment.
Technically yes. But so minor is unnoticible
|
|
||
| _analyzerItems.EndBulkOperation(); | ||
| // Defer actual determination and computation of the items until later. | ||
| public bool HasItems => !_cancellationTokenSource.IsCancellationRequested; |
There was a problem hiding this comment.
correct. This matches what many of our SE nodes do though, which is to guess that they have children while computing asynchronously.
src/VisualStudio/Core/Impl/SolutionExplorer/AnalyzerItem/AnalyzerItemSource.cs
Outdated
Show resolved
Hide resolved
Fixes #43008