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

Commit

Permalink
Fix empty attribute projections for TagHelpers.
Browse files Browse the repository at this point in the history
- Added TagHelperParseTreeRewriter tests, attempted to cover all empty attribute edge cases.
- Added Codegen tests to validate output and DesignTimeLineMappings.

#271
  • Loading branch information
NTaylorMullen committed Feb 2, 2015
1 parent efab52c commit 7afd78b
Show file tree
Hide file tree
Showing 6 changed files with 365 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using Microsoft.AspNet.Razor.Generator;
using Microsoft.AspNet.Razor.Parser.SyntaxTree;
Expand Down Expand Up @@ -90,10 +91,15 @@ private static bool TryParseSpan(
EditHandler = span.EditHandler,
Kind = span.Kind
};

// Will contain symbols that represent a single attribute value: <input| class="btn"| />
var htmlSymbols = span.Symbols.OfType<HtmlSymbol>().ToArray();
var capturedAttributeValueStart = false;
var attributeValueStartLocation = span.Start;
var symbolOffset = 1;

// The symbolOffset is initialized to 0 to expect worst case: "class=". If a quote is found later on for
// the attribute value the symbolOffset is adjusted accordingly.
var symbolOffset = 0;
string name = null;

// Iterate down through the symbols to find the name and the start of the value.
Expand All @@ -104,6 +110,11 @@ private static bool TryParseSpan(

if (afterEquals)
{
// We've captured all leading whitespace, the attribute name, and an equals with an optional
// quote/double quote. We're now at: " asp-for='|...'" or " asp-for=|..."
// The goal here is to capture all symbols until the end of the attribute. Note this will not
// consume an ending quote due to the symbolOffset.

// When symbols are accepted into SpanBuilders, their locations get altered to be offset by the
// parent which is why we need to mark our start location prior to adding the symbol.
// This is needed to know the location of the attribute value start within the document.
Expand All @@ -118,13 +129,24 @@ private static bool TryParseSpan(
}
else if (name == null && symbol.Type == HtmlSymbolType.Text)
{
// We've captured all leading whitespace prior to the attribute name.
// We're now at: " |asp-for='...'" or " |asp-for=..."
// The goal here is to capture the attribute name.

name = symbol.Content;
attributeValueStartLocation = SourceLocation.Advance(span.Start, name);
attributeValueStartLocation = SourceLocation.Advance(attributeValueStartLocation, name);
}
else if (symbol.Type == HtmlSymbolType.Equals)
{
// We've found an '=' symbol, this means that the coming symbols will either be a quote
// or value (in the case that the value is unquoted).
Debug.Assert(
name != null,
"Name should never be null here. The parser should guaruntee an attribute has a name.");

// We've captured all leading whitespace and the attribute name.
// We're now at: " asp-for|='...'" or " asp-for|=..."
// The goal here is to consume the equal sign and the optional single/double-quote.

// The coming symbols will either be a quote or value (in the case that the value is unquoted).
// Spaces after/before the equal symbol are not yet supported:
// https://github.com/aspnet/Razor/issues/123

Expand All @@ -134,27 +156,38 @@ private static bool TryParseSpan(
SourceLocation symbolStartLocation;

// Check for attribute start values, aka single or double quote
if (IsQuote(htmlSymbols[i + 1]))
if ((i + 1) < htmlSymbols.Length && IsQuote(htmlSymbols[i + 1]))
{
// Move past the attribute start so we can accept the true value.
i++;
symbolStartLocation = htmlSymbols[i + 1].Start;
symbolStartLocation = htmlSymbols[i].Start;

// If there's a start quote then there must be an end quote to be valid, skip it.
symbolOffset = 1;
}
else
{
symbolStartLocation = symbol.Start;

// Set the symbol offset to 0 so we don't attempt to skip an end quote that doesn't exist.
symbolOffset = 0;
}

attributeValueStartLocation = symbolStartLocation +
span.Start +
new SourceLocation(absoluteIndex: 1,
lineIndex: 0,
characterIndex: 1);
attributeValueStartLocation =
span.Start +
symbolStartLocation +
new SourceLocation(absoluteIndex: 1, lineIndex: 0, characterIndex: 1);

afterEquals = true;
}
else if (symbol.Type == HtmlSymbolType.WhiteSpace)
{
// We're at the start of the attribute, this branch may be hit on the first iterations of
// the loop since the parser separates attributes with their spaces included as symbols.
// We're at: "| asp-for='...'" or "| asp-for=..."
// Note: This will not be hit even for situations like asp-for ="..." because the core Razor
// parser currently does not know how to handle attributes in that format. This will be addressed
// by https://github.com/aspnet/Razor/issues/123.

attributeValueStartLocation = SourceLocation.Advance(attributeValueStartLocation, symbol.Content);
}
}

// After all symbols have been added we need to set the builders start position so we do not indirectly
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,41 @@ public static TheoryData DesignTimeTagHelperTestData
BuildLineMapping(1094, 30, 18, 5179, 187, 19, 29),
BuildLineMapping(1231, 34, 5279, 192, 0, 1),
}
}
},
{
"EmptyAttributeTagHelpers",
"EmptyAttributeTagHelpers.DesignTime",
PAndInputTagHelperDescriptors,
new List<LineMapping>
{
BuildLineMapping(documentAbsoluteIndex: 14,
documentLineIndex: 0,
generatedAbsoluteIndex: 493,
generatedLineIndex: 15,
characterOffsetIndex: 14,
contentLength: 11),
BuildLineMapping(documentAbsoluteIndex: 62,
documentLineIndex: 3,
documentCharacterOffsetIndex: 26,
generatedAbsoluteIndex: 1289,
generatedLineIndex: 39,
generatedCharacterOffsetIndex: 28,
contentLength: 0),
BuildLineMapping(documentAbsoluteIndex: 122,
documentLineIndex: 5,
generatedAbsoluteIndex: 1634,
generatedLineIndex: 48,
characterOffsetIndex: 30,
contentLength: 0),
BuildLineMapping(documentAbsoluteIndex: 88,
documentLineIndex: 4,
documentCharacterOffsetIndex: 12,
generatedAbsoluteIndex: 1789,
generatedLineIndex: 54,
generatedCharacterOffsetIndex: 19,
contentLength: 0)
}
},
};
}
}
Expand Down Expand Up @@ -245,6 +279,7 @@ public static TheoryData RuntimeTimeTagHelperTestData
{ "BasicTagHelpers", PAndInputTagHelperDescriptors },
{ "BasicTagHelpers.RemoveTagHelper", PAndInputTagHelperDescriptors },
{ "ComplexTagHelpers", PAndInputTagHelperDescriptors },
{ "EmptyAttributeTagHelpers", PAndInputTagHelperDescriptors },
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,77 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
{
public class TagHelperParseTreeRewriterTest : CsHtmlMarkupParserTestBase
{
public static TheoryData EmptyAttributeTagHelperData
{
get
{
var factory = CreateDefaultSpanFactory();

// documentContent, expectedOutput
return new TheoryData<string, MarkupBlock>
{
{
"<p class=\"\"></p>",
new MarkupBlock(
new MarkupTagHelperBlock("p",
new Dictionary<string, SyntaxTreeNode>
{
{ "class", new MarkupBlock() }
}))
},
{
"<p class=''></p>",
new MarkupBlock(
new MarkupTagHelperBlock("p",
new Dictionary<string, SyntaxTreeNode>
{
{ "class", new MarkupBlock() }
}))
},
{
"<p class=></p>",
new MarkupBlock(
new MarkupTagHelperBlock("p",
new Dictionary<string, SyntaxTreeNode>
{
// We expected a markup node here because attribute values without quotes can only ever
// be a single item, hence don't need to be enclosed by a block.
{ "class", factory.Markup("").With(SpanCodeGenerator.Null) },
}))
},
{
"<p class1='' class2= class3=\"\" />",
new MarkupBlock(
new MarkupTagHelperBlock("p",
new Dictionary<string, SyntaxTreeNode>
{
{ "class1", new MarkupBlock() },
{ "class2", factory.Markup("").With(SpanCodeGenerator.Null) },
{ "class3", new MarkupBlock() },
}))
},
{
"<p class1=''class2=\"\"class3= />",
new MarkupBlock(
new MarkupTagHelperBlock("p",
new Dictionary<string, SyntaxTreeNode>
{
{ "class1", new MarkupBlock() },
{ "class2", new MarkupBlock() },
{ "class3", factory.Markup("").With(SpanCodeGenerator.Null) },
}))
},
};
}
}

[Theory]
[MemberData(nameof(EmptyAttributeTagHelperData))]
public void Rewrite_UnderstandsEmptyAttributeTagHelpers(string documentContent, MarkupBlock expectedOutput)
{
RunParseTreeRewriterTest(documentContent, expectedOutput, new RazorError[0], "p");
}

public static TheoryData<string, MarkupBlock, RazorError[]> MalformedTagHelperAttributeBlockData
{
get
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
namespace TestOutput
{
using Microsoft.AspNet.Razor.Runtime.TagHelpers;
using System;
using System.Threading.Tasks;

public class EmptyAttributeTagHelpers
{
private static object @__o;
private void @__RazorDesignTimeHelpers__()
{
#pragma warning disable 219
string __tagHelperDirectiveSyntaxHelper = null;
__tagHelperDirectiveSyntaxHelper =
#line 1 "EmptyAttributeTagHelpers.cshtml"
"something"

#line default
#line hidden
;
#pragma warning restore 219
}
#line hidden
private InputTagHelper __InputTagHelper = null;
private InputTagHelper2 __InputTagHelper2 = null;
private PTagHelper __PTagHelper = null;
#line hidden
public EmptyAttributeTagHelpers()
{
}

#pragma warning disable 1998
public override async Task ExecuteAsync()
{
__InputTagHelper = CreateTagHelper<InputTagHelper>();
__InputTagHelper.Type = "";
__InputTagHelper2 = CreateTagHelper<InputTagHelper2>();
__InputTagHelper2.Type = __InputTagHelper.Type;
#line 4 "EmptyAttributeTagHelpers.cshtml"
__InputTagHelper2.Checked = ;

#line default
#line hidden
__InputTagHelper = CreateTagHelper<InputTagHelper>();
__InputTagHelper.Type = "";
__InputTagHelper2 = CreateTagHelper<InputTagHelper2>();
__InputTagHelper2.Type = __InputTagHelper.Type;
#line 6 "EmptyAttributeTagHelpers.cshtml"
__InputTagHelper2.Checked = ;

#line default
#line hidden
__PTagHelper = CreateTagHelper<PTagHelper>();
#line 5 "EmptyAttributeTagHelpers.cshtml"
__PTagHelper.Age = ;

#line default
#line hidden
}
#pragma warning restore 1998
}
}
Loading

0 comments on commit 7afd78b

Please sign in to comment.