Skip to content
This repository was archived by the owner on Dec 19, 2018. It is now read-only.

Commit dd34a80

Browse files
author
N. Taylor Mullen
committed
Addressed code review comments.
1 parent 333f42d commit dd34a80

File tree

14 files changed

+142
-189
lines changed

14 files changed

+142
-189
lines changed

src/Microsoft.AspNet.Razor.Runtime/Properties/Resources.Designer.cs

Lines changed: 18 additions & 18 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Microsoft.AspNet.Razor.Runtime/Resources.resx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,15 @@
117117
<resheader name="writer">
118118
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
119119
</resheader>
120-
<data name="TagHelperDescriptorResolver_InvalidAddTagHelperLookupText" xml:space="preserve">
121-
<value>Invalid `@addtaghelper` lookup text '{0}'. The correct lookup text formats are:
120+
<data name="TagHelperDescriptorResolver_InvalidTagHelperLookupText" xml:space="preserve">
121+
<value>Invalid tag helper directive look up text '{0}'. The correct look up text formats are:
122122
"assemblyName"
123123
"typeName, assemblyName"</value>
124124
</data>
125-
<data name="TagHelperTypeResolver_AddTagHelperAssemblyNameCannotBeEmptyOrNull" xml:space="preserve">
126-
<value>`@addtaghelper` assembly name cannot be null or empty.</value>
127-
</data>
128125
<data name="TagHelperTypeResolver_CannotResolveTagHelperAssembly" xml:space="preserve">
129126
<value>Cannot resolve TagHelper containing assembly '{0}'. Error: '{1}'.</value>
130127
</data>
128+
<data name="TagHelperTypeResolver_TagHelperAssemblyNameCannotBeEmptyOrNull" xml:space="preserve">
129+
<value>Tag helper directive assembly name cannot be null or empty.</value>
130+
</data>
131131
</root>

src/Microsoft.AspNet.Razor.Runtime/TagHelpers/TagHelperDescriptorFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public static class TagHelperDescriptorFactory
2323
/// Creates a <see cref="TagHelperDescriptor"/> from the given <paramref name="type"/>.
2424
/// </summary>
2525
/// <param name="type">The type to create a <see cref="TagHelperDescriptor"/> from.</param>
26-
/// <returns>A <see cref="TagHelperDescriptor"/> that represents the given <paramref name="type"/>.</returns>
26+
/// <returns>A <see cref="TagHelperDescriptor"/> that describes the given <paramref name="type"/>.</returns>
2727
public static TagHelperDescriptor CreateDescriptor(Type type)
2828
{
2929
var tagName = GetTagName(type);

src/Microsoft.AspNet.Razor.Runtime/TagHelpers/TagHelperDescriptorResolver.cs

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,47 +32,42 @@ public TagHelperDescriptorResolver()
3232
/// <inheritdoc />
3333
public IEnumerable<TagHelperDescriptor> Resolve(string lookupText)
3434
{
35-
if (string.IsNullOrEmpty(lookupText))
35+
var lookupStrings = lookupText?.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
36+
37+
// Ensure that we have valid lookupStrings to work with. Valid formats are:
38+
// "assemblyName"
39+
// "typeName, assemblyName"
40+
if (string.IsNullOrEmpty(lookupText) ||
41+
(lookupStrings.Length != 1 && lookupStrings.Length != 2))
3642
{
37-
throw new InvalidOperationException(
38-
Resources.FormatTagHelperDescriptorResolver_InvalidAddTagHelperLookupText(lookupText));
43+
throw new ArgumentException(
44+
Resources.FormatTagHelperDescriptorResolver_InvalidTagHelperLookupText(lookupText),
45+
nameof(lookupText));
3946
}
4047

41-
var tagHelperTypes = ResolveTagHelperTypes(lookupText);
48+
var tagHelperTypes = ResolveTagHelperTypes(lookupStrings);
4249

4350
// Convert types to TagHelperDescriptors
4451
var descriptors = tagHelperTypes.Select(TagHelperDescriptorFactory.CreateDescriptor);
4552

4653
return descriptors;
4754
}
4855

49-
private IEnumerable<Type> ResolveTagHelperTypes(string lookupText)
56+
private IEnumerable<Type> ResolveTagHelperTypes(string[] lookupStrings)
5057
{
51-
var lookupStrings = lookupText.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
52-
53-
// Ensure that we have valid lookupStrings to work with. Valid formats are:
54-
// "assemblyName"
55-
// "typeName, assemblyName"
56-
if (lookupStrings.Length != 1 && lookupStrings.Length != 2)
57-
{
58-
throw new InvalidOperationException(
59-
Resources.FormatTagHelperDescriptorResolver_InvalidAddTagHelperLookupText(lookupText));
60-
}
61-
6258
// Grab the assembly name from the lookup text strings. Due to our supported lookupText formats it will
6359
// always be the last element provided.
6460
var assemblyName = lookupStrings.Last().Trim();
6561

66-
// Resolve valid tag helper types form the assembly.
62+
// Resolve valid tag helper types from the assembly.
6763
var types = _typeResolver.Resolve(assemblyName);
6864

69-
// If the user provided a type name (lookupStrings.Length == 2) then we need to retrieve the value and
70-
// trim it.
71-
var typeName = lookupStrings.Length == 2 ? lookupStrings[0].Trim() : null;
72-
7365
// Check if the lookupText specifies a type to search for.
74-
if (typeName != null)
66+
if (lookupStrings.Length == 2)
7567
{
68+
// The user provided a type name retrieve the value and trim it.
69+
var typeName = lookupStrings[0].Trim();
70+
7671
types = types.Where(type =>
7772
string.Equals(type.Namespace + "." + type.Name, typeName, StringComparison.Ordinal));
7873
}

src/Microsoft.AspNet.Razor.Runtime/TagHelpers/TagHelperTypeResolver.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
1010
{
1111
/// <summary>
12-
/// Defines a class that locates valid <see cref="ITagHelper"/>s within an assembly.
12+
/// Class that locates valid <see cref="ITagHelper"/>s within an assembly.
1313
/// </summary>
1414
public class TagHelperTypeResolver
1515
{
@@ -32,8 +32,9 @@ public IEnumerable<Type> Resolve(string name)
3232
{
3333
if (string.IsNullOrEmpty(name))
3434
{
35-
throw new InvalidOperationException(
36-
Resources.TagHelperTypeResolver_AddTagHelperAssemblyNameCannotBeEmptyOrNull);
35+
throw new ArgumentException(
36+
Resources.TagHelperTypeResolver_TagHelperAssemblyNameCannotBeEmptyOrNull,
37+
nameof(name));
3738
}
3839

3940
var assemblyName = new AssemblyName(name);

src/Microsoft.AspNet.Razor/Parser/TagHelpers/TagHelperRegistrationVisitor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public override void VisitSpan(Span span)
3939
throw new InvalidOperationException(
4040
RazorResources.FormatTagHelpers_CannotUseDirectiveWithNoTagHelperDescriptorResolver(
4141
SyntaxConstants.CSharp.AddTagHelperKeyword,
42-
nameof(TagHelperDescriptorResolver),
42+
nameof(ITagHelperDescriptorResolver),
4343
nameof(RazorParser)));
4444
}
4545

src/Microsoft.AspNet.Razor/TagHelpers/ITagHelperDescriptorResolver.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
namespace Microsoft.AspNet.Razor.TagHelpers
77
{
88
/// <summary>
9-
/// Defines a contract used to resolve <see cref="TagHelperDescriptor"/>s.
9+
/// Contract used to resolve <see cref="TagHelperDescriptor"/>s.
1010
/// </summary>
1111
public interface ITagHelperDescriptorResolver
1212
{

src/Microsoft.AspNet.Razor/TagHelpers/TagHelperDescriptorComparer.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
namespace Microsoft.AspNet.Razor.TagHelpers
99
{
1010
/// <summary>
11-
/// Defines a an <see cref="IEqualityComparer{TagHelperDescriptor}"/> that is used to check equality between
11+
/// A <see cref="IEqualityComparer{TagHelperDescriptor}"/> that is used to check equality between
1212
/// two <see cref="TagHelperDescriptor"/>s.
1313
/// </summary>
1414
public class TagHelperDescriptorComparer : IEqualityComparer<TagHelperDescriptor>
@@ -18,7 +18,8 @@ public class TagHelperDescriptorComparer : IEqualityComparer<TagHelperDescriptor
1818
/// </summary>
1919
public static readonly TagHelperDescriptorComparer Default = new TagHelperDescriptorComparer();
2020

21-
protected TagHelperDescriptorComparer()
21+
// Internal for testing
22+
internal TagHelperDescriptorComparer()
2223
{
2324
}
2425

test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/CommonTagHelpers.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using Microsoft.AspNet.Razor.Runtime.TagHelpers;
5-
6-
namespace Microsoft.AspNet.Razor.Runtime.Test.TagHelpers
4+
namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
75
{
86
public class Valid_PlainTagHelper : ITagHelper
97
{
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System.Collections.Generic;
5+
using System.Linq;
6+
using Microsoft.AspNet.Razor.TagHelpers;
7+
using Microsoft.Internal.Web.Utils;
8+
9+
namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
10+
{
11+
public class CompleteTagHelperDescriptorComparer : TagHelperDescriptorComparer
12+
{
13+
public new static readonly CompleteTagHelperDescriptorComparer Default =
14+
new CompleteTagHelperDescriptorComparer();
15+
16+
private CompleteTagHelperDescriptorComparer()
17+
{
18+
}
19+
20+
public new bool Equals(TagHelperDescriptor descriptorX, TagHelperDescriptor descriptorY)
21+
{
22+
return base.Equals(descriptorX, descriptorY) &&
23+
descriptorX.Attributes.SequenceEqual(descriptorY.Attributes,
24+
CompleteTagHelperAttributeDescriptorComparer.Default);
25+
}
26+
27+
public new int GetHashCode(TagHelperDescriptor descriptor)
28+
{
29+
return HashCodeCombiner.Start()
30+
.Add(base.GetHashCode())
31+
.Add(descriptor.Attributes)
32+
.CombinedHash;
33+
}
34+
35+
private class CompleteTagHelperAttributeDescriptorComparer : IEqualityComparer<TagHelperAttributeDescriptor>
36+
{
37+
public static readonly CompleteTagHelperAttributeDescriptorComparer Default =
38+
new CompleteTagHelperAttributeDescriptorComparer();
39+
40+
private CompleteTagHelperAttributeDescriptorComparer()
41+
{
42+
}
43+
44+
public bool Equals(TagHelperAttributeDescriptor descriptorX, TagHelperAttributeDescriptor descriptorY)
45+
{
46+
return descriptorX.AttributeName == descriptorY.AttributeName &&
47+
descriptorX.AttributePropertyName == descriptorY.AttributePropertyName;
48+
}
49+
50+
public int GetHashCode(TagHelperAttributeDescriptor descriptor)
51+
{
52+
return HashCodeCombiner.Start()
53+
.Add(descriptor.AttributeName)
54+
.Add(descriptor.AttributePropertyName)
55+
.CombinedHash;
56+
}
57+
}
58+
}
59+
}

0 commit comments

Comments
 (0)