From 5f962be95ab17917d82f6b50781590f30aa447a8 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Wed, 18 Dec 2024 21:14:40 +0000 Subject: [PATCH] implemented __delitem__ for IDictionary and IList fixed crash for all other types (now properly throws TypeError) fixes https://github.com/pythonnet/pythonnet/issues/2530 --- CHANGELOG.md | 5 ++++ src/runtime/ClassManager.cs | 17 ++++++++++++ src/runtime/MethodBinder.cs | 5 ++++ src/runtime/Types/ArrayObject.cs | 6 ++++ src/runtime/Types/ClassBase.cs | 44 ++++++++++++++++++++++++++++++ src/runtime/Util/ReflectionUtil.cs | 7 +++++ tests/test_indexer.py | 36 ++++++++++++++++++++++++ 7 files changed, 120 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 078a6ad6e..d5a357537 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,14 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ## Unreleased ### Added + +- Support `del obj[...]` for types derived from `IList` and `IDictionary` + ### Changed ### Fixed +- Fixed crash when trying to `del clrObj[...]` for non-arrays + ## [3.0.5](https://github.com/pythonnet/pythonnet/releases/tag/v3.0.5) - 2024-12-13 ### Added diff --git a/src/runtime/ClassManager.cs b/src/runtime/ClassManager.cs index d743bc006..b884bfa92 100644 --- a/src/runtime/ClassManager.cs +++ b/src/runtime/ClassManager.cs @@ -213,6 +213,7 @@ internal static void InitClassBase(Type type, ClassBase impl, ReflectedClrType p ClassInfo info = GetClassInfo(type, impl); impl.indexer = info.indexer; + impl.del = info.del; impl.richcompare.Clear(); @@ -538,6 +539,21 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl) ob = new MethodObject(type, name, mlist); ci.members[name] = ob.AllocObject(); + if (name == nameof(IDictionary.Remove) + && mlist.Any(m => m.DeclaringType?.GetInterfaces() + .Any(i => i.TryGetGenericDefinition() == typeof(IDictionary<,>)) is true)) + { + ci.del = new(); + ci.del.AddRange(mlist.Where(m => !m.IsStatic)); + } + else if (name == nameof(IList.RemoveAt) + && mlist.Any(m => m.DeclaringType?.GetInterfaces() + .Any(i => i.TryGetGenericDefinition() == typeof(IList<>)) is true)) + { + ci.del = new(); + ci.del.AddRange(mlist.Where(m => !m.IsStatic)); + } + if (mlist.Any(OperatorMethod.IsOperatorMethod)) { string pyName = OperatorMethod.GetPyMethodName(name); @@ -581,6 +597,7 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl) private class ClassInfo { public Indexer? indexer; + public MethodBinder? del; public readonly Dictionary members = new(); internal ClassInfo() diff --git a/src/runtime/MethodBinder.cs b/src/runtime/MethodBinder.cs index 9a5515c8e..af75a34b4 100644 --- a/src/runtime/MethodBinder.cs +++ b/src/runtime/MethodBinder.cs @@ -54,6 +54,11 @@ internal void AddMethod(MethodBase m) list.Add(m); } + internal void AddRange(IEnumerable methods) + { + list.AddRange(methods.Select(m => new MaybeMethodBase(m))); + } + /// /// Given a sequence of MethodInfo and a sequence of types, return the /// MethodInfo that matches the signature represented by those types. diff --git a/src/runtime/Types/ArrayObject.cs b/src/runtime/Types/ArrayObject.cs index b95934baf..f220d53fb 100644 --- a/src/runtime/Types/ArrayObject.cs +++ b/src/runtime/Types/ArrayObject.cs @@ -247,6 +247,12 @@ public static NewReference mp_subscript(BorrowedReference ob, BorrowedReference /// public static int mp_ass_subscript(BorrowedReference ob, BorrowedReference idx, BorrowedReference v) { + if (v.IsNull) + { + Exceptions.RaiseTypeError("'System.Array' object does not support item deletion"); + return -1; + } + var obj = (CLRObject)GetManagedObject(ob)!; var items = (Array)obj.inst; Type itemType = obj.inst.GetType().GetElementType(); diff --git a/src/runtime/Types/ClassBase.cs b/src/runtime/Types/ClassBase.cs index 8b2a98903..f556f5d18 100644 --- a/src/runtime/Types/ClassBase.cs +++ b/src/runtime/Types/ClassBase.cs @@ -25,6 +25,7 @@ internal class ClassBase : ManagedType, IDeserializationCallback [NonSerialized] internal List dotNetMembers = new(); internal Indexer? indexer; + internal MethodBinder? del; internal readonly Dictionary richcompare = new(); internal MaybeType type; @@ -465,6 +466,11 @@ static int mp_ass_subscript_impl(BorrowedReference ob, BorrowedReference idx, Bo // with the index arg (method binders expect arg tuples). NewReference argsTuple = default; + if (v.IsNull) + { + return DelImpl(ob, idx, cls); + } + if (!Runtime.PyTuple_Check(idx)) { argsTuple = Runtime.PyTuple_New(1); @@ -501,6 +507,44 @@ static int mp_ass_subscript_impl(BorrowedReference ob, BorrowedReference idx, Bo return result.IsNull() ? -1 : 0; } + /// Implements __delitem__ (del x[...]) for IList<T> and IDictionary<TKey, TValue>. + private static int DelImpl(BorrowedReference ob, BorrowedReference idx, ClassBase cls) + { + if (cls.del is null) + { + Exceptions.SetError(Exceptions.TypeError, "object does not support item deletion"); + return -1; + } + + if (Runtime.PyTuple_Check(idx)) + { + Exceptions.SetError(Exceptions.TypeError, "indices must be integers"); + return -1; + } + + using var argsTuple = Runtime.PyTuple_New(1); + Runtime.PyTuple_SetItem(argsTuple.Borrow(), 0, idx); + using var result = cls.del.Invoke(ob, argsTuple.Borrow(), kw: null); + if (result.IsNull()) + return -1; + + if (Runtime.PyBool_CheckExact(result.Borrow())) + { + if (Runtime.PyObject_IsTrue(result.Borrow()) != 0) + return 0; + + Exceptions.SetError(Exceptions.KeyError, "key not found"); + return -1; + } + + if (!result.IsNone()) + { + Exceptions.warn("unsupported return type for __delitem__", Exceptions.TypeError); + } + + return 0; + } + static NewReference tp_call_impl(BorrowedReference ob, BorrowedReference args, BorrowedReference kw) { BorrowedReference tp = Runtime.PyObject_TYPE(ob); diff --git a/src/runtime/Util/ReflectionUtil.cs b/src/runtime/Util/ReflectionUtil.cs index 58d0a506e..0fad2d4b2 100644 --- a/src/runtime/Util/ReflectionUtil.cs +++ b/src/runtime/Util/ReflectionUtil.cs @@ -53,4 +53,11 @@ public static BindingFlags GetBindingFlags(this PropertyInfo property) flags |= accessor.IsPublic ? BindingFlags.Public : BindingFlags.NonPublic; return flags; } + + public static Type? TryGetGenericDefinition(this Type type) + { + if (type is null) throw new ArgumentNullException(nameof(type)); + + return type.IsConstructedGenericType ? type.GetGenericTypeDefinition() : null; + } } diff --git a/tests/test_indexer.py b/tests/test_indexer.py index 8cf3150ba..108573f0d 100644 --- a/tests/test_indexer.py +++ b/tests/test_indexer.py @@ -668,3 +668,39 @@ def test_public_inherited_overloaded_indexer(): with pytest.raises(TypeError): ob[[]] + +def test_del_indexer_dict(): + """Test deleting indexers (__delitem__).""" + from System.Collections.Generic import Dictionary, KeyNotFoundException + d = Dictionary[str, str]() + d["delme"] = "1" + with pytest.raises(KeyError): + del d["nonexistent"] + del d["delme"] + with pytest.raises(KeyError): + del d["delme"] + +def test_del_indexer_list(): + """Test deleting indexers (__delitem__).""" + from System import ArgumentOutOfRangeException + from System.Collections.Generic import List + l = List[str]() + l.Add("1") + with pytest.raises(ArgumentOutOfRangeException): + del l[3] + del l[0] + assert len(l) == 0 + +def test_del_indexer_array(): + """Test deleting indexers (__delitem__).""" + from System import Array + l = Array[str](0) + with pytest.raises(TypeError): + del l[0] + +def test_del_indexer_absent(): + """Test deleting indexers (__delitem__).""" + from System import Uri + l = Uri("http://www.example.com") + with pytest.raises(TypeError): + del l[0]