diff --git a/Directory.Build.props b/Directory.Build.props index e45c16f6a..b3d933914 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -16,9 +16,9 @@ all runtime; build; native; contentfiles; analyzers - + diff --git a/src/embed_tests/Python.EmbeddingTest.csproj b/src/embed_tests/Python.EmbeddingTest.csproj index 4993994d3..4ecb689d7 100644 --- a/src/embed_tests/Python.EmbeddingTest.csproj +++ b/src/embed_tests/Python.EmbeddingTest.csproj @@ -24,7 +24,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - + 1.0.0 all diff --git a/src/embed_tests/TestConverter.cs b/src/embed_tests/TestConverter.cs index a59b9c97b..83540a16a 100644 --- a/src/embed_tests/TestConverter.cs +++ b/src/embed_tests/TestConverter.cs @@ -209,6 +209,90 @@ class PyGetListImpl(test.GetListImpl): List result = inst.GetList(); CollectionAssert.AreEqual(new[] { "testing" }, result); } + + /// + /// Test that when a method returns a concrete type implementing IDisposable, + /// the object is wrapped as the concrete type (not IDisposable interface), + /// preserving access to concrete type members and supporting 'with' statements. + /// + [Test] + public void ConcreteTypeImplementingIDisposable_IsWrappedAsConcreteType() + { + using var scope = Py.CreateScope(); + scope.Import(typeof(ConcreteDisposableResource).Namespace, asname: "test"); + + // Reset static state + ConcreteDisposableResource.IsDisposed = false; + ConcreteDisposableResource.InstanceCount = 0; + + // Test that a method returning IDisposable but actually returning a concrete type + // wraps the object as the concrete type, not the interface + scope.Exec(@" +import clr +clr.AddReference('Python.EmbeddingTest') +from Python.EmbeddingTest import ConcreteDisposableResource + +# Get a resource through a method that declares IDisposable return type +resource = ConcreteDisposableResource.GetResource() + +# Verify it's wrapped as the concrete type, not IDisposable +# The concrete type has a GetValue() method that IDisposable doesn't have +value = resource.GetValue() +assert value == 42, f'Expected 42, got {value}' + +# Verify the concrete type name is accessible +type_name = resource.GetType().Name +assert type_name == 'ConcreteDisposableResource', f'Expected ConcreteDisposableResource, got {type_name}' + +# Verify 'with' statement still works (IDisposable support) +with resource: + inside_value = resource.GetValue() + assert inside_value == 42 + assert ConcreteDisposableResource.IsDisposed == False + +# After 'with' block, should be disposed +assert ConcreteDisposableResource.IsDisposed == True +"); + + // Verify the resource was actually disposed + Assert.IsTrue(ConcreteDisposableResource.IsDisposed, "Resource should be disposed after 'with' statement"); + } + + /// + /// Test that Converter.ToPython wraps concrete types implementing interfaces + /// as the concrete type, not the interface, when the declared type is an interface. + /// + [Test] + public void Converter_ToPython_ConcreteTypeOverInterface() + { + using (Py.GIL()) + { + // Create a concrete type that implements IDisposable + var concreteResource = new ConcreteDisposableResource(100); + + // Convert using IDisposable as the declared type (simulating method return type) + var pyObject = Converter.ToPython(concreteResource, typeof(IDisposable)); + + // Verify it's wrapped as the concrete type, not IDisposable + var wrappedObject = ManagedType.GetManagedObject(pyObject.BorrowOrThrow()); + Assert.IsInstanceOf(wrappedObject); + + var clrObject = (CLRObject)wrappedObject; + var wrappedType = clrObject.inst.GetType(); + + // Should be the concrete type, not IDisposable + Assert.AreEqual(typeof(ConcreteDisposableResource), wrappedType); + Assert.AreNotEqual(typeof(IDisposable), wrappedType); + + // Verify we can access concrete type members from Python + using var scope = Py.CreateScope(); + scope.Set("resource", pyObject.MoveToPyObject()); + var result = scope.Eval("resource.GetValue()"); + Assert.AreEqual(100, result.As()); + + pyObject.Dispose(); + } + } } public interface IGetList @@ -220,4 +304,44 @@ public class GetListImpl : IGetList { public List GetList() => new() { "testing" }; } + + /// + /// A concrete class implementing IDisposable with additional members. + /// Used to test that methods returning IDisposable but actually returning + /// concrete types are wrapped as the concrete type, not the interface. + /// + public class ConcreteDisposableResource : IDisposable + { + public static bool IsDisposed { get; set; } + public static int InstanceCount { get; set; } + + private readonly int _value; + + public ConcreteDisposableResource(int value = 42) + { + _value = value; + InstanceCount++; + IsDisposed = false; + } + + /// + /// A method that exists only on the concrete type, not on IDisposable. + /// This verifies that the object is wrapped as the concrete type. + /// + public int GetValue() => _value; + + public void Dispose() + { + IsDisposed = true; + } + + /// + /// A method that declares IDisposable return type but actually returns + /// the concrete type. This is the scenario we're testing. + /// + public static IDisposable GetResource() + { + return new ConcreteDisposableResource(); + } + } } diff --git a/src/module_tests/Python.ModuleTest.csproj b/src/module_tests/Python.ModuleTest.csproj index 7a8aa9ac3..081b8d871 100644 --- a/src/module_tests/Python.ModuleTest.csproj +++ b/src/module_tests/Python.ModuleTest.csproj @@ -20,7 +20,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - + 1.0.0 all diff --git a/src/perf_tests/Python.PerformanceTests.csproj b/src/perf_tests/Python.PerformanceTests.csproj index b526183cc..5a5ab5455 100644 --- a/src/perf_tests/Python.PerformanceTests.csproj +++ b/src/perf_tests/Python.PerformanceTests.csproj @@ -5,16 +5,10 @@ false x64 x64 - - - - ..\..\pythonnet\runtime\Python.Runtime.dll - true - + - diff --git a/src/python_tests_runner/Python.PythonTestsRunner.csproj b/src/python_tests_runner/Python.PythonTestsRunner.csproj index 63981c424..d5cf106d1 100644 --- a/src/python_tests_runner/Python.PythonTestsRunner.csproj +++ b/src/python_tests_runner/Python.PythonTestsRunner.csproj @@ -11,7 +11,7 @@ - + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/src/runtime/Converter.cs b/src/runtime/Converter.cs index c7154ce36..f093298b8 100644 --- a/src/runtime/Converter.cs +++ b/src/runtime/Converter.cs @@ -118,6 +118,105 @@ private static Func GetIsTransparentProxy() internal static NewReference ToPythonDetectType(object? value) => value is null ? new NewReference(Runtime.PyNone) : ToPython(value, value.GetType()); + + /// + /// Checks if a concrete type has explicit interface implementations for the given interface + /// or any of its base interfaces. Explicit implementations are only accessible through + /// the interface type, so we need to wrap as the interface to preserve access. + /// + private static bool HasExplicitInterfaceImplementations(Type concreteType, Type interfaceType) + { + // Get all interfaces to check: the declared interface and all its base interfaces + var interfacesToCheck = new HashSet { interfaceType }; + foreach (var baseInterface in interfaceType.GetInterfaces()) + { + interfacesToCheck.Add(baseInterface); + } + + foreach (var iface in interfacesToCheck) + { + // Skip if the concrete type doesn't implement this interface + if (!iface.IsAssignableFrom(concreteType)) + { + continue; + } + + try + { + // Get the interface mapping to see how members are implemented + var interfaceMap = concreteType.GetInterfaceMap(iface); + + // Check each interface method/property to see if it's explicitly implemented + for (int i = 0; i < interfaceMap.InterfaceMethods.Length; i++) + { + var interfaceMethod = interfaceMap.InterfaceMethods[i]; + var targetMethod = interfaceMap.TargetMethods[i]; + + // Explicit interface implementations have names like "InterfaceName.MethodName" + // and are not directly accessible on the concrete type + if (targetMethod.Name.Contains(".")) + { + // This is an explicit interface implementation + // Also verify it's not directly accessible on the concrete type + var directMethod = concreteType.GetMethod( + interfaceMethod.Name, + BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly, + null, + interfaceMethod.GetParameters().Select(p => p.ParameterType).ToArray(), + null); + + if (directMethod == null) + { + // Method is explicitly implemented and not directly accessible + return true; + } + } + } + + // Check properties as well + foreach (var prop in iface.GetProperties()) + { + var getter = prop.GetGetMethod(); + var setter = prop.GetSetMethod(); + + if (getter != null) + { + var concreteGetter = concreteType.GetProperty( + prop.Name, + BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly); + + if (concreteGetter == null) + { + // Property getter is explicitly implemented + return true; + } + } + + if (setter != null) + { + var concreteSetter = concreteType.GetProperty( + prop.Name, + BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly); + + if (concreteSetter == null) + { + // Property setter is explicitly implemented + return true; + } + } + } + } + catch + { + // If GetInterfaceMap fails (e.g., for generic interfaces), assume no explicit implementations + // and prefer concrete type + continue; + } + } + + return false; + } + internal static NewReference ToPython(object? value, Type type) { if (value is PyObject pyObj) @@ -140,10 +239,39 @@ internal static NewReference ToPython(object? value, Type type) } } + // If the declared type is an interface, check if the actual runtime type + // is a concrete class. If so, prefer the concrete type to preserve access + // to all members. Only wrap as interface if the runtime type is also an + // interface or if we need explicit interface behavior. if (type.IsInterface) { - var ifaceObj = (InterfaceObject)ClassManager.GetClassImpl(type); - return ifaceObj.TryWrapObject(value); + Type actualType = value.GetType(); + // Prefer concrete types over interface types to preserve full member access. + // This fixes issues where methods return concrete types that implement + // interfaces (e.g., IDisposable) but should be exposed as their + // concrete type for proper member access and Python 'with' statement support. + if (!actualType.IsInterface) + { + // Check if the declared interface (or any of its base interfaces) has + // members that are explicitly implemented in the concrete type. + // If so, we need to wrap as the interface to preserve access to those members. + if (HasExplicitInterfaceImplementations(actualType, type)) + { + // Wrap as interface to preserve access to explicit interface implementations + var ifaceObj = (InterfaceObject)ClassManager.GetClassImpl(type); + return ifaceObj.TryWrapObject(value); + } + + // No explicit interface implementations, use the actual concrete type + // to preserve full member access (e.g., for IDisposable with 'with' statement) + type = actualType; + } + else + { + // Runtime type is also an interface (rare: proxy/dynamic case), wrap as interface + var ifaceObj = (InterfaceObject)ClassManager.GetClassImpl(type); + return ifaceObj.TryWrapObject(value); + } } if (type.IsArray || type.IsEnum) diff --git a/tests/test_array.py b/tests/test_array.py index d207a36fb..e3686147a 100644 --- a/tests/test_array.py +++ b/tests/test_array.py @@ -1335,8 +1335,8 @@ def test_special_array_creation(): assert value[1].__class__ == inst.__class__ assert value.Length == 2 - iface_class = ISayHello1(inst).__class__ value = Array[ISayHello1]([inst, inst]) + iface_class = ISayHello1(inst).__class__ assert value[0].__class__ == iface_class assert value[1].__class__ == iface_class assert value.Length == 2 diff --git a/tests/test_interface.py b/tests/test_interface.py index 969968c55..fc9a217ff 100644 --- a/tests/test_interface.py +++ b/tests/test_interface.py @@ -79,7 +79,7 @@ def test_call_inherited_interface_method(): assert hello1.SayHi() == 'hi' def test_interface_object_returned_through_method(): - """Test interface type is used if method return type is interface""" + """Test interface type is used if method return type is interface and contains explicit interface implementation.""" from Python.Test import InterfaceTest ob = InterfaceTest() @@ -91,13 +91,12 @@ def test_interface_object_returned_through_method(): def test_interface_object_returned_through_out_param(): - """Test interface type is used for out parameters of interface types""" + """Test interface type is used for out parameters of interface types and contains explicit interface implementation.""" from Python.Test import InterfaceTest ob = InterfaceTest() hello2 = ob.GetISayHello2(None) assert type(hello2).__name__ == 'ISayHello2' - assert hello2.SayHello() == 'hello 2' def test_interface_out_param_python_impl(): @@ -125,7 +124,7 @@ def test_null_interface_object_returned(): assert hello2 is None def test_interface_array_returned(): - """Test interface type used for methods returning interface arrays""" + """Test interface type is used for methods returning interface arrays and contains explicit interface implementation.""" from Python.Test import InterfaceTest ob = InterfaceTest() @@ -133,6 +132,8 @@ def test_interface_array_returned(): assert type(hellos[0]).__name__ == 'ISayHello1' assert hellos[0].__implementation__.__class__.__name__ == "InterfaceTest" + assert hellos[0].SayHello() == 'hello 1' + def test_implementation_access(): """Test the __implementation__ and __raw_implementation__ properties""" import System @@ -151,7 +152,7 @@ def test_interface_collection_iteration(): typed_list = List[System.IComparable]() typed_list.Add(elem) for e in typed_list: - assert type(e).__name__ == "IComparable" + assert type(e).__name__ == "int" untyped_list = System.Collections.ArrayList() untyped_list.Add(elem) diff --git a/tests/test_method.py b/tests/test_method.py index b86bbd6b4..013038414 100644 --- a/tests/test_method.py +++ b/tests/test_method.py @@ -585,7 +585,6 @@ def test_explicit_overload_selection(): iface_class = ISayHello1(InterfaceTest()).__class__ value = MethodTest.Overloaded.__overloads__[ISayHello1](inst) - assert value.__class__ != inst.__class__ assert value.__class__ == iface_class atype = Array[System.Object] diff --git a/tests/test_subclass.py b/tests/test_subclass.py index 13a164dfa..9a5206586 100644 --- a/tests/test_subclass.py +++ b/tests/test_subclass.py @@ -200,10 +200,8 @@ def test_interface(): assert ob.bar("bar", 2) == "bar/bar" assert FunctionsTest.test_bar(ob, "bar", 2) == "bar/bar" - # pass_through will convert from InterfaceTestClass -> IInterfaceTest, - # causing a new wrapper object to be created. Hence id will differ. x = FunctionsTest.pass_through_interface(ob) - assert id(x) != id(ob) + assert id(x) == id(ob) def test_derived_class(): @@ -283,7 +281,7 @@ def test_create_instance(): assert FunctionsTest.test_bar(ob2, "bar", 2) == "bar/bar" y = FunctionsTest.pass_through_interface(ob2) - assert id(y) != id(ob2) + assert id(y) == id(ob2) def test_events():