Test mutiple calls to exec / handle resolve.Error (#27)

In #26 another change snuck
in to retain the globals across multiple calls to exec, so that it
could be used to define multiple things. While writing a test for this
behavor, I discovered that it is possible for starlark-go's Eval to
return a resolve.ErrorList, in addition to a starlark.EvalError or
a syntax.Error, so I had to add handling for that as well.

src/pystarlark/__init__.py

@@ -2,9 +2,9 @@
 from typing import Any, Optional
 
 from pystarlark._lib import StarlarkGo
-from pystarlark.errors import EvalError, StarlarkError, SyntaxError
+from pystarlark.errors import EvalError, ResolveError, StarlarkError, SyntaxError
 
-__all__ = ["Starlark", "StarlarkError", "EvalError", "SyntaxError"]
+__all__ = ["Starlark", "StarlarkError", "EvalError", "ResolveError", "SyntaxError"]
 __version__ = "0.0.2"
 
 

src/pystarlark/errors.py

@@ -1,4 +1,4 @@
-from typing import Any
+from typing import Any, Tuple
 
 __all__ = ["StarlarkError", "SyntaxError", "EvalError"]
 
@@ -34,3 +34,18 @@ class EvalError(StarlarkError):
     def __init__(self, error: str, error_type: str, backtrace: str):
         super().__init__(error, error_type, backtrace)
         self.backtrace = backtrace
+
+
+class ResolveErrorItem:
+    def __init__(self, msg: str, line: int, column: int):
+        self.msg = msg
+        self.line = line
+        self.column = column
+
+
+class ResolveError(StarlarkError):
+    def __init__(
+        self, error: str, error_type: str, errors: Tuple[ResolveErrorItem, ...]
+    ):
+        super().__init__(error, error_type)
+        self.errors = list(errors)

starlark.c

@@ -4,10 +4,16 @@
 PyObject *StarlarkError;
 PyObject *SyntaxError;
 PyObject *EvalError;
+PyObject *ResolveError;
+PyObject *ResolveErrorItem;
+
+/* For use with CgoPyBuildOneValue */
+const char *buildStr = "s";
+const char *buildUint = "I";
 
 /* Argument names for our methods */
-static const char *eval_keywords[] = {"expr", "filename", "parse"};
-static const char *exec_keywords[] = {"defs", "filename"};
+static char *eval_keywords[] = {"expr", "filename", "parse", NULL};
+static char *exec_keywords[] = {"defs", "filename", NULL};
 
 /* Helpers to parse method arguments */
 int CgoParseEvalArgs(PyObject *args, PyObject *kwargs, char **expr,
@@ -24,7 +30,7 @@ int GgoParseExecArgs(PyObject *args, PyObject *kwargs, char **defs,
 
 /* This stuff is in the Go file */
 StarlarkGo *StarlarkGo_new(PyTypeObject *type, PyObject *args, PyObject *kwds);
-static void StarlarkGo_dealloc(StarlarkGo *self);
+void StarlarkGo_dealloc(StarlarkGo *self);
 PyObject *StarlarkGo_eval(StarlarkGo *self, PyObject *args);
 PyObject *StarlarkGo_exec(StarlarkGo *self, PyObject *args);
 
@@ -75,6 +81,21 @@ PyObject *CgoEvalErrorArgs(const char *error_msg, const char *error_type,
   return Py_BuildValue("sss", error_msg, error_type, backtrace);
 }
 
+PyObject *CgoResolveErrorItem(const char *msg, const unsigned int line,
+                              const unsigned int column) {
+  PyObject *args = Py_BuildValue("sII", msg, line, column);
+  PyGILState_STATE gilstate = PyGILState_Ensure();
+  PyObject *obj = PyObject_CallObject(ResolveErrorItem, args);
+  PyGILState_Release(gilstate);
+  Py_XDECREF(args);
+
+  return obj;
+}
+
+PyObject *CgoResolveErrorArgs(const char *error_msg, const char *error_type,
+                              PyObject *errors) {
+  return Py_BuildValue("ssO", error_msg, error_type, errors);
+}
 /* Other assorted helpers for Cgo, which can't handle varargs or macros */
 StarlarkGo *CgoStarlarkGoAlloc(PyTypeObject *type) {
   return (StarlarkGo *)type->tp_alloc(type, 0);
@@ -94,6 +115,9 @@ PyObject *CgoPyNone() { Py_RETURN_NONE; }
 
 PyTypeObject *CgoPyType(PyObject *obj) { return Py_TYPE(obj); }
 
+void CgoPyTuple_SET_ITEM(PyObject *tuple, Py_ssize_t pos, PyObject *item) {
+  PyTuple_SET_ITEM(tuple, pos, item);
+}
 /* Helper to fetch exception classes */
 static PyObject *get_exception_class(PyObject *errors, const char *name) {
   PyObject *retval = PyObject_GetAttrString(errors, name);
@@ -123,6 +147,14 @@ PyMODINIT_FUNC PyInit__lib(void) {
   if (EvalError == NULL)
     return NULL;
 
+  ResolveError = get_exception_class(errors, "ResolveError");
+  if (ResolveError == NULL)
+    return NULL;
+
+  ResolveErrorItem = get_exception_class(errors, "ResolveErrorItem");
+  if (ResolveErrorItem == NULL)
+    return NULL;
+
   PyObject *m;
   if (PyType_Ready(&StarlarkGoType) < 0)
     return NULL;

starlark.go

@@ -7,6 +7,10 @@ package main
 extern PyObject *StarlarkError;
 extern PyObject *SyntaxError;
 extern PyObject *EvalError;
+extern PyObject *ResolveError;
+
+extern const char *buildStr;
+extern const char *buildUint;
 */
 import "C"
 
@@ -16,6 +20,7 @@ import (
 	"reflect"
 	"unsafe"
 
+	"go.starlark.net/resolve"
 	"go.starlark.net/starlark"
 	"go.starlark.net/syntax"
 )
@@ -27,10 +32,11 @@ var (
 
 func raisePythonException(err error) {
 	var (
-		exc_args  *C.PyObject
-		exc_type  *C.PyObject
-		syntaxErr syntax.Error
-		evalErr   *starlark.EvalError
+		exc_args   *C.PyObject
+		exc_type   *C.PyObject
+		syntaxErr  syntax.Error
+		evalErr    *starlark.EvalError
+		resolveErr resolve.ErrorList
 	)
 
 	error_msg := C.CString(err.Error())
@@ -58,6 +64,19 @@ func raisePythonException(err error) {
 
 		exc_args = C.CgoEvalErrorArgs(error_msg, error_type, backtrace)
 		exc_type = C.EvalError
+	case errors.As(err, &resolveErr):
+		items := C.PyTuple_New(C.Py_ssize_t(len(resolveErr)))
+		defer C.CgoPyDecRef(items)
+
+		for i, err := range resolveErr {
+			msg := C.CString(err.Msg)
+			defer C.free(unsafe.Pointer(msg))
+
+			C.CgoPyTuple_SET_ITEM(items, C.Py_ssize_t(i), C.CgoResolveErrorItem(msg, C.uint(err.Pos.Line), C.uint(err.Pos.Col)))
+		}
+
+		exc_args = C.CgoResolveErrorArgs(error_msg, error_type, items)
+		exc_type = C.ResolveError
 	default:
 		exc_args = C.CgoStarlarkErrorArgs(error_msg, error_type)
 		exc_type = C.StarlarkError
@@ -123,10 +142,8 @@ func StarlarkGo_eval(self *C.StarlarkGo, args *C.PyObject, kwargs *C.PyObject) *
 
 	cstr := C.CString(result.String())
 	defer C.free(unsafe.Pointer(cstr))
-	fmt := C.CString("U")
-	defer C.free(unsafe.Pointer(fmt))
 
-	return C.CgoPyBuildOneValue(fmt, unsafe.Pointer(cstr))
+	return C.CgoPyBuildOneValue(C.buildStr, unsafe.Pointer(cstr))
 }
 
 //export StarlarkGo_exec

starlark.h

@@ -24,6 +24,12 @@ PyObject *CgoSyntaxErrorArgs(const char *error_msg, const char *error_type,
 PyObject *CgoEvalErrorArgs(const char *error_msg, const char *error_type,
                            const char *backtrace);
 
+PyObject *CgoResolveErrorItem(const char *msg, const unsigned int line,
+                              const unsigned int column);
+
+PyObject *CgoResolveErrorArgs(const char *error_msg, const char *error_type,
+                              PyObject *errors);
+
 void CgoPyDecRef(PyObject *obj);
 
 PyObject *CgoPyBuildOneValue(const char *fmt, const void *src);
@@ -38,4 +44,6 @@ int GgoParseExecArgs(PyObject *args, PyObject *kwargs, char **defs,
 
 PyTypeObject *CgoPyType(PyObject *obj);
 
+void CgoPyTuple_SET_ITEM(PyObject *tuple, Py_ssize_t pos, PyObject *item);
+
 #endif /* PYTHON_STARLARK_GO_H */

tests/test_multi_exec.py

@@ -0,0 +1,28 @@
+import pytest
+
+from pystarlark import ResolveError, Starlark
+
+ADD_ONE = """
+def add_one(x):
+    return x + 1
+"""
+
+ADD_TWO = """
+def add_two(x):
+    return add_one(add_one(x))
+"""
+
+
+def test_multi_exec():
+    s = Starlark()
+
+    s.exec(ADD_ONE)
+
+    assert s.eval("add_one(1)") == 2
+
+    with pytest.raises(ResolveError):
+        s.eval("add_two(1)")
+
+    s.exec(ADD_TWO)
+
+    assert s.eval("add_two(1)") == 3

tests/test_resolveerror.py

@@ -0,0 +1,61 @@
+from pystarlark import ResolveError, Starlark
+
+
+def test_eval_resolveerror():
+    s = Starlark()
+    raised = False
+
+    try:
+        s.eval("add_one(1)")
+    except ResolveError as e:
+        assert isinstance(e.errors, list)
+        assert len(e.errors) == 1
+        assert e.errors[0].line == 1
+        assert e.errors[0].column == 1
+        assert e.errors[0].msg == "undefined: add_one"
+        raised = True
+
+    try:
+        s.eval("from_bad(True) + to_worse(True)")
+    except ResolveError as e:
+        assert isinstance(e.errors, list)
+        assert len(e.errors) == 2
+        assert e.errors[0].line == 1
+        assert e.errors[0].column == 1
+        assert e.errors[0].msg == "undefined: from_bad"
+        assert e.errors[1].line == 1
+        assert e.errors[1].column == 18
+        assert e.errors[1].msg == "undefined: to_worse"
+        raised = True
+
+    assert raised
+
+
+def test_exec_resolveerror():
+    s = Starlark()
+    raised = False
+
+    try:
+        s.exec("add_one(1)")
+    except ResolveError as e:
+        assert isinstance(e.errors, list)
+        assert len(e.errors) == 1
+        assert e.errors[0].line == 1
+        assert e.errors[0].column == 1
+        assert e.errors[0].msg == "undefined: add_one"
+        raised = True
+
+    try:
+        s.exec("from_bad(True) + to_worse(True)")
+    except ResolveError as e:
+        assert isinstance(e.errors, list)
+        assert len(e.errors) == 2
+        assert e.errors[0].line == 1
+        assert e.errors[0].column == 1
+        assert e.errors[0].msg == "undefined: from_bad"
+        assert e.errors[1].line == 1
+        assert e.errors[1].column == 18
+        assert e.errors[1].msg == "undefined: to_worse"
+        raised = True
+
+    assert raised