Merge pull request #1806 from riganti/inner-element-property-errors

Improve error reporting on tag names
riganti · Jun 8, 2024 · 8c73edf · 8c73edf
2 parents 717c62e + e585938
commit 8c73edf
Show file tree

Hide file tree

Showing 25 changed files with 353 additions and 68 deletions.
diff --git a/src/Directory.Build.props b/src/Directory.Build.props
@@ -20,7 +20,7 @@
   </PropertyGroup>
 
   <PropertyGroup Label="Building">
-    <LangVersion>11.0</LangVersion>
+    <LangVersion>12.0</LangVersion>
     <!-- Disable warning for missing XML doc comments. -->
     <NoWarn>$(NoWarn);CS1591;CS1573</NoWarn>
     <Deterministic>true</Deterministic>

diff --git a/src/Framework/Framework/Compilation/ControlTree/ControlResolverBase.cs b/src/Framework/Framework/Compilation/ControlTree/ControlResolverBase.cs
@@ -200,6 +200,10 @@ public IControlResolverMetadata ResolveControl(IControlType controlType)
 		/// </summary>
 		protected abstract IControlType FindMarkupControl(string file);
 
+		/// <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, string? tagName, IControlType type)> EnumerateControlTypes();
+
 		/// <summary>
 		/// Gets the control metadata.
 		/// </summary>

diff --git a/src/Framework/Framework/Compilation/ControlTree/ControlTreeHelper.cs b/src/Framework/Framework/Compilation/ControlTree/ControlTreeHelper.cs
@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Linq;
 using DotVVM.Framework.Compilation.Parser.Dothtml.Parser;
 
@@ -7,7 +7,7 @@ namespace DotVVM.Framework.Compilation.ControlTree
     public static class ControlTreeHelper
     {
         public static bool HasEmptyContent(this IAbstractControl control)
-            => control.Content.All(c => !DothtmlNodeHelper.IsNotEmpty(c.DothtmlNode)); // allow only whitespace literals
+            => control.Content.All(c => DothtmlNodeHelper.IsEmpty(c.DothtmlNode)); // allow only whitespace literals
 
         public static bool HasProperty(this IAbstractControl control, IPropertyDescriptor property)
         {

diff --git a/src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs b/src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs
diff --git a/src/Framework/Framework/Compilation/ControlTree/DefaultControlResolver.cs b/src/Framework/Framework/Compilation/ControlTree/DefaultControlResolver.cs
@@ -300,6 +300,47 @@ public override IControlResolverMetadata BuildControlMetadata(IControlType type)
             return new ControlResolverMetadata((ControlType)type);
         }
 
+        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))
+                {
+                    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);
+                }
+            }
+
+            foreach (var assemblyGroup in configuration.Markup.Controls.Where(c => !string.IsNullOrEmpty(c.Assembly) && string.IsNullOrEmpty(c.Src)).GroupBy(c => c.Assembly!))
+            {
+                var assembly = compiledAssemblyCache.GetAssembly(assemblyGroup.Key);
+                if (assembly is null)
+                    continue;
+
+                var namespaces = assemblyGroup.GroupBy(c => c.Namespace ?? "").ToDictionary(g => g.Key, g => g.First());
+                foreach (var type in assembly.GetLoadableTypes())
+                {
+                    if (type.IsPublic && !type.IsAbstract &&
+                        type.DeclaringType is null &&
+                        typeof(DotvvmBindableObject).IsAssignableFrom(type) &&
+                        namespaces.TryGetValue(type.Namespace ?? "", out var controlConfig))
+                    {
+                        if (!markupControls.Contains((controlConfig.TagPrefix!, type.Name)))
+                            yield return (controlConfig.TagPrefix!, null, new ControlType(type));
+                    }
+                }
+            }
+        }
+
         protected override IPropertyDescriptor? FindGlobalPropertyOrGroup(string name, MappingMode requiredMode)
         {
             // try to find property

diff --git a/src/Framework/Framework/Compilation/ControlTree/DefaultControlTreeResolver.cs b/src/Framework/Framework/Compilation/ControlTree/DefaultControlTreeResolver.cs
@@ -5,6 +5,7 @@
 using DotVVM.Framework.Compilation.Directives;
 using DotVVM.Framework.Compilation.Parser.Dothtml.Parser;
 using DotVVM.Framework.Compilation.ViewCompiler;
+using DotVVM.Framework.Configuration;
 using DotVVM.Framework.Utils;
 
 namespace DotVVM.Framework.Compilation.ControlTree
@@ -23,8 +24,9 @@ public DefaultControlTreeResolver(
             IControlResolver controlResolver,
             IAbstractTreeBuilder treeBuilder,
             IControlBuilderFactory controlBuilderFactory,
-            IMarkupDirectiveCompilerPipeline direrectiveCompilerPipeline)
-            : base(controlResolver, treeBuilder, direrectiveCompilerPipeline)
+            IMarkupDirectiveCompilerPipeline direrectiveCompilerPipeline,
+            DotvvmConfiguration configuration)
+            : base(controlResolver, treeBuilder, direrectiveCompilerPipeline, configuration)
         {
             this.controlBuilderFactory = controlBuilderFactory;
         }

diff --git a/src/Framework/Framework/Compilation/ControlTree/IControlResolver.cs b/src/Framework/Framework/Compilation/ControlTree/IControlResolver.cs
@@ -1,3 +1,4 @@
+using System.Collections.Generic;
 using DotVVM.Framework.Controls;
 using DotVVM.Framework.Runtime;
 
@@ -26,6 +27,10 @@ public interface IControlResolver
         /// </summary>
         IControlResolverMetadata ResolveControl(ITypeDescriptor 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>
+        IEnumerable<(string tagPrefix, string? tagName, IControlType type)> EnumerateControlTypes();
+
         /// <summary>
         /// Resolves the binding type.
         /// </summary>

diff --git a/src/Framework/Framework/Compilation/ControlTree/ResolvedTreeHelpers.cs b/src/Framework/Framework/Compilation/ControlTree/ResolvedTreeHelpers.cs
@@ -55,7 +55,7 @@ public static DataContextStack GetDataContextStack(this ResolvedPropertySetter s
 
 
         public static bool IsOnlyWhitespace(this IAbstractControl control) =>
-            control.Metadata.Type.IsEqualTo(ResolvedTypeDescriptor.Create(typeof(RawLiteral))) && control.DothtmlNode?.IsNotEmpty() == false;
+            control.Metadata.Type.IsEqualTo(ResolvedTypeDescriptor.Create(typeof(RawLiteral))) && control.DothtmlNode?.IsEmpty() == true;
 
         public static bool HasOnlyWhiteSpaceContent(this IAbstractContentNode control) =>
             control.Content.All(IsOnlyWhitespace);

diff --git a/src/Framework/Framework/Compilation/ControlType.cs b/src/Framework/Framework/Compilation/ControlType.cs
@@ -1,7 +1,9 @@
 using System;
 using System.Diagnostics;
+using System.Reflection;
 using DotVVM.Framework.Compilation.ControlTree;
 using DotVVM.Framework.Compilation.ControlTree.Resolved;
+using DotVVM.Framework.Controls;
 
 namespace DotVVM.Framework.Compilation
 {
@@ -17,6 +19,10 @@ public sealed class ControlType : IControlType
 
         ITypeDescriptor? IControlType.DataContextRequirement => ResolvedTypeDescriptor.Create(DataContextRequirement);
 
+        public string PrimaryName => GetControlNames(Type).primary;
+
+        public string[] AlternativeNames => GetControlNames(Type).alternative;
+
         static void ValidateControlClass(Type control)
         {
             if (!control.IsPublic && !control.IsNestedPublic)
@@ -35,6 +41,19 @@ public ControlType(Type type, string? virtualPath = null, Type? dataContextRequi
                 DataContextRequirement = dataContextRequirement;
         }
 
+        public static (string primary, string[] alternative) GetControlNames(Type controlType)
+        {
+            var attr = controlType.GetCustomAttribute<ControlMarkupOptionsAttribute>();
+            if (attr is null)
+            {
+                return (controlType.Name, Array.Empty<string>());
+            }
+            else
+            {
+                return (attr.PrimaryName ?? controlType.Name, attr.AlternativeNames ?? Array.Empty<string>());
+            }
+        }
+
 
         public override bool Equals(object? obj)
         {

diff --git a/src/Framework/Framework/Compilation/IControlType.cs b/src/Framework/Framework/Compilation/IControlType.cs
@@ -11,5 +11,9 @@ public interface IControlType
 
         ITypeDescriptor? DataContextRequirement { get; }
 
+        string PrimaryName { get; }
+
+        string[] AlternativeNames { get; }
+
     }
 }
diff --git a/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DotHtmlCommentNode.cs b/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DotHtmlCommentNode.cs
@@ -39,5 +39,8 @@ public override IEnumerable<DothtmlNode> EnumerateNodes()
         {
             return base.EnumerateNodes().Concat(EnumerateChildNodes().SelectMany(node => node.EnumerateNodes()));
         }
+
+        public override string ToString() =>
+            IsServerSide ? $"<%-- {Value} --%>" : $"<!-- {Value} -->";
     }
 }
diff --git a/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlAttributeNode.cs b/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlAttributeNode.cs
@@ -5,14 +5,14 @@
 
 namespace DotVVM.Framework.Compilation.Parser.Dothtml.Parser
 {
-    [DebuggerDisplay("{debuggerDisplay,nq}{ValueNode}")]
     public sealed class DothtmlAttributeNode : DothtmlNode
     {
-        #region debugger display
-        [DebuggerBrowsable(DebuggerBrowsableState.Never)]
-        private string debuggerDisplay =>
-            AttributeFullName + (ValueNode == null ? "" : "=");
-        #endregion
+        public override string ToString() =>
+            AttributeFullName + ValueNode switch {
+                null => "",
+                DothtmlValueBindingNode => $"={ValueNode}",
+                _ => $"=\"{ValueNode}\""
+            };
         public string? AttributePrefix => AttributePrefixNode?.Text;
 
         public string AttributeName => AttributeNameNode.Text;

diff --git a/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlBindingNode.cs b/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlBindingNode.cs
@@ -5,21 +5,12 @@
 
 namespace DotVVM.Framework.Compilation.Parser.Dothtml.Parser
 {
-    [DebuggerDisplay("{debuggerDisplay,nq}")]
     public class DothtmlBindingNode : DothtmlNode
     {
-
-        #region debugger display
-        [DebuggerBrowsable(DebuggerBrowsableState.Never)]
-        private string debuggerDisplay
+        public override string ToString()
         {
-            get
-            {
-                return "{" + Name + ": " + Value + "}";
-            }
+            return "{" + Name + ": " + Value + "}";
         }
-        #endregion
-
 
         public DothtmlBindingNode(DothtmlToken startToken, DothtmlToken endToken, DothtmlToken separatorToken, DothtmlNameNode nameNode, DothtmlValueTextNode valueNode)
         {

diff --git a/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlDirectiveNode.cs b/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlDirectiveNode.cs
@@ -39,5 +39,7 @@ public override IEnumerable<DothtmlNode> EnumerateNodes()
         {
             return base.EnumerateNodes().Concat( EnumerateChildNodes().SelectMany(node => node.EnumerateNodes() ) );
         }
+
+        public override string ToString() => $"@{Name} {Value}";
     }
 }
diff --git a/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlElementNode.cs b/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlElementNode.cs
@@ -5,19 +5,12 @@
 
 namespace DotVVM.Framework.Compilation.Parser.Dothtml.Parser
 {
-    [DebuggerDisplay("{debuggerDisplay,nq}")]
     public sealed class DothtmlElementNode : DothtmlNodeWithContent
     {
-        #region debugger display
-        [DebuggerBrowsable(DebuggerBrowsableState.Never)]
-        private string debuggerDisplay
+        public override string ToString()
         {
-            get
-            {
-                return "<" + (IsClosingTag ? "/" : "") + FullTagName + (Attributes.Any() ? " ..." : "") + (IsSelfClosingTag ? " /" : "") + ">";
-            }
+            return "<" + (IsClosingTag ? "/" : "") + FullTagName + (Attributes.Any() ? " ..." : "") + (IsSelfClosingTag ? " /" : "") + ">";
         }
-        #endregion
 
         public string TagName => TagNameNode.Text;
         public string? TagPrefix => TagPrefixNode?.Text;

diff --git a/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlLiteralNode.cs b/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlLiteralNode.cs
@@ -5,7 +5,6 @@
 
 namespace DotVVM.Framework.Compilation.Parser.Dothtml.Parser
 {
-    [DebuggerDisplay("{Value}")]
     public sealed class DothtmlLiteralNode : DothtmlNode
     {
         public DothtmlToken? MainValueToken { get; set; }
@@ -20,5 +19,7 @@ public override void Accept(IDothtmlSyntaxTreeVisitor visitor)
         {
             visitor.Visit(this);
         }
+
+        public override string ToString() => Value;
     }
 }
diff --git a/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlNodeHelper.cs b/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlNodeHelper.cs
@@ -5,11 +5,13 @@ namespace DotVVM.Framework.Compilation.Parser.Dothtml.Parser
 {
     public static class DothtmlNodeHelper
     {
-        public static bool IsNotEmpty([NotNullWhen(true)] this DothtmlNode? node)
+        public static bool IsNotEmpty([NotNullWhen(true)] this DothtmlNode? node) =>
+            !IsEmpty(node);
+
+        public static bool IsEmpty([NotNullWhen(false)] this DothtmlNode? node)
         {
-            return node is object &&
-                   !(node is DotHtmlCommentNode) &&
-                   !(node is DothtmlLiteralNode literalNode && string.IsNullOrWhiteSpace(literalNode.Value));
+            return node is null or DotHtmlCommentNode ||
+                   (node is DothtmlLiteralNode literalNode && string.IsNullOrWhiteSpace(literalNode.Value));
         }
 
         public static int GetContentStartPosition(this DothtmlElementNode node)

diff --git a/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlValueBindingNode.cs b/src/Framework/Framework/Compilation/Parser/Dothtml/Parser/DothtmlValueBindingNode.cs
@@ -39,5 +39,7 @@ public override IEnumerable<DothtmlNode> EnumerateNodes()
         {
             return base.EnumerateNodes().Concat(EnumerateChildNodes().SelectMany(n=> n.EnumerateNodes()));
         }
+
+        public override string ToString() => BindingNode.ToString();
     }
 }
diff --git a/src/Framework/Framework/Utils/StringSimilarity.cs b/src/Framework/Framework/Utils/StringSimilarity.cs
@@ -4,7 +4,7 @@ namespace DotVVM.Framework.Utils
 {
     internal static class StringSimilarity
     {
-        /// <summary> Edit distance with deletion (Visble), insertion (Visivble), substitution (Visine) and transposition (Visilbe) </summary>
+        /// <summary> Edit distance with deletion (Visble), insertion (Visivble), substitution (Visinle) and transposition (Visilbe) </summary>
         public static int DamerauLevenshteinDistance(string a, string b)
         {
             // https://en.wikipedia.org/wiki/Damerau%E2%80%93Levenshtein_distance

diff --git a/src/Samples/Tests/Tests/ErrorsTests.cs b/src/Samples/Tests/Tests/ErrorsTests.cs
@@ -57,7 +57,7 @@ public void Error_NonExistingControl()
                             s.Contains("could not be resolved")
                             );
                 AssertUI.InnerTextEquals(browser.First("[class='errorUnderline']")
-                    , "<dot:NonExistingControl />", false);
+                    , "NonExistingControl", false);
             });
         }
 

diff --git a/src/Tests/ControlTests/CompositeControlTests.cs b/src/Tests/ControlTests/CompositeControlTests.cs
@@ -282,7 +282,7 @@ public async Task ControlWithCollection_WrongType()
                     <cc:ControlWithCollectionProperty> <Repeaters> <bazmek /> </Repeaters> </cc:ControlWithCollectionProperty>
                 "));
 
-            Assert.AreEqual("Control type DotVVM.Framework.Controls.HtmlGenericControl can't be used in collection of type DotVVM.Framework.Controls.Repeater.", e.Message);
+            Assert.AreEqual("Control type DotVVM.Framework.Controls.HtmlGenericControl can't be used in a property of type DotVVM.Framework.Controls.Repeater.", e.Message);
         }
 
         [TestMethod]

diff --git a/src/Tests/Runtime/ControlTree/DefaultControlTreeResolver/CompilationWarningsTests.cs b/src/Tests/Runtime/ControlTree/DefaultControlTreeResolver/CompilationWarningsTests.cs
@@ -90,6 +90,64 @@ @viewModel System.Boolean
             Assert.AreEqual("HTML attribute name 'IncludeInPage' should not contain uppercase letters. Did you intent to use a DotVVM property instead?", XAssert.Single(attribute2.AttributeNameNode.NodeWarnings));
         }
 
+        [TestMethod]
+        public void DefaultViewCompiler_NonExistentPropertyWarning_InnerElement()
+        {
+           var markup = $@"
+@viewModel bool
+<dot:Repeater>
+    <EmptyDataTemplate>empty</EmptyDataTemplate>
+    <SepratrorTemplate> --- </SepratrorTemplate>
+    <TextBox Text=AA />
+    test
+    <SeparatorTemplate> ---- </SeparatorTemplate>
+</dot:Repeater>
+";
+            var repeater = ParseSource(markup)
+                .Content.SelectRecursively(c => c.Content)
+                .Single(c => c.Metadata.Type == typeof(Repeater));
+
+            var elementNode = (DothtmlElementNode)repeater.DothtmlNode;
+            var correctTemplateElement = elementNode.Content.OfType<DothtmlElementNode>().Single(e => e.TagName == "EmptyDataTemplate");
+            var mistakeTemplateElement = elementNode.Content.OfType<DothtmlElementNode>().Single(e => e.TagName == "SepratrorTemplate");
+            var mistakeTextBoxElement = elementNode.Content.OfType<DothtmlElementNode>().Single(e => e.TagName == "TextBox");
+            var lateTemplate = elementNode.Content.OfType<DothtmlElementNode>().Single(e => e.TagName == "SeparatorTemplate");
+
+            XAssert.Empty(correctTemplateElement.TagNameNode.NodeWarnings);
+            Assert.AreEqual("HTML element name 'SepratrorTemplate' should not contain uppercase letters. Did you mean SeparatorTemplate, or another DotVVM property?", XAssert.Single(mistakeTemplateElement.TagNameNode.NodeWarnings));
+            Assert.AreEqual("HTML element name 'TextBox' should not contain uppercase letters. Did you mean dot:CheckBox, dot:ListBox, dot:TextBox, or another DotVVM control?", XAssert.Single(mistakeTextBoxElement.TagNameNode.NodeWarnings));
+            Assert.AreEqual("This element looks like an inner element property Repeater.SeparatorTemplate, but it isn't, because it is prefixed by other content ('<SepratrorTemplate>')).", XAssert.Single(lateTemplate.TagNameNode.NodeWarnings));
+        }
+
+        [TestMethod]
+        public void DefaultViewCompiler_DisallowedContentControlType()
+        {
+           var markup = $@"
+@viewModel System.Collections.Generic.IEnumerable<string>
+<dot:GridView DataSource={{value: _this}}>
+    <EmtyDataTemplate>empty</EmtyDataTemplate>
+    <dot:GridViewTemplateColumn>test</dot:GridViewTemplateColumn>
+    <dot:TextBox Text={{value: _this}} />
+</dot:GridView>
+";
+            var repeater = ParseSource(markup)
+                .Content.SelectRecursively(c => c.Content)
+                .Single(c => c.Metadata.Type == typeof(GridView));
+
+            var elementNode = (DothtmlElementNode)repeater.DothtmlNode;
+            var fine = elementNode.Content.OfType<DothtmlElementNode>().Single(e => e.TagName == "GridViewTemplateColumn");
+            var unallowedType = elementNode.Content.OfType<DothtmlElementNode>().Single(e => e.TagName == "TextBox");
+            var mistypedTemplate = elementNode.Content.OfType<DothtmlElementNode>().Single(e => e.TagName == "EmtyDataTemplate");
+
+            XAssert.Empty(fine.TagNameNode.NodeWarnings);
+            XAssert.Empty(fine.TagNameNode.NodeErrors);
+
+            Assert.AreEqual("Control type DotVVM.Framework.Controls.TextBox can't be used in a property of type DotVVM.Framework.Controls.GridViewColumn.", XAssert.Single(unallowedType.TagNameNode.NodeErrors));
+            Assert.AreEqual("Control type DotVVM.Framework.Controls.HtmlGenericControl can't be used in a property of type DotVVM.Framework.Controls.GridViewColumn. Did you mean EmptyDataTemplate, or another DotVVM property?", XAssert.Single(mistypedTemplate.TagNameNode.NodeErrors));
+            Assert.AreEqual("HTML element name 'EmtyDataTemplate' should not contain uppercase letters. Did you mean EmptyDataTemplate, or another DotVVM property?", XAssert.Single(mistypedTemplate.TagNameNode.NodeWarnings));
+        }
+
+
         [TestMethod]
         public void DefaultViewCompiler_UnsupportedCallSite_ResourceBinding_Warning()
         {
-Original file line number
+Diff line change
@@ Expand Up / @@ -39,5 +39,7 @@ public override IEnumerable<DothtmlNode> EnumerateNodes() @@
             {
                 return base.EnumerateNodes().Concat( EnumerateChildNodes().SelectMany(node => node.EnumerateNodes() ) );
             }
+            public override string ToString() => $"@{Name} {Value}";
         }
     }