Repository: caketop/python-starlark-go (language: python)

Issue:
Evaluation errors missing context while syntax errors have them
When I encounter an evaluation error, I am not given any context, e.g. method name, line, etc.

```
$ python3 test.py .vela.star 
Traceback (most recent call last):
  File "/Users/colin/Source/myproject/python-poetry/test.py", line 10, in <module>
    print(s.eval("main([])"))
starlark_go.errors.EvalError: unknown binary op: NoneType | dict
```

I am given this information for syntax errors:
```
$ python3 test.py .vela.star 
Traceback (most recent call last):
  File "/Users/colin/Source/myproject/python-poetry/test.py", line 8, in <module>
    s.exec(input_file.read())
starlark_go.errors.ResolveError: <expr>:140:1: if statement not within a function
```

Perhaps the user can be given the context.


Discussion / hints:
If you catch the EvalError, is there anything in the .backtrace property? That should map to https://pkg.go.dev/go.starlark.net@v0.0.0-20230122040757-066229b0515d/starlark#EvalError.Backtrace
Yes! 

```python
from starlark_go.errors import EvalError
try:
    print(s.eval("main([])"))
except EvalError as e:
    print(e.backtrace)
```

produces

```
$ python3 test.py .vela.star 
Traceback (most recent call last):
  <expr>:1:5: in <expr>
  <expr>:143:10: in main
Error: unknown binary op: NoneType | dict
```

when I inject an error. I fixed the original error; there was a missing return statement ;-)

Could the top of that stack be added to the EvalError message text?
Probably, it looks like [EvalError](https://pkg.go.dev/go.starlark.net@v0.0.0-20230122040757-066229b0515d/starlark#EvalError) exposes a [CallStack](https://pkg.go.dev/go.starlark.net@v0.0.0-20230122040757-066229b0515d/starlark#CallStack) full of [CallFrames](https://pkg.go.dev/go.starlark.net@v0.0.0-20230122040757-066229b0515d/starlark#CallFrame), each of which has a [syntax.Position](https://pkg.go.dev/go.starlark.net@v0.0.0-20230122040757-066229b0515d/syntax#Position).

We would probably want to add `function_name`, `line`, and `column` members to the Python [EvalError](https://github.com/caketop/python-starlark-go/blob/main/src/starlark_go/errors.py#L79), add additional parameters to hold them to the [C wrapper for its constructor](https://github.com/caketop/python-starlark-go/blob/main/starlark.c#L424), and then populate them from either the first or the last CallFrame [when an EvalError is raised](https://github.com/caketop/python-starlark-go/blob/main/python_exceptions.go#L51). Not sure what the right thing to do would be if the CallStack is empty, probably just "unknown", 0, 0.

Fix patch:
diff --git a/python_exceptions.go b/python_exceptions.go
index 9327f8f..ab6adc4 100644
--- a/python_exceptions.go
+++ b/python_exceptions.go
@@ -52,7 +52,34 @@ func raisePythonException(err error) {
 		backtrace := C.CString(evalErr.Backtrace())
 		defer C.free(unsafe.Pointer(backtrace))
 
-		exc_args = C.makeEvalErrorArgs(error_msg, error_type, backtrace)
+		var (
+			function_name *C.char
+			filename      *C.char
+			line          C.uint
+			column        C.uint
+		)
+
+		if len(evalErr.CallStack) > 0 {
+			frame := evalErr.CallStack[len(evalErr.CallStack)-1]
+
+			filename = C.CString(frame.Pos.Filename())
+			defer C.free(unsafe.Pointer((filename)))
+
+			line = C.uint(frame.Pos.Line)
+			column = C.uint(frame.Pos.Col)
+
+			function_name = C.CString(frame.Name)
+			defer C.free(unsafe.Pointer(function_name))
+		} else {
+			filename = C.CString("unknown")
+			defer C.free(unsafe.Pointer(filename))
+
+			line = 0
+			column = 0
+			function_name = filename
+		}
+
+		exc_args = C.makeEvalErrorArgs(error_msg, error_type, filename, line, column, function_name, backtrace)
 		exc_type = C.EvalError
 	case errors.As(err, &resolveErr):
 		items := C.PyTuple_New(C.Py_ssize_t(len(resolveErr)))
diff --git a/src/starlark_go/errors.py b/src/starlark_go/errors.py
index ed638a0..d311d46 100644
--- a/src/starlark_go/errors.py
+++ b/src/starlark_go/errors.py
@@ -84,8 +84,43 @@ class EvalError(StarlarkError):
     such as adding a string to an integer.
     """
 
-    def __init__(self, error: str, error_type: str, backtrace: str):
-        super().__init__(error, error_type, backtrace)
+    def __init__(
+        self,
+        error: str,
+        error_type: str,
+        filename: str,
+        line: int,
+        column: int,
+        function_name: str,
+        backtrace: str,
+    ):
+        super().__init__(
+            error, error_type, filename, line, column, function_name, backtrace
+        )
+        self.filename = filename
+        """
+        The name of the file that the error occurred in.
+
+        :type: str
+        """
+        self.line = line
+        """
+        The line number that the error occurred on (1-based)
+
+        :type: int
+        """
+        self.column = column
+        """
+        The column that the error occurred on (1-based)
+
+        :type: int
+        """
+        self.function_name = function_name
+        """
+        The name of the function that the error occurred in
+
+        :type: str
+        """
         self.backtrace = backtrace
         """
         A backtrace through Starlark's stack leading up to the error.
@@ -93,6 +128,12 @@ def __init__(self, error: str, error_type: str, backtrace: str):
         :type: str
         """
 
+        context = self.filename
+        if self.function_name != "<unknown>":
+            context += " in " + self.function_name
+
+        self.error = f"{context}:{self.line}:{self.column}: {self.error}"
+
 
 class ResolveErrorItem:
     """
diff --git a/starlark.c b/starlark.c
index d874a6d..efbebe6 100644
--- a/starlark.c
+++ b/starlark.c
@@ -422,12 +422,20 @@ PyObject *makeSyntaxErrorArgs(
 }
 
 PyObject *makeEvalErrorArgs(
-    const char *error_msg, const char *error_type, const char *backtrace
+    const char *error_msg,
+    const char *error_type,
+    const char *filename,
+    const unsigned int line,
+    const unsigned int column,
+    const char *function_name,
+    const char *backtrace
 )
 {
   /* Necessary because Cgo can't do varargs */
   /* Three strings */
-  return Py_BuildValue("sss", error_msg, error_type, backtrace);
+  return Py_BuildValue(
+      "sssIIss", error_msg, error_type, filename, line, column, function_name, backtrace
+  );
 }
 
 PyObject *makeResolveErrorItem(
diff --git a/starlark.h b/starlark.h
index d93674a..36b2131 100644
--- a/starlark.h
+++ b/starlark.h
@@ -54,7 +54,13 @@ PyObject *makeSyntaxErrorArgs(
 );
 
 PyObject *makeEvalErrorArgs(
-    const char *error_msg, const char *error_type, const char *backtrace
+    const char *error_msg,
+    const char *error_type,
+    const char *filename,
+    const unsigned int line,
+    const unsigned int column,
+    const char *function_name,
+    const char *backtrace
 );
 
 PyObject *makeResolveErrorItem(


Test patch:
diff --git a/tests/test_evalerror.py b/tests/test_evalerror.py
index a125a6f..8efee3b 100644
--- a/tests/test_evalerror.py
+++ b/tests/test_evalerror.py
@@ -2,6 +2,11 @@
 
 from starlark_go import EvalError, Starlark
 
+STARLARK_SRC = """
+def wrong():
+  return 1 + "2"
+"""
+
 
 def test_raises_evalerror():
     s = Starlark()
@@ -17,16 +22,35 @@ def test_eval_attrs():
     s = Starlark()
     raised = False
 
+    s.exec(STARLARK_SRC, filename="fake.star")
+
     try:
-        s.eval('1 + "2"')
+        s.eval("wrong()")
     except EvalError as e:
         assert hasattr(e, "error")
         assert isinstance(e.error, str)
         assert hasattr(e, "error_type")
         assert isinstance(e.error_type, str)
         assert e.error_type == "*starlark.EvalError"
+        assert hasattr(e, "filename")
+        assert isinstance(e.filename, str)
+        assert e.filename == "fake.star"
+        assert hasattr(e, "line")
+        assert isinstance(e.line, int)
+        assert e.line == 3
+        assert hasattr(e, "column")
+        assert isinstance(e.column, int)
+        assert e.column == 12
+        assert hasattr(e, "function_name")
+        assert isinstance(e.function_name, str)
+        assert e.function_name == "wrong"
         assert hasattr(e, "backtrace")
         assert isinstance(e.backtrace, str)
+
+        strerror = str(e)
+        assert strerror.startswith(
+            f"{e.filename} in {e.function_name}:{e.line}:{e.column}: "
+        )
         raised = True
 
     assert raised