Skip to content

Commit 34bcb6b

Browse files
davidwengierdavidwengier
authored
Improve go to definition for mvc tag helpers (dotnet#12216)
Fixes: dotnet#12210 Fixes: dotnet#12211 We now support this: ![14f84abe-59cc-4280-9813-61da95cfc45a](https://github.com/user-attachments/assets/fa4323a9-5217-4674-a20c-49a7309129e8)
2 parents f3543e7 + 3ce38bd commit 34bcb6b

File tree

10 files changed

+331
-68
lines changed

10 files changed

+331
-68
lines changed

‎src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Definition/DefinitionEndpoint.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,12 @@ public void ApplyCapabilities(VSInternalServerCapabilities serverCapabilities, V
6868
}
6969

7070
// If single server support is on, then we ignore attributes, as they are better handled by delegating to Roslyn
71-
return await _definitionService
71+
var results = await _definitionService
7272
.GetDefinitionAsync(documentContext.Snapshot, positionInfo, _projectManager.GetQueryOperations(), ignoreComponentAttributes: SingleServerSupport, includeMvcTagHelpers: false, cancellationToken)
7373
.ConfigureAwait(false);
74+
75+
// We know there will only be one result, because without tag helper support there can't be anything else
76+
return results?.FirstOrDefault();
7477
}
7578

7679
protected override Task<IDelegatedParams?> CreateDelegatedParamsAsync(

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/GoToDefinition/AbstractDefinitionService.cs‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ internal abstract class AbstractDefinitionService(
2424
private readonly IDocumentMappingService _documentMappingService = documentMappingService;
2525
private readonly ILogger _logger = logger;
2626

27-
public async Task<LspLocation?> GetDefinitionAsync(
27+
public async Task<LspLocation[]?> GetDefinitionAsync(
2828
IDocumentSnapshot documentSnapshot,
2929
DocumentPositionInfo positionInfo,
3030
ISolutionQueryOperations solutionQueryOperations,
@@ -47,7 +47,7 @@ internal abstract class AbstractDefinitionService(
4747

4848
var codeDocument = await documentSnapshot.GetGeneratedOutputAsync(cancellationToken).ConfigureAwait(false);
4949

50-
if (!RazorComponentDefinitionHelpers.TryGetBoundTagHelpers(codeDocument, positionInfo.HostDocumentIndex, ignoreComponentAttributes, _logger, out var boundTagHelper, out var boundAttribute))
50+
if (!RazorComponentDefinitionHelpers.TryGetBoundTagHelpers(codeDocument, positionInfo.HostDocumentIndex, ignoreComponentAttributes, _logger, out var boundTagHelperResults))
5151
{
5252
_logger.LogInformation($"Could not retrieve bound tag helper information.");
5353
return null;
@@ -57,13 +57,16 @@ internal abstract class AbstractDefinitionService(
5757
{
5858
Debug.Assert(_tagHelperSearchEngine is not null, "If includeMvcTagHelpers is true, _tagHelperSearchEngine must not be null.");
5959

60-
var tagHelperLocation = await _tagHelperSearchEngine.TryLocateTagHelperDefinitionAsync(boundTagHelper, boundAttribute, documentSnapshot, solutionQueryOperations, cancellationToken).ConfigureAwait(false);
61-
if (tagHelperLocation is not null)
60+
var tagHelperLocations = await _tagHelperSearchEngine.TryLocateTagHelperDefinitionsAsync(boundTagHelperResults, documentSnapshot, solutionQueryOperations, cancellationToken).ConfigureAwait(false);
61+
if (tagHelperLocations is { Length: > 0 })
6262
{
63-
return tagHelperLocation;
63+
return tagHelperLocations;
6464
}
6565
}
6666

67+
// For Razor components, there can only ever be one tag helper result
68+
var (boundTagHelper, boundAttribute) = boundTagHelperResults[0];
69+
6770
var componentDocument = await _componentSearchEngine
6871
.TryLocateComponentAsync(boundTagHelper, solutionQueryOperations, cancellationToken)
6972
.ConfigureAwait(false);
@@ -80,7 +83,7 @@ internal abstract class AbstractDefinitionService(
8083

8184
var range = await GetNavigateRangeAsync(componentDocument, boundAttribute, cancellationToken).ConfigureAwait(false);
8285

83-
return LspFactory.CreateLocation(componentFilePath, range);
86+
return [LspFactory.CreateLocation(componentFilePath, range)];
8487
}
8588

8689
private async Task<LspRange> GetNavigateRangeAsync(IDocumentSnapshot documentSnapshot, BoundAttributeDescriptor? attributeDescriptor, CancellationToken cancellationToken)

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/GoToDefinition/IDefinitionService.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace Microsoft.CodeAnalysis.Razor.GoToDefinition;
1313
/// </summary>
1414
internal interface IDefinitionService
1515
{
16-
Task<LspLocation?> GetDefinitionAsync(
16+
Task<LspLocation[]?> GetDefinitionAsync(
1717
IDocumentSnapshot documentSnapshot,
1818
DocumentPositionInfo positionInfo,
1919
ISolutionQueryOperations solutionQueryOperations,

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/GoToDefinition/RazorComponentDefinitionHelpers.cs‎

Lines changed: 53 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,15 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
5+
using System.Collections.Generic;
6+
using System.Collections.Immutable;
57
using System.Diagnostics.CodeAnalysis;
68
using System.Linq;
79
using System.Threading;
810
using System.Threading.Tasks;
911
using Microsoft.AspNetCore.Razor.Language;
1012
using Microsoft.AspNetCore.Razor.Language.Syntax;
13+
using Microsoft.AspNetCore.Razor.PooledObjects;
1114
using Microsoft.CodeAnalysis.CSharp.Syntax;
1215
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
1316
using Microsoft.CodeAnalysis.Razor.Logging;
@@ -19,15 +22,15 @@
1922

2023
namespace Microsoft.CodeAnalysis.Razor.GoToDefinition;
2124

25+
internal sealed record BoundTagHelperResult(TagHelperDescriptor ElementDescriptor, BoundAttributeDescriptor? AttributeDescriptor);
26+
2227
internal static class RazorComponentDefinitionHelpers
2328
{
2429
public static bool TryGetBoundTagHelpers(
2530
RazorCodeDocument codeDocument, int absoluteIndex, bool ignoreComponentAttributes, ILogger logger,
26-
[NotNullWhen(true)] out TagHelperDescriptor? boundTagHelper,
27-
[MaybeNullWhen(true)] out BoundAttributeDescriptor? boundAttribute)
31+
out ImmutableArray<BoundTagHelperResult> descriptors)
2832
{
29-
boundTagHelper = null;
30-
boundAttribute = null;
33+
descriptors = default;
3134

3235
var root = codeDocument.GetRequiredSyntaxRoot();
3336

@@ -66,47 +69,63 @@ public static bool TryGetBoundTagHelpers(
6669
return false;
6770
}
6871

69-
boundTagHelper = binding.Descriptors.FirstOrDefault(static d => !d.IsAttributeDescriptor());
70-
if (boundTagHelper is null)
71-
{
72-
logger.LogInformation($"Could not locate bound TagHelperDescriptor.");
73-
return false;
74-
}
72+
using var descriptorsBuilder = new PooledArrayBuilder<BoundTagHelperResult>();
7573

76-
if ((!ignoreComponentAttributes || boundTagHelper.Kind != TagHelperKind.Component) &&
77-
tagHelperNode is MarkupTagHelperStartTagSyntax startTag)
74+
foreach (var boundTagHelper in binding.Descriptors.Where(d => !d.IsAttributeDescriptor()))
7875
{
79-
// Include attributes where the end index also matches, since GetSyntaxNodeAsync will consider that the start tag but we behave
80-
// as if the user wants to go to the attribute definition.
81-
// ie: <Component attribute$$></Component>
82-
var selectedAttribute = startTag.Attributes.FirstOrDefault(a => a.Span.Contains(absoluteIndex) || a.Span.End == absoluteIndex);
76+
var requireAttributeMatch = false;
77+
if ((!ignoreComponentAttributes || boundTagHelper.Kind != TagHelperKind.Component) &&
78+
tagHelperNode is MarkupTagHelperStartTagSyntax startTag)
79+
{
80+
// Include attributes where the end index also matches, since GetSyntaxNodeAsync will consider that the start tag but we behave
81+
// as if the user wants to go to the attribute definition.
82+
// ie: <Component attribute$$></Component>
83+
var selectedAttribute = startTag.Attributes.FirstOrDefault(absoluteIndex, static (a, absoluteIndex) => a.Span.Contains(absoluteIndex) || a.Span.End == absoluteIndex);
84+
85+
requireAttributeMatch = selectedAttribute is not null;
86+
87+
// If we're on an attribute then just validate against the attribute name
88+
switch (selectedAttribute)
89+
{
90+
case MarkupTagHelperAttributeSyntax attribute:
91+
// Normal attribute, ie <Component attribute=value />
92+
nameSpan = attribute.Name.Span;
93+
propertyName = attribute.TagHelperAttributeInfo.Name;
94+
break;
95+
96+
case MarkupMinimizedTagHelperAttributeSyntax minimizedAttribute:
97+
// Minimized attribute, ie <Component attribute />
98+
nameSpan = minimizedAttribute.Name.Span;
99+
propertyName = minimizedAttribute.TagHelperAttributeInfo.Name;
100+
break;
101+
}
102+
}
83103

84-
// If we're on an attribute then just validate against the attribute name
85-
switch (selectedAttribute)
104+
if (!nameSpan.IntersectsWith(absoluteIndex))
86105
{
87-
case MarkupTagHelperAttributeSyntax attribute:
88-
// Normal attribute, ie <Component attribute=value />
89-
nameSpan = attribute.Name.Span;
90-
propertyName = attribute.TagHelperAttributeInfo.Name;
91-
break;
92-
93-
case MarkupMinimizedTagHelperAttributeSyntax minimizedAttribute:
94-
// Minimized attribute, ie <Component attribute />
95-
nameSpan = minimizedAttribute.Name.Span;
96-
propertyName = minimizedAttribute.TagHelperAttributeInfo.Name;
97-
break;
106+
logger.LogInformation($"Tag name or attributes' span does not intersect with index, {absoluteIndex}.");
107+
continue;
98108
}
109+
110+
var boundAttribute = propertyName is not null
111+
? boundTagHelper.BoundAttributes.FirstOrDefault(propertyName, static (a, propertyName) => a.Name == propertyName)
112+
: null;
113+
114+
if (requireAttributeMatch && boundAttribute is null)
115+
{
116+
// The user is on an attribute, but we couldn't find a matching BoundAttributeDescriptor.
117+
continue;
118+
}
119+
120+
descriptorsBuilder.Add(new BoundTagHelperResult(boundTagHelper, boundAttribute));
99121
}
100122

101-
if (!nameSpan.IntersectsWith(absoluteIndex))
123+
if (descriptorsBuilder.Count == 0)
102124
{
103-
logger.LogInformation($"Tag name or attributes' span does not intersect with index, {absoluteIndex}.");
104125
return false;
105126
}
106127

107-
boundAttribute = propertyName is not null
108-
? boundTagHelper.BoundAttributes.FirstOrDefault(a => a.Name?.Equals(propertyName, StringComparison.Ordinal) == true)
109-
: null;
128+
descriptors = descriptorsBuilder.ToImmutableAndClear();
110129

111130
return true;
112131

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ITagHelperSearchEngine.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Collections.Immutable;
45
using System.Threading;
56
using System.Threading.Tasks;
6-
using Microsoft.AspNetCore.Razor.Language;
7+
using Microsoft.CodeAnalysis.Razor.GoToDefinition;
78
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
89

910
namespace Microsoft.CodeAnalysis.Razor.Workspaces;
1011

1112
internal interface ITagHelperSearchEngine
1213
{
13-
Task<LspLocation?> TryLocateTagHelperDefinitionAsync(TagHelperDescriptor boundTagHelper, BoundAttributeDescriptor? boundAttribute, IDocumentSnapshot documentSnapshot, ISolutionQueryOperations solutionQueryOperations, CancellationToken cancellationToken);
14+
Task<LspLocation[]?> TryLocateTagHelperDefinitionsAsync(ImmutableArray<BoundTagHelperResult> boundTagHelpers, IDocumentSnapshot documentSnapshot, ISolutionQueryOperations solutionQueryOperations, CancellationToken cancellationToken);
1415
}

‎src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/GoToDefinition/RemoteGoToDefinitionService.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ protected override IRemoteGoToDefinitionService CreateService(in ServiceArgs arg
5959
if (positionInfo.LanguageKind is RazorLanguageKind.Html or RazorLanguageKind.Razor)
6060
{
6161
// First, see if this is a tag helper. We ignore component attributes here, because they're better served by the C# handler.
62-
var componentLocation = await _definitionService.GetDefinitionAsync(
62+
var componentLocations = await _definitionService.GetDefinitionAsync(
6363
context.Snapshot,
6464
positionInfo,
6565
context.GetSolutionQueryOperations(),
@@ -68,9 +68,9 @@ protected override IRemoteGoToDefinitionService CreateService(in ServiceArgs arg
6868
cancellationToken)
6969
.ConfigureAwait(false);
7070

71-
if (componentLocation is not null)
71+
if (componentLocations is { Length: > 0 })
7272
{
73-
return Results([componentLocation]);
73+
return Results(componentLocations);
7474
}
7575

7676
// If it isn't a Razor construct, and it isn't C#, let the server know to delegate to HTML.

‎src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/RemoteTagHelperSearchEngine.cs‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Collections.Immutable;
45
using System.Composition;
56
using System.Diagnostics;
67
using System.Threading;
78
using System.Threading.Tasks;
89
using Microsoft.AspNetCore.Razor.Language;
10+
using Microsoft.AspNetCore.Razor.PooledObjects;
911
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
12+
using Microsoft.CodeAnalysis.Razor.GoToDefinition;
1013
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
1114
using Microsoft.CodeAnalysis.Razor.Workspaces;
1215
using Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem;
@@ -16,19 +19,8 @@ namespace Microsoft.CodeAnalysis.Remote.Razor;
1619
[Export(typeof(ITagHelperSearchEngine)), Shared]
1720
internal sealed class RemoteTagHelperSearchEngine : ITagHelperSearchEngine
1821
{
19-
public async Task<LspLocation?> TryLocateTagHelperDefinitionAsync(TagHelperDescriptor boundTagHelper, BoundAttributeDescriptor? boundAttribute, IDocumentSnapshot documentSnapshot, ISolutionQueryOperations solutionQueryOperations, CancellationToken cancellationToken)
22+
public async Task<LspLocation[]?> TryLocateTagHelperDefinitionsAsync(ImmutableArray<BoundTagHelperResult> boundTagHelperResults, IDocumentSnapshot documentSnapshot, ISolutionQueryOperations solutionQueryOperations, CancellationToken cancellationToken)
2023
{
21-
if (boundTagHelper.Kind == TagHelperKind.Component)
22-
{
23-
return null;
24-
}
25-
26-
var typeName = boundTagHelper.TypeName;
27-
if (typeName is null)
28-
{
29-
return null;
30-
}
31-
3224
Debug.Assert(documentSnapshot is RemoteDocumentSnapshot);
3325

3426
var project = ((RemoteDocumentSnapshot)documentSnapshot).TextDocument.Project;
@@ -38,19 +30,42 @@ internal sealed class RemoteTagHelperSearchEngine : ITagHelperSearchEngine
3830
return null;
3931
}
4032

41-
foreach (var type in compilation.GetTypesByMetadataName(typeName))
33+
using var locations = new PooledArrayBuilder<LspLocation>();
34+
35+
foreach (var (boundTagHelper, boundAttribute) in boundTagHelperResults)
36+
{
37+
if (boundTagHelper.Kind == TagHelperKind.Component)
38+
{
39+
return null;
40+
}
41+
42+
var location = await TryLocateTagHelperDefinitionAsync(boundTagHelper, boundAttribute, compilation, project.Solution, cancellationToken).ConfigureAwait(false);
43+
if (location is not null)
44+
{
45+
locations.Add(location);
46+
}
47+
}
48+
49+
return locations.ToArrayAndClear();
50+
}
51+
52+
private async Task<LspLocation?> TryLocateTagHelperDefinitionAsync(TagHelperDescriptor boundTagHelper, BoundAttributeDescriptor? boundAttribute, Compilation compilation, Solution solution, CancellationToken cancellationToken)
53+
{
54+
foreach (var type in compilation.GetTypesByMetadataName(boundTagHelper.TypeName))
4255
{
4356
var locations = type.Locations;
44-
if (boundAttribute is { PropertyName: string propertyName } &&
45-
type.GetMembers(propertyName) is [{ } property])
57+
58+
// If we're on an attribute, then lets try to navigate them to the property it represents, rather than just the type.
59+
if (boundAttribute is not null &&
60+
type.GetMembers(boundAttribute.PropertyName) is [{ } property])
4661
{
4762
locations = property.Locations;
4863
}
4964

5065
foreach (var location in locations)
5166
{
5267
if (location.IsInSource &&
53-
project.Solution.GetDocument(location.SourceTree) is { } document)
68+
solution.GetDocument(location.SourceTree) is { } document)
5469
{
5570
var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
5671
return new LspLocation

‎src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Definition/RazorComponentDefinitionHelpersTest.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,13 +356,13 @@ private void VerifyTryGetBoundTagHelpers(
356356
string? tagHelperDescriptorName = null,
357357
string? attributeDescriptorPropertyName = null,
358358
bool isRazorFile = true,
359-
bool ignoreComponentAttributes= false)
359+
bool ignoreComponentAttributes = false)
360360
{
361361
TestFileMarkupParser.GetPosition(content, out content, out var position);
362362

363363
var codeDocument = CreateCodeDocument(content, isRazorFile);
364364

365-
var result = RazorComponentDefinitionHelpers.TryGetBoundTagHelpers(codeDocument, position, ignoreComponentAttributes, Logger, out var boundTagHelper, out var boundAttribute);
365+
var result = RazorComponentDefinitionHelpers.TryGetBoundTagHelpers(codeDocument, position, ignoreComponentAttributes, Logger, out var boundTagHelperResults);
366366

367367
if (tagHelperDescriptorName is null)
368368
{
@@ -371,13 +371,15 @@ private void VerifyTryGetBoundTagHelpers(
371371
else
372372
{
373373
Assert.True(result);
374+
var boundTagHelper = Assert.Single(boundTagHelperResults).ElementDescriptor;
374375
Assert.NotNull(boundTagHelper);
375376
Assert.Equal(tagHelperDescriptorName, boundTagHelper.Name);
376377
}
377378

378379
if (attributeDescriptorPropertyName is not null)
379380
{
380381
Assert.True(result);
382+
var boundAttribute = Assert.Single(boundTagHelperResults).AttributeDescriptor;
381383
Assert.NotNull(boundAttribute);
382384
Assert.Equal(attributeDescriptorPropertyName, boundAttribute.PropertyName);
383385
}

‎src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/DefinitionServiceTest.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,16 @@ private async Task VerifyDefinitionAsync(TestCode input, TestCode expectedDocume
7171
var codeDocument = await documentSnapshot.GetGeneratedOutputAsync(DisposalToken);
7272
var positionInfo = documentMappingService.GetPositionInfo(codeDocument, input.Position);
7373

74-
var location = await service.GetDefinitionAsync(
74+
var locations = await service.GetDefinitionAsync(
7575
documentSnapshot,
7676
positionInfo,
7777
solutionQueryOperations: documentSnapshot.ProjectSnapshot.SolutionSnapshot,
7878
ignoreComponentAttributes: false,
7979
includeMvcTagHelpers: true,
8080
DisposalToken);
8181

82-
Assert.NotNull(location);
82+
Assert.NotNull(locations);
83+
var location = Assert.Single(locations);
8384

8485
var text = SourceText.From(expectedDocument.Text);
8586
var range = text.GetRange(expectedDocument.Span);

0 commit comments

Comments
 (0)