From 01ac9272b3c1c00a71f32438793822287cacd7a8 Mon Sep 17 00:00:00 2001 From: Gen Lu Date: Thu, 24 Sep 2020 15:55:54 -0700 Subject: [PATCH] Address review comments --- ...tTypeImportCompletionService.CacheEntry.cs | 7 +- .../AbstractTypeImportCompletionService.cs | 68 +--- .../ObjectBrowser/AbstractListItemFactory.cs | 13 +- .../Shared/Extensions/ISymbolExtensions.cs | 304 ------------------ .../Utilities/EditorBrowsableHelpers.cs | 288 ++++++++++++++++- 5 files changed, 307 insertions(+), 373 deletions(-) diff --git a/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionService.CacheEntry.cs b/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionService.CacheEntry.cs index 22993109e8c..390351bc25a 100644 --- a/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionService.CacheEntry.cs +++ b/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionService.CacheEntry.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; +using static Microsoft.CodeAnalysis.Shared.Utilities.EditorBrowsableHelpers; namespace Microsoft.CodeAnalysis.Completion.Providers.ImportCompletion { @@ -131,11 +132,7 @@ public void AddItem(INamedTypeSymbol symbol, string containingNamespace, bool is var (isBrowsable, isEditorBrowsableStateAdvanced) = symbol.IsEditorBrowsableWithState( hideAdvancedMembers: false, _editorBrowsableInfo.Compilation, - _editorBrowsableInfo.EditorBrowsableAttributeConstructor, - _editorBrowsableInfo.TypeLibTypeAttributeConstructors, - _editorBrowsableInfo.TypeLibFuncAttributeConstructors, - _editorBrowsableInfo.TypeLibVarAttributeConstructors, - _editorBrowsableInfo.HideModuleNameAttribute); + _editorBrowsableInfo); if (!isBrowsable) { diff --git a/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionService.cs b/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionService.cs index 6b2bcd352fc..85e3d4e2ea5 100644 --- a/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionService.cs +++ b/src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionService.cs @@ -16,9 +16,10 @@ using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Extensions.ContextQuery; -using Microsoft.CodeAnalysis.Shared.Utilities; using Roslyn.Utilities; +using static Microsoft.CodeAnalysis.Shared.Utilities.EditorBrowsableHelpers; + namespace Microsoft.CodeAnalysis.Completion.Providers.ImportCompletion { internal abstract partial class AbstractTypeImportCompletionService : ITypeImportCompletionService @@ -80,7 +81,7 @@ ImmutableArray GetItemsFromCacheResult(GetCacheResult cacheResul var _ = ArrayBuilder.GetInstance(out var builder); var currentCompilation = await currentProject.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false); - var editorBrowsableInfo = new EditorBrowsableInfo(currentCompilation); + var editorBrowsableInfo = new Lazy(() => new EditorBrowsableInfo(currentCompilation)); var cacheResult = await GetCacheForProjectAsync(currentProject, syntaxContext, forceCacheCreation: true, editorBrowsableInfo, cancellationToken).ConfigureAwait(false); @@ -124,7 +125,7 @@ ImmutableArray GetItemsFromCacheResult(GetCacheResult cacheResul { if (HasGlobalAlias(peReference) && currentCompilation.GetAssemblyOrModuleSymbol(peReference) is IAssemblySymbol assembly && - TryGetCacheForPEReference(solution, editorBrowsableInfo, peReference, syntaxContext, forceCacheCreation, cancellationToken, out cacheResult)) + TryGetCacheForPEReference(solution, assembly, editorBrowsableInfo, peReference, syntaxContext, forceCacheCreation, cancellationToken, out cacheResult)) { if (cacheResult.HasValue) { @@ -154,7 +155,7 @@ static bool HasGlobalAlias(MetadataReference? metadataReference) Project project, SyntaxContext syntaxContext, bool forceCacheCreation, - EditorBrowsableInfo? editorBrowsableInfo, + Lazy? editorBrowsableInfo, CancellationToken cancellationToken) { var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false); @@ -169,7 +170,7 @@ static bool HasGlobalAlias(MetadataReference? metadataReference) syntaxContext, forceCacheCreation, CacheService.ProjectItemsCache, - editorBrowsableInfo ?? new EditorBrowsableInfo(compilation), + editorBrowsableInfo ?? new Lazy(() => new EditorBrowsableInfo(compilation)), cancellationToken); } @@ -178,7 +179,8 @@ static bool HasGlobalAlias(MetadataReference? metadataReference) /// private bool TryGetCacheForPEReference( Solution solution, - EditorBrowsableInfo editorBrowsableInfo, + IAssemblySymbol assemblySymbol, + Lazy editorBrowsableInfo, PortableExecutableReference peReference, SyntaxContext syntaxContext, bool forceCacheCreation, @@ -196,12 +198,6 @@ static bool HasGlobalAlias(MetadataReference? metadataReference) return false; } - if (!(editorBrowsableInfo.Compilation.GetAssemblyOrModuleSymbol(peReference) is IAssemblySymbol assemblySymbol)) - { - result = null; - return false; - } - var checksum = SymbolTreeInfo.GetMetadataChecksum(solution, peReference, cancellationToken); result = GetCacheWorker( key, @@ -216,6 +212,12 @@ static bool HasGlobalAlias(MetadataReference? metadataReference) } // Returns null if cache miss and forceCacheCreation == false + // + // PERF: + // Based on profiling results, initializing EditorBrowsableInfo upfront for each referenced + // project every time a completion is triggered is expensive. Making them lazy would + // eliminate this overhead when we have a cache hit while keeping it easy to share + // between original projects and PE references when trying to get completion items. private GetCacheResult? GetCacheWorker( TKey key, IAssemblySymbol assembly, @@ -223,7 +225,7 @@ static bool HasGlobalAlias(MetadataReference? metadataReference) SyntaxContext syntaxContext, bool forceCacheCreation, IDictionary cache, - EditorBrowsableInfo editorBrowsableInfo, + Lazy editorBrowsableInfo, CancellationToken cancellationToken) where TKey : notnull { @@ -238,7 +240,7 @@ static bool HasGlobalAlias(MetadataReference? metadataReference) // Cache miss, create all items only when asked. if (forceCacheCreation) { - using var builder = new CacheEntry.Builder(checksum, language, GenericTypeSuffix, editorBrowsableInfo); + using var builder = new CacheEntry.Builder(checksum, language, GenericTypeSuffix, editorBrowsableInfo.Value); GetCompletionItemsForTopLevelTypeDeclarations(assembly.GlobalNamespace, builder, cancellationToken); cacheEntry = builder.ToReferenceCacheEntry(); cache[key] = cacheEntry; @@ -398,43 +400,5 @@ private enum ItemPropertyKind : byte IsEditorBrowsableStateAdvanced = 0x8, } } - - // Things needed for determining whether a symbol is EditorBrowsable. - // Grouped together and reused within each compilation. - // - // Based on profiling results, initializing these symbols upfront for each referenced - // project every time a completion is triggered is expensive. Making them lazy would - // eliminate this overhead when we have a cache hit while keeping it easy to share - // between original projects and PE references when trying to get completion items. - private class EditorBrowsableInfo - { - private Optional? _hideModuleNameAttribute; - private Optional? _editorBrowsableAttributeConstructor; - private ImmutableArray? _typeLibTypeAttributeConstructors; - private ImmutableArray? _typeLibFuncAttributeConstructors; - private ImmutableArray? _typeLibVarAttributeConstructors; - - public Compilation Compilation { get; } - - public Optional HideModuleNameAttribute - => _hideModuleNameAttribute ??= new(Compilation.HideModuleNameAttribute()); - - public Optional EditorBrowsableAttributeConstructor - => _editorBrowsableAttributeConstructor ??= new(EditorBrowsableHelpers.GetSpecialEditorBrowsableAttributeConstructor(Compilation)); - - public ImmutableArray TypeLibTypeAttributeConstructors - => _typeLibTypeAttributeConstructors ??= EditorBrowsableHelpers.GetSpecialTypeLibTypeAttributeConstructors(Compilation); - - public ImmutableArray TypeLibFuncAttributeConstructors - => _typeLibFuncAttributeConstructors ??= EditorBrowsableHelpers.GetSpecialTypeLibFuncAttributeConstructors(Compilation); - - public ImmutableArray TypeLibVarAttributeConstructors - => _typeLibVarAttributeConstructors ??= EditorBrowsableHelpers.GetSpecialTypeLibVarAttributeConstructors(Compilation); - - public EditorBrowsableInfo(Compilation compilation) - { - Compilation = compilation; - } - } } } diff --git a/src/VisualStudio/Core/Def/Implementation/Library/ObjectBrowser/AbstractListItemFactory.cs b/src/VisualStudio/Core/Def/Implementation/Library/ObjectBrowser/AbstractListItemFactory.cs index 1a620f1133b..926d8225ad6 100644 --- a/src/VisualStudio/Core/Def/Implementation/Library/ObjectBrowser/AbstractListItemFactory.cs +++ b/src/VisualStudio/Core/Def/Implementation/Library/ObjectBrowser/AbstractListItemFactory.cs @@ -184,10 +184,7 @@ private static bool IncludeMemberSymbol(ISymbol symbol, IAssemblySymbol assembly ImmutableArray.Builder builder) where TSymbol : class, ISymbol { - var editorBrowsableAttributeConstructor = EditorBrowsableHelpers.GetSpecialEditorBrowsableAttributeConstructor(compilation); - var typeLibFuncAttributeConstructors = EditorBrowsableHelpers.GetSpecialTypeLibFuncAttributeConstructors(compilation); - var typeLibTypeAttributeConstructors = EditorBrowsableHelpers.GetSpecialTypeLibTypeAttributeConstructors(compilation); - var typeLibVarAttributeConstructors = EditorBrowsableHelpers.GetSpecialTypeLibVarAttributeConstructors(compilation); + var editorBrowsableInfo = new EditorBrowsableHelpers.EditorBrowsableInfo(compilation); foreach (var symbol in symbols) { @@ -197,13 +194,7 @@ private static bool IncludeMemberSymbol(ISymbol symbol, IAssemblySymbol assembly } var hideAdvancedMembers = false; - var isHidden = !symbol.IsEditorBrowsable( - hideAdvancedMembers, - compilation, - new(editorBrowsableAttributeConstructor), - typeLibFuncAttributeConstructors, - typeLibTypeAttributeConstructors, - typeLibVarAttributeConstructors); + var isHidden = !symbol.IsEditorBrowsable(hideAdvancedMembers, compilation, editorBrowsableInfo); builder.Add(listItemCreator(symbol, projectId, isHidden)); } diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs index 40bb96b70f9..0d2853c5634 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs @@ -7,8 +7,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Collections.Immutable; -using System.ComponentModel; using System.Diagnostics; using System.Globalization; using System.Linq; @@ -35,246 +33,6 @@ public static DeclarationModifiers GetSymbolModifiers(this ISymbol symbol) isSealed: symbol.IsSealed); } - /// - /// Checks a given symbol for browsability based on its declaration location, attributes - /// explicitly limiting browsability, and whether showing of advanced members is enabled. - /// The optional attribute constructor parameters may be used to specify the symbols of the - /// constructors of the various browsability limiting attributes because finding these - /// repeatedly over a large list of symbols can be slow. If providing these constructor - /// symbols, they should be in the format provided by - /// EditorBrowsableHelpers.GetSpecial*AttributeConstructor(). If these are not provided, - /// they will be found in the compilation. - /// - public static bool IsEditorBrowsable( - this ISymbol symbol, - bool hideAdvancedMembers, - Compilation compilation, - Optional editorBrowsableAttributeConstructor = default, - ImmutableArray typeLibTypeAttributeConstructors = default, - ImmutableArray typeLibFuncAttributeConstructors = default, - ImmutableArray typeLibVarAttributeConstructors = default, - Optional hideModuleNameAttribute = default) - { - 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, - Optional editorBrowsableAttributeConstructor = default, - ImmutableArray typeLibTypeAttributeConstructors = default, - ImmutableArray typeLibFuncAttributeConstructors = default, - ImmutableArray typeLibVarAttributeConstructors = default, - Optional hideModuleNameAttribute = default) - { - // 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 (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 (isBrowsable: true, isEditorBrowsableStateAdvanced: false); - } - - // Ignore browsability limiting attributes if the symbol is declared in source. - // Check all locations since some of VB's embedded My symbols are declared in - // both source and the MyTemplateLocation. - if (symbol.Locations.All(loc => loc.IsInSource)) - { - // The HideModuleNameAttribute still applies to Modules defined in source - return (!IsBrowsingProhibitedByHideModuleNameAttribute(symbol, compilation, hideModuleNameAttribute), isEditorBrowsableStateAdvanced: false); - } - - var (isProhibited, isEditorBrowsableStateAdvanced) = IsBrowsingProhibited( - symbol, - hideAdvancedMembers, - compilation, - editorBrowsableAttributeConstructor, - typeLibTypeAttributeConstructors, - typeLibFuncAttributeConstructors, - typeLibVarAttributeConstructors, - hideModuleNameAttribute); - - return (!isProhibited, isEditorBrowsableStateAdvanced); - } - - private static (bool isProhibited, bool isEditorBrowsableStateAdvanced) IsBrowsingProhibited( - ISymbol symbol, - bool hideAdvancedMembers, - Compilation compilation, - Optional editorBrowsableAttributeConstructor, - ImmutableArray typeLibTypeAttributeConstructors, - ImmutableArray typeLibFuncAttributeConstructors, - ImmutableArray typeLibVarAttributeConstructors, - Optional hideModuleNameAttribute) - { - var attributes = symbol.GetAttributes(); - if (attributes.Length == 0) - { - return (isProhibited: false, isEditorBrowsableStateAdvanced: false); - } - - 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)), isEditorBrowsableStateAdvanced); - } - - private static bool IsBrowsingProhibitedByHideModuleNameAttribute( - ISymbol symbol, Compilation compilation, Optional hideModuleNameAttribute, ImmutableArray attributes = default) - { - if (!symbol.IsModuleType()) - { - return false; - } - - attributes = attributes.IsDefault ? symbol.GetAttributes() : attributes; - - if (!hideModuleNameAttribute.HasValue) - { - hideModuleNameAttribute = new(compilation.HideModuleNameAttribute()); - } - - foreach (var attribute in attributes) - { - if (Equals(attribute.AttributeClass, hideModuleNameAttribute.Value)) - { - return true; - } - } - - return false; - } - - private static (bool isProhibited, bool isEditorBrowsableStateAdvanced) IsBrowsingProhibitedByEditorBrowsableAttribute( - ImmutableArray attributes, bool hideAdvancedMembers, Compilation compilation, Optional constructor) - { - if (!constructor.HasValue) - { - constructor = new(EditorBrowsableHelpers.GetSpecialEditorBrowsableAttributeConstructor(compilation)); - } - - if (constructor.Value == null) - { - return (isProhibited: false, isEditorBrowsableStateAdvanced: false); - } - - foreach (var attribute in attributes) - { - if (Equals(attribute.AttributeConstructor, constructor.Value) && - attribute.ConstructorArguments.Length == 1 && - attribute.ConstructorArguments.First().Value is int) - { -#nullable disable // Should use unboxed value from previous 'is int' https://github.com/dotnet/roslyn/issues/39166 - var state = (EditorBrowsableState)attribute.ConstructorArguments.First().Value; -#nullable enable - - if (EditorBrowsableState.Never == state) - { - return (isProhibited: true, isEditorBrowsableStateAdvanced: false); - } - - if (EditorBrowsableState.Advanced == state) - { - return (isProhibited: hideAdvancedMembers, isEditorBrowsableStateAdvanced: true); - } - } - } - - return (isProhibited: false, isEditorBrowsableStateAdvanced: false); - } - - private static bool IsBrowsingProhibitedByTypeLibTypeAttribute( - ImmutableArray attributes, Compilation compilation, ImmutableArray constructors) - { - return IsBrowsingProhibitedByTypeLibAttributeWorker( - attributes, - constructors.IsDefault ? EditorBrowsableHelpers.GetSpecialTypeLibTypeAttributeConstructors(compilation) : constructors, - TypeLibTypeFlagsFHidden); - } - - private static bool IsBrowsingProhibitedByTypeLibFuncAttribute( - ImmutableArray attributes, Compilation compilation, ImmutableArray constructors) - { - return IsBrowsingProhibitedByTypeLibAttributeWorker( - attributes, - constructors.IsDefault ? EditorBrowsableHelpers.GetSpecialTypeLibFuncAttributeConstructors(compilation) : constructors, - TypeLibFuncFlagsFHidden); - } - - private static bool IsBrowsingProhibitedByTypeLibVarAttribute( - ImmutableArray attributes, Compilation compilation, ImmutableArray constructors) - { - return IsBrowsingProhibitedByTypeLibAttributeWorker( - attributes, - constructors.IsDefault ? EditorBrowsableHelpers.GetSpecialTypeLibVarAttributeConstructors(compilation) : constructors, - TypeLibVarFlagsFHidden); - } - - private const int TypeLibTypeFlagsFHidden = 0x0010; - private const int TypeLibFuncFlagsFHidden = 0x0040; - private const int TypeLibVarFlagsFHidden = 0x0040; - - private static bool IsBrowsingProhibitedByTypeLibAttributeWorker( - ImmutableArray attributes, ImmutableArray attributeConstructors, int hiddenFlag) - { - foreach (var attribute in attributes) - { - if (attribute.ConstructorArguments.Length == 1) - { - foreach (var constructor in attributeConstructors) - { - if (Equals(attribute.AttributeConstructor, constructor)) - { - // Check for both constructor signatures. The constructor that takes a TypeLib*Flags reports an int argument. - var argumentValue = attribute.ConstructorArguments.First().Value; - - int actualFlags; - if (argumentValue is int i) - { - actualFlags = i; - } - else if (argumentValue is short sh) - { - actualFlags = sh; - } - else - { - continue; - } - - if ((actualFlags & hiddenFlag) == hiddenFlag) - { - return true; - } - } - } - } - } - - return false; - } - public static DocumentationComment GetDocumentationComment(this ISymbol symbol, Compilation compilation, CultureInfo? preferredCulture = null, bool expandIncludes = false, bool expandInheritdoc = false, CancellationToken cancellationToken = default) => GetDocumentationComment(symbol, visitedSymbols: null, compilation, preferredCulture, expandIncludes, expandInheritdoc, cancellationToken); @@ -662,67 +420,5 @@ private static void CopyAnnotations(XObject source, XObject target) private static bool ElementNameIs(XElement element, string name) => string.IsNullOrEmpty(element.Name.NamespaceName) && DocumentationCommentXmlNames.ElementEquals(element.Name.LocalName, name); - - /// - /// First, remove symbols from the set if they are overridden by other symbols in the set. - /// If a symbol is overridden only by symbols outside of the set, then it is not removed. - /// This is useful for filtering out symbols that cannot be accessed in a given context due - /// to the existence of overriding members. Second, remove remaining symbols that are - /// unsupported (e.g. pointer types in VB) or not editor browsable based on the EditorBrowsable - /// attribute. - /// - public static ImmutableArray FilterToVisibleAndBrowsableSymbols( - this ImmutableArray symbols, bool hideAdvancedMembers, Compilation compilation) where T : ISymbol - { - symbols = symbols.RemoveOverriddenSymbolsWithinSet(); - - // Since all symbols are from the same compilation, find the required attribute - // constructors once and reuse. - - var editorBrowsableAttributeConstructor = new Optional(EditorBrowsableHelpers.GetSpecialEditorBrowsableAttributeConstructor(compilation)); - var typeLibTypeAttributeConstructors = EditorBrowsableHelpers.GetSpecialTypeLibTypeAttributeConstructors(compilation); - var typeLibFuncAttributeConstructors = EditorBrowsableHelpers.GetSpecialTypeLibFuncAttributeConstructors(compilation); - var typeLibVarAttributeConstructors = EditorBrowsableHelpers.GetSpecialTypeLibVarAttributeConstructors(compilation); - var hideModuleNameAttribute = new Optional(compilation.HideModuleNameAttribute()); - - // PERF: HasUnsupportedMetadata may require recreating the syntax tree to get the base class, so first - // check to see if we're referencing a symbol defined in source. - static bool isSymbolDefinedInSource(Location l) => l.IsInSource; - return symbols.WhereAsArray((s, arg) => - (s.Locations.Any(isSymbolDefinedInSource) || !s.HasUnsupportedMetadata) && - !s.IsDestructor() && - s.IsEditorBrowsable( - arg.hideAdvancedMembers, - arg.compilation, - arg.editorBrowsableAttributeConstructor, - arg.typeLibTypeAttributeConstructors, - arg.typeLibFuncAttributeConstructors, - arg.typeLibVarAttributeConstructors, - arg.hideModuleNameAttribute), - (hideAdvancedMembers, compilation, editorBrowsableAttributeConstructor, typeLibTypeAttributeConstructors, typeLibFuncAttributeConstructors, typeLibVarAttributeConstructors, hideModuleNameAttribute)); - } - - private static ImmutableArray RemoveOverriddenSymbolsWithinSet(this ImmutableArray symbols) where T : ISymbol - { - var overriddenSymbols = new HashSet(); - - foreach (var symbol in symbols) - { - var overriddenMember = symbol.OverriddenMember(); - if (overriddenMember != null && !overriddenSymbols.Contains(overriddenMember)) - { - overriddenSymbols.Add(overriddenMember); - } - } - - return symbols.WhereAsArray(s => !overriddenSymbols.Contains(s)); - } - - public static ImmutableArray FilterToVisibleAndBrowsableSymbolsAndNotUnsafeSymbols( - this ImmutableArray symbols, bool hideAdvancedMembers, Compilation compilation) where T : ISymbol - { - return symbols.FilterToVisibleAndBrowsableSymbols(hideAdvancedMembers, compilation) - .WhereAsArray(s => !s.RequiresUnsafeModifier()); - } } } diff --git a/src/Workspaces/Core/Portable/Shared/Utilities/EditorBrowsableHelpers.cs b/src/Workspaces/Core/Portable/Shared/Utilities/EditorBrowsableHelpers.cs index 0764138d9ea..cbd87d89ca5 100644 --- a/src/Workspaces/Core/Portable/Shared/Utilities/EditorBrowsableHelpers.cs +++ b/src/Workspaces/Core/Portable/Shared/Utilities/EditorBrowsableHelpers.cs @@ -2,16 +2,302 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections.Generic; using System.Collections.Immutable; +using System.ComponentModel; using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; +#nullable enable + namespace Microsoft.CodeAnalysis.Shared.Utilities { internal static class EditorBrowsableHelpers { + public struct EditorBrowsableInfo + { + public Compilation Compilation { get; } + public INamedTypeSymbol? HideModuleNameAttribute { get; } + public IMethodSymbol? EditorBrowsableAttributeConstructor { get; } + public ImmutableArray TypeLibTypeAttributeConstructors { get; } + public ImmutableArray TypeLibFuncAttributeConstructors { get; } + public ImmutableArray TypeLibVarAttributeConstructors { get; } + public bool IsDefault => Compilation == null; + + public EditorBrowsableInfo(Compilation compilation) + { + Compilation = compilation; + HideModuleNameAttribute = compilation.HideModuleNameAttribute(); + EditorBrowsableAttributeConstructor = GetSpecialEditorBrowsableAttributeConstructor(compilation); + TypeLibTypeAttributeConstructors = GetSpecialTypeLibTypeAttributeConstructors(compilation); + TypeLibFuncAttributeConstructors = GetSpecialTypeLibFuncAttributeConstructors(compilation); + TypeLibVarAttributeConstructors = GetSpecialTypeLibVarAttributeConstructors(compilation); + } + } + + /// + /// Checks a given symbol for browsability based on its declaration location, attributes + /// explicitly limiting browsability, and whether showing of advanced members is enabled. + /// The optional editorBrowsableInfo parameters may be used to specify the symbols of the + /// constructors of the various browsability limiting attributes because finding these + /// repeatedly over a large list of symbols can be slow. If these are not provided, + /// they will be found in the compilation. + /// + public static bool IsEditorBrowsable( + this ISymbol symbol, + bool hideAdvancedMembers, + Compilation compilation, + EditorBrowsableInfo editorBrowsableInfo = default) + { + return IsEditorBrowsableWithState( + symbol, + hideAdvancedMembers, + compilation, + editorBrowsableInfo).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, + EditorBrowsableInfo editorBrowsableInfo = default) + { + // 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 (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 (isBrowsable: true, isEditorBrowsableStateAdvanced: false); + } + + if (editorBrowsableInfo.IsDefault) + { + editorBrowsableInfo = new EditorBrowsableInfo(compilation); + } + + // Ignore browsability limiting attributes if the symbol is declared in source. + // Check all locations since some of VB's embedded My symbols are declared in + // both source and the MyTemplateLocation. + if (symbol.Locations.All(loc => loc.IsInSource)) + { + // The HideModuleNameAttribute still applies to Modules defined in source + return (!IsBrowsingProhibitedByHideModuleNameAttribute(symbol, editorBrowsableInfo.HideModuleNameAttribute), isEditorBrowsableStateAdvanced: false); + } + + var (isProhibited, isEditorBrowsableStateAdvanced) = IsBrowsingProhibited(symbol, hideAdvancedMembers, editorBrowsableInfo); + + return (!isProhibited, isEditorBrowsableStateAdvanced); + } + + private static (bool isProhibited, bool isEditorBrowsableStateAdvanced) IsBrowsingProhibited( + ISymbol symbol, + bool hideAdvancedMembers, + EditorBrowsableInfo editorBrowsableInfo) + { + var attributes = symbol.GetAttributes(); + if (attributes.Length == 0) + { + return (isProhibited: false, isEditorBrowsableStateAdvanced: false); + } + + var (isProhibited, isEditorBrowsableStateAdvanced) = IsBrowsingProhibitedByEditorBrowsableAttribute(attributes, hideAdvancedMembers, editorBrowsableInfo.EditorBrowsableAttributeConstructor); + + return ((isProhibited + || IsBrowsingProhibitedByTypeLibTypeAttribute(attributes, editorBrowsableInfo.TypeLibTypeAttributeConstructors) + || IsBrowsingProhibitedByTypeLibFuncAttribute(attributes, editorBrowsableInfo.TypeLibFuncAttributeConstructors) + || IsBrowsingProhibitedByTypeLibVarAttribute(attributes, editorBrowsableInfo.TypeLibVarAttributeConstructors) + || IsBrowsingProhibitedByHideModuleNameAttribute(symbol, editorBrowsableInfo.HideModuleNameAttribute, attributes)), isEditorBrowsableStateAdvanced); + } + + private static bool IsBrowsingProhibitedByHideModuleNameAttribute( + ISymbol symbol, INamedTypeSymbol? hideModuleNameAttribute, ImmutableArray attributes = default) + { + if (hideModuleNameAttribute == null || !symbol.IsModuleType()) + { + return false; + } + + attributes = attributes.IsDefault ? symbol.GetAttributes() : attributes; + + foreach (var attribute in attributes) + { + if (Equals(attribute.AttributeClass, hideModuleNameAttribute)) + { + return true; + } + } + + return false; + } + + private static (bool isProhibited, bool isEditorBrowsableStateAdvanced) IsBrowsingProhibitedByEditorBrowsableAttribute( + ImmutableArray attributes, bool hideAdvancedMembers, IMethodSymbol? constructor) + { + if (constructor == null) + { + return (isProhibited: false, isEditorBrowsableStateAdvanced: false); + } + + foreach (var attribute in attributes) + { + if (Equals(attribute.AttributeConstructor, constructor) && + attribute.ConstructorArguments.Length == 1 && + attribute.ConstructorArguments.First().Value is int) + { +#nullable disable // Should use unboxed value from previous 'is int' https://github.com/dotnet/roslyn/issues/39166 + var state = (EditorBrowsableState)attribute.ConstructorArguments.First().Value; +#nullable enable + + if (EditorBrowsableState.Never == state) + { + return (isProhibited: true, isEditorBrowsableStateAdvanced: false); + } + + if (EditorBrowsableState.Advanced == state) + { + return (isProhibited: hideAdvancedMembers, isEditorBrowsableStateAdvanced: true); + } + } + } + + return (isProhibited: false, isEditorBrowsableStateAdvanced: false); + } + + private static bool IsBrowsingProhibitedByTypeLibTypeAttribute( + ImmutableArray attributes, ImmutableArray constructors) + { + return IsBrowsingProhibitedByTypeLibAttributeWorker( + attributes, + constructors, + TypeLibTypeFlagsFHidden); + } + + private static bool IsBrowsingProhibitedByTypeLibFuncAttribute( + ImmutableArray attributes, ImmutableArray constructors) + { + return IsBrowsingProhibitedByTypeLibAttributeWorker( + attributes, + constructors, + TypeLibFuncFlagsFHidden); + } + + private static bool IsBrowsingProhibitedByTypeLibVarAttribute( + ImmutableArray attributes, ImmutableArray constructors) + { + return IsBrowsingProhibitedByTypeLibAttributeWorker( + attributes, + constructors, + TypeLibVarFlagsFHidden); + } + + private const int TypeLibTypeFlagsFHidden = 0x0010; + private const int TypeLibFuncFlagsFHidden = 0x0040; + private const int TypeLibVarFlagsFHidden = 0x0040; + + private static bool IsBrowsingProhibitedByTypeLibAttributeWorker( + ImmutableArray attributes, ImmutableArray attributeConstructors, int hiddenFlag) + { + foreach (var attribute in attributes) + { + if (attribute.ConstructorArguments.Length == 1) + { + foreach (var constructor in attributeConstructors) + { + if (Equals(attribute.AttributeConstructor, constructor)) + { + // Check for both constructor signatures. The constructor that takes a TypeLib*Flags reports an int argument. + var argumentValue = attribute.ConstructorArguments.First().Value; + + int actualFlags; + if (argumentValue is int i) + { + actualFlags = i; + } + else if (argumentValue is short sh) + { + actualFlags = sh; + } + else + { + continue; + } + + if ((actualFlags & hiddenFlag) == hiddenFlag) + { + return true; + } + } + } + } + } + + return false; + } + + /// + /// First, remove symbols from the set if they are overridden by other symbols in the set. + /// If a symbol is overridden only by symbols outside of the set, then it is not removed. + /// This is useful for filtering out symbols that cannot be accessed in a given context due + /// to the existence of overriding members. Second, remove remaining symbols that are + /// unsupported (e.g. pointer types in VB) or not editor browsable based on the EditorBrowsable + /// attribute. + /// + public static ImmutableArray FilterToVisibleAndBrowsableSymbols( + this ImmutableArray symbols, bool hideAdvancedMembers, Compilation compilation) where T : ISymbol + { + symbols = symbols.RemoveOverriddenSymbolsWithinSet(); + + // Since all symbols are from the same compilation, find the required attribute + // constructors once and reuse. + var editorBrowsableInfo = new EditorBrowsableInfo(compilation); + + // PERF: HasUnsupportedMetadata may require recreating the syntax tree to get the base class, so first + // check to see if we're referencing a symbol defined in source. + static bool isSymbolDefinedInSource(Location l) => l.IsInSource; + return symbols.WhereAsArray((s, arg) => + (s.Locations.Any(isSymbolDefinedInSource) || !s.HasUnsupportedMetadata) && + !s.IsDestructor() && + s.IsEditorBrowsable( + arg.hideAdvancedMembers, + arg.editorBrowsableInfo.Compilation, + arg.editorBrowsableInfo), + (hideAdvancedMembers, editorBrowsableInfo)); + } + + public static ImmutableArray FilterToVisibleAndBrowsableSymbolsAndNotUnsafeSymbols( + this ImmutableArray symbols, bool hideAdvancedMembers, Compilation compilation) where T : ISymbol + { + return symbols.FilterToVisibleAndBrowsableSymbols(hideAdvancedMembers, compilation) + .WhereAsArray(s => !s.RequiresUnsafeModifier()); + } + + private static ImmutableArray RemoveOverriddenSymbolsWithinSet(this ImmutableArray symbols) where T : ISymbol + { + var overriddenSymbols = new HashSet(); + + foreach (var symbol in symbols) + { + var overriddenMember = symbol.OverriddenMember(); + if (overriddenMember != null && !overriddenSymbols.Contains(overriddenMember)) + { + overriddenSymbols.Add(overriddenMember); + } + } + + return symbols.WhereAsArray(s => !overriddenSymbols.Contains(s)); + } + /// /// Finds the constructor which takes exactly one argument, which must be of type EditorBrowsableState. /// It does not require that the EditorBrowsableAttribute and EditorBrowsableState types be those @@ -19,7 +305,7 @@ internal static class EditorBrowsableHelpers /// point that pattern appears to be violated, return null to indicate that an appropriate constructor /// could not be found. /// - public static IMethodSymbol GetSpecialEditorBrowsableAttributeConstructor(Compilation compilation) + public static IMethodSymbol? GetSpecialEditorBrowsableAttributeConstructor(Compilation compilation) { var editorBrowsableAttributeType = compilation.EditorBrowsableAttributeType(); var editorBrowsableStateType = compilation.EditorBrowsableStateType(); -- GitLab