Skip to content

Commit

Permalink
Improve text validation.
Browse files Browse the repository at this point in the history
  • Loading branch information
SebastianStehle committed Sep 18, 2024
1 parent c6645da commit 46fa063
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 29 deletions.
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
<RepositoryUrl>https://github.com/SebastianStehle/mjml-net.git</RepositoryUrl>
<RepositoryType>git</RepositoryType>
<SymbolPackageFormat>snupkg</SymbolPackageFormat>
<Version>3.13.0</Version>
<Version>3.14.0</Version>
</PropertyGroup>
</Project>
4 changes: 4 additions & 0 deletions Mjml.Net/IHtmlReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ public interface IHtmlReader

string Text { get; }

ReadOnlySpan<char> NameAsSpan { get; }

ReadOnlySpan<char> TextAsSpan { get; }

bool SelfClosingElement { get; }

HtmlTokenKind TokenKind { get; }
Expand Down
5 changes: 5 additions & 0 deletions Mjml.Net/Internal/HtmlReaderWrapper.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using HtmlPerformanceKit;
using System;

Check warning on line 2 in Mjml.Net/Internal/HtmlReaderWrapper.cs

View workflow job for this annotation

GitHub Actions / build

Using directive for 'System' should appear before directive for 'HtmlPerformanceKit' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

Check warning on line 2 in Mjml.Net/Internal/HtmlReaderWrapper.cs

View workflow job for this annotation

GitHub Actions / build

Using directive for 'System' should appear before directive for 'HtmlPerformanceKit' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

Check warning on line 2 in Mjml.Net/Internal/HtmlReaderWrapper.cs

View workflow job for this annotation

GitHub Actions / build

Using directive for 'System' should appear before directive for 'HtmlPerformanceKit' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

Check warning on line 2 in Mjml.Net/Internal/HtmlReaderWrapper.cs

View workflow job for this annotation

GitHub Actions / build

Using directive for 'System' should appear before directive for 'HtmlPerformanceKit' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

Check warning on line 2 in Mjml.Net/Internal/HtmlReaderWrapper.cs

View workflow job for this annotation

GitHub Actions / build

Using directive for 'System' should appear before directive for 'HtmlPerformanceKit' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)

Check warning on line 2 in Mjml.Net/Internal/HtmlReaderWrapper.cs

View workflow job for this annotation

GitHub Actions / build

Using directive for 'System' should appear before directive for 'HtmlPerformanceKit' (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1208.md)
using HtmlReaderImpl = HtmlPerformanceKit.HtmlReader;

namespace Mjml.Net.Internal;
Expand All @@ -24,6 +25,10 @@ internal class HtmlReaderWrapper : IHtmlReader

public string Text => impl.Text;

public ReadOnlySpan<char> NameAsSpan => impl.NameAsMemory.Span;

public ReadOnlySpan<char> TextAsSpan => impl.TextAsMemory.Span;

public bool SelfClosingElement => impl.SelfClosingElement;

public HtmlTokenKind TokenKind => impl.TokenKind;
Expand Down
53 changes: 35 additions & 18 deletions Mjml.Net/MjmlRenderContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ public void Read(IHtmlReader reader, IComponent? parent, string? file)
{
var substree = reader.ReadSubtree();

// Only add the text error once per parent.
var hasAddedError = false;

while (substree.Read())
{
switch (substree.TokenKind)
Expand All @@ -74,6 +77,15 @@ public void Read(IHtmlReader reader, IComponent? parent, string? file)
case HtmlTokenKind.Comment when mjmlOptions.KeepComments && parent != null:
ReadComment(substree, parent);
break;
case HtmlTokenKind.Text when !hasAddedError && substree.TextAsSpan.Trim().Length > 0:
errors.Add(
"Unexpected text content.",
ValidationErrorType.UnexpectedText,
Position(reader, file));

// Only add the text error once per parent.
hasAddedClosingError = true;
break;
}
}
}
Expand All @@ -94,9 +106,10 @@ private void ReadElement(string name, IHtmlReader reader, IComponent? parent, st

if (component == null)
{
errors.Add($"Invalid element '{name}'.",
errors.Add(
$"Invalid element '{name}'.",
ValidationErrorType.UnknownElement,
position);
Position(reader, file));

// Just skip unknown elements.
reader.ReadInnerHtml();
Expand All @@ -110,8 +123,8 @@ private void ReadElement(string name, IHtmlReader reader, IComponent? parent, st
// Add all binders to list, so that we can return them later to the pool.
allBinders.Add(binder);

BindAttributes(reader, component, binder, file);
BindContent(reader, component, binder);
BindComponentAttributes(reader, component, binder, file);
BindComponentContent(reader, component, binder);

component.SetBinder(binder);
component.Read(reader, this, context);
Expand All @@ -131,7 +144,7 @@ private void ReadElement(string name, IHtmlReader reader, IComponent? parent, st
}
}

private void BindAttributes(IHtmlReader reader, IComponent component, Binder binder, string? file)
private void BindComponentAttributes(IHtmlReader reader, IComponent component, Binder binder, string? file)
{
for (var i = 0; i < reader.AttributeCount; i++)
{
Expand All @@ -149,7 +162,7 @@ private void BindAttributes(IHtmlReader reader, IComponent component, Binder bin
}
}

private static void BindContent(IHtmlReader reader, IComponent component, Binder binder)
private static void BindComponentContent(IHtmlReader reader, IComponent component, Binder binder)
{
if (component.ContentType == ContentType.Text)
{
Expand Down Expand Up @@ -185,29 +198,33 @@ private void ValidatingClosingState(string name, IHtmlReader reader, string? fil
return;
}

void AddClosingError(string message)
if (reader.TokenKind == HtmlTokenKind.EndTag && reader.Name != name)
{
errors.Add(message,
errors.Add(
$"Unexpected end element, expected '{name}', got '{reader.Name}'.",
ValidationErrorType.InvalidHtml,
new SourcePosition(
reader.LineNumber,
reader.LinePosition,
file));
Position(reader, name));

// Only show one closing error, otherwise we could get one for every item in the hierarchy.
hasAddedClosingError = true;
}

if (reader.TokenKind == HtmlTokenKind.EndTag && reader.Name != name)
{
AddClosingError($"Unexpected end element, expected '{name}', got '{reader.Name}'.");
}
else if (reader.TokenKind != HtmlTokenKind.EndTag)
{
AddClosingError($"Unexpected end element, expected '{name}', got '{reader.TokenKind}' token.");
errors.Add(
$"Unexpected end element, expected '{name}', got '{reader.TokenKind}' token.",
ValidationErrorType.InvalidHtml,
Position(reader, name));

// Only show one closing error, otherwise we could get one for every item in the hierarchy.
hasAddedClosingError = true;
}
}

private static SourcePosition Position(IHtmlReader reader, string? file)
{
return new SourcePosition(reader.LineNumber, reader.LinePosition, file);
}

private void Cleanup()
{
foreach (var usedBinder in allBinders)
Expand Down
1 change: 1 addition & 0 deletions Mjml.Net/ValidationErrors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public enum ValidationErrorType
InvalidParent,
UnknownAttribute,
UnknownElement,
UnexpectedText,
InvalidHtml
}

Expand Down
1 change: 0 additions & 1 deletion Mjml.Net/Validators/SoftValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ public sealed class SoftValidator : ValidatorBase
public static readonly SoftValidator Instance = new SoftValidator();

private SoftValidator()
: base(false)
{
}
}
11 changes: 10 additions & 1 deletion Mjml.Net/Validators/StrictValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,16 @@ public sealed class StrictValidator : ValidatorBase
public static readonly StrictValidator Instance = new StrictValidator();

private StrictValidator()
: base(true)
{
}

public override void AttributeValue(string name, string value, IComponent component, IType type, ValidationErrors errors, ref ValidationContext context)
{
if (!type.Validate(value, ref context))
{
errors.Add($"'{value}' is not a valid attribute '{name}' of '{component.ComponentName}'.",
ValidationErrorType.InvalidAttribute,
context.Position);
}
}
}
15 changes: 7 additions & 8 deletions Mjml.Net/Validators/ValidatorBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,8 @@ namespace Mjml.Net.Validators;

public abstract class ValidatorBase : IValidator
{
private readonly bool validateAttributeValue;

protected ValidatorBase(bool validateAttributeValue)
protected ValidatorBase()
{
this.validateAttributeValue = validateAttributeValue;
}

public void Attribute(string name, string value, IComponent component, ValidationErrors errors, ref ValidationContext context)
Expand All @@ -27,14 +24,16 @@ public void Attribute(string name, string value, IComponent component, Validatio
ValidationErrorType.UnknownAttribute,
context.Position);
}
else if (validateAttributeValue && !attribute.Validate(value, ref context))
else
{
errors.Add($"'{value}' is not a valid attribute '{name}' of '{component.ComponentName}'.",
ValidationErrorType.InvalidAttribute,
context.Position);
AttributeValue(name, value, component, attribute, errors, ref context);
}
}

public virtual void AttributeValue(string name, string value, IComponent component, IType type, ValidationErrors errors, ref ValidationContext context)
{
}

public void Components(IComponent root, ValidationErrors errors, ref ValidationContext context)
{
if (root is RootComponent rootComponent)
Expand Down
34 changes: 34 additions & 0 deletions Tests/ValidationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,40 @@ public void Should_add_error_if_component_has_invalid_attribute_value()
Assert.Equal(new[] { "'red' is not a valid attribute 'width' of 'mj-button'." }, errors);
}

[Fact]
public void Should_add_error_if_component_has_unexpected_text()
{
var source = @"
<mjml>
<mj-body>
<mj-section>
hello
</mj-section>
</mj-body>
</mjml>
";
var errors = Render(source);

Assert.Equal(new[] { "Unexpected text content." }, errors);
}

[Fact]
public void Should_add_error_if_component_has_unexpected_html_child()
{
var source = @"
<mjml>
<mj-body>
<mj-section>
<button></button>
</mj-section>
</mj-body>
</mjml>
";
var errors = Render(source);

Assert.Equal(new[] { "Invalid element 'button'." }, errors);
}

[Fact]
public void Should_not_add_error_for_self_closing_element()
{
Expand Down

0 comments on commit 46fa063

Please sign in to comment.