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.

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

Support EditorBrowsable in completion for unimported types #47771

Merged
merged 11 commits into from
Sep 25, 2020
Prev Previous commit
Next Next commit
Only show editor browsable types from unimported namespace
genlu committed Sep 16, 2020
commit 37f0eb9ac1b064dec89554214593bdf0f78f80d2
Original file line number Diff line number Diff line change
@@ -25,31 +25,38 @@ private readonly struct CacheEntry

private ImmutableArray<TypeImportCompletionItemInfo> ItemInfos { get; }

public bool ContainsAdvancedMembers { get; }
genlu marked this conversation as resolved.
Show resolved Hide resolved

private CacheEntry(
Checksum checksum,
string language,
bool containsAdvancedmembers,
ImmutableArray<TypeImportCompletionItemInfo> items)
{
Checksum = checksum;
Language = language;

ContainsAdvancedMembers = containsAdvancedmembers;
ItemInfos = items;

genlu marked this conversation as resolved.
Show resolved Hide resolved
}

public ImmutableArray<CompletionItem> GetItemsForContext(
string language,
string genericTypeSuffix,
bool isInternalsVisible,
bool isAttributeContext,
bool isCaseSensitive)
bool isCaseSensitive,
bool hideAdvancedMembers)
{
// We will need to adjust some items if the request is made in:
// 1. attribute context, then we will not show or complete with "Attribute" suffix.
// 2. a project with different language than when the cache entry was created,
// then we will change the generic suffix accordingly.
// 3. asked to hide advanced members and there is advanced member in the cache
// Otherwise, we can simply return cached items.
var isSameLanguage = Language == language;
if (isSameLanguage && !isAttributeContext)
if (isSameLanguage && !isAttributeContext && !(hideAdvancedMembers && ContainsAdvancedMembers))
genlu marked this conversation as resolved.
Show resolved Hide resolved
{
return ItemInfos.Where(info => info.IsPublic || isInternalsVisible).SelectAsArray(info => info.Item);
}
genlu marked this conversation as resolved.
Show resolved Hide resolved
@@ -59,6 +66,11 @@ public ImmutableArray<CompletionItem> GetItemsForContext(
{
if (info.IsPublic || isInternalsVisible)
{
if (hideAdvancedMembers && info.IsEditorBrowsableStateAdvanced)
{
continue;
}

var item = info.Item;
if (isAttributeContext)
{
@@ -99,14 +111,18 @@ public class Builder : IDisposable
private readonly string _language;
private readonly string _genericTypeSuffix;
private readonly Checksum _checksum;
private readonly EditorBrowsableInfo _editorBrowsableInfo;

private readonly ArrayBuilder<TypeImportCompletionItemInfo> _itemsBuilder;

public Builder(Checksum checksum, string language, string genericTypeSuffix)
private bool _containsAdvancedMembers;

public Builder(Checksum checksum, string language, string genericTypeSuffix, EditorBrowsableInfo editorBrowsableInfo)
{
_checksum = checksum;
_language = language;
_genericTypeSuffix = genericTypeSuffix;
_editorBrowsableInfo = editorBrowsableInfo;

_itemsBuilder = ArrayBuilder<TypeImportCompletionItemInfo>.GetInstance();
}
@@ -116,11 +132,28 @@ public CacheEntry ToReferenceCacheEntry()
return new CacheEntry(
_checksum,
_language,
_containsAdvancedMembers,
_itemsBuilder.ToImmutable());
}

public void AddItem(INamedTypeSymbol symbol, string containingNamespace, bool isPublic)
{
// We want to cache items with EditoBrowsableState == Advanced regarless of current "hide adv members" option value
genlu marked this conversation as resolved.
Show resolved Hide resolved
var (isBrowsable, isEditorBrowsableStateAdvanced) = symbol.IsEditorBrowsableWithState(
hideAdvancedMembers: false,
_editorBrowsableInfo.Compilation,
_editorBrowsableInfo.EditorBrowsableAttributeConstructor,
_editorBrowsableInfo.typeLibTypeAttributeConstructors,
_editorBrowsableInfo.TypeLibFuncAttributeConstructors,
genlu marked this conversation as resolved.
Show resolved Hide resolved
_editorBrowsableInfo.TypeLibVarAttributeConstructors,
_editorBrowsableInfo.HideModuleNameAttribute);

if (!isBrowsable)
{
// Hide this item from completion
return;
}

var isGeneric = symbol.Arity > 0;

// Need to determine if a type is an attribute up front since we want to filter out
@@ -140,7 +173,8 @@ public void AddItem(INamedTypeSymbol symbol, string containingNamespace, bool is
CompletionItemFlags.CachedAndExpanded,
extensionMethodData: null);

_itemsBuilder.Add(new TypeImportCompletionItemInfo(item, isPublic, isGeneric, isAttribute));
_containsAdvancedMembers = _containsAdvancedMembers || isEditorBrowsableStateAdvanced;
_itemsBuilder.Add(new TypeImportCompletionItemInfo(item, isPublic, isGeneric, isAttribute, isEditorBrowsableStateAdvanced));
}

public void Dispose()
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Extensions.ContextQuery;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Completion.Providers.ImportCompletion
@@ -40,6 +41,7 @@ internal AbstractTypeImportCompletionService(Workspace workspace)
bool forceCacheCreation,
CancellationToken cancellationToken)
{
var hideAdvancedmembers = currentProject.Solution.Workspace.Options.GetOption(CompletionOptions.HideAdvancedMembers, currentProject.Language);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var hideAdvancedmembers = currentProject.Solution.Workspace.Options.GetOption(CompletionOptions.HideAdvancedMembers, currentProject.Language);
var hideAdvancedmembers = currentProject.Solution.Options.GetOption(CompletionOptions.HideAdvancedMembers, currentProject.Language);

(fix elsewhere if you used teh former).

the former is mutable and may change as the feature is running. the latter is an immutable snapshot.

Copy link
Member Author

Choose a reason for hiding this comment

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

var getCacheResults = await GetCacheEntriesAsync(currentProject, syntaxContext, forceCacheCreation, cancellationToken).ConfigureAwait(false);

if (getCacheResults == null)
@@ -68,7 +70,8 @@ ImmutableArray<CompletionItem> GetItemsFromCacheResult(GetCacheResult cacheResul
GenericTypeSuffix,
currentCompilation.Assembly.IsSameAssemblyOrHasFriendAccessTo(cacheResult.Assembly),
syntaxContext.IsAttributeNameContext,
IsCaseSensitive);
IsCaseSensitive,
hideAdvancedmembers);
}
}

@@ -114,11 +117,13 @@ ImmutableArray<CompletionItem> GetItemsFromCacheResult(GetCacheResult cacheResul
}
}

var editorBrowsableInfo = new EditorBrowsableInfo(currentCompilation);

foreach (var peReference in currentProject.MetadataReferences.OfType<PortableExecutableReference>())
{
if (HasGlobalAlias(peReference) &&
currentCompilation.GetAssemblyOrModuleSymbol(peReference) is IAssemblySymbol assembly &&
TryGetCacheForPEReference(solution, currentCompilation, peReference, syntaxContext, forceCacheCreation, cancellationToken, out cacheResult))
TryGetCacheForPEReference(solution, editorBrowsableInfo, peReference, syntaxContext, forceCacheCreation, cancellationToken, out cacheResult))
{
if (cacheResult.HasValue)
{
@@ -162,6 +167,7 @@ static bool HasGlobalAlias(MetadataReference? metadataReference)
syntaxContext,
forceCacheCreation,
CacheService.ProjectItemsCache,
new EditorBrowsableInfo(compilation),
cancellationToken);
}

@@ -170,7 +176,7 @@ static bool HasGlobalAlias(MetadataReference? metadataReference)
/// </summary>
private bool TryGetCacheForPEReference(
Solution solution,
Compilation compilation,
EditorBrowsableInfo editorBrowsableInfo,
PortableExecutableReference peReference,
SyntaxContext syntaxContext,
bool forceCacheCreation,
@@ -188,7 +194,7 @@ private bool TryGetCacheForPEReference(
return false;
}

if (!(compilation.GetAssemblyOrModuleSymbol(peReference) is IAssemblySymbol assemblySymbol))
if (!(editorBrowsableInfo.Compilation.GetAssemblyOrModuleSymbol(peReference) is IAssemblySymbol assemblySymbol))
{
result = null;
return false;
@@ -202,6 +208,7 @@ private bool TryGetCacheForPEReference(
syntaxContext,
forceCacheCreation,
CacheService.PEItemsCache,
editorBrowsableInfo,
cancellationToken);
return true;
}
@@ -214,6 +221,7 @@ private bool TryGetCacheForPEReference(
SyntaxContext syntaxContext,
bool forceCacheCreation,
IDictionary<TKey, CacheEntry> cache,
EditorBrowsableInfo editorBrowsableInfo,
CancellationToken cancellationToken)
where TKey : notnull
{
@@ -228,7 +236,7 @@ private bool TryGetCacheForPEReference(
// Cache miss, create all items only when asked.
if (forceCacheCreation)
{
using var builder = new CacheEntry.Builder(checksum, language, GenericTypeSuffix);
using var builder = new CacheEntry.Builder(checksum, language, GenericTypeSuffix, editorBrowsableInfo);
GetCompletionItemsForTopLevelTypeDeclarations(assembly.GlobalNamespace, builder, cancellationToken);
cacheEntry = builder.ToReferenceCacheEntry();
cache[key] = cacheEntry;
@@ -353,12 +361,13 @@ private readonly struct TypeImportCompletionItemInfo
{
private readonly ItemPropertyKind _properties;

public TypeImportCompletionItemInfo(CompletionItem item, bool isPublic, bool isGeneric, bool isAttribute)
public TypeImportCompletionItemInfo(CompletionItem item, bool isPublic, bool isGeneric, bool isAttribute, bool isEditorBrowsableStateAdvanced = false)
genlu marked this conversation as resolved.
Show resolved Hide resolved
{
Item = item;
_properties = (isPublic ? ItemPropertyKind.IsPublic : 0)
| (isGeneric ? ItemPropertyKind.IsGeneric : 0)
| (isAttribute ? ItemPropertyKind.IsAttribute : 0);
| (isAttribute ? ItemPropertyKind.IsAttribute : 0)
| (isEditorBrowsableStateAdvanced ? ItemPropertyKind.IsEditorBrowsableStateAdvanced : 0);
}

public CompletionItem Item { get; }
@@ -372,16 +381,49 @@ public bool IsGeneric
public bool IsAttribute
=> (_properties & ItemPropertyKind.IsAttribute) != 0;

public bool IsEditorBrowsableStateAdvanced
=> (_properties & ItemPropertyKind.IsEditorBrowsableStateAdvanced) != 0;

public TypeImportCompletionItemInfo WithItem(CompletionItem item)
=> new TypeImportCompletionItemInfo(item, IsPublic, IsGeneric, IsAttribute);
=> new TypeImportCompletionItemInfo(item, IsPublic, IsGeneric, IsAttribute, IsEditorBrowsableStateAdvanced);

[Flags]
private enum ItemPropertyKind : byte
{
IsPublic = 0x1,
IsGeneric = 0x2,
IsAttribute = 0x4,
IsEditorBrowsableStateAdvanced = 0x8,
}
}

private class EditorBrowsableInfo
{
public Compilation Compilation { get; }
private IMethodSymbol? _editorBrowsableAttributeConstructor;
private List<IMethodSymbol>? _typeLibTypeAttributeConstructors;
private List<IMethodSymbol>? _typeLibFuncAttributeConstructors;
private List<IMethodSymbol>? _typeLibVarAttributeConstructors;

public EditorBrowsableInfo(Compilation compilation)
{
Compilation = compilation;
HideModuleNameAttribute = Compilation.HideModuleNameAttribute();
}

public IMethodSymbol EditorBrowsableAttributeConstructor
=> _editorBrowsableAttributeConstructor ??= EditorBrowsableHelpers.GetSpecialEditorBrowsableAttributeConstructor(Compilation);

public List<IMethodSymbol> typeLibTypeAttributeConstructors
=> _typeLibTypeAttributeConstructors ??= EditorBrowsableHelpers.GetSpecialTypeLibTypeAttributeConstructors(Compilation);

public List<IMethodSymbol> TypeLibFuncAttributeConstructors
=> _typeLibFuncAttributeConstructors ??= EditorBrowsableHelpers.GetSpecialTypeLibFuncAttributeConstructors(Compilation);

public List<IMethodSymbol> TypeLibVarAttributeConstructors
=> _typeLibVarAttributeConstructors ??= EditorBrowsableHelpers.GetSpecialTypeLibVarAttributeConstructors(Compilation);

public INamedTypeSymbol? HideModuleNameAttribute { get; }
}
}
}
58 changes: 44 additions & 14 deletions src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs
Original file line number Diff line number Diff line change
@@ -54,21 +54,43 @@ public static bool IsEditorBrowsable(
List<IMethodSymbol>? typeLibFuncAttributeConstructors = null,
List<IMethodSymbol>? typeLibVarAttributeConstructors = null,
INamedTypeSymbol? hideModuleNameAttribute = null)
{
return IsEditorBrowsableWithState(
symbol,
hideAdvancedMembers,
compilation,
editorBrowsableAttributeConstructor,
typeLibTypeAttributeConstructors,
typeLibFuncAttributeConstructors,
typeLibVarAttributeConstructors,
hideModuleNameAttribute).isBrowsable;
}

// In addition to given symbol's browsability, also returns its EditorBrowsableState if it contains EditorBrowsableAttribute.
public static (bool isBrowsable, bool isEditorBrowsableStateAdvanced) IsEditorBrowsableWithState(
this ISymbol symbol,
bool hideAdvancedMembers,
Compilation compilation,
IMethodSymbol? editorBrowsableAttributeConstructor = null,
List<IMethodSymbol>? typeLibTypeAttributeConstructors = null,
List<IMethodSymbol>? typeLibFuncAttributeConstructors = null,
List<IMethodSymbol>? typeLibVarAttributeConstructors = null,
INamedTypeSymbol? hideModuleNameAttribute = null)
{
// Namespaces can't have attributes, so just return true here. This also saves us a
// costly check if this namespace has any locations in source (since a merged namespace
// needs to go collect all the locations).
if (symbol.Kind == SymbolKind.Namespace)
{
return true;
return (isBrowsable: true, isEditorBrowsableStateAdvanced: false);
}

// check for IsImplicitlyDeclared so we don't spend time examining VB's embedded types.
// This saves a few percent in typing scenarios. An implicitly declared symbol can't
// have attributes, so it can't be hidden by them.
if (symbol.IsImplicitlyDeclared)
{
return true;
return (isBrowsable: true, isEditorBrowsableStateAdvanced: false);
}

// Ignore browsability limiting attributes if the symbol is declared in source.
@@ -77,10 +99,10 @@ public static bool IsEditorBrowsable(
if (symbol.Locations.All(loc => loc.IsInSource))
{
// The HideModuleNameAttribute still applies to Modules defined in source
return !IsBrowsingProhibitedByHideModuleNameAttribute(symbol, compilation, hideModuleNameAttribute);
return (!IsBrowsingProhibitedByHideModuleNameAttribute(symbol, compilation, hideModuleNameAttribute), isEditorBrowsableStateAdvanced: false);
}

return !IsBrowsingProhibited(
var (isProhibited, isEditorBrowsableStateAdvanced) = IsBrowsingProhibited(
symbol,
hideAdvancedMembers,
compilation,
@@ -89,9 +111,11 @@ public static bool IsEditorBrowsable(
typeLibFuncAttributeConstructors,
typeLibVarAttributeConstructors,
hideModuleNameAttribute);

return (!isProhibited, isEditorBrowsableStateAdvanced);
}

private static bool IsBrowsingProhibited(
private static (bool isProhibited, bool isEditorBrowsableStateAdvanced) IsBrowsingProhibited(
ISymbol symbol,
bool hideAdvancedMembers,
Compilation compilation,
@@ -104,14 +128,16 @@ private static bool IsBrowsingProhibited(
var attributes = symbol.GetAttributes();
if (attributes.Length == 0)
{
return false;
return (isProhibited: false, isEditorBrowsableStateAdvanced: false);
}

return IsBrowsingProhibitedByEditorBrowsableAttribute(attributes, hideAdvancedMembers, compilation, editorBrowsableAttributeConstructor)
var (isProhibited, isEditorBrowsableStateAdvanced) = IsBrowsingProhibitedByEditorBrowsableAttribute(attributes, hideAdvancedMembers, compilation, editorBrowsableAttributeConstructor);

return ((isProhibited
|| IsBrowsingProhibitedByTypeLibTypeAttribute(attributes, compilation, typeLibTypeAttributeConstructors)
|| IsBrowsingProhibitedByTypeLibFuncAttribute(attributes, compilation, typeLibFuncAttributeConstructors)
|| IsBrowsingProhibitedByTypeLibVarAttribute(attributes, compilation, typeLibVarAttributeConstructors)
|| IsBrowsingProhibitedByHideModuleNameAttribute(symbol, compilation, hideModuleNameAttribute, attributes);
|| IsBrowsingProhibitedByHideModuleNameAttribute(symbol, compilation, hideModuleNameAttribute, attributes)), isEditorBrowsableStateAdvanced);
}

private static bool IsBrowsingProhibitedByHideModuleNameAttribute(
@@ -135,13 +161,13 @@ private static bool IsBrowsingProhibitedByHideModuleNameAttribute(
return false;
}

private static bool IsBrowsingProhibitedByEditorBrowsableAttribute(
private static (bool isProhibited, bool isEditorBrowsableStateAdvanced) IsBrowsingProhibitedByEditorBrowsableAttribute(
ImmutableArray<AttributeData> attributes, bool hideAdvancedMembers, Compilation compilation, IMethodSymbol? constructor)
{
constructor ??= EditorBrowsableHelpers.GetSpecialEditorBrowsableAttributeConstructor(compilation);
if (constructor == null)
{
return false;
return (isProhibited: false, isEditorBrowsableStateAdvanced: false);
}

foreach (var attribute in attributes)
@@ -154,15 +180,19 @@ private static bool IsBrowsingProhibitedByEditorBrowsableAttribute(
var state = (EditorBrowsableState)attribute.ConstructorArguments.First().Value;
#nullable enable

if (EditorBrowsableState.Never == state ||
(hideAdvancedMembers && EditorBrowsableState.Advanced == state))
if (EditorBrowsableState.Never == state)
{
return true;
return (isProhibited: true, isEditorBrowsableStateAdvanced: false);
}

if (EditorBrowsableState.Advanced == state)
{
return (isProhibited: hideAdvancedMembers, isEditorBrowsableStateAdvanced: true);
}
}
}

return false;
return (isProhibited: false, isEditorBrowsableStateAdvanced: false);
}

private static bool IsBrowsingProhibitedByTypeLibTypeAttribute(