Skip to content

Commit

Permalink
Fixed tag name similarity suggestion for markup controls
Browse files Browse the repository at this point in the history
  • Loading branch information
exyi committed Jun 8, 2024
1 parent 1855e4e commit c44275a
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ public IControlResolverMetadata ResolveControl(IControlType controlType)

/// <summary> Returns a list of possible DotVVM controls. </summary>
/// <remark>Used only for smart error handling, the list isn't necessarily complete, but doesn't contain false positives.</remark>
public abstract IEnumerable<(string tagPrefix, IControlType type)> EnumerateControlTypes();
public abstract IEnumerable<(string tagPrefix, string? tagName, IControlType type)> EnumerateControlTypes();

/// <summary>
/// Gets the control metadata.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ private IAbstractControl ProcessObjectElement(DothtmlElementNode element, IDataC
var similarNameHelp = similarControls.Any() ? $" Did you mean {string.Join(", ", similarControls.Select(c => c.tagPrefix + ":" + c.name))}, or other DotVVM control?" : "";
var tagPrefixHelp = configuration.Markup.Controls.Any(c => string.Equals(c.TagPrefix, element.TagPrefix, StringComparison.OrdinalIgnoreCase))
? ""
: $" {(similarNameHelp is null ? "Make" : "Otherwise, make")} sure that the tagPrefix '{element.TagPrefix}' is registered in DotvvmConfiguration.Markup.Controls collection!";
: $" {(similarNameHelp is "" ? "Make" : "Otherwise, make")} sure that the tagPrefix '{element.TagPrefix}' is registered in DotvvmConfiguration.Markup.Controls collection!";
element.TagNameNode.AddError($"The control <{element.FullTagName}> could not be resolved!{similarNameHelp}{tagPrefixHelp}");
}
if (controlMetadata.VirtualPath is {} && controlMetadata.Type.IsAssignableTo(ResolvedTypeDescriptor.Create(typeof(DotvvmView))))
Expand Down Expand Up @@ -512,10 +512,10 @@ from c in this.controlResolver.EnumerateControlTypes()
where controlBaseType is null || c.type.Type.IsAssignableTo(controlBaseType)
let prefixScore = tagPrefix is null ? 0 : StringSimilarity.DamerauLevenshteinDistance(c.tagPrefix, tagPrefix)
where prefixScore <= threshold
from controlName in Enumerable.Concat([ c.type.Type.Name ], c.type.AlternativeNames)
from controlName in Enumerable.Concat([ c.tagName ?? c.type.PrimaryName ], c.type.AlternativeNames)
let nameScore = StringSimilarity.DamerauLevenshteinDistance(elementName.ToLowerInvariant(), controlName.ToLowerInvariant())
where prefixScore + nameScore <= threshold
orderby prefixScore + nameScore descending
orderby (prefixScore + nameScore, controlName, c.tagPrefix) descending
select (c.tagPrefix, controlName, c.type)
).Take(limit).ToArray();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,16 +300,23 @@ public override IControlResolverMetadata BuildControlMetadata(IControlType type)
return new ControlResolverMetadata((ControlType)type);
}

public override IEnumerable<(string tagPrefix, IControlType type)> EnumerateControlTypes()
public override IEnumerable<(string tagPrefix, string? tagName, IControlType type)> EnumerateControlTypes()
{
var markupControls = new HashSet<(string, string)>(); // don't report MarkupControl with @baseType twice

foreach (var control in configuration.Markup.Controls)
{
if (!string.IsNullOrEmpty(control.Src))
{
var markupControl = FindMarkupControl(control.Src);
markupControls.Add((control.TagPrefix!, control.TagName!));
IControlType? markupControl = null;
try
{
markupControl = FindMarkupControl(control.Src);
}
catch { } // ignore the error, we should not crash here
if (markupControl != null)
yield return (control.TagPrefix!, control.TagName, markupControl);
}
}

Expand All @@ -327,7 +334,8 @@ type.DeclaringType is null &&
typeof(DotvvmBindableObject).IsAssignableFrom(type) &&
namespaces.TryGetValue(type.Namespace ?? "", out var controlConfig))
{
yield return (controlConfig.TagPrefix!, new ControlType(type));
if (!markupControls.Contains((controlConfig.TagPrefix!, type.Name)))
yield return (controlConfig.TagPrefix!, null, new ControlType(type));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public interface IControlResolver

/// <summary> Returns a list of possible DotVVM controls. </summary>
/// <remark>Used only for smart error handling, the list isn't necessarily complete, but doesn't contain false positives.</remark>
IEnumerable<(string tagPrefix, IControlType type)> EnumerateControlTypes();
IEnumerable<(string tagPrefix, string? tagName, IControlType type)> EnumerateControlTypes();

/// <summary>
/// Resolves the binding type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,22 @@ public void ResolvedTree_UnknownPrefix()
Assert.AreEqual("The control <bp:Button> could not be resolved! Did you mean dot:Button, or other DotVVM control? Otherwise, make sure that the tagPrefix 'bp' is registered in DotvvmConfiguration.Markup.Controls collection!", XAssert.Single(node.NodeErrors));
}


[TestMethod]
public void ResolvedTree_SimilarToMarkupControl()
{
var root = ParseSource(@"@viewModel string
<cmc:ControlWithPropertyDirectives />
<cmc:ControlWithGazeType />
");

var controls = root.Content.Where(c => !c.IsOnlyWhitespace()).ToArray();
var node = (controls[0].DothtmlNode as DothtmlElementNode).TagNameNode;
Assert.AreEqual("The control <cmc:ControlWithPropertyDirectives> could not be resolved! Did you mean cmc:ControlWithPropertyDirective, or other DotVVM control?", XAssert.Single(node.NodeErrors));
node = (controls[1].DothtmlNode as DothtmlElementNode).TagNameNode;
Assert.AreEqual("The control <cmc:ControlWithGazeType> could not be resolved! Did you mean cmc:ControlWithBaseType, or other DotVVM control?", XAssert.Single(node.NodeErrors));
}

[TestMethod]
public void ResolvedTree_ElementProperty()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
using DotVVM.Framework.Compilation.ControlTree.Resolved;
using DotVVM.Framework.Binding;
using DotVVM.Framework.Compilation.ControlTree.Resolved;
using DotVVM.Framework.Configuration;
using DotVVM.Framework.Controls;
using DotVVM.Framework.Hosting;
using DotVVM.Framework.Testing;
using DotVVM.Framework.Tests.Runtime.ControlTree.DefaultControlTreeResolver;
using Microsoft.Extensions.DependencyInjection;

namespace DotVVM.Framework.Tests.Runtime.ControlTree
{
Expand All @@ -11,13 +15,43 @@ public abstract class DefaultControlTreeResolverTestsBase

static DefaultControlTreeResolverTestsBase()
{
configuration = DotvvmTestHelper.CreateConfiguration();
var fakeMarkupFileLoader = new FakeMarkupFileLoader() {
MarkupFiles = {
["ControlWithBaseType.dotcontrol"] = """
@viewModel object
@baseType DotVVM.Framework.Tests.Runtime.ControlTree.DefaultControlTreeResolverTestsBase.TestMarkupControl1
{{value: Text}}
""",
["ControlWithPropertyDirective.dotcontrol"] = """
@viewModel object
@property string Text
{{value: Text}}
"""
}
};
configuration = DotvvmTestHelper.CreateConfiguration(s => {
s.AddSingleton<IMarkupFileLoader>(fakeMarkupFileLoader);
});
configuration.Markup.AddCodeControls("cc", typeof(ClassWithInnerElementProperty));
configuration.Markup.AddMarkupControl("cmc", "ControlWithBaseType", "ControlWithBaseType.dotcontrol");
configuration.Markup.AddMarkupControl("cmc", "ControlWithPropertyDirective", "ControlWithPropertyDirective.dotcontrol");
configuration.Freeze();
}

protected ResolvedTreeRoot ParseSource(string markup, string fileName = "default.dothtml", bool checkErrors = false) =>
DotvvmTestHelper.ParseResolvedTree(markup, fileName, configuration, checkErrors);

public class TestMarkupControl1: DotvvmMarkupControl
{
public string Text
{
get { return (string)GetValue(TextProperty); }
set { SetValue(TextProperty, value); }
}
public static readonly DotvvmProperty TextProperty =
DotvvmProperty.Register<string, TestMarkupControl1>(nameof(Text));
}
}
}

0 comments on commit c44275a

Please sign in to comment.