Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed ambiguous namespace resolution and matching non-public types #1877

Merged
merged 4 commits into from
Nov 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading