- Updated naming for methods to be more intuitive.

- Added extra tests for clarity
- Added extra comments to clarify complex areas
This commit is contained in:
Artak Mkrtchyan 2018-03-09 18:18:16 -08:00
parent 3e22c16930
commit ae94c3c452
No known key found for this signature in database
GPG Key ID: 64D580ACBA8CA645
4 changed files with 51 additions and 32 deletions

View File

@ -19,7 +19,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
// Special // Special
Comment = 8, Comment = 8,
Tag = 9, Tag = 9,
HtmlComment = 10 HtmlComment = 10
} }
} }

View File

@ -12,7 +12,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
{ {
private const string ScriptTagName = "script"; private const string ScriptTagName = "script";
private static readonly HtmlSymbol[] nonAllowedHtmlCommentEnding = new[] { new HtmlSymbol("-", HtmlSymbolType.Text), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("<", HtmlSymbolType.OpenAngle) }; private static readonly HtmlSymbol[] nonAllowedHtmlCommentEnding = new[] { HtmlSymbol.Hyphen, new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("<", HtmlSymbolType.OpenAngle) };
private static readonly HtmlSymbol[] singleHyphenArray = new[] { HtmlSymbol.Hyphen };
private static readonly char[] ValidAfterTypeAttributeNameCharacters = { ' ', '\t', '\r', '\n', '\f', '=' }; private static readonly char[] ValidAfterTypeAttributeNameCharacters = { ' ', '\t', '\r', '\n', '\f', '=' };
private SourceLocation _lastTagStart = SourceLocation.Zero; private SourceLocation _lastTagStart = SourceLocation.Zero;
@ -506,7 +507,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
while (!EndOfFile) while (!EndOfFile)
{ {
SkipToAndParseCode(HtmlSymbolType.DoubleHyphen); SkipToAndParseCode(HtmlSymbolType.DoubleHyphen);
var lastDoubleHyphen = AcceptAllButLastDoubleHypens(); var lastDoubleHyphen = AcceptAllButLastDoubleHyphens();
if (At(HtmlSymbolType.CloseAngle)) if (At(HtmlSymbolType.CloseAngle))
{ {
@ -543,7 +544,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
return false; return false;
} }
protected HtmlSymbol AcceptAllButLastDoubleHypens() protected HtmlSymbol AcceptAllButLastDoubleHyphens()
{ {
var lastDoubleHyphen = CurrentSymbol; var lastDoubleHyphen = CurrentSymbol;
AcceptWhile(s => AcceptWhile(s =>
@ -558,9 +559,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
}); });
NextToken(); NextToken();
EnsureCurrent();
if (At(HtmlSymbolType.Text) && IsDashSymbol(CurrentSymbol)) if (At(HtmlSymbolType.Text) && IsHyphen(CurrentSymbol))
{ {
// Doing this here to maintain the order of symbols // Doing this here to maintain the order of symbols
if (!NextIs(HtmlSymbolType.CloseAngle)) if (!NextIs(HtmlSymbolType.CloseAngle))
@ -575,9 +575,9 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
return lastDoubleHyphen; return lastDoubleHyphen;
} }
internal static bool IsDashSymbol(HtmlSymbol symbol) internal static bool IsHyphen(HtmlSymbol symbol)
{ {
return string.Equals(symbol.Content, "-", StringComparison.Ordinal); return symbol.Equals(HtmlSymbol.Hyphen);
} }
protected bool IsHtmlCommentAhead() protected bool IsHtmlCommentAhead()
@ -589,7 +589,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
* 1. The string "<!--" * 1. The string "<!--"
* 2. Optionally, text, with the additional restriction that the text * 2. Optionally, text, with the additional restriction that the text
* 2.1 must not start with the string ">" nor start with the string "->" * 2.1 must not start with the string ">" nor start with the string "->"
* 2.2 nor contain the strings "<!--", "-->", or "--!>" * 2.2 nor contain the strings
* 2.2.1 "<!--"
* 2.2.2 "-->" // As we will be treating this as a comment ending, there is no need to handle this case at all.
* 2.2.3 "--!>"
* 2.3 nor end with the string "<!-". * 2.3 nor end with the string "<!-".
* 3. The string "-->" * 3. The string "-->"
* *
@ -601,37 +604,42 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
} }
// Check condition 2.1 // Check condition 2.1
if (NextIs(HtmlSymbolType.CloseAngle) || NextIs(next => IsDashSymbol(next) && NextIs(HtmlSymbolType.CloseAngle))) if (NextIs(HtmlSymbolType.CloseAngle) || NextIs(next => IsHyphen(next) && NextIs(HtmlSymbolType.CloseAngle)))
{ {
return false; return false;
} }
// Check condition 2.2 // Check condition 2.2
var isValidComment = false; var isValidComment = false;
LookaheadUntil((s, p) => LookaheadUntil((symbol, prevSymbols) =>
{ {
if (s.Type == HtmlSymbolType.DoubleHyphen) if (symbol.Type == HtmlSymbolType.DoubleHyphen)
{ {
if (NextIs(HtmlSymbolType.CloseAngle)) if (NextIs(HtmlSymbolType.CloseAngle))
{ {
// Check condition 2.3: We're at the end of a comment. Check to make sure the text ending is allowed. // Check condition 2.3: We're at the end of a comment. Check to make sure the text ending is allowed.
isValidComment = !IsCommentContentDisallowed(p); isValidComment = !IsCommentContentEndingInvalid(prevSymbols);
return true; return true;
} }
else if (NextIs(ns => IsDashSymbol(ns) && NextIs(HtmlSymbolType.CloseAngle))) else if (NextIs(ns => IsHyphen(ns) && NextIs(HtmlSymbolType.CloseAngle)))
{ {
// This is also a valid closing comment case, as the dashes lookup is treated with DoubleHyphen symbols first. // Check condition 2.3: we're at the end of a comment, which has an extra dash.
// Need to treat the dash as part of the content and check the ending.
// However, that case would have already been checked as part of check from 2.2.1 which
// would already fail this iteration and we wouldn't get here
isValidComment = true; isValidComment = true;
return true; return true;
} }
else if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.CloseAngle))) else if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.CloseAngle)))
{ {
// This is condition 2.2.3
isValidComment = false; isValidComment = false;
return true; return true;
} }
} }
else if (s.Type == HtmlSymbolType.OpenAngle) else if (symbol.Type == HtmlSymbolType.OpenAngle)
{ {
// Checking condition 2.2.1
if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.DoubleHyphen))) if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.DoubleHyphen)))
{ {
isValidComment = false; isValidComment = false;
@ -648,7 +656,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
/// <summary> /// <summary>
/// Verifies, that the sequence doesn't end with the "&lt;!-" HtmlSymbols. Note, the first symbol is an opening bracket symbol /// Verifies, that the sequence doesn't end with the "&lt;!-" HtmlSymbols. Note, the first symbol is an opening bracket symbol
/// </summary> /// </summary>
internal static bool IsCommentContentDisallowed(IEnumerable<HtmlSymbol> sequence) internal static bool IsCommentContentEndingInvalid(IEnumerable<HtmlSymbol> sequence)
{ {
var reversedSequence = sequence.Reverse(); var reversedSequence = sequence.Reverse();
var index = 0; var index = 0;

View File

@ -8,6 +8,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
{ {
internal class HtmlSymbol : SymbolBase<HtmlSymbolType> internal class HtmlSymbol : SymbolBase<HtmlSymbolType>
{ {
internal static readonly HtmlSymbol Hyphen = new HtmlSymbol("-", HtmlSymbolType.Text);
public HtmlSymbol(string content, HtmlSymbolType type) public HtmlSymbol(string content, HtmlSymbolType type)
: base(content, type, RazorDiagnostic.EmptyArray) : base(content, type, RazorDiagnostic.EmptyArray)
{ {

View File

@ -24,23 +24,23 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
[Theory] [Theory]
[MemberData(nameof(NonDashSymbols))] [MemberData(nameof(NonDashSymbols))]
public void IsDashSymbol_ReturnsFalseForNonDashSymbol(object symbol) public void IsHyphen_ReturnsFalseForNonDashSymbol(object symbol)
{ {
// Arrange // Arrange
var convertedSymbol = (HtmlSymbol)symbol; var convertedSymbol = (HtmlSymbol)symbol;
// Act & Assert // Act & Assert
Assert.False(HtmlMarkupParser.IsDashSymbol(convertedSymbol)); Assert.False(HtmlMarkupParser.IsHyphen(convertedSymbol));
} }
[Fact] [Fact]
public void IsDashSymbol_ReturnsTrueForADashSymbol() public void IsHyphen_ReturnsTrueForADashSymbol()
{ {
// Arrange // Arrange
var dashSymbol = new HtmlSymbol("-", HtmlSymbolType.Text); var dashSymbol = new HtmlSymbol("-", HtmlSymbolType.Text);
// Act & Assert // Act & Assert
Assert.True(HtmlMarkupParser.IsDashSymbol(dashSymbol)); Assert.True(HtmlMarkupParser.IsHyphen(dashSymbol));
} }
[Fact] [Fact]
@ -50,7 +50,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
var sut = CreateTestParserForContent("-->"); var sut = CreateTestParserForContent("-->");
// Act // Act
var symbol = sut.AcceptAllButLastDoubleHypens(); var symbol = sut.AcceptAllButLastDoubleHyphens();
// Assert // Assert
Assert.Equal(doubleHyphenSymbol, symbol); Assert.Equal(doubleHyphenSymbol, symbol);
@ -65,12 +65,12 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
var sut = CreateTestParserForContent("--->"); var sut = CreateTestParserForContent("--->");
// Act // Act
var symbol = sut.AcceptAllButLastDoubleHypens(); var symbol = sut.AcceptAllButLastDoubleHyphens();
// Assert // Assert
Assert.Equal(doubleHyphenSymbol, symbol); Assert.Equal(doubleHyphenSymbol, symbol);
Assert.True(sut.At(HtmlSymbolType.CloseAngle)); Assert.True(sut.At(HtmlSymbolType.CloseAngle));
Assert.True(HtmlMarkupParser.IsDashSymbol(sut.PreviousSymbol)); Assert.True(HtmlMarkupParser.IsHyphen(sut.PreviousSymbol));
} }
[Fact] [Fact]
@ -103,6 +103,16 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
Assert.True(sut.IsHtmlCommentAhead()); Assert.True(sut.IsHtmlCommentAhead());
} }
[Fact]
public void IsHtmlCommentAhead_ReturnsFalseForContentWithBadEndingAndExtraDash()
{
// Arrange
var sut = CreateTestParserForContent("-- Some comment content in here <!--->");
// Act & Assert
Assert.False(sut.IsHtmlCommentAhead());
}
[Fact] [Fact]
public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraInfoAfter() public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraInfoAfter()
{ {
@ -144,36 +154,36 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
} }
[Fact] [Fact]
public void IsCommentContentDisallowed_ReturnsFalseForAllowedContent() public void IsCommentContentEndingInvalid_ReturnsFalseForAllowedContent()
{ {
// Arrange // Arrange
var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text); var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text);
var sequence = Enumerable.Range((int)'a', 26).Select(item => new HtmlSymbol(((char)item).ToString(), HtmlSymbolType.Text)); var sequence = Enumerable.Range((int)'a', 26).Select(item => new HtmlSymbol(((char)item).ToString(), HtmlSymbolType.Text));
// Act & Assert // Act & Assert
Assert.False(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); Assert.False(HtmlMarkupParser.IsCommentContentEndingInvalid(sequence));
} }
[Fact] [Fact]
public void IsCommentContentDisallowed_ReturnsTrueForDisallowedContent() public void IsCommentContentEndingInvalid_ReturnsTrueForDisallowedContent()
{ {
// Arrange // Arrange
var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text); var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text);
var sequence = new[] { new HtmlSymbol("<", HtmlSymbolType.OpenAngle), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("-", HtmlSymbolType.Text) }; var sequence = new[] { new HtmlSymbol("<", HtmlSymbolType.OpenAngle), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("-", HtmlSymbolType.Text) };
// Act & Assert // Act & Assert
Assert.True(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); Assert.True(HtmlMarkupParser.IsCommentContentEndingInvalid(sequence));
} }
[Fact] [Fact]
public void IsCommentContentDisallowed_ReturnsFalseForEmptyContent() public void IsCommentContentEndingInvalid_ReturnsFalseForEmptyContent()
{ {
// Arrange // Arrange
var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text); var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text);
var sequence = Array.Empty<HtmlSymbol>(); var sequence = Array.Empty<HtmlSymbol>();
// Act & Assert // Act & Assert
Assert.False(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); Assert.False(HtmlMarkupParser.IsCommentContentEndingInvalid(sequence));
} }
private class TestHtmlMarkupParser : HtmlMarkupParser private class TestHtmlMarkupParser : HtmlMarkupParser
@ -193,9 +203,9 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
this.EnsureCurrent(); this.EnsureCurrent();
} }
public new HtmlSymbol AcceptAllButLastDoubleHypens() public new HtmlSymbol AcceptAllButLastDoubleHyphens()
{ {
return base.AcceptAllButLastDoubleHypens(); return base.AcceptAllButLastDoubleHyphens();
} }
public override void BuildSpan(SpanBuilder span, SourceLocation start, string content) public override void BuildSpan(SpanBuilder span, SourceLocation start, string content)