Skip to content

Commit

Permalink
Update FAR and Go to Def to work on indexers (#76220)
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi authored Dec 3, 2024
2 parents 417f332 + 0134be4 commit 2d7ee0d
Show file tree
Hide file tree
Showing 18 changed files with 146 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ await TryGetSurroundingNodeSpanAsync<MemberDeclarationSyntax>(renameDefinition.D
if (symbolService is not null)
{
var textSpan = inlineRenameInfo.TriggerSpan;
var semanticModel = await renameDefinition.Document.GetRequiredNullableDisabledSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var (symbol, _, _) = await symbolService.GetSymbolProjectAndBoundSpanAsync(
renameDefinition.Document, textSpan.Start, cancellationToken)
.ConfigureAwait(true);
renameDefinition.Document, semanticModel, textSpan.Start, cancellationToken).ConfigureAwait(true);
var docComment = symbol?.GetDocumentationCommentXml(expandIncludes: true, cancellationToken: cancellationToken);
if (!string.IsNullOrWhiteSpace(docComment))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,17 @@ internal abstract partial class AbstractDefinitionLocationService(

async ValueTask<DefinitionLocation?> GetDefinitionLocationWorkerAsync(Document document)
{
return await GetControlFlowTargetLocationAsync(document).ConfigureAwait(false) ??
await GetSymbolLocationAsync(document).ConfigureAwait(false);
// We don't need nullable information to compute the symbol. So avoid expensive work computing this.
var semanticModel = await document.GetRequiredNullableDisabledSemanticModelAsync(cancellationToken).ConfigureAwait(false);
return await GetControlFlowTargetLocationAsync(document, semanticModel).ConfigureAwait(false) ??
await GetSymbolLocationAsync(document, semanticModel).ConfigureAwait(false);
}

async ValueTask<DefinitionLocation?> GetControlFlowTargetLocationAsync(Document document)
async ValueTask<DefinitionLocation?> GetControlFlowTargetLocationAsync(
Document document, SemanticModel semanticModel)
{
var (controlFlowTarget, controlFlowSpan) = await symbolService.GetTargetIfControlFlowAsync(
document, position, cancellationToken).ConfigureAwait(false);
document, semanticModel, position, cancellationToken).ConfigureAwait(false);
if (controlFlowTarget == null)
return null;

Expand All @@ -66,11 +69,12 @@ internal abstract partial class AbstractDefinitionLocationService(
return location is null ? null : new DefinitionLocation(location, new DocumentSpan(document, controlFlowSpan));
}

async ValueTask<DefinitionLocation?> GetSymbolLocationAsync(Document document)
async ValueTask<DefinitionLocation?> GetSymbolLocationAsync(
Document document, SemanticModel semanticModel)
{
// Try to compute the referenced symbol and attempt to go to definition for the symbol.
var (symbol, project, span) = await symbolService.GetSymbolProjectAndBoundSpanAsync(
document, position, cancellationToken).ConfigureAwait(false);
document, semanticModel, position, cancellationToken).ConfigureAwait(false);
if (symbol is null)
return null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,32 @@ public class Test
}
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function

<WpfTheory, CombinatorialData>
<WorkItem("https://github.com/dotnet/roslyn/issues/31819")>
Public Async Function TestCSharp_Indexer_AtReferenceLocation(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferences="true">
<Document>
class C
{
public int {|Definition:this|}[int y] { get { } }
}

class D
{
void Goo()
{
var q = new C();
var b = q[||]$$[4];
}
}
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3819,6 +3819,50 @@ class Program
Await TestAsync(workspace)
End Function

<WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/69916")>
Public Async Function TestCSharpGoToDefinition_OnElementAccessExpression1() As Task
Dim workspace =
<Workspace>
<Project Language="C#">
<Document>
class C
{
int [||]this[int i] => i;

void M(C c)
{
var v = c$$[0];
}
}s
</Document>
</Project>
</Workspace>

Await TestAsync(workspace)
End Function

<WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/69916")>
Public Async Function TestCSharpGoToDefinition_OnElementAccessExpression2() As Task
Dim workspace =
<Workspace>
<Project Language="C#">
<Document>
class C
{
int [||]this[int i] => i;

void M(C c)
{
var v = c[0]$$;
}
}s
</Document>
</Project>
</Workspace>

Await TestAsync(workspace)
End Function

<WpfFact>
Public Async Function TestInterceptors_AttributeMissingVersion() As Task
Dim workspace =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
Imports Microsoft.CodeAnalysis.Remote.Testing

Namespace Microsoft.CodeAnalysis.Editor.UnitTests.ReferenceHighlighting
Public Class CSharpReferenceHighlightingTests
Public NotInheritable Class CSharpReferenceHighlightingTests
Inherits AbstractReferenceHighlightingTests

<WpfTheory, CombinatorialData>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,9 @@ protected override NullableFlowState GetNullabilityAnalysis(SemanticModel semant
}

var symbolService = document.GetRequiredLanguageService<IGoToDefinitionSymbolService>();
var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var (symbol, _, _) = await symbolService.GetSymbolProjectAndBoundSpanAsync(
document, position, cancellationToken).ConfigureAwait(false);
document, semanticModel, position, cancellationToken).ConfigureAwait(false);

// Don't show on-the-fly-docs for namespace symbols.
if (symbol is null || symbol.IsNamespace())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,9 @@ private static async Task<bool> TryFindLiteralReferencesAsync(
// bother with true/false/null as those are likely to have way too many results
// to be useful.
var token = await syntaxTree.GetTouchingTokenAsync(
semanticModel: null,
position,
t => syntaxFacts.IsNumericLiteral(t) ||
(_, t) => syntaxFacts.IsNumericLiteral(t) ||
syntaxFacts.IsCharacterLiteral(t) ||
syntaxFacts.IsStringLiteral(t),
cancellationToken).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@ internal abstract class AbstractGoToDefinitionSymbolService : IGoToDefinitionSym

protected abstract int? GetTargetPositionIfControlFlow(SemanticModel semanticModel, SyntaxToken token);

public async Task<(ISymbol? symbol, Project project, TextSpan boundSpan)> GetSymbolProjectAndBoundSpanAsync(Document document, int position, CancellationToken cancellationToken)
public async Task<(ISymbol? symbol, Project project, TextSpan boundSpan)> GetSymbolProjectAndBoundSpanAsync(
Document document, SemanticModel semanticModel, int position, CancellationToken cancellationToken)
{
var project = document.Project;
var services = document.Project.Solution.Services;

// We don't need nullable information to compute the symbol. So avoid expensive work computing this.
var semanticModel = await document.GetRequiredNullableDisabledSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var semanticInfo = await SymbolFinder.GetSemanticInfoAtPositionAsync(semanticModel, position, services, cancellationToken).ConfigureAwait(false);

// Prefer references to declarations. It's more likely that the user is attempting to
Expand Down Expand Up @@ -62,18 +61,17 @@ internal abstract class AbstractGoToDefinitionSymbolService : IGoToDefinitionSym
return (explicitlyDeclaredSymbol, project, semanticInfo.Span);
}

public async Task<(int? targetPosition, TextSpan tokenSpan)> GetTargetIfControlFlowAsync(Document document, int position, CancellationToken cancellationToken)
public async Task<(int? targetPosition, TextSpan tokenSpan)> GetTargetIfControlFlowAsync(
Document document, SemanticModel semanticModel, int position, CancellationToken cancellationToken)
{
var syntaxTree = await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
var syntaxTree = semanticModel.SyntaxTree;
var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
var token = await syntaxTree.GetTouchingTokenAsync(position, syntaxFacts.IsBindableToken, cancellationToken, findInsideTrivia: true).ConfigureAwait(false);
var token = await syntaxTree.GetTouchingTokenAsync(
semanticModel, position, syntaxFacts.IsBindableToken, cancellationToken, findInsideTrivia: true).ConfigureAwait(false);

if (token == default)
return default;

// We don't need nullable information to compute control flow targets. So avoid expensive work computing this.
var semanticModel = await document.GetRequiredNullableDisabledSemanticModelAsync(cancellationToken).ConfigureAwait(false);

return (GetTargetPositionIfControlFlow(semanticModel, token), token.Span);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ namespace Microsoft.CodeAnalysis.GoToDefinition;

internal interface IGoToDefinitionSymbolService : ILanguageService
{
Task<(ISymbol? symbol, Project project, TextSpan boundSpan)> GetSymbolProjectAndBoundSpanAsync(Document document, int position, CancellationToken cancellationToken);
Task<(ISymbol? symbol, Project project, TextSpan boundSpan)> GetSymbolProjectAndBoundSpanAsync(
Document document, SemanticModel semanticModel, int position, CancellationToken cancellationToken);

/// <summary>
/// If the position is on a control flow keyword (continue, break, yield, return , etc), returns the relevant position in the corresponding control flow statement.
/// Otherwise, returns null.
/// </summary>
Task<(int? targetPosition, TextSpan tokenSpan)> GetTargetIfControlFlowAsync(Document document, int position, CancellationToken cancellationToken);
Task<(int? targetPosition, TextSpan tokenSpan)> GetTargetIfControlFlowAsync(
Document document, SemanticModel semanticModel, int position, CancellationToken cancellationToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ await GetSymbolAsync(document.WithFrozenPartialSemantics(cancellationToken)).Con

async Task<(ISymbol symbol, Solution solution)?> GetSymbolAsync(Document document)
{
var (symbol, project, _) = await symbolService.GetSymbolProjectAndBoundSpanAsync(document, position, cancellationToken).ConfigureAwait(false);
// No need for NRT analysis here as it doesn't affect navigation.
var semanticModel = await document.GetRequiredNullableDisabledSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var (symbol, project, _) = await symbolService.GetSymbolProjectAndBoundSpanAsync(document, semanticModel, position, cancellationToken).ConfigureAwait(false);

var solution = project.Solution;

Expand Down
2 changes: 1 addition & 1 deletion src/Features/Lsif/Generator/Generator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ SymbolKind.RangeVariable or

ISymbol? referencedSymbol = null;

if (syntaxFactsService.IsBindableToken(syntaxToken))
if (syntaxFactsService.IsBindableToken(semanticModel, syntaxToken))
{
var bindableParent = syntaxFactsService.TryGetBindableParent(syntaxToken);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ Imports System.Threading
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.Editor.Shared.Utilities
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Utilities.GoToHelpers
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.GoToDefinition
Imports Microsoft.CodeAnalysis.Navigation
Imports Microsoft.CodeAnalysis.Test.Utilities
Expand Down
3 changes: 2 additions & 1 deletion src/Workspaces/Core/Portable/FindSymbols/SymbolFinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ private static Task<SyntaxToken> GetTokenAtPositionAsync(
var syntaxTree = semanticModel.SyntaxTree;
var syntaxFacts = services.GetRequiredLanguageService<ISyntaxFactsService>(semanticModel.Language);

return syntaxTree.GetTouchingTokenAsync(position, syntaxFacts.IsBindableToken, cancellationToken, findInsideTrivia: true);
return syntaxTree.GetTouchingTokenAsync(
semanticModel, position, syntaxFacts.IsBindableToken, cancellationToken, findInsideTrivia: true);
}

public static async Task<ISymbol> FindSymbolAtPositionAsync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ public static TokenSemanticInfo GetSemanticInfo(
var syntaxFacts = languageServices.GetRequiredService<ISyntaxFactsService>();
var syntaxKinds = languageServices.GetRequiredService<ISyntaxKindsService>();

if (!syntaxFacts.IsBindableToken(token))
if (!syntaxFacts.IsBindableToken(semanticModel, token))
return TokenSemanticInfo.Empty;

var semanticFacts = languageServices.GetRequiredService<ISemanticFactsService>();
var overriddingIdentifier = syntaxFacts.GetDeclarationIdentifierIfOverride(token);
var overridingIdentifier = syntaxFacts.GetDeclarationIdentifierIfOverride(token);

IAliasSymbol? aliasSymbol = null;
ITypeSymbol? type = null;
Expand All @@ -97,11 +97,11 @@ public static TokenSemanticInfo GetSemanticInfo(
var usingStatement = token.Parent;
declaredSymbol = semanticFacts.TryGetDisposeMethod(semanticModel, tokenParent, cancellationToken);
}
else if (overriddingIdentifier.HasValue)
else if (overridingIdentifier.HasValue)
{
// on an "override" token, we'll find the overridden symbol
var overriddingSymbol = semanticFacts.GetDeclaredSymbol(semanticModel, overriddingIdentifier.Value, cancellationToken);
var overriddenSymbol = overriddingSymbol.GetOverriddenMember(allowLooseMatch: true);
var overridingSymbol = semanticFacts.GetDeclaredSymbol(semanticModel, overridingIdentifier.Value, cancellationToken);
var overriddenSymbol = overridingSymbol.GetOverriddenMember(allowLooseMatch: true);

allSymbols = overriddenSymbol is null ? [] : [overriddenSymbol];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ public bool IsLiteral(SyntaxToken token)
public bool IsStringLiteralOrInterpolatedStringLiteral(SyntaxToken token)
=> token.Kind() is SyntaxKind.StringLiteralToken or SyntaxKind.InterpolatedStringTextToken;

public bool IsBindableToken(SyntaxToken token)
public bool IsBindableToken(SemanticModel? semanticModel, SyntaxToken token)
{
if (this.IsWord(token) || this.IsLiteral(token) || this.IsOperator(token))
{
Expand All @@ -525,14 +525,15 @@ public bool IsBindableToken(SyntaxToken token)

// In the order by clause a comma might be bound to ThenBy or ThenByDescending
if (token.Kind() == SyntaxKind.CommaToken && token.Parent.IsKind(SyntaxKind.OrderByClause))
{
return true;
}

if (token.Kind() is SyntaxKind.OpenBracketToken or SyntaxKind.CloseBracketToken
&& token.Parent.IsKind(SyntaxKind.CollectionExpression))
if (token.Kind() is SyntaxKind.OpenBracketToken or SyntaxKind.CloseBracketToken)
{
return true;
if (token.Parent.IsKind(SyntaxKind.CollectionExpression))
return true;

if (semanticModel != null && token.Parent is BracketedArgumentListSyntax { Parent: ElementAccessExpressionSyntax elementAccessExpression })
return semanticModel.GetSymbolInfo(elementAccessExpression).GetAnySymbol() != null;
}

return false;
Expand Down Expand Up @@ -1006,45 +1007,34 @@ private static TextSpan GetBlockBodySpan(BlockSyntax body)
{
var parent = node.Parent;

// If this node is on the left side of a member access expression, don't ascend
// further or we'll end up binding to something else.
if (parent is MemberAccessExpressionSyntax memberAccess)
{
if (memberAccess.Expression == node)
{
break;
}
}
// If this node is on the left side of a member access expression, don't ascend further or we'll end up
// binding to something else.
if (parent is MemberAccessExpressionSyntax memberAccess && memberAccess.Expression == node)
break;

// If this node is on the left side of a qualified name, don't ascend
// further or we'll end up binding to something else.
if (parent is QualifiedNameSyntax qualifiedName)
{
if (qualifiedName.Left == node)
{
break;
}
}
// If this node is on the left side of a qualified name, don't ascend further or we'll end up binding to
// something else.
if (parent is QualifiedNameSyntax qualifiedName && qualifiedName.Left == node)
break;

// If this node is on the left side of a alias-qualified name, don't ascend further or we'll end up binding
// to something else.
if (parent is AliasQualifiedNameSyntax aliasQualifiedName && aliasQualifiedName.Alias == node)
break;

// If this node is on the left side of a alias-qualified name, don't ascend
// further or we'll end up binding to something else.
if (parent is AliasQualifiedNameSyntax aliasQualifiedName)
// If this node is the type of an object creation expression, return the object creation expression.
if (parent is ObjectCreationExpressionSyntax objectCreation && objectCreation.Type == node)
{
if (aliasQualifiedName.Alias == node)
{
break;
}
node = parent;
break;
}

// If this node is the type of an object creation expression, return the
// object creation expression.
if (parent is ObjectCreationExpressionSyntax objectCreation)
// If we're on the argument list `[...]` of an index expression, return the index expression itself as we
// want to bind to whatever symbol that binds to.
if (parent is ElementAccessExpressionSyntax elementAccess && elementAccess.ArgumentList == node)
{
if (objectCreation.Type == node)
{
node = parent;
break;
}
node = parent;
break;
}

// The inside of an interpolated string is treated as its own token so we
Expand Down
Loading

0 comments on commit 2d7ee0d

Please sign in to comment.