Add parser error for empty TagHelper bound attributes.
- Errors are only created for TagHelper bound attributes that are not bound to string. - Added tests to validate proper errors for expected input. #289
This commit is contained in:
N. Taylor Mullen
parent
cf3d049272
commit
e14dbdf9be
|
|
@ -15,6 +15,8 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
|
|||
{
|
||||
public static class TagHelperBlockRewriter
|
||||
{
|
||||
private static readonly string StringTypeName = typeof(string).FullName;
|
||||
|
||||
public static TagHelperBlockBuilder Rewrite(string tagName,
|
||||
bool validStructure,
|
||||
Block tag,
|
||||
|
|
@ -68,6 +70,23 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
|
|||
// Only want to track the attribute if we succeeded in parsing its corresponding Block/Span.
|
||||
if (succeeded)
|
||||
{
|
||||
// Check if it's a bound attribute that is not of type string and happens to be null or whitespace.
|
||||
string attributeValueType;
|
||||
if (attributeValueTypes.TryGetValue(attribute.Key, out attributeValueType) &&
|
||||
!IsStringAttribute(attributeValueType) &&
|
||||
IsNullOrWhitespaceAttributeValue(attribute.Value))
|
||||
{
|
||||
var errorLocation = GetAttributeNameStartLocation(child);
|
||||
|
||||
errorSink.OnError(
|
||||
errorLocation,
|
||||
RazorResources.FormatRewriterError_EmptyTagHelperBoundAttribute(
|
||||
attribute.Key,
|
||||
tagName,
|
||||
attributeValueType),
|
||||
attribute.Key.Length);
|
||||
}
|
||||
|
||||
attributes[attribute.Key] = attribute.Value;
|
||||
}
|
||||
}
|
||||
|
|
@ -363,6 +382,34 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
|
|||
return builder.Build();
|
||||
}
|
||||
|
||||
private static SourceLocation GetAttributeNameStartLocation(SyntaxTreeNode node)
|
||||
{
|
||||
Span span;
|
||||
|
||||
if (node.IsBlock)
|
||||
{
|
||||
span = ((Block)node).FindFirstDescendentSpan();
|
||||
}
|
||||
else
|
||||
{
|
||||
span = (Span)node;
|
||||
}
|
||||
|
||||
// Span should never be null here, this should only ever be called if an attribute was successfully parsed.
|
||||
Debug.Assert(span != null);
|
||||
|
||||
var nodeStart = span.Parent.Start;
|
||||
|
||||
// Attributes must have at least one non-whitespace character to represent the tagName (even if its a C#
|
||||
// expression).
|
||||
var firstNonWhitespaceSymbol = span
|
||||
.Symbols
|
||||
.OfType<HtmlSymbol>()
|
||||
.First(sym => sym.Type != HtmlSymbolType.WhiteSpace && sym.Type != HtmlSymbolType.NewLine);
|
||||
|
||||
return nodeStart + firstNonWhitespaceSymbol.Start;
|
||||
}
|
||||
|
||||
private static KeyValuePair<string, SyntaxTreeNode> CreateMarkupAttribute(
|
||||
string name,
|
||||
SpanBuilder builder,
|
||||
|
|
@ -374,7 +421,7 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
|
|||
// its value as code. Any non-string value can be any C# value so we need to ensure the SyntaxTreeNode
|
||||
// reflects that.
|
||||
if (attributeValueTypes.TryGetValue(name, out attributeTypeName) &&
|
||||
!string.Equals(attributeTypeName, typeof(string).FullName, StringComparison.OrdinalIgnoreCase))
|
||||
!IsStringAttribute(attributeTypeName))
|
||||
{
|
||||
builder.Kind = SpanKind.Code;
|
||||
}
|
||||
|
|
@ -382,6 +429,31 @@ namespace Microsoft.AspNet.Razor.Parser.TagHelpers.Internal
|
|||
return new KeyValuePair<string, SyntaxTreeNode>(name, builder.Build());
|
||||
}
|
||||
|
||||
private static bool IsNullOrWhitespaceAttributeValue(SyntaxTreeNode attributeValue)
|
||||
{
|
||||
if (attributeValue.IsBlock)
|
||||
{
|
||||
foreach (var span in ((Block)attributeValue).Flatten())
|
||||
{
|
||||
if (!string.IsNullOrWhiteSpace(span.Content))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
else
|
||||
{
|
||||
return string.IsNullOrWhiteSpace(((Span)attributeValue).Content);
|
||||
}
|
||||
}
|
||||
|
||||
private static bool IsStringAttribute(string attributeTypeName)
|
||||
{
|
||||
return string.Equals(attributeTypeName, StringTypeName, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
private static bool IsQuote(HtmlSymbol htmlSymbol)
|
||||
{
|
||||
return htmlSymbol.Type == HtmlSymbolType.DoubleQuote ||
|
||||
|
|
|
|||
|
|
@ -1530,6 +1530,22 @@ namespace Microsoft.AspNet.Razor
|
|||
return string.Format(CultureInfo.CurrentCulture, GetString("TagHelpers_InlineMarkupBlocks_NotSupported_InAttributes"), p0);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Attribute '{0}' on tag helper element '{1}' requires a value. Tag helper bound attributes of type '{2}' cannot be empty or contain only whitespace.
|
||||
/// </summary>
|
||||
internal static string RewriterError_EmptyTagHelperBoundAttribute
|
||||
{
|
||||
get { return GetString("RewriterError_EmptyTagHelperBoundAttribute"); }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Attribute '{0}' on tag helper element '{1}' requires a value. Tag helper bound attributes of type '{2}' cannot be empty or contain only whitespace.
|
||||
/// </summary>
|
||||
internal static string FormatRewriterError_EmptyTagHelperBoundAttribute(object p0, object p1, object p2)
|
||||
{
|
||||
return string.Format(CultureInfo.CurrentCulture, GetString("RewriterError_EmptyTagHelperBoundAttribute"), p0, p1, p2);
|
||||
}
|
||||
|
||||
private static string GetString(string name, params string[] formatterNames)
|
||||
{
|
||||
var value = _resourceManager.GetString(name);
|
||||
|
|
|
|||
|
|
@ -422,4 +422,7 @@ Instead, wrap the contents of the block in "{{}}":
|
|||
<value>Inline markup blocks (e.g. @<p>content</p>) must not appear in non-string tag helper attribute values.
|
||||
Expected a '{0}' attribute value, not a string.</value>
|
||||
</data>
|
||||
<data name="RewriterError_EmptyTagHelperBoundAttribute" xml:space="preserve">
|
||||
<value>Attribute '{0}' on tag helper element '{1}' requires a value. Tag helper bound attributes of type '{2}' cannot be empty or contain only whitespace.</value>
|
||||
</data>
|
||||
</root>
|
||||
|
|
@ -19,6 +19,299 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
|
|||
{
|
||||
public class TagHelperParseTreeRewriterTest : CsHtmlMarkupParserTestBase
|
||||
{
|
||||
public static TheoryData EmptyTagHelperBoundAttributeData
|
||||
{
|
||||
get
|
||||
{
|
||||
var factory = CreateDefaultSpanFactory();
|
||||
var emptyAttributeError =
|
||||
"Attribute '{0}' on tag helper element '{1}' requires a value. Tag helper bound attributes of " +
|
||||
"type '{2}' cannot be empty or contain only whitespace.";
|
||||
var boolTypeName = typeof(bool).FullName;
|
||||
|
||||
// documentContent, expectedOutput, expectedErrors
|
||||
return new TheoryData<string, MarkupBlock, RazorError[]>
|
||||
{
|
||||
{
|
||||
"<myth bound='' />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "bound", new MarkupBlock() }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bound", "myth", boolTypeName),
|
||||
absoluteIndex: 6, lineIndex: 0, columnIndex: 6, length: 5)
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth bound=' true' />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "bound", factory.CodeMarkup(" true") }
|
||||
})),
|
||||
new RazorError[0]
|
||||
},
|
||||
{
|
||||
"<myth bound=' ' />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "bound", factory.CodeMarkup(" ") }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bound", "myth", boolTypeName),
|
||||
absoluteIndex: 6, lineIndex: 0, columnIndex: 6, length: 5)
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth bound='' bound=\"\" />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "bound", new MarkupBlock() }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bound", "myth", boolTypeName),
|
||||
absoluteIndex: 6, lineIndex: 0, columnIndex: 6, length: 5),
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bound", "myth", boolTypeName),
|
||||
absoluteIndex: 16, lineIndex: 0, columnIndex: 16, length: 5)
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth bound=' ' bound=\" \" />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "bound", factory.CodeMarkup(" ") }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bound", "myth", boolTypeName),
|
||||
absoluteIndex: 6, lineIndex: 0, columnIndex: 6, length: 5),
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bound", "myth", boolTypeName),
|
||||
absoluteIndex: 17, lineIndex: 0, columnIndex: 17, length: 5)
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth bound='true' bound= />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "bound", factory.CodeMarkup(string.Empty).With(SpanCodeGenerator.Null) }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bound", "myth", boolTypeName),
|
||||
absoluteIndex: 19, lineIndex: 0, columnIndex: 19, length: 5)
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth bound= name='' />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "bound", factory.CodeMarkup(string.Empty).With(SpanCodeGenerator.Null) },
|
||||
{ "name", new MarkupBlock() }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bound", "myth", boolTypeName),
|
||||
absoluteIndex: 6, lineIndex: 0, columnIndex: 6, length: 5),
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth bound= name=' ' />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "bound", factory.CodeMarkup(string.Empty).With(SpanCodeGenerator.Null) },
|
||||
{ "name", factory.Markup(" ") }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bound", "myth", boolTypeName),
|
||||
absoluteIndex: 6, lineIndex: 0, columnIndex: 6, length: 5),
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth bound='true' name='john' bound= name= />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "bound", factory.CodeMarkup(string.Empty).With(SpanCodeGenerator.Null) },
|
||||
{ "name", factory.Markup(string.Empty).With(SpanCodeGenerator.Null) }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bound", "myth", boolTypeName),
|
||||
absoluteIndex: 31, lineIndex: 0, columnIndex: 31, length: 5),
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth BouND='' />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "BouND", new MarkupBlock() }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "BouND", "myth", boolTypeName),
|
||||
absoluteIndex: 6, lineIndex: 0, columnIndex: 6, length: 5),
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth BOUND='' bOUnd=\"\" />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "BOUND", new MarkupBlock() }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "BOUND", "myth", boolTypeName),
|
||||
absoluteIndex: 6, lineIndex: 0, columnIndex: 6, length: 5),
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "bOUnd", "myth", boolTypeName),
|
||||
absoluteIndex: 18, lineIndex: 0, columnIndex: 18, length: 5)
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth BOUND= nAMe='john'></myth>",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{ "BOUND", factory.CodeMarkup(string.Empty).With(SpanCodeGenerator.Null) },
|
||||
{ "nAMe", factory.Markup("john") }
|
||||
})),
|
||||
new[]
|
||||
{
|
||||
new RazorError(
|
||||
string.Format(emptyAttributeError, "BOUND", "myth", boolTypeName),
|
||||
absoluteIndex: 6, lineIndex: 0, columnIndex: 6, length: 5)
|
||||
}
|
||||
},
|
||||
{
|
||||
"<myth bound=' @true ' />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{
|
||||
"bound",
|
||||
new MarkupBlock(
|
||||
new MarkupBlock(
|
||||
factory.Markup(" "),
|
||||
new ExpressionBlock(
|
||||
factory.CodeTransition(),
|
||||
factory.Code("true")
|
||||
.AsImplicitExpression(CSharpCodeParser.DefaultKeywords)
|
||||
.Accepts(AcceptedCharacters.NonWhiteSpace))),
|
||||
factory.Markup(" "))
|
||||
}
|
||||
})),
|
||||
new RazorError[0]
|
||||
},
|
||||
{
|
||||
"<myth bound=' @(true) ' />",
|
||||
new MarkupBlock(
|
||||
new MarkupTagHelperBlock(
|
||||
"myth",
|
||||
new Dictionary<string, SyntaxTreeNode>
|
||||
{
|
||||
{
|
||||
"bound",
|
||||
new MarkupBlock(
|
||||
new MarkupBlock(
|
||||
factory.Markup(" "),
|
||||
new ExpressionBlock(
|
||||
factory.CodeTransition(),
|
||||
factory.MetaCode("(").Accepts(AcceptedCharacters.None),
|
||||
factory.Code("true").AsExpression(),
|
||||
factory.MetaCode(")").Accepts(AcceptedCharacters.None))),
|
||||
factory.Markup(" "))
|
||||
}
|
||||
})),
|
||||
new RazorError[0]
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[MemberData(nameof(EmptyTagHelperBoundAttributeData))]
|
||||
public void Rewrite_CreatesErrorForEmptyTagHelperBoundAttributes(
|
||||
string documentContent,
|
||||
MarkupBlock expectedOutput,
|
||||
RazorError[] expectedErrors)
|
||||
{
|
||||
// Arrange
|
||||
var descriptors = new TagHelperDescriptor[]
|
||||
{
|
||||
new TagHelperDescriptor(
|
||||
tagName: "myth",
|
||||
typeName: "mythTagHelper",
|
||||
assemblyName: "SomeAssembly",
|
||||
attributes: new[]
|
||||
{
|
||||
new TagHelperAttributeDescriptor(
|
||||
name: "bound",
|
||||
propertyName: "Bound",
|
||||
typeName: typeof(bool).FullName),
|
||||
new TagHelperAttributeDescriptor(
|
||||
name: "name",
|
||||
propertyName: "Name",
|
||||
typeName: typeof(string).FullName)
|
||||
})
|
||||
};
|
||||
var descriptorProvider = new TagHelperDescriptorProvider(descriptors);
|
||||
|
||||
// Act & Assert
|
||||
EvaluateData(descriptorProvider, documentContent, expectedOutput, expectedErrors);
|
||||
}
|
||||
|
||||
public static TheoryData OptOut_WithAttributeTextTagData
|
||||
{
|
||||
get
|
||||
|
|
@ -80,7 +373,7 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
|
|||
errorMatchingBrace,
|
||||
absoluteIndex: 1, lineIndex: 0, columnIndex: 1),
|
||||
new RazorError(
|
||||
string.Format(errorFormatNormalUnclosed, "!text"),
|
||||
string.Format(errorFormatNormalUnclosed, "!text"),
|
||||
absoluteIndex: 2, lineIndex: 0, columnIndex: 2)
|
||||
}
|
||||
},
|
||||
|
|
@ -1654,7 +1947,7 @@ namespace Microsoft.AspNet.Razor.Test.TagHelpers
|
|||
{
|
||||
RunParseTreeRewriterTest(documentContent, expectedOutput, expectedErrors, "strong", "p");
|
||||
}
|
||||
|
||||
|
||||
public static TheoryData EmptyAttributeTagHelperData
|
||||
{
|
||||
get
|
||||
|
|
|
|||
Loading…
Reference in New Issue