Skip to content

Commit

Permalink
Merge pull request #1877 from riganti/fix/binding-ambiguous-namespaces
Browse files Browse the repository at this point in the history
Fixed ambiguous namespace resolution and matching non-public types
  • Loading branch information
exyi authored Nov 3, 2024
2 parents 22f9e4b + 4fc4930 commit 86a56fc
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 28 deletions.
4 changes: 2 additions & 2 deletions src/Adapters/Tests/WebForms/HybridRouteLinkTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public async Task HybridRouteLink_SuffixAndQueryString()
}
}

class ControlTestViewModel
public class ControlTestViewModel
{
public int Value { get; set; } = 15;

Expand All @@ -90,7 +90,7 @@ class ControlTestViewModel
};
}

class ControlTestChildViewModel
public class ControlTestChildViewModel
{
public int Id { get; set; }
public string Name { get; set; }
Expand Down
27 changes: 24 additions & 3 deletions src/Framework/Framework/Compilation/Binding/TypeRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq;
using System.Linq.Expressions;
using DotVVM.Framework.Utils;
using FastExpressionCompiler;

namespace DotVVM.Framework.Compilation.Binding
{
Expand Down Expand Up @@ -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)).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.");
}

if (matchedExpressions.Count == 1) return matchedExpressions[0];
if (throwOnNotFound) throw new InvalidOperationException($"The identifier '{ name }' could not be resolved!");
return null;
}
Expand All @@ -56,7 +62,7 @@ public TypeRegistry AddSymbols(IEnumerable<Func<string, Expression?>> symbols)
[return: NotNullIfNotNull("type")]
public static Expression? CreateStatic(Type? type)
{
return type == null ? null : new StaticClassIdentifierExpression(type);
return type?.IsPublicType() == true ? new StaticClassIdentifierExpression(type) : null;
}

private static readonly ImmutableDictionary<string, Expression> predefinedTypes =
Expand Down Expand Up @@ -122,5 +128,20 @@ public TypeRegistry AddImportedTypes(CompiledAssemblyCache compiledAssemblyCache
};
else return t => TypeRegistry.CreateStatic(compiledAssemblyCache.FindType(import.Namespace + "." + t));
}

class ExpressionTypeComparer : IEqualityComparer<Expression>
{
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();
}
}
}
13 changes: 13 additions & 0 deletions src/Framework/Framework/Utils/ReflectionUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -778,5 +778,18 @@ internal static Type AssignGenericParameters(Type t, IReadOnlyDictionary<Type, T
return t;
}
}

/// <summary>
/// Determines whether the type is public and has the entire chain of nested parents public.
/// </summary>
public static bool IsPublicType(this Type type)
{
if (type.IsPublic) return true;
if (type.IsNested)
{
return type.IsNestedPublic && IsPublicType(type.DeclaringType!);
}
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

This file was deleted.

44 changes: 36 additions & 8 deletions src/Tests/Binding/BindingCompilationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down Expand Up @@ -1452,7 +1467,7 @@ public T GenericDefault<T>(T something, T somethingElse = default)
}


record struct VehicleNumber(
public record struct VehicleNumber(
[property: Range(100, 999)]
int Value
): IDotvvmPrimitiveType
Expand Down Expand Up @@ -1499,7 +1514,7 @@ public string CustomGenericDelegateInvoker<T>(T item, CustomGenericDelegate<T> f
string.Join(",", func(new List<T>() { item, item }));
}

class TestViewModel2
public class TestViewModel2
{
public int MyProperty { get; set; }
public string SomeString { get; set; }
Expand All @@ -1520,7 +1535,7 @@ public class TestViewModel3 : DotvvmViewModelBase
public string SomeString { get; set; }
}

class TestViewModel4
public class TestViewModel4
{
public int Number { get; set; }

Expand All @@ -1537,7 +1552,7 @@ public Task Multiply()
}
}

class TestViewModel5
public class TestViewModel5
{
public Dictionary<int, int> Dictionary { get; set; } = new Dictionary<int, int>()
{
Expand All @@ -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,
Expand All @@ -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";
}
}
2 changes: 1 addition & 1 deletion src/Tests/Runtime/ControlTree/ControlValidationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
namespace DotVVM.Framework.Tests.Runtime.ControlTree
{

internal class TestViewModel : Tests.Binding.TestViewModel
public class TestViewModel : Tests.Binding.TestViewModel
{
public StructList<DateTime?>? NullableDateList { get; }
public StructList<DateTime>? DateList { get; }
Expand Down

0 comments on commit 86a56fc

Please sign in to comment.