Skip to content

Commit

Permalink
implemented __delitem__ for IDictionary<K,V> and IList<T>
Browse files Browse the repository at this point in the history
fixed crash for all other types (now properly throws TypeError)

fixes pythonnet#2530
  • Loading branch information
lostmsu committed Dec 18, 2024
1 parent 4eafbfa commit 5f962be
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 0 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>` and `IDictionary<K, V>`

### 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
Expand Down
17 changes: 17 additions & 0 deletions src/runtime/ClassManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();


Expand Down Expand Up @@ -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<int, int>.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<int>.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);
Expand Down Expand Up @@ -581,6 +597,7 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
private class ClassInfo
{
public Indexer? indexer;
public MethodBinder? del;
public readonly Dictionary<string, PyObject> members = new();

internal ClassInfo()
Expand Down
5 changes: 5 additions & 0 deletions src/runtime/MethodBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ internal void AddMethod(MethodBase m)
list.Add(m);
}

internal void AddRange(IEnumerable<MethodBase> methods)
{
list.AddRange(methods.Select(m => new MaybeMethodBase(m)));
}

/// <summary>
/// Given a sequence of MethodInfo and a sequence of types, return the
/// MethodInfo that matches the signature represented by those types.
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/Types/ArrayObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,12 @@ public static NewReference mp_subscript(BorrowedReference ob, BorrowedReference
/// </summary>
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();
Expand Down
44 changes: 44 additions & 0 deletions src/runtime/Types/ClassBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ internal class ClassBase : ManagedType, IDeserializationCallback
[NonSerialized]
internal List<string> dotNetMembers = new();
internal Indexer? indexer;
internal MethodBinder? del;
internal readonly Dictionary<int, MethodObject> richcompare = new();
internal MaybeType type;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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&lt;T&gt; and IDictionary&lt;TKey, TValue&gt;.
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);
Expand Down
7 changes: 7 additions & 0 deletions src/runtime/Util/ReflectionUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
36 changes: 36 additions & 0 deletions tests/test_indexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]

0 comments on commit 5f962be

Please sign in to comment.