From 98ffb28a37e2893dc02134370e9e71d41fc051e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Herceg?= Date: Sat, 26 Oct 2024 15:05:06 +0200 Subject: [PATCH 1/4] Fixed ambiguous namespace resolution and matching non-public types --- .../Compilation/Binding/TypeRegistry.cs | 27 ++++++++++-- src/Tests/Binding/BindingCompilationTests.cs | 44 +++++++++++++++---- .../ControlTree/ControlValidationTests.cs | 2 +- 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/src/Framework/Framework/Compilation/Binding/TypeRegistry.cs b/src/Framework/Framework/Compilation/Binding/TypeRegistry.cs index f84bc42948..50c28b67fc 100644 --- a/src/Framework/Framework/Compilation/Binding/TypeRegistry.cs +++ b/src/Framework/Framework/Compilation/Binding/TypeRegistry.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Linq.Expressions; using DotVVM.Framework.Utils; +using FastExpressionCompiler; namespace DotVVM.Framework.Compilation.Binding { @@ -34,8 +35,13 @@ public TypeRegistry(CompiledAssemblyCache compiledAssemblyCache, ImmutableDictio { return expr; } - expr = resolvers.Select(r => r(name)).FirstOrDefault(e => e != null); - if (expr != null) return expr; + var matchedExpressions = resolvers.Select(r => r(name)).Where(e => e != null).Distinct(ExpressionTypeComparer.Instance).ToList(); + if (matchedExpressions.Count > 1) + { + throw new InvalidOperationException($"The identifier '{name}' is ambiguous between the following types: {string.Join(", ", matchedExpressions.Select(e => e!.Type.ToCode()))}. Please specify the namespace explicitly."); + } + + if (matchedExpressions.Count == 1) return matchedExpressions[0]; if (throwOnNotFound) throw new InvalidOperationException($"The identifier '{ name }' could not be resolved!"); return null; } @@ -56,7 +62,7 @@ public TypeRegistry AddSymbols(IEnumerable> symbols) [return: NotNullIfNotNull("type")] public static Expression? CreateStatic(Type? type) { - return type == null ? null : new StaticClassIdentifierExpression(type); + return type is { IsPublic: true } or { IsNestedPublic: true } ? new StaticClassIdentifierExpression(type) : null; } private static readonly ImmutableDictionary predefinedTypes = @@ -122,5 +128,20 @@ public TypeRegistry AddImportedTypes(CompiledAssemblyCache compiledAssemblyCache }; else return t => TypeRegistry.CreateStatic(compiledAssemblyCache.FindType(import.Namespace + "." + t)); } + + class ExpressionTypeComparer : IEqualityComparer + { + public static readonly ExpressionTypeComparer Instance = new(); + + public bool Equals(Expression? x, Expression? y) + { + if (ReferenceEquals(x, y)) return true; + if (x is null) return false; + if (y is null) return false; + return ReferenceEquals(x.Type, y.Type); + } + + public int GetHashCode(Expression? obj) => obj?.Type.GetHashCode() ?? 0; + } } } diff --git a/src/Tests/Binding/BindingCompilationTests.cs b/src/Tests/Binding/BindingCompilationTests.cs index 07f9551e48..a78724992f 100755 --- a/src/Tests/Binding/BindingCompilationTests.cs +++ b/src/Tests/Binding/BindingCompilationTests.cs @@ -1339,8 +1339,23 @@ public void BindingCompiler_InvalidStructComparison() => [ExpectedExceptionMessageSubstring(typeof(BindingPropertyException), "Cannot apply And operator to types TestEnum and Boolean")] public void BindingCompiler_InvalidBitAndComparison() => ExecuteBinding("EnumProperty & 2 == 0", new TestViewModel()); + + [TestMethod] + public void BindingCompiler_DoNotMatchInternalClasses() + { + var result = ExecuteBinding("Strings.SomeResource", new[] { new NamespaceImport("System.Linq"), new NamespaceImport("DotVVM.Framework.Tests") }, new TestViewModel()); + Assert.AreEqual("hello", result); + } + + [TestMethod] + [ExpectedExceptionMessageSubstring(typeof(BindingPropertyException), "ambiguous")] + public void BindingCompiler_AmbiguousMatches() + { + var result = ExecuteBinding("Strings.SomeResource", new[] { new NamespaceImport("DotVVM.Framework.Tests.Ambiguous"), new NamespaceImport("DotVVM.Framework.Tests") }, new TestViewModel()); + Assert.AreEqual("hello", result); + } } - class TestViewModel + public class TestViewModel { public bool BoolProp { get; set; } public string StringProp { get; set; } @@ -1452,7 +1467,7 @@ public T GenericDefault(T something, T somethingElse = default) } - record struct VehicleNumber( + public record struct VehicleNumber( [property: Range(100, 999)] int Value ): IDotvvmPrimitiveType @@ -1499,7 +1514,7 @@ public string CustomGenericDelegateInvoker(T item, CustomGenericDelegate f string.Join(",", func(new List() { item, item })); } - class TestViewModel2 + public class TestViewModel2 { public int MyProperty { get; set; } public string SomeString { get; set; } @@ -1520,7 +1535,7 @@ public class TestViewModel3 : DotvvmViewModelBase public string SomeString { get; set; } } - class TestViewModel4 + public class TestViewModel4 { public int Number { get; set; } @@ -1537,7 +1552,7 @@ public Task Multiply() } } - class TestViewModel5 + public class TestViewModel5 { public Dictionary Dictionary { get; set; } = new Dictionary() { @@ -1559,16 +1574,16 @@ class TestViewModel5 public int[] Array { get; set; } = new int[] { 1, 2, 3 }; } - struct TestStruct + public struct TestStruct { public int Int { get; set; } } - class Something + public class Something { public bool Value { get; set; } public string StringValue { get; set; } } - enum TestEnum + public enum TestEnum { A, B, @@ -1584,4 +1599,17 @@ public static class TestStaticClass { public static string GetSomeString() => "string 123"; } + + public class Strings + { + public static string SomeResource = "hello"; + } +} + +namespace DotVVM.Framework.Tests.Ambiguous +{ + public class Strings + { + public static string SomeResource = "hello2"; + } } diff --git a/src/Tests/Runtime/ControlTree/ControlValidationTests.cs b/src/Tests/Runtime/ControlTree/ControlValidationTests.cs index 01ff229187..56f6bceb90 100644 --- a/src/Tests/Runtime/ControlTree/ControlValidationTests.cs +++ b/src/Tests/Runtime/ControlTree/ControlValidationTests.cs @@ -11,7 +11,7 @@ namespace DotVVM.Framework.Tests.Runtime.ControlTree { - internal class TestViewModel : Tests.Binding.TestViewModel + public class TestViewModel : Tests.Binding.TestViewModel { public StructList? NullableDateList { get; } public StructList? DateList { get; } From 2f3ced4ba488fafa14c18c1ab0cfca05e6112547 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Herceg=20=28RIGANTI=20s=2Er=2Eo=2E=29?= Date: Sun, 3 Nov 2024 10:56:50 +0100 Subject: [PATCH 2/4] Removed unused sample --- .../EventPropagation/EventPropagation.dothtml | 13 ------------- 1 file changed, 13 deletions(-) delete mode 100644 src/Samples/Common/Views/ComplexSamples/EventPropagation/EventPropagation.dothtml diff --git a/src/Samples/Common/Views/ComplexSamples/EventPropagation/EventPropagation.dothtml b/src/Samples/Common/Views/ComplexSamples/EventPropagation/EventPropagation.dothtml deleted file mode 100644 index 49b5ef89ac..0000000000 --- a/src/Samples/Common/Views/ComplexSamples/EventPropagation/EventPropagation.dothtml +++ /dev/null @@ -1,13 +0,0 @@ -@viewModel DotVVM.Samples.Common.Views.ComplexSamples.EventPropagation.EventPropagationViewModel, DotVVM.Samples.Common.Views.ComplexSamples.EventPropagation - - - - - - - - - - - - \ No newline at end of file From e09359d66aacd2570f30231d0685615e6ac24608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Herceg=20=28RIGANTI=20s=2Er=2Eo=2E=29?= Date: Sun, 3 Nov 2024 10:57:16 +0100 Subject: [PATCH 3/4] Fixed review --- src/Adapters/Tests/WebForms/HybridRouteLinkTests.cs | 4 ++-- .../Framework/Compilation/Binding/TypeRegistry.cs | 8 ++++---- src/Framework/Framework/Utils/ReflectionUtils.cs | 13 +++++++++++++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/Adapters/Tests/WebForms/HybridRouteLinkTests.cs b/src/Adapters/Tests/WebForms/HybridRouteLinkTests.cs index fe3cc1616d..fa368198c0 100644 --- a/src/Adapters/Tests/WebForms/HybridRouteLinkTests.cs +++ b/src/Adapters/Tests/WebForms/HybridRouteLinkTests.cs @@ -78,7 +78,7 @@ public async Task HybridRouteLink_SuffixAndQueryString() } } - class ControlTestViewModel + public class ControlTestViewModel { public int Value { get; set; } = 15; @@ -90,7 +90,7 @@ class ControlTestViewModel }; } - class ControlTestChildViewModel + public class ControlTestChildViewModel { public int Id { get; set; } public string Name { get; set; } diff --git a/src/Framework/Framework/Compilation/Binding/TypeRegistry.cs b/src/Framework/Framework/Compilation/Binding/TypeRegistry.cs index 50c28b67fc..55d154a2cf 100644 --- a/src/Framework/Framework/Compilation/Binding/TypeRegistry.cs +++ b/src/Framework/Framework/Compilation/Binding/TypeRegistry.cs @@ -35,7 +35,7 @@ public TypeRegistry(CompiledAssemblyCache compiledAssemblyCache, ImmutableDictio { return expr; } - var matchedExpressions = resolvers.Select(r => r(name)).Where(e => e != null).Distinct(ExpressionTypeComparer.Instance).ToList(); + var matchedExpressions = resolvers.Select(r => r(name)).WhereNotNull().Distinct(ExpressionTypeComparer.Instance).ToList(); if (matchedExpressions.Count > 1) { throw new InvalidOperationException($"The identifier '{name}' is ambiguous between the following types: {string.Join(", ", matchedExpressions.Select(e => e!.Type.ToCode()))}. Please specify the namespace explicitly."); @@ -62,7 +62,7 @@ public TypeRegistry AddSymbols(IEnumerable> symbols) [return: NotNullIfNotNull("type")] public static Expression? CreateStatic(Type? type) { - return type is { IsPublic: true } or { IsNestedPublic: true } ? new StaticClassIdentifierExpression(type) : null; + return type?.IsPublicType() == true ? new StaticClassIdentifierExpression(type) : null; } private static readonly ImmutableDictionary predefinedTypes = @@ -129,7 +129,7 @@ public TypeRegistry AddImportedTypes(CompiledAssemblyCache compiledAssemblyCache else return t => TypeRegistry.CreateStatic(compiledAssemblyCache.FindType(import.Namespace + "." + t)); } - class ExpressionTypeComparer : IEqualityComparer + class ExpressionTypeComparer : IEqualityComparer { public static readonly ExpressionTypeComparer Instance = new(); @@ -141,7 +141,7 @@ public bool Equals(Expression? x, Expression? y) return ReferenceEquals(x.Type, y.Type); } - public int GetHashCode(Expression? obj) => obj?.Type.GetHashCode() ?? 0; + public int GetHashCode(Expression obj) => obj.Type.GetHashCode(); } } } diff --git a/src/Framework/Framework/Utils/ReflectionUtils.cs b/src/Framework/Framework/Utils/ReflectionUtils.cs index 91d6c2aa00..3eaf1ebe5f 100644 --- a/src/Framework/Framework/Utils/ReflectionUtils.cs +++ b/src/Framework/Framework/Utils/ReflectionUtils.cs @@ -778,5 +778,18 @@ internal static Type AssignGenericParameters(Type t, IReadOnlyDictionary + /// Determines whether the type is public and has the entire chain of nested parents public. + /// + public static bool IsPublicType(this Type type) + { + if (type.IsPublic) return true; + if (type.IsNested) + { + return type.IsNestedPublic && IsPublicType(type.DeclaringType!); + } + return false; + } } } From 4fc49309e4b552a281f637b1354d1a8e53fdc45b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Herceg=20=28RIGANTI=20s=2Er=2Eo=2E=29?= Date: Sun, 3 Nov 2024 11:32:48 +0100 Subject: [PATCH 4/4] Fixed bug in samples --- .../Redirect/RedirectPostbackConcurrencyViewModel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Samples/Common/ViewModels/FeatureSamples/Redirect/RedirectPostbackConcurrencyViewModel.cs b/src/Samples/Common/ViewModels/FeatureSamples/Redirect/RedirectPostbackConcurrencyViewModel.cs index 75f53ba0b1..5eb95c352b 100644 --- a/src/Samples/Common/ViewModels/FeatureSamples/Redirect/RedirectPostbackConcurrencyViewModel.cs +++ b/src/Samples/Common/ViewModels/FeatureSamples/Redirect/RedirectPostbackConcurrencyViewModel.cs @@ -7,7 +7,7 @@ namespace DotVVM.Samples.BasicSamples.ViewModels.FeatureSamples.Redirect { - class RedirectPostbackConcurrencyViewModel : DotvvmViewModelBase + public class RedirectPostbackConcurrencyViewModel : DotvvmViewModelBase { public static int GlobalCounter = 0; private readonly IReturnedFileStorage returnedFileStorage;