Skip to content

Commit e11f855

Browse files
DustinCampbellDustinCampbell
authored
Rework required attributes (#11971)
> [!IMPORTANT] > This affects the Razor SDK. When the compiler flows to the .NET VMR, this commit will be needed: dotnet/dotnet@a10383a. This change makes a few updates `RequiredAttributeDescriptor`: - Un-nest and rename the `RequiredAttributeDescriptor.NameComparisonMode` and `RequiredAttributeDescriptor.ValueComparisonMode` enums to `RequiredAttributeNameComparison` and `RequiredAttributeValueComparison`, respectively. - Remove the `Metadata` property. There's only one bit of data that was ever stored in the `Metadata` property: `IsDirectiveAttribute`. This has been moved to a property on `RequiredAttributeDescriptor` itself. Interestingly, the compiler doesn't actually use `IsDirectiveAttribute`. However, it might sometime, so I've left it. - Don't serialize the `DisplayName` property. This always has exactly the same format for any `RequiredAttributeDescriptor`. ---- CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2736049&view=results Test Insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/645854 Toolset Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2736052&view=results
2 parents 85af466 + dfc3dce commit e11f855

File tree

47 files changed

+9183
-15716
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+9183
-15716
lines changed

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/legacyTest/Legacy/TagHelperBlockRewriterTest.cs

Lines changed: 52 additions & 103 deletions
Large diffs are not rendered by default.

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -265,12 +265,10 @@ public void Execute_DirectiveWithoutQuotes_RewritesTagHelpers_TagHelperMatchesEl
265265
[
266266
ruleBuilder => ruleBuilder
267267
.RequireAttributeDescriptor(attribute => attribute
268-
.Name("a")
269-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch)),
268+
.Name("a", RequiredAttributeNameComparison.FullMatch)),
270269
ruleBuilder => ruleBuilder
271270
.RequireAttributeDescriptor(attribute => attribute
272-
.Name("b")
273-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch)),
271+
.Name("b", RequiredAttributeNameComparison.FullMatch)),
274272
]);
275273

276274
var content = @"
@@ -309,12 +307,10 @@ public void Execute_DirectiveWithQuotes_RewritesTagHelpers_TagHelperMatchesEleme
309307
[
310308
ruleBuilder => ruleBuilder
311309
.RequireAttributeDescriptor(attribute => attribute
312-
.Name("a")
313-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch)),
310+
.Name("a", RequiredAttributeNameComparison.FullMatch)),
314311
ruleBuilder => ruleBuilder
315312
.RequireAttributeDescriptor(attribute => attribute
316-
.Name("b")
317-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch)),
313+
.Name("b", RequiredAttributeNameComparison.FullMatch)),
318314
]);
319315

320316
var content = @"

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRequiredAttributeDescriptorBuilderTest.cs

Lines changed: 14 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using Xunit;
5-
using static Microsoft.AspNetCore.Razor.Language.CommonMetadata;
65

76
namespace Microsoft.AspNetCore.Razor.Language;
87

@@ -12,87 +11,33 @@ public class DefaultRequiredAttributeDescriptorBuilderTest
1211
public void Build_DisplayNameIsName_NameComparisonFullMatch()
1312
{
1413
// Arrange
15-
var tagHelperBuilder = new TagHelperDescriptorBuilder(TagHelperConventions.DefaultKind, "TestTagHelper", "Test");
16-
var tagMatchingRuleBuilder = new TagMatchingRuleDescriptorBuilder(tagHelperBuilder);
17-
var builder = new RequiredAttributeDescriptorBuilder(tagMatchingRuleBuilder);
18-
19-
builder
20-
.Name("asp-action")
21-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch);
14+
var builder = TagHelperDescriptorBuilder.Create(TagHelperConventions.DefaultKind, "TestTagHelper", "Test")
15+
.TagMatchingRuleDescriptor(rule => rule
16+
.RequireAttributeDescriptor(attribute => attribute
17+
.Name("asp-action", RequiredAttributeNameComparison.FullMatch)));
2218

2319
// Act
24-
var descriptor = builder.Build();
20+
var tagHelper = builder.Build();
21+
var attribute = tagHelper.TagMatchingRules[0].Attributes[0];
2522

2623
// Assert
27-
Assert.Equal("asp-action", descriptor.DisplayName);
24+
Assert.Equal("asp-action", attribute.DisplayName);
2825
}
2926

3027
[Fact]
3128
public void Build_DisplayNameIsNameWithDots_NameComparisonPrefixMatch()
3229
{
3330
// Arrange
34-
var tagHelperBuilder = new TagHelperDescriptorBuilder(TagHelperConventions.DefaultKind, "TestTagHelper", "Test");
35-
var tagMatchingRuleBuilder = new TagMatchingRuleDescriptorBuilder(tagHelperBuilder);
36-
var builder = new RequiredAttributeDescriptorBuilder(tagMatchingRuleBuilder);
37-
38-
builder
39-
.Name("asp-route-")
40-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.PrefixMatch);
41-
42-
// Act
43-
var descriptor = builder.Build();
44-
45-
// Assert
46-
Assert.Equal("asp-route-...", descriptor.DisplayName);
47-
}
48-
49-
[Fact]
50-
public void Metadata_Same()
51-
{
52-
// When SetMetadata is called on multiple builders with the same metadata collection,
53-
// they should share the instance.
54-
55-
// Arrange
56-
var tagHelperBuilder = new TagHelperDescriptorBuilder(TagHelperConventions.DefaultKind, "TestTagHelper", "Test");
57-
var tagMatchingRuleBuilder = new TagMatchingRuleDescriptorBuilder(tagHelperBuilder);
58-
59-
var metadata = MetadataCollection.Create(PropertyName("SomeProperty"));
60-
61-
var builder1 = new RequiredAttributeDescriptorBuilder(tagMatchingRuleBuilder);
62-
var builder2 = new RequiredAttributeDescriptorBuilder(tagMatchingRuleBuilder);
63-
64-
builder1.SetMetadata(metadata);
65-
builder2.SetMetadata(metadata);
66-
67-
// Act
68-
var descriptor1 = builder1.Build();
69-
var descriptor2 = builder2.Build();
70-
71-
// Assert
72-
Assert.Same(descriptor1.Metadata, descriptor2.Metadata);
73-
}
74-
75-
[Fact]
76-
public void Metadata_NotSame()
77-
{
78-
// When Metadata is accessed on multiple builders with the same metadata,
79-
// they do not share the instance.
80-
81-
// Arrange
82-
var tagHelperBuilder = new TagHelperDescriptorBuilder(TagHelperConventions.DefaultKind, "TestTagHelper", "Test");
83-
var tagMatchingRuleBuilder = new TagMatchingRuleDescriptorBuilder(tagHelperBuilder);
84-
85-
var builder1 = new RequiredAttributeDescriptorBuilder(tagMatchingRuleBuilder);
86-
var builder2 = new RequiredAttributeDescriptorBuilder(tagMatchingRuleBuilder);
87-
88-
builder1.Metadata.Add(PropertyName("SomeProperty"));
89-
builder2.Metadata.Add(PropertyName("SomeProperty"));
31+
var builder = TagHelperDescriptorBuilder.Create(TagHelperConventions.DefaultKind, "TestTagHelper", "Test")
32+
.TagMatchingRuleDescriptor(rule => rule
33+
.RequireAttributeDescriptor(attribute => attribute
34+
.Name("asp-route-", RequiredAttributeNameComparison.PrefixMatch)));
9035

9136
// Act
92-
var descriptor1 = builder1.Build();
93-
var descriptor2 = builder2.Build();
37+
var tagHelper = builder.Build();
38+
var attribute = tagHelper.TagMatchingRules[0].Attributes[0];
9439

9540
// Assert
96-
Assert.NotSame(descriptor1.Metadata, descriptor2.Metadata);
41+
Assert.Equal("asp-route-...", attribute.DisplayName);
9742
}
9843
}

‎src/Compiler/Microsoft.AspNetCore.Razor.Language/test/IntegrationTests/TestTagHelperDescriptors.cs

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,8 @@ public static IEnumerable<TagHelperDescriptor> CssSelectorTagHelperDescriptors
116116
{
117117
builder => builder
118118
.RequireAttributeDescriptor(attribute => attribute
119-
.Name("href")
120-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch)
121-
.Value("~/")
122-
.ValueComparisonMode(RequiredAttributeDescriptor.ValueComparisonMode.FullMatch)),
119+
.Name("href", RequiredAttributeNameComparison.FullMatch)
120+
.Value("~/", RequiredAttributeValueComparison.FullMatch)),
123121
}),
124122
CreateTagHelperDescriptor(
125123
tagName: "a",
@@ -129,15 +127,11 @@ public static IEnumerable<TagHelperDescriptor> CssSelectorTagHelperDescriptors
129127
{
130128
builder => builder
131129
.RequireAttributeDescriptor(attribute => attribute
132-
.Name("href")
133-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch)
134-
.Value("~/")
135-
.ValueComparisonMode(RequiredAttributeDescriptor.ValueComparisonMode.PrefixMatch))
130+
.Name("href", RequiredAttributeNameComparison.FullMatch)
131+
.Value("~/", RequiredAttributeValueComparison.PrefixMatch))
136132
.RequireAttributeDescriptor(attribute => attribute
137-
.Name("href")
138-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch)
139-
.Value("?hello=world")
140-
.ValueComparisonMode(RequiredAttributeDescriptor.ValueComparisonMode.SuffixMatch)),
133+
.Name("href", RequiredAttributeNameComparison.FullMatch)
134+
.Value("?hello=world", RequiredAttributeValueComparison.SuffixMatch)),
141135
}),
142136
CreateTagHelperDescriptor(
143137
tagName: "input",
@@ -151,10 +145,8 @@ public static IEnumerable<TagHelperDescriptor> CssSelectorTagHelperDescriptors
151145
{
152146
builder => builder
153147
.RequireAttributeDescriptor(attribute => attribute
154-
.Name("type")
155-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch)
156-
.Value("text")
157-
.ValueComparisonMode(RequiredAttributeDescriptor.ValueComparisonMode.FullMatch)),
148+
.Name("type", RequiredAttributeNameComparison.FullMatch)
149+
.Value("text", RequiredAttributeValueComparison.FullMatch)),
158150
}),
159151
CreateTagHelperDescriptor(
160152
tagName: "input",
@@ -168,8 +160,7 @@ public static IEnumerable<TagHelperDescriptor> CssSelectorTagHelperDescriptors
168160
{
169161
builder => builder
170162
.RequireAttributeDescriptor(attribute => attribute
171-
.Name("ty")
172-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.PrefixMatch)),
163+
.Name("ty", RequiredAttributeNameComparison.PrefixMatch)),
173164
}),
174165
CreateTagHelperDescriptor(
175166
tagName: "*",
@@ -179,10 +170,8 @@ public static IEnumerable<TagHelperDescriptor> CssSelectorTagHelperDescriptors
179170
{
180171
builder => builder
181172
.RequireAttributeDescriptor(attribute => attribute
182-
.Name("href")
183-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch)
184-
.Value("~/")
185-
.ValueComparisonMode(RequiredAttributeDescriptor.ValueComparisonMode.PrefixMatch)),
173+
.Name("href", RequiredAttributeNameComparison.FullMatch)
174+
.Value("~/", RequiredAttributeValueComparison.PrefixMatch)),
186175
}),
187176
CreateTagHelperDescriptor(
188177
tagName: "*",
@@ -192,8 +181,7 @@ public static IEnumerable<TagHelperDescriptor> CssSelectorTagHelperDescriptors
192181
{
193182
builder => builder
194183
.RequireAttributeDescriptor(attribute => attribute
195-
.Name("type")
196-
.NameComparisonMode(RequiredAttributeDescriptor.NameComparisonMode.FullMatch)),
184+
.Name("type", RequiredAttributeNameComparison.FullMatch)),
197185
}),
198186
};
199187
}

0 commit comments

Comments
 (0)