From de95f67400c8f20cf9cfe063c291720bf7d71cce Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Fri, 24 Apr 2015 22:57:41 -0700 Subject: [PATCH] Add `HtmlAttributeNotBoundAttribute` - #182 - ignore otherwise-bound (i.e. `public`) properties in tag helpers nits: - add more `TagHelperDescriptorFactory` tests e.g. of `internal` properties - remove `private` setter from `HtmlAttributeNameAttribute` - use `null` propagation to shorten `IsAccessibleProperty` expression - clean up some trailing whitespace --- .../TagHelpers/HtmlAttributeNameAttribute.cs | 2 +- .../HtmlAttributeNotBoundAttribute.cs | 21 +++++ .../TagHelpers/TagHelperDescriptorFactory.cs | 14 ++-- .../TagHelpers/CommonTagHelpers.cs | 5 +- .../TagHelperDescriptorFactoryTest.cs | 77 +++++++++++++++++-- 5 files changed, 104 insertions(+), 15 deletions(-) create mode 100644 src/Microsoft.AspNet.Razor.Runtime/TagHelpers/HtmlAttributeNotBoundAttribute.cs diff --git a/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/HtmlAttributeNameAttribute.cs b/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/HtmlAttributeNameAttribute.cs index ccd2948a61..faad959a28 100644 --- a/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/HtmlAttributeNameAttribute.cs +++ b/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/HtmlAttributeNameAttribute.cs @@ -28,6 +28,6 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers /// /// HTML attribute name of the associated property. /// - public string Name { get; private set; } + public string Name { get; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/HtmlAttributeNotBoundAttribute.cs b/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/HtmlAttributeNotBoundAttribute.cs new file mode 100644 index 0000000000..9697af06c1 --- /dev/null +++ b/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/HtmlAttributeNotBoundAttribute.cs @@ -0,0 +1,21 @@ +// 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; + +namespace Microsoft.AspNet.Razor.Runtime.TagHelpers +{ + /// + /// Indicates the associated property should not be bound to HTML attributes. + /// + [AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = false)] + public sealed class HtmlAttributeNotBoundAttribute : Attribute + { + /// + /// Instantiates a new instance of the class. + /// + public HtmlAttributeNotBoundAttribute() + { + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/TagHelperDescriptorFactory.cs b/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/TagHelperDescriptorFactory.cs index 6eb7ddeae8..0bb8f9aff5 100644 --- a/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/TagHelperDescriptorFactory.cs +++ b/src/Microsoft.AspNet.Razor.Runtime/TagHelpers/TagHelperDescriptorFactory.cs @@ -203,7 +203,7 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers } private static IEnumerable GetAttributeDescriptors( - Type type, + Type type, ErrorSink errorSink) { var accessibleProperties = type.GetRuntimeProperties().Where(IsAccessibleProperty); @@ -226,12 +226,12 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers Type parentType, ErrorSink errorSink) { - // data-* attributes are explicitly not implemented by user agents and are not intended for use on + // data-* attributes are explicitly not implemented by user agents and are not intended for use on // the server; therefore it's invalid for TagHelpers to bind to them. if (attributeDescriptor.Name.StartsWith(DataDashPrefix, StringComparison.OrdinalIgnoreCase)) { errorSink.OnError( - SourceLocation.Zero, + SourceLocation.Zero, Resources.FormatTagHelperDescriptorFactory_InvalidBoundAttributeName( attributeDescriptor.PropertyName, parentType.FullName, @@ -255,10 +255,10 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers private static bool IsAccessibleProperty(PropertyInfo property) { - return property.GetMethod != null && - property.GetMethod.IsPublic && - property.SetMethod != null && - property.SetMethod.IsPublic; + // Accessible properties are those with public getters and setters and without [HtmlAttributeNotBound]. + return property.GetMethod?.IsPublic == true && + property.SetMethod?.IsPublic == true && + property.GetCustomAttribute(inherit: false) == null; } /// diff --git a/test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/CommonTagHelpers.cs b/test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/CommonTagHelpers.cs index d9edeca8ca..e16d08a4a8 100644 --- a/test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/CommonTagHelpers.cs +++ b/test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/CommonTagHelpers.cs @@ -23,10 +23,13 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers public string InvalidNoSetAttribute { get { return string.Empty; } } } - public class PrivateAccessorTagHelper : TagHelper + public class NonPublicAccessorTagHelper : TagHelper { public string ValidAttribute { get; set; } public string InvalidPrivateSetAttribute { get; private set; } public string InvalidPrivateGetAttribute { private get; set; } + protected string InvalidProtectedAttribute { get; set; } + internal string InvalidInternalAttribute { get; set; } + protected internal string InvalidProtectedInternalAttribute { get; set; } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/TagHelperDescriptorFactoryTest.cs b/test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/TagHelperDescriptorFactoryTest.cs index f6b29b5947..162010e6d7 100644 --- a/test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/TagHelperDescriptorFactoryTest.cs +++ b/test/Microsoft.AspNet.Razor.Runtime.Test/TagHelpers/TagHelperDescriptorFactoryTest.cs @@ -443,11 +443,11 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers { // Arrange var errorSink = new ErrorSink(); - var validProperty = typeof(PrivateAccessorTagHelper).GetProperty( - nameof(PrivateAccessorTagHelper.ValidAttribute)); + var validProperty = typeof(NonPublicAccessorTagHelper).GetProperty( + nameof(NonPublicAccessorTagHelper.ValidAttribute)); var expectedDescriptor = new TagHelperDescriptor( - "private-accessor", - typeof(PrivateAccessorTagHelper).FullName, + "non-public-accessor", + typeof(NonPublicAccessorTagHelper).FullName, AssemblyName, new[] { new TagHelperAttributeDescriptor( @@ -457,7 +457,7 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers // Act var descriptors = TagHelperDescriptorFactory.CreateDescriptors( AssemblyName, - typeof(PrivateAccessorTagHelper), + typeof(NonPublicAccessorTagHelper), errorSink); // Assert @@ -466,6 +466,51 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers Assert.Equal(expectedDescriptor, descriptor, CaseSensitiveTagHelperDescriptorComparer.Default); } + [Fact] + public void CreateDescriptor_DoesNotIncludePropertiesWithNotBound() + { + // Arrange + var errorSink = new ErrorSink(); + var boundProperty = typeof(NotBoundAttributeTagHelper).GetProperty( + nameof(NotBoundAttributeTagHelper.BoundProperty)); + var expectedDescriptor = new TagHelperDescriptor( + "not-bound-attribute", + typeof(NotBoundAttributeTagHelper).FullName, + AssemblyName, + new[] + { + new TagHelperAttributeDescriptor("bound-property", boundProperty) + }); + + // Act + var descriptors = TagHelperDescriptorFactory.CreateDescriptors( + AssemblyName, + typeof(NotBoundAttributeTagHelper), + errorSink); + + // Assert + Assert.Empty(errorSink.Errors); + var descriptor = Assert.Single(descriptors); + Assert.Equal(expectedDescriptor, descriptor, CaseSensitiveTagHelperDescriptorComparer.Default); + } + + [Fact(Skip = "#364")] + public void CreateDescriptor_AddsErrorForTagHelperWithDuplicateAttributeNames() + { + // Arrange + var errorSink = new ErrorSink(); + + // Act + var descriptors = TagHelperDescriptorFactory.CreateDescriptors( + AssemblyName, + typeof(DuplicateAttributeNameTagHelper), + errorSink); + + // Assert + Assert.Empty(descriptors); + var error = Assert.Single(errorSink.Errors); + } + [Fact] public void CreateDescriptor_ResolvesMultipleTagHelperDescriptorsFromSingleType() { @@ -867,7 +912,7 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers { var errorFormat = "Invalid tag helper bound property '{0}.{1}'. Tag helpers cannot bind to HTML " + "attributes beginning with 'data-'."; - + // type, expectedAttributeDescriptors, expectedErrors return new TheoryData, string[]> { @@ -1049,6 +1094,26 @@ namespace Microsoft.AspNet.Razor.Runtime.TagHelpers { } + private class DuplicateAttributeNameTagHelper + { + public string MyNameIsLegion { get; set; } + + [HtmlAttributeName("my-name-is-legion")] + public string Fred { get; set; } + } + + private class NotBoundAttributeTagHelper + { + public string BoundProperty { get; set; } + + [HtmlAttributeNotBound] + public string NotBoundProperty { get; set; } + + [HtmlAttributeName("unused")] + [HtmlAttributeNotBound] + public string NamedNotBoundProperty { get; set; } + } + private class OverriddenAttributeTagHelper { [HtmlAttributeName("SomethingElse")]