Merge pull request #1772 from riganti/markup-properties-same-name-error

Properties with the same name defined by `@property` directive
riganti · Feb 13, 2024 · a348360 · a348360
2 parents 7faf5a0 + ffaf508
commit a348360
Show file tree

Hide file tree

Showing 10 changed files with 120 additions and 2 deletions.
diff --git a/src/Framework/Framework/Compilation/Directives/PropertyDeclarationDirectiveCompiler.cs b/src/Framework/Framework/Compilation/Directives/PropertyDeclarationDirectiveCompiler.cs
@@ -112,6 +112,19 @@ protected override ImmutableList<DotvvmProperty> CreateArtefact(IReadOnlyList<IA
 
             return directives
             .Where(HasPropertyType)
+            .GroupBy(
+                directive => directive.NameSyntax.Name,
+                directive => directive,
+                (name, directiveOfSameName) => {
+                    if (directiveOfSameName.Count() > 1)
+                    {
+                        foreach (var sameNameDirective in directiveOfSameName.Skip(1))
+                        {
+                            sameNameDirective.DothtmlNode?.AddError("Property with the same name is already defined.");
+                        };
+                    }
+                    return directiveOfSameName.First();
+                })
             .Select(TryCreateDotvvmPropertyFromDirective)
             .ToImmutableList();
         }

diff --git a/src/Samples/Common/DotVVM.Samples.Common.csproj b/src/Samples/Common/DotVVM.Samples.Common.csproj
@@ -74,6 +74,10 @@
     <None Remove="Views\Errors\ExceptionInRender.dothtml" />
     <None Remove="Views\Errors\InvalidServiceDirective.dothtml" />
     <None Remove="Views\Errors\InvalidLocationFallback.dothtml" />
+    <None Remove="Views\Errors\MarkupControlInvalidViewModel.dothtml" />
+    <None Remove="Views\Errors\MarkupControlPropertiesSameName.dotcontrol" />
+    <None Remove="Views\Errors\MarkupControlPropertiesSameNameWithBase.dotcontrol" />
+    <None Remove="Views\Errors\MarkupControlPropertiesSameNameWithBase.dothtml" />
     <None Remove="Views\Errors\ResourceCircularDependency.dothtml" />
     <None Remove="Views\FeatureSamples\Api\ApiInSpa_Master.dotmaster" />
     <None Remove="Views\FeatureSamples\Api\ApiInSpa_PageA.dothtml" />

diff --git a/src/Samples/Common/ViewModels/Errors/MarkupControlPropertiesSameNameWithBase.cs b/src/Samples/Common/ViewModels/Errors/MarkupControlPropertiesSameNameWithBase.cs
@@ -0,0 +1,20 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+using DotVVM.Framework.Binding;
+using DotVVM.Framework.Controls;
+
+namespace DotVVM.Samples.Common.ViewModels.Errors
+{
+    public class MarkupControlPropertiesSameNameWithBase : DotvvmMarkupControl
+    {
+        public string MyProperty
+        {
+            get { return (string)GetValue(MyPropertyProperty); }
+            set { SetValue(MyPropertyProperty, value); }
+        }
+        public static readonly DotvvmProperty MyPropertyProperty
+            = DotvvmProperty.Register<string, MarkupControlPropertiesSameNameWithBase>(c => c.MyProperty, null);
+
+    }
+}
diff --git a/src/Samples/Common/Views/Errors/MarkupControlPropertiesSameName.dotcontrol b/src/Samples/Common/Views/Errors/MarkupControlPropertiesSameName.dotcontrol
@@ -0,0 +1,6 @@
+@viewModel object
+
+@property int MyProperty
+@property bool MyProperty
+
+{{value: _control.MyProperty}}
diff --git a/src/Samples/Common/Views/Errors/MarkupControlPropertiesSameName.dothtml b/src/Samples/Common/Views/Errors/MarkupControlPropertiesSameName.dothtml
@@ -0,0 +1,15 @@
+@viewModel object
+
+<!DOCTYPE html>
+
+<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
+<head>
+    <meta charset="utf-8" />
+    <title></title>
+</head>
+<body>
+    <sample:MarkupControlPropertiesSameName MyProperty="1" />
+</body>
+</html>
+
+
diff --git a/src/Samples/Common/Views/Errors/MarkupControlPropertiesSameNameWithBase.dotcontrol b/src/Samples/Common/Views/Errors/MarkupControlPropertiesSameNameWithBase.dotcontrol
@@ -0,0 +1,6 @@
+@viewModel object
+@baseType DotVVM.Samples.Common.ViewModels.Errors.MarkupControlPropertiesSameNameWithBase
+
+@property bool MyProperty
+
+{{value: _control.MyProperty}}
diff --git a/src/Samples/Common/Views/Errors/MarkupControlPropertiesSameNameWithBase.dothtml b/src/Samples/Common/Views/Errors/MarkupControlPropertiesSameNameWithBase.dothtml
@@ -0,0 +1,15 @@
+@viewModel object
+
+<!DOCTYPE html>
+
+<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
+<head>
+    <meta charset="utf-8" />
+    <title></title>
+</head>
+<body>
+    <sample:MarkupControlPropertiesSameNameWithBase MyProperty="1" />
+</body>
+</html>
+
+
diff --git a/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs b/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs
diff --git a/src/Samples/Tests/Tests/ErrorsTests.cs b/src/Samples/Tests/Tests/ErrorsTests.cs
@@ -487,6 +487,17 @@ public void Error_ExceptionInLifecycle()
             });
         }
 
+        [Fact]
+        public void Error_MarkupControlPropertiesSameName()
+        {
+            RunInAllBrowsers(browser => {
+                browser.NavigateToUrl(SamplesRouteUrls.Errors_MarkupControlPropertiesSameName);
+
+                AssertUI.InnerText(browser.First(".exceptionMessage"), s => s.Contains("already defined"));
+                AssertUI.InnerText(browser.First(".errorUnderline"), s => s.Contains("@property bool MyProperty"));
+            });
+        }
+
         public ErrorsTests(ITestOutputHelper output) : base(output)
         {
         }

diff --git a/src/Tests/Runtime/ControlTree/DefaultControlTreeResolver/PropertyDirectiveTests.cs b/src/Tests/Runtime/ControlTree/DefaultControlTreeResolver/PropertyDirectiveTests.cs
@@ -1,8 +1,6 @@
 using Microsoft.VisualStudio.TestTools.UnitTesting;
-using DotVVM.Framework.Tests.Runtime.ControlTree.DefaultControlTreeResolver;
 using System.Linq;
 using DotVVM.Framework.Compilation.ControlTree;
-using DotVVM.Framework.Controls;
 
 namespace DotVVM.Framework.Tests.Runtime.ControlTree
 {
@@ -82,5 +80,33 @@ @viewModel object
             AssertEx.BindingNode(property.Attributes[0].Initializer, "True", 62, 5);
             AssertEx.BindingNode(property.Attributes[1].Initializer, "False", 106, 6);
         }
+
+        [TestMethod]
+        public void ResolvedTree_PropertyDirectiveSameName_ErrorReported()
+        {
+            var root = ParseSource("""
+                @viewModel object
+                @property int MyProperty
+                @property bool MyProperty
+                @property string MyProperty
+
+                {{value: _control.MyProperty}}
+""", "control.dotcontrol");
+
+            var firstProperty = root.Directives["property"][0] as IAbstractPropertyDeclarationDirective;
+            var secondProperty = root.Directives["property"][1] as IAbstractPropertyDeclarationDirective;
+            var thirdProperty = root.Directives["property"][2] as IAbstractPropertyDeclarationDirective;
+
+            var property = root.Metadata.FindProperty("MyProperty");
+
+            //First property wins
+            Assert.AreEqual(property.PropertyType, typeof(int));
+
+            Assert.IsFalse(firstProperty.DothtmlNode.HasNodeErrors);
+            Assert.IsTrue(secondProperty.DothtmlNode.HasNodeErrors);
+            Assert.IsTrue(thirdProperty.DothtmlNode.HasNodeErrors);
+
+
+        }
     }
 }