Add `AddHtmlAttributeValues` for `TagHelper`s.

- Refactored `WriteAttributeTo` to allow re-use of some of the core attribute writing logic. The refactoring was based on removing the `Begin`/`EndContext` for instrumentation bits which isn't valid in a `TagHelper` attribute scenario.
- Added unit tests to validate attributes are properly added to `TagHelperExecutionContext`.
- Added functional test to validate everything is output as expected with dynamic attributes (encoded and non-encoded).

aspnet/Razor#247

src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs

@@ -524,25 +524,26 @@ namespace Microsoft.AspNet.Mvc.Razor
                 // Explicitly empty attribute, so write the prefix
                 WritePositionTaggedLiteral(writer, prefix);
             }
-            else if (values.Length == 1 && values[0].Prefix == "" &&
+            else if (IsSingleBoolFalseOrNullValue(values))
-                (values[0].Value.Value is bool || values[0].Value.Value == null))
+            {
+                // Value is either null or the bool 'false' with no prefix; don't render the attribute.
+                return;
+            }
+            else if (UseAttributeNameAsValue(values))
             {
-                // Value is either null or a bool with no prefix.
                 var attributeValue = values[0];
                 var positionTaggedAttributeValue = attributeValue.Value;
 
-                if (positionTaggedAttributeValue.Value == null || !(bool)positionTaggedAttributeValue.Value)
-                {
-                    // The value is null or just the bool 'false', don't write anything.
-                    return;
-                }
-
                 WritePositionTaggedLiteral(writer, prefix);
 
                 var sourceLength = suffix.Position - positionTaggedAttributeValue.Position;
+                var nameAttributeValue = new AttributeValue(
+                    attributeValue.Prefix,
+                    new PositionTagged<object>(name, attributeValue.Value.Position),
+                    literal: attributeValue.Literal);
 
                 // The value is just the bool 'true', write the attribute name instead of the string 'True'.
-                WriteAttributeValue(writer, attributeValue, name, sourceLength);
+                WriteAttributeValue(writer, nameAttributeValue, sourceLength);
             }
             else
             {
@@ -568,15 +569,60 @@ namespace Microsoft.AspNet.Mvc.Razor
                     // Calculate length of the source span by the position of the next value (or suffix)
                     var sourceLength = next.Position - attributeValue.Value.Position;
 
-                    var stringValue = positionTaggedAttributeValue.Value as string;
+                    WriteAttributeValue(writer, attributeValue, sourceLength);
-
-                    WriteAttributeValue(writer, attributeValue, stringValue, sourceLength);
                 }
             }
 
             WritePositionTaggedLiteral(writer, suffix);
         }
 
+        public void AddHtmlAttributeValues(
+            string attributeName,
+            TagHelperExecutionContext executionContext,
+            params AttributeValue[] values)
+        {
+            if (IsSingleBoolFalseOrNullValue(values))
+            {
+                // The first value was 'null' or 'false' indicating that we shouldn't render the attribute. The
+                // attribute is treated as a TagHelper attribute so it's only available in
+                // TagHelperContext.AllAttributes for TagHelper authors to see (if they want to see why the attribute
+                // was removed from TagHelperOutput.Attributes).
+                executionContext.AddTagHelperAttribute(
+                    attributeName,
+                    values[0].Value.Value?.ToString() ?? string.Empty);
+
+                return;
+            }
+            else if (UseAttributeNameAsValue(values))
+            {
+                executionContext.AddHtmlAttribute(attributeName, attributeName);
+            }
+            else
+            {
+                var valueBuffer = new StringCollectionTextWriter(Output.Encoding);
+
+                foreach (var value in values)
+                {
+                    if (value.Value.Value == null)
+                    {
+                        // Skip null values
+                        continue;
+                    }
+
+                    if (!string.IsNullOrEmpty(value.Prefix))
+                    {
+                        WriteLiteralTo(valueBuffer, value.Prefix);
+                    }
+
+                    WriteUnprefixedAttributeValueTo(valueBuffer, value);
+                }
+
+                var htmlString = new HtmlString(valueBuffer.ToString());
+
+                executionContext.AddHtmlAttribute(attributeName, htmlString);
+            }
+        }
+
         public virtual string Href([NotNull] string contentPath)
         {
             if (_urlHelper == null)
@@ -587,14 +633,8 @@ namespace Microsoft.AspNet.Mvc.Razor
             return _urlHelper.Content(contentPath);
         }
 
-        private void WriteAttributeValue(
+        private void WriteAttributeValue(TextWriter writer, AttributeValue attributeValue, int sourceLength)
-            TextWriter writer,
-            AttributeValue attributeValue,
-            string stringValue,
-            int sourceLength)
         {
-            var positionTaggedAttributeValue = attributeValue.Value;
-
             if (!string.IsNullOrEmpty(attributeValue.Prefix))
             {
                 WritePositionTaggedLiteral(writer, attributeValue.Prefix);
@@ -602,8 +642,17 @@ namespace Microsoft.AspNet.Mvc.Razor
 
             BeginContext(attributeValue.Value.Position, sourceLength, isLiteral: attributeValue.Literal);
 
-            // The extra branching here is to ensure that we call the Write*To(string) overload where
+            WriteUnprefixedAttributeValueTo(writer, attributeValue);
-            // possible.
+
+            EndContext();
+        }
+
+        private void WriteUnprefixedAttributeValueTo(TextWriter writer, AttributeValue attributeValue)
+        {
+            var positionTaggedAttributeValue = attributeValue.Value;
+            var stringValue = positionTaggedAttributeValue.Value as string;
+
+            // The extra branching here is to ensure that we call the Write*To(string) overload where possible.
             if (attributeValue.Literal && stringValue != null)
             {
                 WriteLiteralTo(writer, stringValue);
@@ -620,8 +669,6 @@ namespace Microsoft.AspNet.Mvc.Razor
             {
                 WriteTo(writer, positionTaggedAttributeValue.Value);
             }
-
-            EndContext();
         }
 
         private void WritePositionTaggedLiteral(TextWriter writer, string value, int position)
@@ -846,6 +893,32 @@ namespace Microsoft.AspNet.Mvc.Razor
             return HtmlString.Empty;
         }
 
+        private bool IsSingleBoolFalseOrNullValue(AttributeValue[] values)
+        {
+            if (values.Length == 1 && string.IsNullOrEmpty(values[0].Prefix) &&
+                (values[0].Value.Value is bool || values[0].Value.Value == null))
+            {
+                var attributeValue = values[0];
+                var positionTaggedAttributeValue = attributeValue.Value;
+
+                if (positionTaggedAttributeValue.Value == null || !(bool)positionTaggedAttributeValue.Value)
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        private bool UseAttributeNameAsValue(AttributeValue[] values)
+        {
+            // If the value is just the bool 'true', use the attribute name as the value.
+            return values.Length == 1 &&
+                string.IsNullOrEmpty(values[0].Prefix) &&
+                values[0].Value.Value is bool &&
+                (bool)values[0].Value.Value;
+        }
+
         private void EnsureMethodCanBeInvoked(string methodName)
         {
             if (PreviousSectionWriters == null)

test/Microsoft.AspNet.Mvc.FunctionalTests/TagHelpersTest.cs

@@ -11,6 +11,7 @@ using System.Threading.Tasks;
 using BasicWebSite;
 using Microsoft.AspNet.Builder;
 using Microsoft.Framework.DependencyInjection;
+using Microsoft.Framework.WebEncoders;
 using Xunit;
 
 namespace Microsoft.AspNet.Mvc.FunctionalTests
@@ -32,6 +33,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
         [InlineData("Index")]
         [InlineData("About")]
         [InlineData("Help")]
+        [InlineData("UnboundDynamicAttributes")]
         public async Task CanRenderViewsWithTagHelpers(string action)
         {
             // Arrange
@@ -58,6 +60,37 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
 #endif
         }
 
+        [Fact]
+        public async Task CanRenderViewsWithTagHelpersAndUnboundDynamicAttributes_Encoded()
+        {
+            // Arrange
+            var server = TestHelper.CreateServer(_app, SiteName, services =>
+            {
+                _configureServices(services);
+                services.AddTransient<IHtmlEncoder, TestHtmlEncoder>();
+            });
+            var client = server.CreateClient();
+            var expectedMediaType = MediaTypeHeaderValue.Parse("text/html; charset=utf-8");
+            var outputFile = "compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.Encoded.html";
+            var expectedContent =
+                await ResourceFile.ReadResourceAsync(_resourcesAssembly, outputFile, sourceFile: false);
+
+            // Act
+            // The host is not important as everything runs in memory and tests are isolated from each other.
+            var response = await client.GetAsync("http://localhost/Home/UnboundDynamicAttributes");
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            Assert.Equal(expectedMediaType, response.Content.Headers.ContentType);
+
+            var responseContent = await response.Content.ReadAsStringAsync();
+#if GENERATE_BASELINES
+            ResourceFile.UpdateFile(_resourcesAssembly, outputFile, expectedContent, responseContent);
+#else
+            Assert.Equal(expectedContent, responseContent, ignoreLineEndingDifferences: true);
+#endif
+        }
+
         public static TheoryData TagHelpersAreInheritedFromViewImportsPagesData
         {
             get

test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.Encoded.html

@@ -0,0 +1,17 @@
+
+
+
+<input checked="HtmlEncode[[checked]]" processed />
+<input checked="HtmlEncode[[checked]]" processed />
+<input processed />
+<input processed />
+<input checked="  HtmlEncode[[True]]    " processed />
+<input checked="  HtmlEncode[[False]]    " processed />
+<input checked="    HtmlEncode[[value]]: HtmlEncode[[True]]   " processed />
+<input checked="    value: HtmlEncode[[False]]   " processed />
+<input checked="HtmlEncode[[True]] HtmlEncode[[True]]" processed />
+<input checked="   HtmlEncode[[False]]  HtmlEncode[[True]]" processed />
+<input processed />
+<input checked="" processed />
+<input checked="   " processed />
+<input checked=" HtmlEncode[[value]] HtmlEncode[[True]]" processed />

test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.html

@@ -0,0 +1,17 @@
+
+
+
+<input checked="checked" processed />
+<input checked="checked" processed />
+<input processed />
+<input processed />
+<input checked="  True    " processed />
+<input checked="  False    " processed />
+<input checked="    value: True   " processed />
+<input checked="    value: False   " processed />
+<input checked="True True" processed />
+<input checked="   False  True" processed />
+<input processed />
+<input checked="" processed />
+<input checked="   " processed />
+<input checked=" value True" processed />

test/Microsoft.AspNet.Mvc.Razor.Test/RazorPageTest.cs

@@ -742,6 +742,189 @@ namespace Microsoft.AspNet.Mvc.Razor
             context.Verify();
         }
 
+        public static TheoryData AddHtmlAttributeValues_ValueData
+        {
+            get
+            {
+                // attributeValues, expectedValue
+                return new TheoryData<AttributeValue[], string>
+                {
+                    {
+                        new AttributeValue[] {
+                            new AttributeValue(
+                                new PositionTagged<string>("", 9),
+                                new PositionTagged<object>("Hello", 9),
+                                literal: true)
+                        },
+                        "Hello"
+                    },
+                    {
+                        new AttributeValue[] {
+                            new AttributeValue(
+                                new PositionTagged<string>(" ", 9),
+                                new PositionTagged<object>("Hello", 10),
+                                literal: true)
+                        },
+                        " Hello"
+                    },
+                    {
+                        new AttributeValue[] {
+                            new AttributeValue(
+                                new PositionTagged<string>(" ", 9),
+                                new PositionTagged<object>(null, 10),
+                                literal: false)
+                        },
+                        ""
+                    },
+                    {
+                        new AttributeValue[] {
+                            new AttributeValue(
+                                new PositionTagged<string>(" ", 9),
+                                new PositionTagged<object>(false, 10),
+                                literal: false)
+                        },
+                        " HtmlEncode[[False]]"
+                    },
+                    {
+                        new AttributeValue[] {
+                            new AttributeValue(
+                                new PositionTagged<string>("  ", 9),
+                                new PositionTagged<object>(true, 11),
+                                literal: false),
+                            new AttributeValue(
+                                new PositionTagged<string>("  ", 15),
+                                new PositionTagged<object>("abcd", 17),
+                                literal: true),
+                        },
+                        "  HtmlEncode[[True]]  abcd"
+                    },
+
+                    {
+                        new AttributeValue[] {
+                            new AttributeValue(
+                                new PositionTagged<string>("", 9),
+                                new PositionTagged<object>("prefix", 9),
+                                literal: true),
+                            new AttributeValue(
+                                new PositionTagged<string>("  ", 15),
+                                new PositionTagged<object>(null, 17),
+                                literal: false),
+                            new AttributeValue(
+                                new PositionTagged<string>(" ", 21),
+                                new PositionTagged<object>("suffix", 22),
+                                literal: false),
+                        },
+                        "prefix HtmlEncode[[suffix]]"
+                    },
+                };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(AddHtmlAttributeValues_ValueData))]
+        public void AddHtmlAttributeValues_AddsToHtmlAttributesAsExpected(
+            AttributeValue[] attributeValues,
+            string expectedValue)
+        {
+            // Arrange
+            var page = CreatePage(p => { });
+            page.HtmlEncoder = new CommonTestEncoder();
+            var executionContext = new TagHelperExecutionContext(
+                "p",
+                selfClosing: false,
+                items: null,
+                uniqueId: string.Empty,
+                executeChildContentAsync: () => Task.FromResult(result: true),
+                startTagHelperWritingScope: () => { },
+                endTagHelperWritingScope: () => new DefaultTagHelperContent());
+
+            // Act
+            page.AddHtmlAttributeValues("someattr", executionContext, attributeValues);
+
+            // Assert
+            var htmlAttribute = Assert.Single(executionContext.HTMLAttributes);
+            Assert.Equal("someattr", htmlAttribute.Name, StringComparer.Ordinal);
+            Assert.IsType<HtmlString>(htmlAttribute.Value);
+            Assert.Equal(expectedValue, htmlAttribute.Value.ToString(), StringComparer.Ordinal);
+            Assert.False(htmlAttribute.Minimized);
+            var allAttribute = Assert.Single(executionContext.AllAttributes);
+            Assert.Equal("someattr", allAttribute.Name, StringComparer.Ordinal);
+            Assert.IsType<HtmlString>(allAttribute.Value);
+            Assert.Equal(expectedValue, allAttribute.Value.ToString(), StringComparer.Ordinal);
+            Assert.False(allAttribute.Minimized);
+        }
+
+        [Theory]
+        [InlineData(null, "")]
+        [InlineData(false, "False")]
+        public void AddHtmlAttributeValues_OnlyAddsToAllAttributesWhenAttributeRemoved(
+            object attributeValue,
+            string expectedValue)
+        {
+            // Arrange
+            var page = CreatePage(p => { });
+            page.HtmlEncoder = new CommonTestEncoder();
+            var executionContext = new TagHelperExecutionContext(
+                "p",
+                selfClosing: false,
+                items: null,
+                uniqueId: string.Empty,
+                executeChildContentAsync: () => Task.FromResult(result: true),
+                startTagHelperWritingScope: () => { },
+                endTagHelperWritingScope: () => new DefaultTagHelperContent());
+
+            // Act
+            page.AddHtmlAttributeValues(
+                "someattr",
+                executionContext,
+                new AttributeValue(
+                    prefix: new PositionTagged<string>(string.Empty, 9),
+                    value: new PositionTagged<object>(attributeValue, 9),
+                    literal: false));
+
+            // Assert
+            Assert.Empty(executionContext.HTMLAttributes);
+            var attribute = Assert.Single(executionContext.AllAttributes);
+            Assert.Equal("someattr", attribute.Name, StringComparer.Ordinal);
+            Assert.Equal(expectedValue, (string)attribute.Value, StringComparer.Ordinal);
+            Assert.False(attribute.Minimized);
+        }
+
+        [Fact]
+        public void AddHtmlAttributeValues_AddsAttributeNameAsValueWhenValueIsUnprefixedTrue()
+        {
+            // Arrange
+            var page = CreatePage(p => { });
+            page.HtmlEncoder = new CommonTestEncoder();
+            var executionContext = new TagHelperExecutionContext(
+                "p",
+                selfClosing: false,
+                items: null,
+                uniqueId: string.Empty,
+                executeChildContentAsync: () => Task.FromResult(result: true),
+                startTagHelperWritingScope: () => { },
+                endTagHelperWritingScope: () => new DefaultTagHelperContent());
+
+            // Act
+            page.AddHtmlAttributeValues(
+                "someattr",
+                executionContext,
+                new AttributeValue(
+                    prefix: new PositionTagged<string>(string.Empty, 9),
+                    value: new PositionTagged<object>(true, 9),
+                    literal: false));
+
+            // Assert
+            var htmlAttribute = Assert.Single(executionContext.HTMLAttributes);
+            Assert.Equal("someattr", htmlAttribute.Name, StringComparer.Ordinal);
+            Assert.Equal("someattr", (string)htmlAttribute.Value, StringComparer.Ordinal);
+            Assert.False(htmlAttribute.Minimized);
+            var allAttribute = Assert.Single(executionContext.AllAttributes);
+            Assert.Equal("someattr", allAttribute.Name, StringComparer.Ordinal);
+            Assert.Equal("someattr", (string)allAttribute.Value, StringComparer.Ordinal);
+            Assert.False(allAttribute.Minimized);
+        }
+
         public static TheoryData<AttributeValue[], string> WriteAttributeData
         {
             get

test/WebSites/TagHelpersWebSite/Controllers/HomeController.cs

@@ -30,6 +30,11 @@ namespace TagHelpersWebSite.Controllers
             return View();
         }
 
+        public ViewResult UnboundDynamicAttributes()
+        {
+            return View();
+        }
+
         public ViewResult NestedViewImportsTagHelper()
         {
             return View();

test/WebSites/TagHelpersWebSite/TagHelpers/AddProcessedAttributeTagHelper.cs

@@ -0,0 +1,16 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Microsoft.AspNet.Razor.Runtime.TagHelpers;
+
+namespace TagHelpersWebSite.TagHelpers
+{
+    [TargetElement("input")]
+    public class AddProcessedAttributeTagHelper : TagHelper
+    {
+        public override void Process(TagHelperContext context, TagHelperOutput output)
+        {
+            output.Attributes.Add(new TagHelperAttribute { Name = "processed", Minimized = true });
+        }
+    }
+}

test/WebSites/TagHelpersWebSite/Views/Home/UnboundDynamicAttributes.cshtml

@@ -0,0 +1,23 @@
+@addTagHelper "TagHelpersWebSite.TagHelpers.AddProcessedAttributeTagHelper, TagHelpersWebSite"
+
+@{
+    Layout = null;
+    var trueVar = true;
+    var falseVar = false;
+    var stringVar = "value";
+    string nullVar = null;
+}
+<input checked="@true" />
+<input checked="@trueVar" />
+<input checked="@false" />
+<input checked="@falseVar" />
+<input checked="  @true    " />
+<input checked="  @falseVar    " />
+<input checked="    @stringVar: @trueVar   " />
+<input checked="    value: @false   " />
+<input checked="@true @trueVar" />
+<input checked="   @falseVar  @true" />
+<input checked="@null" />
+<input checked="  @nullVar" />
+<input checked="@nullVar   " />
+<input checked="  @null @stringVar @trueVar" />