diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 6e0eefb..16aee0f 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -46,11 +46,6 @@ jobs: os: - macos-latest - ubuntu-24.04 - go: - - '1.20' - - '1.21' - - '1.22' - - '1.23' runs-on: ${{matrix.os}} steps: - name: Install Python 3.12 (macOS only) @@ -65,7 +60,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v4 with: - go-version: ${{matrix.go}} + go-version: 1.23 - name: Build run: go build -v ./... @@ -74,7 +69,7 @@ jobs: run: go test -v -coverprofile=coverage.txt -covermode=atomic ./... - name: Upload coverage to Codecov - if: matrix.os == 'ubuntu-24.04' && matrix.go == '1.23' + if: matrix.os == 'ubuntu-24.04' uses: codecov/codecov-action@v4 with: token: ${{ secrets.CODECOV_TOKEN }} diff --git a/convert.go b/convert.go index 3d7e67f..e911cae 100644 --- a/convert.go +++ b/convert.go @@ -149,14 +149,15 @@ func ToValue(obj Object, v reflect.Value) bool { t := v.Type() v.Set(reflect.MakeMap(t)) dict := Cast[Dict](obj) - dict.ForEach(func(key, value Object) { + for key, value := range dict.Items() { vk := reflect.New(t.Key()).Elem() vv := reflect.New(t.Elem()).Elem() if !ToValue(key, vk) || !ToValue(value, vv) { - panic(fmt.Errorf("failed to convert key or value to %v", t.Key())) + return false } v.SetMapIndex(vk, vv) - }) + } + return true } else { return false } @@ -167,9 +168,13 @@ func ToValue(obj Object, v reflect.Value) bool { for i := 0; i < t.NumField(); i++ { field := t.Field(i) key := goNameToPythonName(field.Name) + if !dict.HasKey(MakeStr(key)) { + continue + } value := dict.Get(MakeStr(key)) if !ToValue(value, v.Field(i)) { - panic(fmt.Errorf("failed to convert value to %v", field.Name)) + SetTypeError(fmt.Errorf("failed to convert value to %v", field.Name)) + return false } } } else { diff --git a/dict.go b/dict.go index 5d76ef9..0ee6ab6 100644 --- a/dict.go +++ b/dict.go @@ -38,7 +38,7 @@ func MakeDict(m map[any]any) Dict { return dict } -func (d Dict) Has(key any) bool { +func (d Dict) HasKey(key any) bool { keyObj := From(key) return C.PyDict_Contains(d.obj, keyObj.obj) != 0 } @@ -75,22 +75,26 @@ func (d Dict) Del(key Objecter) { C.PyDict_DelItem(d.obj, key.Obj()) } -func (d Dict) ForEach(fn func(key, value Object)) { - items := C.PyDict_Items(d.obj) - check(items != nil, "failed to get items of dict") - defer C.Py_DecRef(items) - iter := C.PyObject_GetIter(items) - for { - item := C.PyIter_Next(iter) - if item == nil { - break +func (d Dict) Items() func(fn func(key, value Object) bool) { + return func(fn func(key, value Object) bool) { + items := C.PyDict_Items(d.obj) + check(items != nil, "failed to get items of dict") + defer C.Py_DecRef(items) + iter := C.PyObject_GetIter(items) + for { + item := C.PyIter_Next(iter) + if item == nil { + break + } + C.Py_IncRef(item) + key := C.PyTuple_GetItem(item, 0) + value := C.PyTuple_GetItem(item, 1) + C.Py_IncRef(key) + C.Py_IncRef(value) + C.Py_DecRef(item) + if !fn(newObject(key), newObject(value)) { + break + } } - C.Py_IncRef(item) - key := C.PyTuple_GetItem(item, 0) - value := C.PyTuple_GetItem(item, 1) - C.Py_IncRef(key) - C.Py_IncRef(value) - C.Py_DecRef(item) - fn(newObject(key), newObject(value)) } } diff --git a/dict_test.go b/dict_test.go index bc41c92..73111f0 100644 --- a/dict_test.go +++ b/dict_test.go @@ -135,7 +135,7 @@ func TestDictDel(t *testing.T) { dict.Del(key) // After deletion, the key should not exist - if dict.Has(key) { + if dict.HasKey(key) { t.Errorf("After deletion, key %v should not exist", key) } } @@ -155,14 +155,14 @@ func TestDictForEach(t *testing.T) { "key3": "value3", } - dict.ForEach(func(key, value Object) { + for key, value := range dict.Items() { count++ k := key.String() v := value.String() if expectedVal, ok := expectedPairs[k]; !ok || expectedVal != v { t.Errorf("ForEach() unexpected pair: %v: %v", k, v) } - }) + } if count != len(expectedPairs) { t.Errorf("ForEach() visited %d pairs, want %d", count, len(expectedPairs)) diff --git a/extension.go b/extension.go index ca136b4..081365e 100644 --- a/extension.go +++ b/extension.go @@ -131,6 +131,9 @@ func getterMethod(self *C.PyObject, _closure unsafe.Pointer, methodId C.int) *C. fieldType := field.Type() if fieldType.Kind() == reflect.Ptr && fieldType.Elem().Kind() == reflect.Struct { + if field.IsNil() { + return C.Py_None + } if pyType, ok := maps.pyTypes[fieldType.Elem()]; ok { newWrapper := allocWrapper((*C.PyTypeObject)(unsafe.Pointer(pyType)), field.Interface()) if newWrapper == nil { @@ -187,15 +190,24 @@ func setterMethod(self, value *C.PyObject, _closure unsafe.Pointer, methodId C.i fieldType := field.Type() if fieldType.Kind() == reflect.Ptr && fieldType.Elem().Kind() == reflect.Struct { + if C.Py_Is(value, C.Py_None) != 0 { + field.Set(reflect.Zero(fieldType)) + return 0 + } if C.Py_IS_TYPE(value, &C.PyDict_Type) != 0 { if field.IsNil() { field.Set(reflect.New(fieldType.Elem())) } if !ToValue(FromPy(value), field.Elem()) { - SetError(fmt.Errorf("failed to convert dict to %s", fieldType.Elem())) + SetTypeError(fmt.Errorf("failed to convert dict to %s", fieldType.Elem())) return -1 } } else { + pyType := C.Py_TYPE(value) + if _, ok := maps.typeMetas[(*C.PyObject)(unsafe.Pointer(pyType))]; !ok { + SetTypeError(fmt.Errorf("invalid value of type %v for struct pointer field", FromPy((*C.PyObject)(unsafe.Pointer(pyType))))) + return -1 + } valueWrapper := (*wrapperType)(unsafe.Pointer(value)) if valueWrapper == nil { SetError(fmt.Errorf("invalid value for struct pointer field")) @@ -207,10 +219,15 @@ func setterMethod(self, value *C.PyObject, _closure unsafe.Pointer, methodId C.i } else if field.Kind() == reflect.Struct { if C.Py_IS_TYPE(value, &C.PyDict_Type) != 0 { if !ToValue(FromPy(value), field) { - SetError(fmt.Errorf("failed to convert dict to %s", field.Type())) + SetTypeError(fmt.Errorf("failed to convert dict to %s", field.Type())) return -1 } } else { + pyType := (*C.PyTypeObject)(unsafe.Pointer(value.ob_type)) + if _, ok := maps.typeMetas[(*C.PyObject)(unsafe.Pointer(pyType))]; !ok { + SetTypeError(fmt.Errorf("invalid value of type %v for struct field", FromPy((*C.PyObject)(unsafe.Pointer(pyType))))) + return -1 + } valueWrapper := (*wrapperType)(unsafe.Pointer(value)) if valueWrapper == nil { SetError(fmt.Errorf("invalid value for struct field")) @@ -218,14 +235,14 @@ func setterMethod(self, value *C.PyObject, _closure unsafe.Pointer, methodId C.i } baseAddr := goPtr.UnsafePointer() fieldAddr := unsafe.Add(baseAddr, typeMeta.typ.Field(methodMeta.index).Offset) - fieldPtr := reflect.NewAt(fieldType, fieldAddr).Interface() - reflect.ValueOf(fieldPtr).Set(reflect.ValueOf(valueWrapper.goObj)) + fieldPtr := reflect.NewAt(fieldType, fieldAddr) + fieldPtr.Elem().Set(reflect.ValueOf(valueWrapper.goObj).Elem()) } return 0 } if !ToValue(FromPy(value), field) { - SetError(fmt.Errorf("failed to convert value to %s", methodMeta.typ)) + SetTypeError(fmt.Errorf("failed to convert value to %s", methodMeta.typ)) return -1 } return 0 diff --git a/extension_test.go b/extension_test.go index b932565..6f97151 100644 --- a/extension_test.go +++ b/extension_test.go @@ -465,3 +465,375 @@ assert o.inner_list[0].y == "python" t.Fatalf("Test failed: %v", err) } } + +func TestSetterMethodEdgeCases(t *testing.T) { + setupTest(t) + + type ChildStruct struct { + Value int + } + + type ParentStruct struct { + unexported int + Value int + Child *ChildStruct + Nested ChildStruct + } + + m := MainModule() + m.AddType(ChildStruct{}, nil, "ChildStruct", "") + m.AddType(ParentStruct{}, nil, "ParentStruct", "") + + code := ` +obj = ParentStruct() +try: + obj.value = "invalid" # Try to set int with string + assert False, "Should have raised TypeError" +except TypeError: + pass + +try: + obj.child = 123 # Try to set struct pointer with int + assert False, "Should have raised TypeError" +except TypeError: + pass + +try: + obj.nested = 123 # Try to set struct with int + assert False, "Should have raised TypeError" +except TypeError: + pass +` + err := RunString(code) + if err != nil { + t.Fatal(err) + } +} + +func TestGetterMethodEdgeCases(t *testing.T) { + setupTest(t) + + type ChildStruct struct { + Value int + } + + type ParentStruct struct { + Value int + Child *ChildStruct + Nested ChildStruct + } + + m := MainModule() + m.AddType(ChildStruct{}, nil, "ChildStruct", "") + m.AddType(ParentStruct{}, nil, "ParentStruct", "") + + code := ` +obj = ParentStruct() +obj.child = None # Set pointer to nil +val = obj.child # Should return None for nil pointer +assert val is None + +obj.nested = ChildStruct() # Set nested struct +val = obj.nested # Should return wrapper for nested struct +assert isinstance(val, ChildStruct) + +# Test accessing nested struct fields +obj.nested.value = 42 +assert obj.nested.value == 42 +` + err := RunString(code) + if err != nil { + t.Fatal(err) + } +} + +func TestWrapperMethodEdgeCases(t *testing.T) { + setupTest(t) + + type TestStruct struct { + Value int + } + + m := MainModule() + + // Test method with wrong number of arguments + m.AddMethod("test_func", func(x int, y int) int { return x + y }, "") + + code := ` +try: + test_func(1) # Missing argument + assert False, "Should have raised TypeError" +except TypeError: + pass + +try: + test_func(1, 2, 3) # Too many arguments + assert False, "Should have raised TypeError" +except TypeError: + pass + +try: + test_func("invalid", 2) # Invalid argument type + assert False, "Should have raised TypeError" +except TypeError: + pass +` + err := RunString(code) + if err != nil { + t.Fatal(err) + } +} + +func TestAddTypeEdgeCases(t *testing.T) { + setupTest(t) + + // Test adding non-struct type + defer func() { + if r := recover(); r == nil { + t.Error("Expected panic when adding non-struct type") + } + }() + + m := MainModule() + m.AddType(123, nil, "NotAStruct", "") +} + +func TestInitFunctionEdgeCases(t *testing.T) { + setupTest(t) + + type TestStruct struct { + Value int + } + + // Test init function with invalid signature + defer func() { + if r := recover(); r == nil { + t.Error("Expected panic when using invalid init function") + } + }() + + m := MainModule() + invalidInit := func(x string) string { return x } // Wrong signature + m.AddType(TestStruct{}, invalidInit, "TestStruct", "") +} + +func TestNestedStructRegistration(t *testing.T) { + setupTest(t) + + type NestedStruct struct { + Value int + } + + type ParentStruct struct { + Nested NestedStruct + NestedPtr *NestedStruct + } + + m := MainModule() + m.AddType(ParentStruct{}, nil, "ParentStruct", "") + + code := ` +parent = ParentStruct() +assert hasattr(parent, "nested") +assert hasattr(parent, "nested_ptr") + +# Test nested struct manipulation +parent.nested.value = 42 +assert parent.nested.value == 42 + +parent.nested_ptr = None +assert parent.nested_ptr is None + +# Create and assign new nested struct +parent.nested_ptr = NestedStruct() +parent.nested_ptr.value = 100 +assert parent.nested_ptr.value == 100 +` + err := RunString(code) + if err != nil { + t.Fatal(err) + } +} + +func TestAddTypeWithPointerArg(t *testing.T) { + setupTest(t) + m := MainModule() + + type TestStruct struct { + Value int + } + + // Test adding type with pointer argument + typ1 := m.AddType(&TestStruct{}, nil, "TestStruct", "") + if typ1.Nil() { + t.Fatal("Failed to create type with pointer argument") + } + + code := ` +obj = TestStruct() +obj.value = 42 +assert obj.value == 42 +` + err := RunString(code) + if err != nil { + t.Fatal(err) + } +} + +func TestAddTypeDuplicate(t *testing.T) { + setupTest(t) + m := MainModule() + + type TestStruct struct { + Value int + } + + // First registration + typ1 := m.AddType(TestStruct{}, nil, "TestStruct", "") + if typ1.Nil() { + t.Fatal("Failed to create type on first registration") + } + + // Second registration should return the same type object + typ2 := m.AddType(TestStruct{}, nil, "TestStruct", "") + if typ2.Nil() { + t.Fatal("Failed to get type on second registration") + } + + if typ1.Obj() != typ2.Obj() { + t.Fatal("Expected same type object on second registration") + } + + // Both types should work with the same underlying Go type + code := ` +obj1 = TestStruct() +obj1.value = 42 +assert obj1.value == 42 +` + err := RunString(code) + if err != nil { + t.Fatal(err) + } + + // Also test with pointer argument + typ3 := m.AddType(&TestStruct{}, nil, "TestStruct3", "") + if typ3.Nil() { + t.Fatal("Failed to get type on registration with pointer") + } + + if typ1.Obj() != typ3.Obj() { + t.Fatal("Expected same type object on second registration") + } +} + +func TestStructPointerFieldDictAssignment(t *testing.T) { + setupTest(t) + m := MainModule() + + type NestedStruct struct { + IntVal int + StringVal string + } + + type ParentStruct struct { + PtrField *NestedStruct + } + + m.AddType(ParentStruct{}, nil, "ParentStruct", "") + + // Test assigning dict to nil pointer field + code := ` +obj = ParentStruct() +# Initially the pointer should be nil +assert obj.ptr_field is None + +# Assign dict to nil pointer field +obj.ptr_field = {"int_val": 42, "string_val": "hello"} +assert obj.ptr_field.int_val == 42 +assert obj.ptr_field.string_val == "hello" + +# Test invalid dict value type +try: + obj.ptr_field = {"int_val": "not an int", "string_val": "hello"} + assert False, "Should have raised TypeError for invalid int_val" +except TypeError: + pass + +# Test completely wrong type +try: + obj.ptr_field = ["not", "a", "dict"] + assert False, "Should have raised TypeError for list" +except TypeError: + pass + +# Test nested dict with wrong type +try: + obj.ptr_field = {"int_val": {"nested": "dict"}, "string_val": "hello"} + assert False, "Should have raised TypeError for nested dict" +except TypeError: + pass +` + err := RunString(code) + if err != nil { + t.Fatal(err) + } +} + +func TestStructFieldDictAssignment(t *testing.T) { + setupTest(t) + m := MainModule() + + type NestedStruct struct { + IntVal int + StringVal string + } + + type ParentStruct struct { + Field NestedStruct + } + + m.AddType(ParentStruct{}, nil, "ParentStruct", "") + + // Test assigning dict to struct field + code := ` +obj = ParentStruct() + +# Assign valid dict +obj.field = {"int_val": 42, "string_val": "hello"} +assert obj.field.int_val == 42 +assert obj.field.string_val == "hello" + +# Test invalid value type +try: + obj.field = {"int_val": "not an int", "string_val": "hello"} + assert False, "Should have raised TypeError for invalid int_val" +except TypeError: + pass + +# Test completely wrong type +try: + obj.field = ["not", "a", "dict"] + assert False, "Should have raised TypeError for list" +except TypeError: + pass + +# Test nested dict with wrong type +try: + obj.field = {"int_val": {"nested": "dict"}, "string_val": "hello"} + assert False, "Should have raised TypeError for nested dict" +except TypeError: + pass + +# Test with complex nested structure +obj.field = { + "int_val": 100, + "string_val": "test" +} +assert obj.field.int_val == 100 +assert obj.field.string_val == "test" +` + err := RunString(code) + if err != nil { + t.Fatal(err) + } +} diff --git a/go.mod b/go.mod index 3873b45..f37958f 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/cpunion/go-python -go 1.20 +go 1.23 diff --git a/module_test.go b/module_test.go index 7ee7025..8910926 100644 --- a/module_test.go +++ b/module_test.go @@ -19,7 +19,7 @@ func TestModuleImport(t *testing.T) { } // Verify math module has expected attributes - if !modDict.Has("pi") { + if !modDict.HasKey("pi") { t.Error("Math module doesn't contain 'pi' constant") } } @@ -63,7 +63,7 @@ func TestCreateModule(t *testing.T) { // Verify the object was added modDict := mod.Dict() - if !modDict.Has("test_value") { + if !modDict.HasKey("test_value") { t.Error("Module doesn't contain added value") } @@ -90,7 +90,7 @@ func TestGetModuleDict(t *testing.T) { } // Verify the module is in the module dictionary - if !moduleDict.Has("math") { + if !moduleDict.HasKey("math") { t.Error("Module dictionary doesn't contain imported module") } }