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

Provide a way to efficiently get the list of global aliases in a compilation. #59088

Closed
CyrusNajmabadi opened this issue Jan 26, 2022 · 3 comments
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request
Milestone

Comments

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Jan 26, 2022

Background and Motivation

The IDE needs to know the set of global aliases in a compilation in order to address bugs like #59042

While it is possible for us to go to every file, and find/bind every using, this is quite expensive as it will require parsing all files in the project, and it scales linearly with that cost, as opposed to the cost only being relative the number of global-usings actually present.

Proposed API

I have two proposals. One highly symbolic, one syntactic.

The syntactic approach is to have something like the following:

public class CSharpCompilation
{
+    public bool ImmutableArray<SyntaxTree> GetTreesWithGlobalUsings();
}

This function can be powered by the internal decl table (which already has a bit on it stating if a file contains global usings).

Alternatively, we can go for a more symbolic approach like so:

public class CSharpCompilation
{
+    public bool ImmutableArray<INamespaceOrTypeSymbol> GlobalUsings(CancellationToken cancellationToken = default);
+    public bool ImmutableDictionary<string, INamespaceOrTypeSymbol> GlobalUsingAliases(CancellationToken cancellationToken = default);
}

Usage Examples

We currently have an analyzer that walks the user's code and sees if the name in source at least matches the name of some alias prior to do any expensive semantic work. This syntactic check is critical (it made this feature go from insanely expensive, to totally reasonable in terms of running time).

THat anlayzer already populates a set of names that the aliases map to for the usings in the file:

        private static ImmutableHashSet<string> GetAliasedNames(CompilationUnitSyntax? compilationUnit)
        {
            var aliasedNames = s_emptyAliasedNames;
            if (compilationUnit is null)
                return aliasedNames;

            foreach (var usingDirective in compilationUnit.Usings)
                AddAliasedName(usingDirective);

            foreach (var member in compilationUnit.Members)
            {
                if (member is BaseNamespaceDeclarationSyntax namespaceDeclaration)
                    AddAliasedNames(namespaceDeclaration);
            }

+            foreach (var tree in _compilation.GetTreesWithGlobalUsings()
+            {
+                var root = tree.GetRoot(cancellationToken);
+                AddAliasedNames(root, globalOnly: true);
+            }

            return aliasedNames;

            void AddAliasedName(UsingDirectiveSyntax usingDirective, bool globalOnly = false) ...

Risks

Low. We already have the information on the compiler side. this is just about lazily retrieving it for us to use.

@CyrusNajmabadi CyrusNajmabadi added Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request labels Jan 26, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 26, 2022
@CyrusNajmabadi CyrusNajmabadi added the api-ready-for-review API is ready for review, it is NOT ready for implementation label Jan 26, 2022
@333fred
Copy link
Member

333fred commented Feb 3, 2022

API Review

After some discussion, we decided we want to investigate a more symbol-based approach to see if it can answer all the questions the IDE has, and to not separate out global using as special. Instead, @CyrusNajmabadi will investigate whether a SemanticModel API that returns some type of hierarchical list of "here are all the using scopes at a given location", with each scope containing all the usings and alias symbols for the current location. If that will be sufficient for the IDE's purposes, we like it much better as an API than singling out global usings specially.

@333fred 333fred added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Feb 3, 2022
@jcouv jcouv added this to the Backlog milestone Mar 4, 2022
@jcouv jcouv removed the untriaged Issues and PRs which have not yet been triaged by a lead label Mar 4, 2022
@jcouv
Copy link
Member

jcouv commented Mar 4, 2022

Moved to Backlog milestone until the proposal is reviewed/approved.
@CyrusNajmabadi, are you the one driving this? If not, please re-assign.

@CyrusNajmabadi
Copy link
Member Author

Fixed with #59533

@sharwell sharwell modified the milestones: Backlog, 17.3 P2 Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request
Projects
None yet
Development

No branches or pull requests

4 participants