Addressed code review comments.

This commit is contained in:
N. Taylor Mullen 2014-11-05 15:35:21 -08:00 committed by NTaylorMullen
parent bad8b16a2a
commit 64a5b8ee22
12 changed files with 103 additions and 78 deletions

View File

@ -77,7 +77,7 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
attributeNameAttribute.Name :
property.Name;
return new TagHelperAttributeDescriptor(attributeName, property);
return new TagHelperAttributeDescriptor(attributeName, property.Name, property.PropertyType.FullName);
}
private static ContentBehavior GetContentBehavior(Type type)

View File

@ -48,18 +48,16 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
// Grab the assembly name from the lookup text strings. Due to our supported lookupText formats it will
// always be the last element provided.
var assemblyName = lookupStrings.Last().Trim();
// Retrieve all TagHelperDescriptors that exist within the given assemblyName.
var descriptors = ResolveDescriptorsInAssembly(assemblyName);
// Check if the lookupText specifies a type to search for.
if (lookupStrings.Length == 2)
{
// The user provided a type name retrieve it so we can prune our descriptors.
// The user provided a type name. Retrieve it so we can prune our descriptors.
var typeName = lookupStrings[0].Trim();
descriptors = descriptors.Where(descriptor =>
string.Equals(descriptor.TagHelperName, typeName, StringComparison.Ordinal));
string.Equals(descriptor.TypeName, typeName, StringComparison.Ordinal));
}
return descriptors;
@ -72,8 +70,9 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
/// <param name="assemblyName">
/// The name of the assembly to resolve <see cref="TagHelperDescriptor"/>s from.
/// </param>
/// <returns><see cref="TagHelperDescriptor"/>s that represent <see cref="ITagHelper"/>s from the given
/// <returns><see cref="TagHelperDescriptor"/>s for <see cref="ITagHelper"/>s from the given
/// <paramref name="assemblyName"/>.</returns>
// This is meant to be overridden by tooling to enable assembly level caching.
protected virtual IEnumerable<TagHelperDescriptor> ResolveDescriptorsInAssembly(string assemblyName)
{
// Resolve valid tag helper types from the assembly.

View File

@ -69,7 +69,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
RenderTagHelpersCreation(chunk);
var attributeDescriptors = tagHelperDescriptors.SelectMany(descriptor => descriptor.Attributes);
var boundHTMLAttributes = attributeDescriptors.Select(descriptor => descriptor.AttributeName);
var boundHTMLAttributes = attributeDescriptors.Select(descriptor => descriptor.Name);
var htmlAttributes = chunk.Attributes;
var unboundHTMLAttributes =
htmlAttributes.Where(htmlAttribute => !boundHTMLAttributes.Contains(htmlAttribute.Key,
@ -119,7 +119,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
internal static string GetVariableName(TagHelperDescriptor descriptor)
{
return "__" + descriptor.TagHelperName.Replace('.', '_');
return "__" + descriptor.TypeName.Replace('.', '_');
}
private void RenderBeginTagHelperScope(string tagName)
@ -156,7 +156,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
// Create the tag helper
_writer.WriteStartAssignment(tagHelperVariableName)
.WriteStartMethodInvocation(_tagHelperContext.CreateTagHelperMethodName,
tagHelperDescriptor.TagHelperName)
tagHelperDescriptor.TypeName)
.WriteEndMethodInvocation();
// Execution contexts are a runtime feature.
@ -184,12 +184,12 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
{
Chunk attributeValueChunk;
var providedAttribute = chunkAttributes.TryGetValue(attributeDescriptor.AttributeName,
var providedAttribute = chunkAttributes.TryGetValue(attributeDescriptor.Name,
out attributeValueChunk);
if (providedAttribute)
{
var attributeValueRecorded = htmlAttributeValues.ContainsKey(attributeDescriptor.AttributeName);
var attributeValueRecorded = htmlAttributeValues.ContainsKey(attributeDescriptor.Name);
// Bufferable attributes are attributes that can have Razor code inside of them.
var bufferableAttribute = IsStringAttribute(attributeDescriptor);
@ -211,7 +211,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
var valueAccessor = string.Format(CultureInfo.InvariantCulture,
"{0}.{1}",
tagHelperVariableName,
attributeDescriptor.AttributePropertyName);
attributeDescriptor.PropertyName);
_writer.WriteStartAssignment(valueAccessor);
@ -221,7 +221,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
// We only need to create attribute values once per HTML element (not once per tag helper).
// We're saving the value accessor so we can retrieve it later if there are more tag helpers that
// need the value.
htmlAttributeValues.Add(attributeDescriptor.AttributeName, valueAccessor);
htmlAttributeValues.Add(attributeDescriptor.Name, valueAccessor);
if (bufferableAttribute)
{
@ -245,7 +245,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
{
throw new InvalidOperationException(
RazorResources.FormatTagHelpers_AttributesThatAreNotStringsMustNotContainAtSymbols(
attributeDescriptor.AttributePropertyName));
attributeDescriptor.PropertyName));
}
// We aren't a bufferable attribute which means we have no Razor code in our value.
@ -267,7 +267,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
ExecutionContextVariableName,
_tagHelperContext.ExecutionContextAddTagHelperAttributeMethodName);
_writer.WriteStringLiteral(attributeDescriptor.AttributeName)
_writer.WriteStringLiteral(attributeDescriptor.Name)
.WriteParameterSeparator()
.Write(valueAccessor)
.WriteEndMethodInvocation();
@ -275,7 +275,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
else
{
// The attribute value has already been recorded, lets retrieve it from the stored value accessors.
_writer.Write(htmlAttributeValues[attributeDescriptor.AttributeName])
_writer.Write(htmlAttributeValues[attributeDescriptor.Name])
.WriteLine(";");
}
}
@ -497,7 +497,10 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
private static bool IsStringAttribute(TagHelperAttributeDescriptor attributeDescriptor)
{
return attributeDescriptor.AttributeTypeName == typeof(string).FullName;
return string.Equals(
attributeDescriptor.TypeName,
typeof(string).FullName,
StringComparison.Ordinal);
}
private static bool TryGetPlainTextValue(Chunk chunk, out string plainText)
@ -528,12 +531,12 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
{
public bool Equals(TagHelperAttributeDescriptor descriptorX, TagHelperAttributeDescriptor descriptorY)
{
return descriptorX.AttributeName.Equals(descriptorY.AttributeName, StringComparison.OrdinalIgnoreCase);
return string.Equals(descriptorX.Name, descriptorY.Name, StringComparison.OrdinalIgnoreCase);
}
public int GetHashCode(TagHelperAttributeDescriptor descriptor)
{
return StringComparer.OrdinalIgnoreCase.GetHashCode(descriptor.AttributeName);
return StringComparer.OrdinalIgnoreCase.GetHashCode(descriptor.Name);
}
}
}

View File

@ -54,11 +54,11 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
foreach (var descriptor in chunk.Descriptors)
{
if (!_declaredTagHelpers.Contains(descriptor.TagHelperName))
if (!_declaredTagHelpers.Contains(descriptor.TypeName))
{
_declaredTagHelpers.Add(descriptor.TagHelperName);
_declaredTagHelpers.Add(descriptor.TypeName);
WritePrivateField(descriptor.TagHelperName,
WritePrivateField(descriptor.TypeName,
CSharpTagHelperCodeRenderer.GetVariableName(descriptor),
value: null);
}

View File

@ -10,48 +10,45 @@ namespace Microsoft.AspNet.Razor.TagHelpers
/// </summary>
public class TagHelperAttributeDescriptor
{
/// <summary>
/// Instantiates a new <see cref="TagHelperAttributeDescriptor"/> class.
/// </summary>
/// <param name="attributeName">The HTML attribute name.</param>
/// <param name="propertyInfo">The <see cref="System.Reflection.PropertyInfo"/> for the tag
/// helper attribute</param>
public TagHelperAttributeDescriptor(string attributeName, PropertyInfo propertyInfo)
: this(attributeName, propertyInfo.Name, propertyInfo.PropertyType.FullName)
// Internal for testing
internal TagHelperAttributeDescriptor(string name, PropertyInfo propertyInfo)
: this(name, propertyInfo.Name, propertyInfo.PropertyType.FullName)
{
}
/// <summary>
/// Instantiates a new <see cref="TagHelperAttributeDescriptor"/> class.
/// Instantiates a new instance of the <see cref="TagHelperAttributeDescriptor"/> class.
/// </summary>
/// <param name="attributeName">The HTML attribute name.</param>
/// <param name="attributePropertyName">The name of the CLR property name that corresponds to the HTML
/// attribute name.</param>
/// <param name="attributeTypeName">
/// The full name of the <see cref="System.Type"/> that corresponds to the HTML attribute.
/// <param name="name">The HTML attribute name.</param>
/// <param name="propertyName">The name of the CLR property name that corresponds to the HTML
/// attribute.</param>
/// <param name="typeName">
/// The full name of the named (see <paramref name="propertyName"/>) property's
/// <see cref="System.Type"/>.
/// </param>
public TagHelperAttributeDescriptor(string attributeName,
string attributePropertyName,
string attributeTypeName)
public TagHelperAttributeDescriptor(string name,
string propertyName,
string typeName)
{
AttributeName = attributeName;
AttributePropertyName = attributePropertyName;
AttributeTypeName = attributeTypeName;
Name = name;
PropertyName = propertyName;
TypeName = typeName;
}
/// <summary>
/// The HTML attribute name.
/// </summary>
public string AttributeName { get; private set; }
public string Name { get; private set; }
/// <summary>
/// The name of the CLR property name that corresponds to the HTML attribute name.
/// </summary>
public string AttributePropertyName { get; private set; }
public string PropertyName { get; private set; }
/// <summary>
/// The full name of the <see cref="System.Type"/> that corresponds to the HTML attribute.
/// The full name of the named (see <see name="PropertyName"/>) property's
/// <see cref="System.Type"/>.
/// </summary>
public string AttributeTypeName { get; private set; }
public string TypeName { get; private set; }
}
}

View File

@ -1,10 +1,8 @@
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.Internal.Web.Utils;
namespace Microsoft.AspNet.Razor.TagHelpers
{
@ -18,17 +16,17 @@ namespace Microsoft.AspNet.Razor.TagHelpers
/// </summary>
/// <param name="tagName">The tag name that the tag helper targets. '*' indicates a catch-all
/// <see cref="TagHelperDescriptor"/> which applies to every HTML tag.</param>
/// <param name="tagHelperName">The full name of the tag helper class.</param>
/// <param name="tagHelperAssemblyName">The name of the assembly the tag helper was resolved from.</param>
/// <param name="typeName">The full name of the tag helper class.</param>
/// <param name="tagHelperAssemblyName">The name of the assembly containing the tag helper class.</param>
/// <param name="contentBehavior">The <see cref="TagHelpers.ContentBehavior"/>
/// of the tag helper.</param>
public TagHelperDescriptor([NotNull] string tagName,
[NotNull] string tagHelperName,
[NotNull] string tagHelperAssemblyName,
[NotNull] string typeName,
[NotNull] string assemblyName,
ContentBehavior contentBehavior)
: this(tagName,
tagHelperName,
tagHelperAssemblyName,
typeName,
assemblyName,
contentBehavior,
Enumerable.Empty<TagHelperAttributeDescriptor>())
{
@ -40,23 +38,22 @@ namespace Microsoft.AspNet.Razor.TagHelpers
/// </summary>
/// <param name="tagName">The tag name that the tag helper targets. '*' indicates a catch-all
/// <see cref="TagHelperDescriptor"/> which applies to every HTML tag.</param>
/// <param name="tagHelperName">The code class used to render the tag helper. Corresponds to
/// the tag helper's <see cref="Type.FullName"/>.</param>
/// <param name="tagHelperAssemblyName">The name of the assembly the tag helper was resolved from.</param>
/// <param name="typeName">The full name of the tag helper class.</param>
/// <param name="assemblyName">The name of the assembly containing the tag helper class.</param>
/// <param name="contentBehavior">The <see cref="TagHelpers.ContentBehavior"/>
/// of the tag helper.</param>
/// <param name="attributes">
/// The <see cref="TagHelperAttributeDescriptor"/>s to request from the HTML tag.
/// </param>
public TagHelperDescriptor([NotNull] string tagName,
[NotNull] string tagHelperName,
[NotNull] string tagHelperAssemblyName,
[NotNull] string typeName,
[NotNull] string assemblyName,
ContentBehavior contentBehavior,
[NotNull] IEnumerable<TagHelperAttributeDescriptor> attributes)
{
TagName = tagName;
TagHelperName = tagHelperName;
AssemblyName = tagHelperAssemblyName;
TypeName = typeName;
AssemblyName = assemblyName;
ContentBehavior = contentBehavior;
Attributes = new List<TagHelperAttributeDescriptor>(attributes);
}
@ -69,10 +66,10 @@ namespace Microsoft.AspNet.Razor.TagHelpers
/// <summary>
/// The full name of the tag helper class.
/// </summary>
public string TagHelperName { get; private set; }
public string TypeName { get; private set; }
/// <summary>
/// The name of the assembly the tag helper was resolved from.
/// The name of the assembly containing the tag helper class.
/// </summary>
public string AssemblyName { get; private set; }

View File

@ -28,12 +28,13 @@ namespace Microsoft.AspNet.Razor.TagHelpers
/// <returns><c>true</c> if <paramref name="descriptorX"/> and <paramref name="descriptorY"/> are equal,
/// <c>false</c> otherwise.</returns>
/// <remarks>
/// Determines equality based on <see cref="TagHelperDescriptor.TagHelperName"/>,
/// <see cref="TagHelperDescriptor.TagName"/> and <see cref="TagHelperDescriptor.ContentBehavior"/>.
/// Determines equality based on <see cref="TagHelperDescriptor.TypeName"/>,
/// <see cref="TagHelperDescriptor.AssemblyName"/>, <see cref="TagHelperDescriptor.TagName"/> and
/// <see cref="TagHelperDescriptor.ContentBehavior"/>.
/// </remarks>
public bool Equals(TagHelperDescriptor descriptorX, TagHelperDescriptor descriptorY)
{
return string.Equals(descriptorX.TagHelperName, descriptorY.TagHelperName, StringComparison.Ordinal) &&
return string.Equals(descriptorX.TypeName, descriptorY.TypeName, StringComparison.Ordinal) &&
string.Equals(descriptorX.TagName, descriptorY.TagName, StringComparison.OrdinalIgnoreCase) &&
string.Equals(descriptorX.AssemblyName, descriptorY.AssemblyName, StringComparison.Ordinal) &&
descriptorX.ContentBehavior == descriptorY.ContentBehavior;
@ -48,7 +49,7 @@ namespace Microsoft.AspNet.Razor.TagHelpers
{
return HashCodeCombiner.Start()
.Add(descriptor.TagName, StringComparer.OrdinalIgnoreCase)
.Add(descriptor.TagHelperName, StringComparer.Ordinal)
.Add(descriptor.TypeName, StringComparer.Ordinal)
.Add(descriptor.AssemblyName, StringComparer.Ordinal)
.Add(descriptor.ContentBehavior)
.CombinedHash;

View File

@ -43,17 +43,17 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
public bool Equals(TagHelperAttributeDescriptor descriptorX, TagHelperAttributeDescriptor descriptorY)
{
return descriptorX.AttributeName == descriptorY.AttributeName &&
descriptorX.AttributePropertyName == descriptorY.AttributePropertyName &&
descriptorX.AttributeTypeName == descriptorY.AttributeTypeName;
return descriptorX.Name == descriptorY.Name &&
descriptorX.PropertyName == descriptorY.PropertyName &&
descriptorX.TypeName == descriptorY.TypeName;
}
public int GetHashCode(TagHelperAttributeDescriptor descriptor)
{
return HashCodeCombiner.Start()
.Add(descriptor.AttributeName)
.Add(descriptor.AttributePropertyName)
.Add(descriptor.AttributeTypeName)
.Add(descriptor.Name)
.Add(descriptor.PropertyName)
.Add(descriptor.TypeName)
.CombinedHash;
}
}

View File

@ -98,8 +98,9 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
public void CreateDescriptor_BuildsDescriptorsFromSimpleTypes()
{
// Arrange
var assemblyName = typeof(object).GetTypeInfo().Assembly.GetName().Name;
var expectedDescriptor = new TagHelperDescriptor("Object", "System.Object", assemblyName, ContentBehavior.None);
var objectAssemblyName = typeof(object).GetTypeInfo().Assembly.GetName().Name;
var expectedDescriptor =
new TagHelperDescriptor("Object", "System.Object", objectAssemblyName, ContentBehavior.None);
// Act
var descriptors = TagHelperDescriptorFactory.CreateDescriptors(typeof(object));

View File

@ -15,6 +15,21 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
private static readonly string AssemblyName =
typeof(TagHelperDescriptorFactoryTest).GetTypeInfo().Assembly.GetName().Name;
[Theory]
[InlineData("MyType, MyAssembly", "MyAssembly")]
[InlineData("MyAssembly2", "MyAssembly2")]
public void Resolve_AllowsOverridenResolveDescriptorsInAssembly(string lookupText, string expectedAssemblyName)
{
// Arrange
var tagHelperDescriptorResolver = new TestTagHelperDescriptorResolver();
// Act
tagHelperDescriptorResolver.Resolve(lookupText);
// Assert
Assert.Equal(expectedAssemblyName, tagHelperDescriptorResolver.CalledWithAssemblyName);
}
[Fact]
public void DescriptorResolver_DoesNotReturnInvalidTagHelpersWhenSpecified()
{
@ -193,5 +208,17 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers
return types?.Select(type => type.GetTypeInfo()) ?? Enumerable.Empty<TypeInfo>();
}
}
private class TestTagHelperDescriptorResolver : TagHelperDescriptorResolver
{
public string CalledWithAssemblyName { get; set; }
protected override IEnumerable<TagHelperDescriptor> ResolveDescriptorsInAssembly(string assemblyName)
{
CalledWithAssemblyName = assemblyName;
return Enumerable.Empty<TagHelperDescriptor>();
}
}
}
}

View File

@ -26,7 +26,7 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
}
[Fact]
public void TagHelperDescriptorProvider_GetTagHelpersDoesntReturnNonCatchAllTagsForCatchAll()
public void TagHelperDescriptorProvider_GetTagHelpersDoesNotReturnNonCatchAllTagsForCatchAll()
{
// Arrange
var divDescriptor = new TagHelperDescriptor("div", "foo1", "SomeAssembly", ContentBehavior.None);

View File

@ -293,7 +293,7 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
[Theory]
[MemberData(nameof(TextTagsBlockData))]
public void TagHelperParseTreeRewriter_DoesntRewriteTextTagTransitionTagHelpers(
public void TagHelperParseTreeRewriter_DoesNotRewriteTextTagTransitionTagHelpers(
string documentContent,
MarkupBlock expectedOutput)
{
@ -407,7 +407,7 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
[Theory]
[MemberData(nameof(SpecialTagsBlockData))]
public void TagHelperParseTreeRewriter_DoesntRewriteSpecialTagTagHelpers(
public void TagHelperParseTreeRewriter_DoesNotRewriteSpecialTagTagHelpers(
string documentContent,
MarkupBlock expectedOutput)
{