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

Change SupportedPlatformData to use ImmutableArrays #76658

Merged
merged 2 commits into from
Jan 7, 2025
Merged

Change SupportedPlatformData to use ImmutableArrays #76658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f033045
Change SupportedPlatformData to use ImmutableArrays for it's data
ToddGrun Jan 7, 2025
59383e9
prefer the using form of ArrayBuilder.GetInstance.
ToddGrun Jan 7, 2025
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
29 changes: 17 additions & 12 deletions src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ private ImmutableArray<CompletionItem> CreateItems(
CompletionContext completionContext,
ImmutableArray<SymbolAndSelectionInfo> symbols,
Func<SymbolAndSelectionInfo, TSyntaxContext> contextLookup,
Dictionary<ISymbol, List<ProjectId>>? invalidProjectMap,
List<ProjectId>? totalProjects)
Dictionary<ISymbol, ArrayBuilder<ProjectId>>? invalidProjectMap,
ImmutableArray<ProjectId> totalProjects)
{
// We might get symbol w/o name but CanBeReferencedByName is still set to true,
// need to filter them out.
Expand Down Expand Up @@ -197,21 +197,21 @@ protected static bool TryFindFirstSymbolMatchesTargetTypes(
private static SupportedPlatformData? ComputeSupportedPlatformData(
CompletionContext completionContext,
ImmutableArray<SymbolAndSelectionInfo> symbols,
Dictionary<ISymbol, List<ProjectId>>? invalidProjectMap,
List<ProjectId>? totalProjects)
Dictionary<ISymbol, ArrayBuilder<ProjectId>>? invalidProjectMap,
ImmutableArray<ProjectId> totalProjects)
{
SupportedPlatformData? supportedPlatformData = null;
if (invalidProjectMap != null)
{
List<ProjectId>? invalidProjects = null;
ArrayBuilder<ProjectId>? invalidProjects = null;
Copy link
Member

Choose a reason for hiding this comment

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

Preference is the using form for me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change the other ones, I don't think this one can be changed as it doesn't own the builders.

foreach (var symbol in symbols)
{
if (invalidProjectMap.TryGetValue(symbol.Symbol, out invalidProjects))
break;
}

if (invalidProjects != null)
supportedPlatformData = new SupportedPlatformData(completionContext.Document.Project.Solution, invalidProjects, totalProjects);
supportedPlatformData = new SupportedPlatformData(completionContext.Document.Project.Solution, invalidProjects.ToImmutable(), totalProjects);
}

return supportedPlatformData;
Expand Down Expand Up @@ -299,7 +299,7 @@ private async Task<ImmutableArray<CompletionItem>> GetItemsAsync(
if (relatedDocumentIds.IsEmpty)
{
var itemsForCurrentDocument = await GetSymbolsAsync(completionContext, syntaxContext, position, options, cancellationToken).ConfigureAwait(false);
return CreateItems(completionContext, itemsForCurrentDocument, _ => syntaxContext, invalidProjectMap: null, totalProjects: null);
return CreateItems(completionContext, itemsForCurrentDocument, _ => syntaxContext, invalidProjectMap: null, totalProjects: []);
}

using var _ = PooledDictionary<DocumentId, int>.GetInstance(out var documentIdToIndex);
Expand All @@ -315,10 +315,15 @@ private async Task<ImmutableArray<CompletionItem>> GetItemsAsync(

var symbolToContextMap = UnionSymbols(contextAndSymbolLists);
var missingSymbolsMap = FindSymbolsMissingInLinkedContexts(symbolToContextMap, contextAndSymbolLists);
var totalProjects = contextAndSymbolLists.Select(t => t.documentId.ProjectId).ToList();
var totalProjects = contextAndSymbolLists.SelectAsArray(t => t.documentId.ProjectId);

return CreateItems(
var items = CreateItems(
completionContext, [.. symbolToContextMap.Keys], symbol => symbolToContextMap[symbol], missingSymbolsMap, totalProjects);

foreach (var (_, builder) in missingSymbolsMap)
builder.Free();

return items;
}

protected virtual bool IsExclusive()
Expand Down Expand Up @@ -395,17 +400,17 @@ protected async Task<ImmutableArray<SymbolAndSelectionInfo>> TryGetSymbolsForCon
/// <param name="symbolToContext">The symbols recommended in the active context.</param>
/// <param name="linkedContextSymbolLists">The symbols recommended in linked documents</param>
/// <returns>The list of projects each recommended symbol did NOT appear in.</returns>
private static Dictionary<ISymbol, List<ProjectId>> FindSymbolsMissingInLinkedContexts(
private static Dictionary<ISymbol, ArrayBuilder<ProjectId>> FindSymbolsMissingInLinkedContexts(
Copy link
Member

Choose a reason for hiding this comment

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

Does the consumer return these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

caller (line 317) owns the freeing of these builders

Dictionary<SymbolAndSelectionInfo, TSyntaxContext> symbolToContext,
ImmutableArray<(DocumentId documentId, TSyntaxContext syntaxContext, ImmutableArray<SymbolAndSelectionInfo> symbols)> linkedContextSymbolLists)
{
var missingSymbols = new Dictionary<ISymbol, List<ProjectId>>(LinkedFilesSymbolEquivalenceComparer.Instance);
var missingSymbols = new Dictionary<ISymbol, ArrayBuilder<ProjectId>>(LinkedFilesSymbolEquivalenceComparer.Instance);

foreach (var (documentId, syntaxContext, symbols) in linkedContextSymbolLists)
{
var symbolsMissingInLinkedContext = symbolToContext.Keys.Except(symbols);
foreach (var (symbol, _) in symbolsMissingInLinkedContext)
missingSymbols.GetOrAdd(symbol, m => []).Add(documentId.ProjectId);
missingSymbols.GetOrAdd(symbol, m => ArrayBuilder<ProjectId>.GetInstance()).Add(documentId.ProjectId);
}

return missingSymbols;
Expand Down
4 changes: 2 additions & 2 deletions src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,8 @@ private static void AddSupportedPlatforms(ArrayBuilder<KeyValuePair<string, stri
{
return new SupportedPlatformData(
solution,
[.. invalidProjects.Split(s_projectSeperators).Select(s => ProjectId.CreateFromSerialized(Guid.Parse(s)))],
candidateProjects.Split(s_projectSeperators).Select(s => ProjectId.CreateFromSerialized(Guid.Parse(s))).ToList());
invalidProjects.Split(s_projectSeperators).SelectAsArray(s => ProjectId.CreateFromSerialized(Guid.Parse(s))),
candidateProjects.Split(s_projectSeperators).SelectAsArray(s => ProjectId.CreateFromSerialized(Guid.Parse(s))));
}

return null;
Expand Down
11 changes: 6 additions & 5 deletions src/Features/Core/Portable/QuickInfo/CommonSemanticQuickInfoProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Collections;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Utilities;
Expand Down Expand Up @@ -86,9 +87,6 @@ internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInf
var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var mainTokenInformation = BindToken(services, semanticModel, token, cancellationToken);

var candidateProjects = new List<ProjectId> { document.Project.Id };
var invalidProjects = new List<ProjectId>();

var candidateResults = new List<(DocumentId docId, TokenInformation tokenInformation)>
{
(document.Id, mainTokenInformation)
Expand All @@ -103,7 +101,6 @@ internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInf
if (linkedToken != default)
{
// Not in an inactive region, so this file is a candidate.
candidateProjects.Add(linkedDocumentId.ProjectId);
var linkedSymbols = BindToken(services, linkedModel, linkedToken, cancellationToken);
candidateResults.Add((linkedDocumentId, linkedSymbols));
}
Expand All @@ -117,7 +114,11 @@ internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInf
if (bestBinding.tokenInformation.Symbols.IsDefaultOrEmpty)
return default;

// We calculate the set of projects that are candidates for the best binding
var candidateProjects = candidateResults.SelectAsArray(result => result.docId.ProjectId);

// We calculate the set of supported projects
using var _ = ArrayBuilder<ProjectId>.GetInstance(out var invalidProjects);
candidateResults.Remove(bestBinding);
foreach (var (docId, tokenInformation) in candidateResults)
{
Expand All @@ -126,7 +127,7 @@ internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInf
invalidProjects.Add(docId.ProjectId);
}

var supportedPlatforms = new SupportedPlatformData(solution, invalidProjects, candidateProjects);
var supportedPlatforms = new SupportedPlatformData(solution, invalidProjects.ToImmutableAndClear(), candidateProjects);
return (bestBinding.tokenInformation, supportedPlatforms);
}

Expand Down
13 changes: 6 additions & 7 deletions src/Features/Core/Portable/Shared/Utilities/SupportedPlatformData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,26 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Shared.Utilities;

internal sealed class SupportedPlatformData(Solution solution, List<ProjectId> invalidProjects, IEnumerable<ProjectId> candidateProjects)
internal sealed class SupportedPlatformData(Solution solution, ImmutableArray<ProjectId> invalidProjects, ImmutableArray<ProjectId> candidateProjects)
{
// Because completion finds lots of symbols that exist in
// all projects, we'll instead maintain a list of projects
// missing the symbol.
public readonly List<ProjectId> InvalidProjects = invalidProjects;
public readonly IEnumerable<ProjectId> CandidateProjects = candidateProjects;
public readonly ImmutableArray<ProjectId> InvalidProjects = invalidProjects;
public readonly ImmutableArray<ProjectId> CandidateProjects = candidateProjects;
public readonly Solution Solution = solution;

public IList<SymbolDisplayPart> ToDisplayParts()
{
if (InvalidProjects == null || InvalidProjects.Count == 0)
if (InvalidProjects.Length == 0)
return [];

var builder = new List<SymbolDisplayPart>();
Expand All @@ -46,5 +45,5 @@ private static string Supported(bool supported)
=> supported ? FeaturesResources.Available : FeaturesResources.Not_Available;

public bool HasValidAndInvalidProjects()
=> InvalidProjects.Any() && InvalidProjects.Count != CandidateProjects.Count();
=> InvalidProjects.Length > 0 && InvalidProjects.Length != CandidateProjects.Length;
}
6 changes: 3 additions & 3 deletions src/Features/Core/Portable/SignatureHelp/AbstractSignatureHelpProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ private static SignatureHelpSymbolParameter ReplaceStructuralTypes(
return itemsForCurrentDocument;
}

var totalProjects = relatedDocuments.Select(d => d.Project.Id).Concat(document.Project.Id);
var totalProjects = relatedDocuments.Concat(document).SelectAsArray(d => d.Project.Id);

var semanticModel = await document.ReuseExistingSpeculativeModelAsync(position, cancellationToken).ConfigureAwait(false);
var compilation = semanticModel.Compilation;
Expand All @@ -277,7 +277,7 @@ symbolKeyItem.SymbolKey is not SymbolKey symbolKey ||
symbolKey = SymbolKey.Create(methodSymbol.OriginalDefinition, cancellationToken);
}

var invalidProjectsForCurrentSymbol = new List<ProjectId>();
using var _ = ArrayBuilder<ProjectId>.GetInstance(out var invalidProjectsForCurrentSymbol);
foreach (var relatedDocument in relatedDocuments)
{
// Try to resolve symbolKey in each related compilation,
Expand All @@ -289,7 +289,7 @@ symbolKeyItem.SymbolKey is not SymbolKey symbolKey ||
}
}

var platformData = new SupportedPlatformData(document.Project.Solution, invalidProjectsForCurrentSymbol, totalProjects);
var platformData = new SupportedPlatformData(document.Project.Solution, invalidProjectsForCurrentSymbol.ToImmutableAndClear(), totalProjects);
finalItems.Add(UpdateItem(item, platformData));
}

Expand Down
Loading