[Proposal] GetAllTypesByMetadataName

[333fred]

Background and Motivation

We've had a number of requests over the years for a GetTypeByMetadataName that works differently. The IDE has a GetBestTypeByMetadataName extension method that works well for their purposes, and #52399 requested making it a public API on Compilation itself. However, this API is just a different heuristic for picking the "best" among all potential types exposed for a given metadata name in a compilation; it's one that works well for the IDE, but it's not a one-size-fits all algorithm. Rather than making another heuristic API and trying to determine if we made it extensible enough for any given use case, we instead propose an API that will get all types that match a given metadata name in a Compilation and all referenced assemblies: the caller can than look through those instances and decide which one it would like to use.

Proposed API

namespace Microsoft.CodeAnalysis
{
    public class Compilation
    {
+        public ImmutableArray<INamedTypeSymbol> GetTypesByMetadataName(string fullyQualifiedMetadataName);
    }
}

Usage Examples

// Fetching `System.Runtime.CompilerServices.IsExternalInit` from the current project, if it exists
var isExternalInit = compilation.GetTypesByMetadataName("System.Runtime.CompilerServices.IsExternalInit").FirstOrDefault();
if (ReferenceEquals(isExternalInit?.ContainingAssembly, compilation.Assembly) { ... }

// Reimplementing the IDE's GetBestTypeByMetadataName on top of this API
public static INamedTypeSymbol? GetBestTypeByMetadataName(this Compilation compilation, string fullyQualifiedMetadataName)
{
    INamedTypeSymbol type = null;

    foreach (var currentType in compilation.GetTypesByMetadataName(fullyQualifiedMetadataName));
    {
        switch (currentType.GetResultantVisibility())
        {
            case Utilities.SymbolVisibility.Public:
            case Utilities.SymbolVisibility.Internal when currentType.ContainingAssembly.GivesAccessTo(compilation.Assembly):
                break;

            default:
                continue;
        }

        if (ReferenceEquals(currentType.ContainingAssembly, compilation.ContainingAssembly))
        {
            return currentType;
        }

        if (type is not null)
        {
            // Multiple visible types with the same metadata name are present
            return null;
        }

        type = currentType;
    }

    return type;
}

Alternative Designs

I've opted to add a few extensions to try address what I view as the common use case for this API, but it's possible we don't really see those extensions being useful/needed, so we can choose to omit them if needed.

We could soften our "no heuristics" stance slightly by adding a default bool parameter includeInaccessibleTypes that would control whether the returned enumerable includes types that the match the requested FQN, but are not visible to the current Compilation (because they are internal without IVT/private). This could be a common enough heuristic that it would make sense to include in the API, but there's definitely a slippery slope of how far down this rabbit hole do we want to go.

Risks

Any new API adds risks, of course, but I believe the approach here leads to less risk for both the compiler and consumers, and the heuristics expected are clearly defined by the consumer's code.

[333fred]

@AlekseyTs would you take a look at this API and let me know your thoughts? I've marked it needs-work for now until we agree that it's ready to go to full design review.

[AlekseyTs]

would you take a look at this API and let me know your thoughts?

The proposal looks good to me. I don't particularly like the name of the structure, but we can discuss that during review. Also, I would use "GetTypesByMetadataName" as a name for the API.

[333fred]

Also, I would use "GetTypesByMetadataName" as a name for the API.

FWIW, I did consider this, but I felt it was too similar to GetTypeByMetadataName. Definitely something we can discuss during API review though, we'll try to get to it tomorrow.

[CyrusNajmabadi]

Definitely something we can discuss during API review though, we'll try to get to it tomorrow.

FWIW< that doesn't bother me. You'll get very distinct types back, so it should be readily apparent if you're doing something wrong. I would not like it if the signatures were the same (including return type) and tehre was a one letter difference like this :)

[CyrusNajmabadi]

I'd call MetadataNameEnumerable TypesWithMetadataName instead. I do appreciate the idea that you can use this efficiently without alloc.

[333fred]

API Review

We had a lengthy discussion about our philosophies in this area today. We generally think an API shaped similar to this is a good primitive to have for building heuristics, and there is some concern about adding heuristics to the compiler layer itself. We do, however, need to consider how we want to help users with common heuristics, like the equivalent of the compiler's GetWellKnownType internal API. Now that we've agreed on this API concept, we can look at specifics of the API itself in the next design meeting. Some initial comments:

• GetTypesByMetadataName seems to be the preferred naming
• MetadataNameEnumerable implies that we are enumerating metadata names. Perhaps TypesByMetadataNameEnumerable would be a better name.
• We might consider adding a filter delegate to the enumerable, but we will have to carefully consider how that works with arguments and potentially avoiding performance issues. It might be best to simply let perf scenarios filter with a foreach, and non-perf scenarios use LINQ.

[333fred]

I've updated the proposal here: in particular, I've removed the custom lazy enumerable MetadataNameEnumerable. This is because we know that GetTypeByMetadataName can be hit repeatedly in the same Compilation for the same type by analyzers today. If we make this method return a lazy enumerable, attempting to perform the same caching strategy is going to result in a very complex implementation that I don't think is worth it to maintain. We have no current indications that the performance of GetTypeByMetadataName is a significant problem, and since that API is already effectively doing the same implementation (it needs to search all referenced assemblies to determine if there are duplicates, even after the first match is found) I don't think the extra laziness complexity is worth any effort here.

Similarly, while we could add filtering overloads, we can't effectively cache their results, and thus the filtering methods would have to do one of two things:

1. Rerun the search with the filter. This doesn't really speed anything up, we have to look through all the referenced assemblies.
2. Call the unfiltered method to get the full results, and then filter the full results. The user can just do this on the other end.

[333fred]

API Review

API is approved. We should additionally update the docs on GetTypeByMetadataName to clarify the ambiguity scenarios, and look into an analyzer to warn when the result of GetTypeByMetadataName is unconditionally accessed.