Skip to content

Commit

Permalink
Merge pull request #54057 from CyrusNajmabadi/declaredInfos
Browse files Browse the repository at this point in the history 
Share more data in the SyntaxTreeIndex
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi authored Jun 15, 2021
2 parents f31d8b6 + 57c7964 commit a34b3be
Show file tree
Hide file tree
Showing 10 changed files with 536 additions and 371 deletions.
11 changes: 11 additions & 0 deletions src/EditorFeatures/VisualBasicTest/NavigateTo/NavigateToTests.vb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,17 @@ End Class", Async Function(w)
End Function)
End Function

<Theory>
<CombinatorialData>
Public Async Function TestFindAbstractMethod(testHost As TestHost, composition As Composition) As Task
Await TestAsync(testHost, composition, "MustInherit Class A
Public MustOverride Sub M()
End Class", Async Function(w)
Dim item = (Await _aggregator.GetItemsAsync("M")).Single
VerifyNavigateToResultItem(item, "M", "[|M|]()", PatternMatchKind.Exact, NavigateToItemKind.Method, Glyph.MethodPublic, additionalInfo:=String.Format(FeaturesResources.in_0_project_1, "A", "Test"))
End Function)
End Function

<WorkItem(1111131, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1111131")>
<Theory>
<CombinatorialData>
Expand Down
2 changes: 1 addition & 1 deletion ...tures/Core/Portable/Completion/Providers/ImportCompletionProvider/ImportCompletionItem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public static CompletionItem Create(
{
// We don't need arity to recover symbol if we already have SymbolKeyData or it's 0.
// (but it still needed below to decide whether to show generic suffix)
builder.Add(TypeAritySuffixName, AbstractDeclaredSymbolInfoFactoryService.GetMetadataAritySuffix(arity));
builder.Add(TypeAritySuffixName, ArityUtilities.GetMetadataAritySuffix(arity));
}

properties = builder.ToImmutableDictionaryAndFree();
Expand Down
203 changes: 115 additions & 88 deletions src/Workspaces/CSharp/Portable/FindSymbols/CSharpDeclaredSymbolInfoFactoryService.cs
View file

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ private SyntaxTreeIndex(

public static async Task PrecalculateAsync(Document document, CancellationToken cancellationToken)
{
if (!document.SupportsSyntaxTree)
return;

using (Logger.LogBlock(FunctionId.SyntaxTreeIndex_Precalculate, cancellationToken))
{
Debug.Assert(document.IsFromPrimaryBranch());
Expand Down Expand Up @@ -82,6 +85,9 @@ public static async ValueTask<SyntaxTreeIndex> GetRequiredIndexAsync(Document do
bool loadOnly,
CancellationToken cancellationToken)
{
if (!document.SupportsSyntaxTree)
return null;

// See if we already cached an index with this direct document index. If so we can just
// return it with no additional work.
if (!s_documentToIndex.TryGetValue(document, out var index))
Expand All @@ -108,6 +114,9 @@ public static async ValueTask<SyntaxTreeIndex> GetRequiredIndexAsync(Document do
bool loadOnly,
CancellationToken cancellationToken)
{
if (!document.SupportsSyntaxTree)
return null;

var checksum = await GetChecksumAsync(document, cancellationToken).ConfigureAwait(false);

// Check if we have an index for a previous version of this document. If our
Expand Down
166 changes: 25 additions & 141 deletions src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex_Create.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,13 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.PooledObjects;
Expand All @@ -25,15 +22,7 @@ internal interface IDeclaredSymbolInfoFactoryService : ILanguageService
{
// `rootNamespace` is required for VB projects that has non-global namespace as root namespace,
// otherwise we would not be able to get correct data from syntax.
bool TryGetDeclaredSymbolInfo(StringTable stringTable, SyntaxNode node, string rootNamespace, out DeclaredSymbolInfo declaredSymbolInfo);

// Get the name of the receiver type of specified extension method declaration node.
// The returned value would be "" or "[]" for complex types.
string GetReceiverTypeName(SyntaxNode node);

bool TryGetAliasesFromUsingDirective(SyntaxNode node, out ImmutableArray<(string aliasName, string name)> aliases);

string GetRootNamespace(CompilationOptions compilationOptions);
void AddDeclaredSymbolInfos(Document document, SyntaxNode root, ArrayBuilder<DeclaredSymbolInfo> declaredSymbolInfos, Dictionary<string, ArrayBuilder<int>> extensionMethodInfo, CancellationToken cancellationToken);
}

internal sealed partial class SyntaxTreeIndex
Expand All @@ -53,28 +42,32 @@ internal sealed partial class SyntaxTreeIndex
/// this string table. The table will have already served its purpose at that point and
/// doesn't need to be kept around further.
/// </summary>
private static readonly ConditionalWeakTable<Project, StringTable> s_projectStringTable =
new();
private static readonly ConditionalWeakTable<Project, StringTable> s_projectStringTable = new();

private static async Task<SyntaxTreeIndex> CreateIndexAsync(
Document document, Checksum checksum, CancellationToken cancellationToken)
{
var project = document.Project;
var stringTable = GetStringTable(project);
Contract.ThrowIfFalse(document.SupportsSyntaxTree);

var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
return CreateIndex(document, root, checksum, cancellationToken);
}

var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
var infoFactory = document.GetLanguageService<IDeclaredSymbolInfoFactoryService>();
var ignoreCase = syntaxFacts != null && !syntaxFacts.IsCaseSensitive;
private static SyntaxTreeIndex CreateIndex(
Document document, SyntaxNode root, Checksum checksum, CancellationToken cancellationToken)
{
var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
var infoFactory = document.GetRequiredLanguageService<IDeclaredSymbolInfoFactoryService>();
var ignoreCase = !syntaxFacts.IsCaseSensitive;
var isCaseSensitive = !ignoreCase;

GetIdentifierSet(ignoreCase, out var identifiers, out var escapedIdentifiers);

var stringLiterals = StringLiteralHashSetPool.Allocate();
var longLiterals = LongLiteralHashSetPool.Allocate();

var declaredSymbolInfos = ArrayBuilder<DeclaredSymbolInfo>.GetInstance();
var extensionMethodInfoBuilder = PooledDictionary<string, ArrayBuilder<int>>.GetInstance();
using var _ = PooledDictionary<string, string>.GetInstance(out var usingAliases);
using var _1 = ArrayBuilder<DeclaredSymbolInfo>.GetInstance(out var declaredSymbolInfos);
using var _2 = PooledDictionary<string, ArrayBuilder<int>>.GetInstance(out var extensionMethodInfo);

try
{
Expand All @@ -98,14 +91,11 @@ private static async Task<SyntaxTreeIndex> CreateIndexAsync(

if (syntaxFacts != null)
{
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var rootNamespace = infoFactory.GetRootNamespace(project.CompilationOptions);

foreach (var current in root.DescendantNodesAndTokensAndSelf(descendIntoTrivia: true))
{
if (current.IsNode)
{
var node = (SyntaxNode)current;
var node = current.AsNode();

containsForEachStatement = containsForEachStatement || syntaxFacts.IsForEachStatement(node);
containsLockStatement = containsLockStatement || syntaxFacts.IsLockStatement(node);
Expand All @@ -123,69 +113,6 @@ private static async Task<SyntaxTreeIndex> CreateIndexAsync(
containsImplicitObjectCreation = containsImplicitObjectCreation || syntaxFacts.IsImplicitObjectCreationExpression(node);
containsGlobalAttributes = containsGlobalAttributes || syntaxFacts.IsGlobalAttribute(node);
containsConversion = containsConversion || syntaxFacts.IsConversionExpression(node);

if (syntaxFacts.IsUsingAliasDirective(node) && infoFactory.TryGetAliasesFromUsingDirective(node, out var aliases))
{
foreach (var (aliasName, name) in aliases)
{
// In C#, it's valid to declare two alias with identical name,
// as long as they are in different containers.
//
// e.g.
// using X = System.String;
// namespace N
// {
// using X = System.Int32;
// }
//
// If we detect this, we will simply treat extension methods whose
// target type is this alias as complex method.
if (usingAliases.ContainsKey(aliasName))
{
usingAliases[aliasName] = null;
}
else
{
usingAliases[aliasName] = name;
}
}
}

// We've received a number of error reports where DeclaredSymbolInfo.GetSymbolAsync() will
// crash because the document's syntax root doesn't contain the span of the node returned
// by TryGetDeclaredSymbolInfo(). There are two possibilities for this crash:
// 1) syntaxFacts.TryGetDeclaredSymbolInfo() is returning a bad span, or
// 2) Document.GetSyntaxRootAsync() (called from DeclaredSymbolInfo.GetSymbolAsync) is
// returning a bad syntax root that doesn't represent the original parsed document.
// By adding the `root.FullSpan.Contains()` check below, if we get similar crash reports in
// the future then we know the problem lies in (2). If, however, the problem is really in
// TryGetDeclaredSymbolInfo, then this will at least prevent us from returning bad spans
// and will prevent the crash from occurring.
if (infoFactory.TryGetDeclaredSymbolInfo(stringTable, node, rootNamespace, out var declaredSymbolInfo))
{
if (root.FullSpan.Contains(declaredSymbolInfo.Span))
{
var declaredSymbolInfoIndex = declaredSymbolInfos.Count;
declaredSymbolInfos.Add(declaredSymbolInfo);

AddExtensionMethodInfo(
infoFactory,
node,
usingAliases,
declaredSymbolInfoIndex,
declaredSymbolInfo,
extensionMethodInfoBuilder);
}
else
{
var message =
$@"Invalid span in {nameof(declaredSymbolInfo)}.
{nameof(declaredSymbolInfo.Span)} = {declaredSymbolInfo.Span}
{nameof(root.FullSpan)} = {root.FullSpan}";

FatalError.ReportAndCatch(new InvalidOperationException(message));
}
}
}
else
{
Expand Down Expand Up @@ -223,7 +150,7 @@ private static async Task<SyntaxTreeIndex> CreateIndexAsync(

if (syntaxFacts.IsCharacterLiteral(token))
{
longLiterals.Add((char)token.Value);
longLiterals.Add((char)token.Value!);
}

if (syntaxFacts.IsNumericLiteral(token))
Expand All @@ -247,6 +174,9 @@ private static async Task<SyntaxTreeIndex> CreateIndexAsync(
}
}
}

infoFactory.AddDeclaredSymbolInfos(
document, root, declaredSymbolInfos, extensionMethodInfo, cancellationToken);
}

return new SyntaxTreeIndex(
Expand All @@ -273,10 +203,9 @@ private static async Task<SyntaxTreeIndex> CreateIndexAsync(
containsImplicitObjectCreation,
containsGlobalAttributes,
containsConversion),
new DeclarationInfo(
declaredSymbolInfos.ToImmutable()),
new DeclarationInfo(declaredSymbolInfos.ToImmutable()),
new ExtensionMethodInfo(
extensionMethodInfoBuilder.ToImmutableDictionary(
extensionMethodInfo.ToImmutableDictionary(
static kvp => kvp.Key,
static kvp => kvp.Value.ToImmutable())));
}
Expand All @@ -286,58 +215,13 @@ private static async Task<SyntaxTreeIndex> CreateIndexAsync(
StringLiteralHashSetPool.ClearAndFree(stringLiterals);
LongLiteralHashSetPool.ClearAndFree(longLiterals);

foreach (var (_, builder) in extensionMethodInfoBuilder)
{
foreach (var (_, builder) in extensionMethodInfo)
builder.Free();
}

extensionMethodInfoBuilder.Free();
declaredSymbolInfos.Free();
}
}

private static void AddExtensionMethodInfo(
IDeclaredSymbolInfoFactoryService infoFactory,
SyntaxNode node,
PooledDictionary<string, string> aliases,
int declaredSymbolInfoIndex,
DeclaredSymbolInfo declaredSymbolInfo,
PooledDictionary<string, ArrayBuilder<int>> extensionMethodsInfoBuilder)
{
if (declaredSymbolInfo.Kind != DeclaredSymbolInfoKind.ExtensionMethod)
{
return;
}

var receiverTypeName = infoFactory.GetReceiverTypeName(node);

// Target type is an alias
if (aliases.TryGetValue(receiverTypeName, out var originalName))
{
// it is an alias of multiple with identical name,
// simply treat it as a complex method.
if (originalName == null)
{
receiverTypeName = Extensions.ComplexReceiverTypeName;
}
else
{
// replace the alias with its original name.
receiverTypeName = originalName;
}
}

if (!extensionMethodsInfoBuilder.TryGetValue(receiverTypeName, out var arrayBuilder))
{
arrayBuilder = ArrayBuilder<int>.GetInstance();
extensionMethodsInfoBuilder[receiverTypeName] = arrayBuilder;
}

arrayBuilder.Add(declaredSymbolInfoIndex);
}

private static StringTable GetStringTable(Project project)
=> s_projectStringTable.GetValue(project, _ => StringTable.GetInstance());
public static StringTable GetStringTable(Project project)
=> s_projectStringTable.GetValue(project, static _ => StringTable.GetInstance());

private static void GetIdentifierSet(bool ignoreCase, out HashSet<string> identifiers, out HashSet<string> escapedIdentifiers)
{
Expand Down
Loading

0 comments on commit a34b3be

Please sign in to comment.