Skip to content

Commit dc45fbc

Browse files
DustinCampbellDustinCampbell
authored
Remove RazorCompletionItem.Items property (#11360)
Every `RazorCompletionItem` creates an instance of `ItemCollection` and exposes it via an `Items` property. The property lazily constructs the `ItemCollection`, but that doesn't really matter much because the property is always accessed. Every time the `RazorCompletionItem` constructor is called a "description info" object (used to build completion description tooltips) is immediately added to the `Items` property. No other data is stored in the `ItemCollection`. It is unnecessary overhead to store a single object per `RazorCompletionItem` in an `ItemCollection`. Internally, `ItemCollection` is implemented with a `ConcurrentDictionary`, so it's definitely not cheap! This change removes the `Items` property altogether and provides a `DescriptionInfo` property that is set via the constructor. I've also done a fair amount of clean up in separate commits. The first commit contains the meat of removing the `Items` property.
2 parents a91b26f + 1a699b3 commit dc45fbc

26 files changed

+493
-641
lines changed

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/DirectiveAttributeCompletionItemProvider.cs‎

Lines changed: 53 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -16,44 +16,34 @@ internal class DirectiveAttributeCompletionItemProvider : DirectiveAttributeComp
1616
{
1717
public override ImmutableArray<RazorCompletionItem> GetCompletionItems(RazorCompletionContext context)
1818
{
19-
if (context is null)
20-
{
21-
throw new ArgumentNullException(nameof(context));
22-
}
23-
24-
if (context.TagHelperDocumentContext is null)
25-
{
26-
throw new ArgumentNullException(nameof(context.TagHelperDocumentContext));
27-
}
28-
2919
if (!FileKinds.IsComponent(context.SyntaxTree.Options.FileKind))
3020
{
3121
// Directive attributes are only supported in components
32-
return ImmutableArray<RazorCompletionItem>.Empty;
22+
return [];
3323
}
3424

3525
var owner = context.Owner;
3626
if (owner is null)
3727
{
38-
return ImmutableArray<RazorCompletionItem>.Empty;
28+
return [];
3929
}
4030

4131
if (!TryGetAttributeInfo(owner, out _, out var attributeName, out var attributeNameLocation, out _, out _))
4232
{
4333
// Either we're not in an attribute or the attribute is so malformed that we can't provide proper completions.
44-
return ImmutableArray<RazorCompletionItem>.Empty;
34+
return [];
4535
}
4636

4737
if (!attributeNameLocation.IntersectsWith(context.AbsoluteIndex))
4838
{
4939
// We're trying to retrieve completions on a portion of the name that is not supported (such as a parameter).
50-
return ImmutableArray<RazorCompletionItem>.Empty;
40+
return [];
5141
}
5242

5343
if (!TryGetElementInfo(owner.Parent.Parent, out var containingTagName, out var attributes))
5444
{
5545
// This should never be the case, it means that we're operating on an attribute that doesn't have a tag.
56-
return ImmutableArray<RazorCompletionItem>.Empty;
46+
return [];
5747
}
5848

5949
// At this point we've determined that completions have been requested for the name portion of the selected attribute.
@@ -63,16 +53,16 @@ public override ImmutableArray<RazorCompletionItem> GetCompletionItems(RazorComp
6353
// We don't provide Directive Attribute completions when we're in the middle of
6454
// another unrelated (doesn't start with @) partially completed attribute.
6555
// <svg xml:| ></svg> (attributeName = "xml:") should not get any directive attribute completions.
66-
if (string.IsNullOrWhiteSpace(attributeName) || attributeName.StartsWith("@", StringComparison.Ordinal))
56+
if (attributeName.IsNullOrWhiteSpace() || attributeName.StartsWith('@'))
6757
{
6858
return completionItems;
6959
}
7060

71-
return ImmutableArray<RazorCompletionItem>.Empty;
61+
return [];
7262
}
7363

7464
// Internal for testing
75-
internal ImmutableArray<RazorCompletionItem> GetAttributeCompletions(
65+
internal static ImmutableArray<RazorCompletionItem> GetAttributeCompletions(
7666
string selectedAttributeName,
7767
string containingTagName,
7868
ImmutableArray<string> attributes,
@@ -82,11 +72,11 @@ internal ImmutableArray<RazorCompletionItem> GetAttributeCompletions(
8272
if (descriptorsForTag.Length == 0)
8373
{
8474
// If the current tag has no possible descriptors then we can't have any directive attributes.
85-
return ImmutableArray<RazorCompletionItem>.Empty;
75+
return [];
8676
}
8777

88-
// Attributes are case sensitive when matching
89-
var attributeCompletions = new Dictionary<string, (HashSet<BoundAttributeDescriptionInfo>, HashSet<string>)>(StringComparer.Ordinal);
78+
// Use ordinal dictionary because attributes are case sensitive when matching
79+
using var _ = StringDictionaryPool<(HashSet<BoundAttributeDescriptionInfo>, HashSet<string>)>.Ordinal.GetPooledObject(out var attributeCompletions);
9080

9181
foreach (var descriptor in descriptorsForTag)
9282
{
@@ -114,7 +104,7 @@ internal ImmutableArray<RazorCompletionItem> GetAttributeCompletions(
114104
}
115105
}
116106

117-
if (!string.IsNullOrEmpty(attributeDescriptor.IndexerNamePrefix))
107+
if (!attributeDescriptor.IndexerNamePrefix.IsNullOrEmpty())
118108
{
119109
TryAddCompletion(attributeDescriptor.IndexerNamePrefix + "...", attributeDescriptor, descriptor);
120110
}
@@ -123,39 +113,36 @@ internal ImmutableArray<RazorCompletionItem> GetAttributeCompletions(
123113

124114
using var completionItems = new PooledArrayBuilder<RazorCompletionItem>(capacity: attributeCompletions.Count);
125115

126-
foreach (var completion in attributeCompletions)
116+
foreach (var (displayText, (attributeDescriptions, commitCharacters)) in attributeCompletions)
127117
{
128-
var insertText = completion.Key;
129-
if (insertText.EndsWith("...", StringComparison.Ordinal))
130-
{
131-
// Indexer attribute, we don't want to insert with the triple dot.
132-
insertText = insertText[..^3];
133-
}
118+
var insertText = displayText;
119+
120+
// Strip off the @ from the insertion text. This change is here to align the insertion text with the
121+
// completion hooks into VS and VSCode. Basically, completion triggers when `@` is typed so we don't
122+
// want to insert `@bind` because `@` already exists.
123+
var startIndex = insertText.StartsWith('@') ? 1 : 0;
134124

135-
if (insertText.StartsWith("@", StringComparison.Ordinal))
125+
// Indexer attribute, we don't want to insert with the triple dot.
126+
var endIndex = insertText.EndsWith("...", StringComparison.Ordinal) ? ^3 : ^0;
127+
128+
// Don't allocate a new string unless we need to make a change.
129+
if (startIndex > 0 || endIndex.Value > 0)
136130
{
137-
// Strip off the @ from the insertion text. This change is here to align the insertion text with the
138-
// completion hooks into VS and VSCode. Basically, completion triggers when `@` is typed so we don't
139-
// want to insert `@bind` because `@` already exists.
140-
insertText = insertText[1..];
131+
insertText = insertText[startIndex..endIndex];
141132
}
142133

143-
var (attributeDescriptionInfos, commitCharacters) = completion.Value;
144-
145134
using var razorCommitCharacters = new PooledArrayBuilder<RazorCommitCharacter>(capacity: commitCharacters.Count);
146135

147136
foreach (var c in commitCharacters)
148137
{
149138
razorCommitCharacters.Add(new(c));
150139
}
151140

152-
var razorCompletionItem = new RazorCompletionItem(
153-
completion.Key,
141+
var razorCompletionItem = RazorCompletionItem.CreateDirectiveAttribute(
142+
displayText,
154143
insertText,
155-
RazorCompletionItemKind.DirectiveAttribute,
144+
descriptionInfo: new([.. attributeDescriptions]),
156145
commitCharacters: razorCommitCharacters.DrainToImmutable());
157-
var completionDescription = new AggregateBoundAttributeDescription(attributeDescriptionInfos.ToImmutableArray());
158-
razorCompletionItem.SetAttributeCompletionDescription(completionDescription);
159146

160147
completionItems.Add(razorCompletionItem);
161148
}
@@ -164,8 +151,8 @@ internal ImmutableArray<RazorCompletionItem> GetAttributeCompletions(
164151

165152
bool TryAddCompletion(string attributeName, BoundAttributeDescriptor boundAttributeDescriptor, TagHelperDescriptor tagHelperDescriptor)
166153
{
167-
if (attributes.Any(name => string.Equals(name, attributeName, StringComparison.Ordinal)) &&
168-
!string.Equals(selectedAttributeName, attributeName, StringComparison.Ordinal))
154+
if (selectedAttributeName != attributeName &&
155+
attributes.Any(attributeName, static (name, attributeName) => name == attributeName))
169156
{
170157
// Attribute is already present on this element and it is not the selected attribute.
171158
// It shouldn't exist in the completion list.
@@ -180,16 +167,16 @@ void AddCompletion(string attributeName, BoundAttributeDescriptor boundAttribute
180167
{
181168
if (!attributeCompletions.TryGetValue(attributeName, out var attributeDetails))
182169
{
183-
attributeDetails = (new HashSet<BoundAttributeDescriptionInfo>(), new HashSet<string>());
170+
attributeDetails = ([], []);
184171
attributeCompletions[attributeName] = attributeDetails;
185172
}
186173

187-
(var attributeDescriptionInfos, var commitCharacters) = attributeDetails;
174+
(var attributeDescriptions, var commitCharacters) = attributeDetails;
188175

189176
var indexerCompletion = attributeName.EndsWith("...", StringComparison.Ordinal);
190177
var tagHelperTypeName = tagHelperDescriptor.GetTypeName();
191178
var descriptionInfo = BoundAttributeDescriptionInfo.From(boundAttributeDescriptor, isIndexer: indexerCompletion, tagHelperTypeName);
192-
attributeDescriptionInfos.Add(descriptionInfo);
179+
attributeDescriptions.Add(descriptionInfo);
193180

194181
if (indexerCompletion)
195182
{
@@ -199,14 +186,28 @@ void AddCompletion(string attributeName, BoundAttributeDescriptor boundAttribute
199186

200187
commitCharacters.Add("=");
201188

202-
if (tagHelperDescriptor.BoundAttributes.Any(b => b.IsBooleanProperty))
203-
{
204-
commitCharacters.Add(" ");
205-
}
189+
var spaceAdded = commitCharacters.Contains(" ");
190+
var colonAdded = commitCharacters.Contains(":");
206191

207-
if (tagHelperDescriptor.BoundAttributes.Any(b => b.Parameters.Length > 0))
192+
if (!spaceAdded || !colonAdded)
208193
{
209-
commitCharacters.Add(":");
194+
foreach (var boundAttribute in tagHelperDescriptor.BoundAttributes)
195+
{
196+
if (!spaceAdded && boundAttribute.IsBooleanProperty)
197+
{
198+
commitCharacters.Add(" ");
199+
spaceAdded = true;
200+
}
201+
else if (!colonAdded && boundAttribute.Parameters.Length > 0)
202+
{
203+
commitCharacters.Add(":");
204+
colonAdded = true;
205+
}
206+
else if (spaceAdded && colonAdded)
207+
{
208+
break;
209+
}
210+
}
210211
}
211212
}
212213
}

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/DirectiveAttributeParameterCompletionItemProvider.cs‎

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the MIT license. See License.txt in the project root for license information.
33

4-
using System;
54
using System.Collections.Generic;
65
using System.Collections.Immutable;
76
using System.Linq;
@@ -16,51 +15,41 @@ internal class DirectiveAttributeParameterCompletionItemProvider : DirectiveAttr
1615
{
1716
public override ImmutableArray<RazorCompletionItem> GetCompletionItems(RazorCompletionContext context)
1817
{
19-
if (context is null)
20-
{
21-
throw new ArgumentNullException(nameof(context));
22-
}
23-
24-
if (context.TagHelperDocumentContext is null)
25-
{
26-
throw new ArgumentNullException(nameof(context.TagHelperDocumentContext));
27-
}
28-
2918
if (!FileKinds.IsComponent(context.SyntaxTree.Options.FileKind))
3019
{
3120
// Directive attribute parameters are only supported in components
32-
return ImmutableArray<RazorCompletionItem>.Empty;
21+
return [];
3322
}
3423

3524
var owner = context.Owner;
3625
if (owner is null)
3726
{
38-
return ImmutableArray<RazorCompletionItem>.Empty;
27+
return [];
3928
}
4029

4130
if (!TryGetAttributeInfo(owner, out _, out var attributeName, out _, out var parameterName, out var parameterNameLocation))
4231
{
4332
// Either we're not in an attribute or the attribute is so malformed that we can't provide proper completions.
44-
return ImmutableArray<RazorCompletionItem>.Empty;
33+
return [];
4534
}
4635

4736
if (!parameterNameLocation.IntersectsWith(context.AbsoluteIndex))
4837
{
4938
// We're trying to retrieve completions on a portion of the name that is not supported (such as the name, i.e., |@bind|:format).
50-
return ImmutableArray<RazorCompletionItem>.Empty;
39+
return [];
5140
}
5241

5342
if (!TryGetElementInfo(owner.Parent.Parent, out var containingTagName, out var attributes))
5443
{
5544
// This should never be the case, it means that we're operating on an attribute that doesn't have a tag.
56-
return ImmutableArray<RazorCompletionItem>.Empty;
45+
return [];
5746
}
5847

5948
return GetAttributeParameterCompletions(attributeName, parameterName, containingTagName, attributes, context.TagHelperDocumentContext);
6049
}
6150

6251
// Internal for testing
63-
internal ImmutableArray<RazorCompletionItem> GetAttributeParameterCompletions(
52+
internal static ImmutableArray<RazorCompletionItem> GetAttributeParameterCompletions(
6453
string attributeName,
6554
string? parameterName,
6655
string containingTagName,
@@ -71,17 +60,16 @@ internal ImmutableArray<RazorCompletionItem> GetAttributeParameterCompletions(
7160
if (descriptorsForTag.Length == 0)
7261
{
7362
// If the current tag has no possible descriptors then we can't have any additional attributes.
74-
return ImmutableArray<RazorCompletionItem>.Empty;
63+
return [];
7564
}
7665

77-
// Attribute parameters are case sensitive when matching
78-
var attributeCompletions = new Dictionary<string, HashSet<BoundAttributeDescriptionInfo>>(StringComparer.Ordinal);
66+
// Use ordinal dictionary because attributes are case sensitive when matching
67+
using var _ = StringDictionaryPool<HashSet<BoundAttributeDescriptionInfo>>.Ordinal.GetPooledObject(out var attributeCompletions);
7968

8069
foreach (var descriptor in descriptorsForTag)
8170
{
82-
for (var i = 0; i < descriptor.BoundAttributes.Length; i++)
71+
foreach (var attributeDescriptor in descriptor.BoundAttributes)
8372
{
84-
var attributeDescriptor = descriptor.BoundAttributes[i];
8573
var boundAttributeParameters = attributeDescriptor.Parameters;
8674
if (boundAttributeParameters.Length == 0)
8775
{
@@ -92,43 +80,44 @@ internal ImmutableArray<RazorCompletionItem> GetAttributeParameterCompletions(
9280
{
9381
foreach (var parameterDescriptor in boundAttributeParameters)
9482
{
95-
if (attributes.Any(name => TagHelperMatchingConventions.SatisfiesBoundAttributeWithParameter(parameterDescriptor, name, attributeDescriptor)))
83+
if (attributes.Any(
84+
(parameterDescriptor, attributeDescriptor),
85+
static (name, arg) =>
86+
TagHelperMatchingConventions.SatisfiesBoundAttributeWithParameter(arg.parameterDescriptor, name, arg.attributeDescriptor)))
9687
{
9788
// There's already an existing attribute that satisfies this parameter, don't show it in the completion list.
9889
continue;
9990
}
10091

101-
if (!attributeCompletions.TryGetValue(parameterDescriptor.Name, out var attributeDescriptionInfos))
92+
if (!attributeCompletions.TryGetValue(parameterDescriptor.Name, out var attributeDescriptions))
10293
{
103-
attributeDescriptionInfos = new HashSet<BoundAttributeDescriptionInfo>();
104-
attributeCompletions[parameterDescriptor.Name] = attributeDescriptionInfos;
94+
attributeDescriptions = [];
95+
attributeCompletions[parameterDescriptor.Name] = attributeDescriptions;
10596
}
10697

10798
var tagHelperTypeName = descriptor.GetTypeName();
10899
var descriptionInfo = BoundAttributeDescriptionInfo.From(parameterDescriptor, tagHelperTypeName);
109-
attributeDescriptionInfos.Add(descriptionInfo);
100+
attributeDescriptions.Add(descriptionInfo);
110101
}
111102
}
112103
}
113104
}
114105

115-
using var completionItems = new PooledArrayBuilder<RazorCompletionItem>();
106+
using var completionItems = new PooledArrayBuilder<RazorCompletionItem>(capacity: attributeCompletions.Count);
116107

117-
foreach (var completion in attributeCompletions)
108+
foreach (var (displayText, value) in attributeCompletions)
118109
{
119-
if (string.Equals(completion.Key, parameterName, StringComparison.Ordinal))
110+
if (displayText == parameterName)
120111
{
121112
// This completion is identical to the selected parameter, don't provide for completions for what's already
122113
// present in the document.
123114
continue;
124115
}
125116

126-
var razorCompletionItem = new RazorCompletionItem(
127-
completion.Key,
128-
completion.Key,
129-
RazorCompletionItemKind.DirectiveAttributeParameter);
130-
var completionDescription = new AggregateBoundAttributeDescription(completion.Value.ToImmutableArray());
131-
razorCompletionItem.SetAttributeCompletionDescription(completionDescription);
117+
var razorCompletionItem = RazorCompletionItem.CreateDirectiveAttributeParameter(
118+
displayText: displayText,
119+
insertText: displayText,
120+
descriptionInfo: new([.. value]));
132121

133122
completionItems.Add(razorCompletionItem);
134123
}

0 commit comments

Comments
 (0)