API for exposing imports (including global imports) from the semantic model#59533
API for exposing imports (including global imports) from the semantic model#59533CyrusNajmabadi merged 67 commits intodotnet:mainfrom
Conversation
| /// <summary> | ||
| /// Represents a chain of symbols that are imported to a particular position in a source file. Symbols may be | ||
| /// imported, but may not necessarily be available at that location (for example, an alias symbol hidden by another | ||
| /// symbol). |
There was a problem hiding this comment.
note: i hope we keep the last part. or, if not, we have some really easy way of determining which import symbols should be stripped out because they're not available :)
| { | ||
| [DebuggerDisplay("{GetDebuggerDisplay(),nq}")] | ||
| internal sealed class ImportChain : Cci.IImportScope | ||
| internal sealed class ImportChain : Cci.IImportScope, IImportChain |
There was a problem hiding this comment.
that c# already had this 'ImportChain' type was a lot of the inspiration of this this approach. It has already stitched together the entire chain, so why not just use it?
| ''' </summary> | ||
| Friend Class ImportAliasesBinder | ||
| Inherits Binder | ||
| Implements IImportChain |
There was a problem hiding this comment.
in VB things are a bit different. the binder is itself the link in the chain. So two of the binders then expose the relevant information. Specifically ImportAliasesBinder and TypesOfImportedNamespacesMembersBinder. I got thes two binders from CreateBinderForSourceFile which showed me how you guys stitch up that chain yourself.
There was a problem hiding this comment.
not sure how acceptable/appropriate this approach is for you two.
|
API shape itself seems perfectly reasonable to me, and was about what I was expecting to see. |
|
@333fred Cool! Any concern about the impl side? I'm not an expert on binders and i wasn't sure if this was a reasonable way to go. |
Exposing binders directly like this definitely gives me a bit of pause. I'd want to think on it a bit. |
src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs
Outdated
Show resolved
Hide resolved
src/Compilers/VisualBasic/Portable/Binding/ImportAliasesBinder.vb
Outdated
Show resolved
Hide resolved
src/Compilers/VisualBasic/Portable/Binding/ImportAliasesBinder.vb
Outdated
Show resolved
Hide resolved
src/Compilers/VisualBasic/Portable/Binding/ImportAliasesBinder.vb
Outdated
Show resolved
Hide resolved
src/Compilers/VisualBasic/Portable/Binding/TypesOfImportedNamespacesMembersBinder.vb
Outdated
Show resolved
Hide resolved
|
Done with review pass (commit 2) |
src/Compilers/VisualBasic/Portable/Compilation/SemanticModel.vb
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs
Outdated
Show resolved
Hide resolved
jcouv
left a comment
There was a problem hiding this comment.
LGTM Thanks (iteration 62). Please make sure to squash (after second sign-off). Much appreciate
|
@AlekseyTs for eyes. thanks :) |
| End Sub | ||
|
|
||
| Public Function GetImportChainData() As ImmutableArray(Of IAliasSymbol) | ||
| Return ImmutableArray(Of IAliasSymbol).CastUp(_importedAliases.SelectAsArray(Function(kvp) kvp.Value.Alias)) |
There was a problem hiding this comment.
Currently, that's intentional. I've explicitly stated in the docs that order is not guaranteed. Do you think that's sufficient/ or would you prefer we stated some guarantee here and then coded that in?
There was a problem hiding this comment.
Currently, that's intentional. I've explicitly stated in the docs that order is not guaranteed. Do you think that's sufficient/ or would you prefer we stated some guarantee here and then coded that in?
In general we are trying to avoid behavior like that in a public API. Even if we do not document any particular order, we are trying to avoid "randomization" for the same scenario, which is likely to happen in this case. I think we don't need to block this PR, but let's bring this issue for discussion at the API review meeting.
|
Done with review pass (commit 62) |
AlekseyTs
left a comment
There was a problem hiding this comment.
LGTM (commit 62) with a follow-up on the ordering issue.
|
@AlekseyTs @jcouv if you'd like to take a look after the latest changes. This applies the outcome of the api review that it's ok that things not be ordered. Note: the docs still say that things are not ordered. Would you like me to remove those docs and just provide no statement on it? Or should we leave the docs in? Thanks! |
|
Thanks all! Set to squash and merge. |
Adding a new API to provide the set of imported symbols at a particular location in a code file.
The API shape is:
public class SemanticModel { + public ImmutableArray<IImportScope> GetImportScopes(int position, CancellationToken cancellationToken = default); } + /// <summary> + /// Represents a chain of symbols that are imported to a particular position in a source file. Symbols may be + /// imported, but may not necessarily be available at that location (for example, an alias symbol hidden by another + /// symbol). There is no guarantee that the same chain will be returned from successive calls to <see + /// cref="SemanticModel.GetImportScopes"/>. Each import has a reference to the location the import directive was + /// declared at. For the <see cref="IAliasSymbol"/> import, the location can be found using either <see + /// cref="ISymbol.Locations"/> or <see cref="ISymbol.DeclaringSyntaxReferences"/> on the <see cref="IAliasSymbol"/> + /// itself. For <see cref="Imports"/> or <see cref="XmlNamespaces"/> the location is found through <see + /// cref="ImportedNamespaceOrType.DeclaringSyntaxReference"/> or <see + /// cref="ImportedXmlNamespace.DeclaringSyntaxReference"/> respectively. + /// </summary> + /// <remarks> + /// <list type="bullet"> + /// Scopes returned will always have at least one non-empty property value in them. + /// <item> + /// In C# there will be an <see cref="IImportScope"/> for every containing namespace-declarations that include any + /// import directives. There will also be an <see cref="IImportScope"/> for the containing compilation-unit if it + /// includes any import directives or if there are global import directives pulled in from other files. + /// </item> + /// <item> + /// In Visual Basic there will only be at most two <see cref="IImportScope"/>s returned for any position. There + /// will be a scope for the containing compilation unit if it includes any import directives. There can also be a + /// scope representing any imports specified at the project level. + /// </item> + /// <item> + /// Elements of any property have no defined order. Even if they represent items from a single document, they are + /// not guaranteed to be returned in any specific file-oriented order. + /// </item> + /// </list> + /// </remarks> + public interface IImportScope + { + /// <summary> + /// Aliases defined at this level of the chain. This corresponds to <c>using X = TypeOrNamespace;</c> in C# or + /// <c>Imports X = TypeOrNamespace</c> in Visual Basic. + /// </summary> + ImmutableArray<IAliasSymbol> Aliases { get; } + + /// <summary> + /// Extern aliases defined at this level of the chain. This corresponds to <c>extern alias X;</c> in C#. It + /// will be empty in Visual Basic. + /// </summary> + ImmutableArray<IAliasSymbol> ExternAliases { get; } + + /// <summary> + /// Types or namespaces imported at this level of the chain. This corresponds to <c>using Namespace;</c> or + /// <c>using static Type;</c> in C#, or <c>Imports TypeOrNamespace</c> in Visual Basic. + /// </summary> + ImmutableArray<ImportedNamespaceOrType> Imports { get; } + + /// <summary> + /// Xml namespaces imported at this level of the chain. This corresponds to <c>Imports <xmlns:prefix = + /// "name"></c> in Visual Basic. It will be empty in C#. + /// </summary> + ImmutableArray<ImportedXmlNamespace> XmlNamespaces { get; } + } + + /// <summary> + /// Represents an <see cref="INamespaceOrTypeSymbol"/> that has been imported, and the location the import was + /// declared at. This corresponds to <c>using Namespace;</c> or <c>using static Type;</c> in C#, or <c>Imports + /// TypeOrNamespace</c> in Visual Basic. + /// </summary> + public readonly struct ImportedNamespaceOrType + { + public INamespaceOrTypeSymbol NamespaceOrType { get; } + public SyntaxReference? DeclaringSyntaxReference { get; } + + internal ImportedNamespaceOrType(INamespaceOrTypeSymbol namespaceOrType, SyntaxReference? declaringSyntaxReference) + { + NamespaceOrType = namespaceOrType; + DeclaringSyntaxReference = declaringSyntaxReference; + } + } + + /// <summary> + /// Represents an imported xml namespace name. This corresponds to <c>Imports <xmlns:prefix = "name"></c> in + /// Visual Basic. It does not exist for C#. + /// </summary> + public readonly struct ImportedXmlNamespace + { + public string XmlNamespace { get; } + public SyntaxReference? DeclaringSyntaxReference { get; } + + internal ImportedXmlNamespace(string xmlNamespace, SyntaxReference? declaringSyntaxReference) + { + XmlNamespace = xmlNamespace; + DeclaringSyntaxReference = declaringSyntaxReference; + } + }TODO: