Repository: google/starlark-go
Pull request #243 (2019-10-03 14:18:03)

go.mod: init Go module
Initialize Go module support.

Close: #232

zchee: cc @alandonovan

Patch:
diff --git a/doc/spec.md b/doc/spec.md
index 66d4a14..211650e 100644
--- a/doc/spec.md
+++ b/doc/spec.md
@@ -1257,7 +1257,7 @@ The [dot expression](#dot-expressions) `.split` is a dynamic operation
 on the value returned by `get_filename()`.
 
 
-## Value concepts {#value-concepts}
+## Value concepts
 
 Starlark has eleven core [data types](#data-types).  An application
 that embeds the Starlark intepreter may define additional types that
@@ -1543,7 +1543,7 @@ Lookup of locals and globals may fail if not yet defined.
 
 ### Literals
 
-Starlark supports string literals of three different kinds:
+Starlark supports literals of three different kinds:
 
 ```grammar {.good}
 Primary = int | float | string
@@ -2349,9 +2349,9 @@ the global dictionary of the current module.
 
 The name of a function created by a lambda expression is `"lambda"`.
 
-The two statements below are essentially equivalent, but that the
+The two statements below are essentially equivalent, but the
 function created by the `def` statement is named `twice` and the
-function created by the lambda expression is called `lambda`.
+function created by the lambda expression is named `lambda`.
 
 ```python
 def twice(x):
@@ -2439,7 +2439,8 @@ pi, e = 3.141, 2.718
 (x, y) = f()
 [zero, one, two] = range(3)
 
-[(a, b), (c, d)] = ("ab", "cd")
+[(a, b), (c, d)] = {"a": "b", "c": "d"}.items()
+a, b = {"a": 1, "b": 2}
 ```
 
 The same process for assigning a value to a target expression is used
@@ -3006,6 +3007,9 @@ It is a dynamic error if x has no such attribute.
 getattr("banana", "split")("a")	       # ["b", "n", "n", ""], equivalent to "banana".split("a")
 ```
 
+The three-argument form `getattr(x, name, default)` returns the
+provided `default` value instead of failing.
+
 ### hasattr
 
 `hasattr(x, name)` reports whether x has an attribute (field or method) named `name`.
diff --git a/go.mod b/go.mod
new file mode 100644
index 0000000..2e64352
--- /dev/null
+++ b/go.mod
@@ -0,0 +1,10 @@
+module go.starlark.net
+
+go 1.13
+
+require (
+	github.com/chzyer/logex v1.1.10 // indirect
+	github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e
+	github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1 // indirect
+	golang.org/x/sys v0.0.0-20191002063906-3421d5a6bb1c // indirect
+)
diff --git a/go.sum b/go.sum
new file mode 100644
index 0000000..3a8dac4
--- /dev/null
+++ b/go.sum
@@ -0,0 +1,8 @@
+github.com/chzyer/logex v1.1.10 h1:Swpa1K6QvQznwJRcfTfQJmTE72DqScAa40E+fbHEXEE=
+github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
+github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e h1:fY5BOSpyZCqRo5OhCuC+XN+r/bBCmeuuJtjz+bCNIf8=
+github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
+github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1 h1:q763qf9huN11kDQavWsoZXJNW3xEE4JJyHa5Q25/sd8=
+github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
+golang.org/x/sys v0.0.0-20191002063906-3421d5a6bb1c h1:Vco5b+cuG5NNfORVxZy6bYZQ7rsigisU1WQFkvQ0L5E=
+golang.org/x/sys v0.0.0-20191002063906-3421d5a6bb1c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
diff --git a/repl/repl.go b/repl/repl.go
index d766eff..5f2ee36 100644
--- a/repl/repl.go
+++ b/repl/repl.go
@@ -133,26 +133,9 @@ func rep(rl *readline.Instance, thread *starlark.Thread, globals starlark.String
 		if v != starlark.None {
 			fmt.Println(v)
 		}
-	} else {
-		// compile
-		prog, err := starlark.FileProgram(f, globals.Has)
-		if err != nil {
-			PrintError(err)
-			return nil
-		}
-
-		// execute (but do not freeze)
-		res, err := prog.Init(thread, globals)
-		if err != nil {
-			PrintError(err)
-		}
-
-		// The global names from the previous call become
-		// the predeclared names of this call.
-		// If execution failed, some globals may be undefined.
-		for k, v := range res {
-			globals[k] = v
-		}
+	} else if err := starlark.ExecREPLChunk(f, thread, globals); err != nil {
+		PrintError(err)
+		return nil
 	}
 
 	return nil
diff --git a/resolve/resolve.go b/resolve/resolve.go
index 13ddfe6..440bcf0 100644
--- a/resolve/resolve.go
+++ b/resolve/resolve.go
@@ -122,7 +122,13 @@ var (
 // dependency upon starlark.Universe, not because users should ever need
 // to redefine it.
 func File(file *syntax.File, isPredeclared, isUniversal func(name string) bool) error {
-	r := newResolver(isPredeclared, isUniversal)
+	return REPLChunk(file, nil, isPredeclared, isUniversal)
+}
+
+// REPLChunk is a generalization of the File function that supports a
+// non-empty initial global block, as occurs in a REPL.
+func REPLChunk(file *syntax.File, isGlobal, isPredeclared, isUniversal func(name string) bool) error {
+	r := newResolver(isGlobal, isPredeclared, isUniversal)
 	r.stmts(file.Stmts)
 
 	r.env.resolveLocalUses()
@@ -148,7 +154,7 @@ func File(file *syntax.File, isPredeclared, isUniversal func(name string) bool)
 //
 // The isPredeclared and isUniversal predicates behave as for the File function.
 func Expr(expr syntax.Expr, isPredeclared, isUniversal func(name string) bool) ([]*Binding, error) {
-	r := newResolver(isPredeclared, isUniversal)
+	r := newResolver(nil, isPredeclared, isUniversal)
 	r.expr(expr)
 	r.env.resolveLocalUses()
 	r.resolveNonLocalUses(r.env) // globals & universals
@@ -171,11 +177,12 @@ type Error struct {
 
 func (e Error) Error() string { return e.Pos.String() + ": " + e.Msg }
 
-func newResolver(isPredeclared, isUniversal func(name string) bool) *resolver {
+func newResolver(isGlobal, isPredeclared, isUniversal func(name string) bool) *resolver {
 	file := new(block)
 	return &resolver{
 		file:          file,
 		env:           file,
+		isGlobal:      isGlobal,
 		isPredeclared: isPredeclared,
 		isUniversal:   isUniversal,
 		globals:       make(map[string]*Binding),
@@ -202,8 +209,10 @@ type resolver struct {
 	predeclared map[string]*Binding
 
 	// These predicates report whether a name is
-	// pre-declared, either in this module or universally.
-	isPredeclared, isUniversal func(name string) bool
+	// pre-declared, either in this module or universally,
+	// or already declared in the module globals (as in a REPL).
+	// isGlobal may be nil.
+	isGlobal, isPredeclared, isUniversal func(name string) bool
 
 	loops int // number of enclosing for loops
 
@@ -390,6 +399,15 @@ func (r *resolver) useToplevel(use use) (bind *Binding) {
 	} else if prev, ok := r.globals[id.Name]; ok {
 		// use of global declared by module
 		bind = prev
+	} else if r.isGlobal != nil && r.isGlobal(id.Name) {
+		// use of global defined in a previous REPL chunk
+		bind = &Binding{
+			First: id, // wrong: this is not even a binding use
+			Scope: Global,
+			Index: len(r.moduleGlobals),
+		}
+		r.globals[id.Name] = bind
+		r.moduleGlobals = append(r.moduleGlobals, bind)
 	} else if prev, ok := r.predeclared[id.Name]; ok {
 		// repeated use of predeclared or universal
 		bind = prev
@@ -433,7 +451,8 @@ func (r *resolver) spellcheck(use use) string {
 
 	// globals
 	//
-	// We have no way to enumerate predeclared/universe,
+	// We have no way to enumerate the sets whose membership
+	// tests are isPredeclared, isUniverse, and isGlobal,
 	// which includes prior names in the REPL session.
 	for _, bind := range r.moduleGlobals {
 		names = append(names, bind.First.Name)
diff --git a/starlark/eval.go b/starlark/eval.go
index e10ead0..de492ca 100644
--- a/starlark/eval.go
+++ b/starlark/eval.go
@@ -190,6 +190,7 @@ func (stack CallStack) String() string {
 type EvalError struct {
 	Msg       string
 	CallStack CallStack
+	cause     error
 }
 
 // A CallFrame represents the function name and current
@@ -210,6 +211,7 @@ func (thread *Thread) evalError(err error) *EvalError {
 	return &EvalError{
 		Msg:       err.Error(),
 		CallStack: thread.CallStack(),
+		cause:     err,
 	}
 }
 
@@ -221,6 +223,8 @@ func (e *EvalError) Backtrace() string {
 	return fmt.Sprintf("%sError: %s", e.CallStack, e.Msg)
 }
 
+func (e *EvalError) Unwrap() error { return e.cause }
+
 // A Program is a compiled Starlark program.
 //
 // Programs are immutable, and contain no Values.
@@ -362,6 +366,57 @@ func (prog *Program) Init(thread *Thread, predeclared StringDict) (StringDict, e
 	return toplevel.Globals(), err
 }
 
+// ExecREPLChunk compiles and executes file f in the specified thread
+// and global environment. This is a variant of ExecFile specialized to
+// the needs of a REPL, in which a sequence of input chunks, each
+// syntactically a File, manipulates the same set of module globals,
+// which are not frozen after execution.
+//
+// This function is intended to support only go.starlark.net/repl.
+// Its API stability is not guaranteed.
+func ExecREPLChunk(f *syntax.File, thread *Thread, globals StringDict) error {
+	var predeclared StringDict
+
+	// -- variant of FileProgram --
+
+	if err := resolve.REPLChunk(f, globals.Has, predeclared.Has, Universe.Has); err != nil {
+		return err
+	}
+
+	var pos syntax.Position
+	if len(f.Stmts) > 0 {
+		pos = syntax.Start(f.Stmts[0])
+	} else {
+		pos = syntax.MakePosition(&f.Path, 1, 1)
+	}
+
+	module := f.Module.(*resolve.Module)
+	compiled := compile.File(f.Stmts, pos, "<toplevel>", module.Locals, module.Globals)
+	prog := &Program{compiled}
+
+	// -- variant of Program.Init --
+
+	toplevel := makeToplevelFunction(prog.compiled, predeclared)
+
+	// Initialize module globals from parameter.
+	for i, id := range prog.compiled.Globals {
+		if v := globals[id.Name]; v != nil {
+			toplevel.module.globals[i] = v
+		}
+	}
+
+	_, err := Call(thread, toplevel, nil, nil)
+
+	// Reflect changes to globals back to parameter, even after an error.
+	for i, id := range prog.compiled.Globals {
+		if v := toplevel.module.globals[i]; v != nil {
+			globals[id.Name] = v
+		}
+	}
+
+	return err
+}
+
 func makeToplevelFunction(prog *compile.Program, predeclared StringDict) *Function {
 	// Create the Starlark value denoted by each program constant c.
 	constants := make([]Value, len(prog.Constants))
diff --git a/starlark/interp.go b/starlark/interp.go
index bad4887..5290730 100644
--- a/starlark/interp.go
+++ b/starlark/interp.go
@@ -503,7 +503,10 @@ loop:
 			dict, err2 := thread.Load(thread, module)
 			thread.beginProfSpan()
 			if err2 != nil {
-				err = fmt.Errorf("cannot load %s: %v", module, err2)
+				err = wrappedError{
+					msg:   fmt.Sprintf("cannot load %s: %v", module, err2),
+					cause: err2,
+				}
 				break loop
 			}
 
@@ -590,6 +593,21 @@ loop:
 	return result, err
 }
 
+type wrappedError struct {
+	msg   string
+	cause error
+}
+
+func (e wrappedError) Error() string {
+	return e.msg
+}
+
+// Implements the xerrors.Wrapper interface
+// https://godoc.org/golang.org/x/xerrors#Wrapper
+func (e wrappedError) Unwrap() error {
+	return e.cause
+}
+
 // mandatory is a sentinel value used in a function's defaults tuple
 // to indicate that a (keyword-only) parameter is mandatory.
 type mandatory struct{}


Test patch:
diff --git a/starlark/eval_test.go b/starlark/eval_test.go
index 960f3b8..f5a667a 100644
--- a/starlark/eval_test.go
+++ b/starlark/eval_test.go
@@ -35,6 +35,11 @@ func option(chunk, name string) bool {
 	return strings.Contains(chunk, "option:"+name)
 }
 
+// Wrapper is the type of errors with an Unwrap method; see https://golang.org/pkg/errors.
+type Wrapper interface {
+	Unwrap() error
+}
+
 func TestEvalExpr(t *testing.T) {
 	// This is mostly redundant with the new *.star tests.
 	// TODO(adonovan): move checks into *.star files and
@@ -481,19 +486,19 @@ func TestInt(t *testing.T) {
 	}
 }
 
-func TestBacktrace(t *testing.T) {
-	getBacktrace := func(err error) string {
-		switch err := err.(type) {
-		case *starlark.EvalError:
-			return err.Backtrace()
-		case nil:
-			t.Fatalf("ExecFile succeeded unexpectedly")
-		default:
-			t.Fatalf("ExecFile failed with %v, wanted *EvalError", err)
-		}
-		panic("unreachable")
+func backtrace(t *testing.T, err error) string {
+	switch err := err.(type) {
+	case *starlark.EvalError:
+		return err.Backtrace()
+	case nil:
+		t.Fatalf("ExecFile succeeded unexpectedly")
+	default:
+		t.Fatalf("ExecFile failed with %v, wanted *EvalError", err)
 	}
+	panic("unreachable")
+}
 
+func TestBacktrace(t *testing.T) {
 	// This test ensures continuity of the stack of active Starlark
 	// functions, including propagation through built-ins such as 'min'.
 	const src = `
@@ -514,7 +519,7 @@ i()
   crash.star:3:12: in g
   crash.star:2:19: in f
 Error: floored division by zero`
-	if got := getBacktrace(err); got != want {
+	if got := backtrace(t, err); got != want {
 		t.Errorf("error was %s, want %s", got, want)
 	}
 
@@ -540,12 +545,67 @@ Error: join: in list, want string, got int`,
 	} {
 		globals := starlark.StringDict{"i": starlark.MakeInt(i)}
 		_, err := starlark.ExecFile(thread, "crash.star", src2, globals)
-		if got := getBacktrace(err); got != want {
+		if got := backtrace(t, err); got != want {
 			t.Errorf("error was %s, want %s", got, want)
 		}
 	}
 }
 
+func TestLoadBacktrace(t *testing.T) {
+	// This test ensures that load() does NOT preserve stack traces,
+	// but that API callers can get them with Unwrap().
+	// For discussion, see:
+	// https://github.com/google/starlark-go/pull/244
+	const src = `
+load('crash.star', 'x')
+`
+	const loadedSrc = `
+def f(x):
+  return 1 // x
+
+f(0)
+`
+	thread := new(starlark.Thread)
+	thread.Load = func(t *starlark.Thread, module string) (starlark.StringDict, error) {
+		return starlark.ExecFile(new(starlark.Thread), module, loadedSrc, nil)
+	}
+	_, err := starlark.ExecFile(thread, "root.star", src, nil)
+
+	const want = `Traceback (most recent call last):
+  root.star:2:1: in <toplevel>
+Error: cannot load crash.star: floored division by zero`
+	if got := backtrace(t, err); got != want {
+		t.Errorf("error was %s, want %s", got, want)
+	}
+
+	unwrapEvalError := func(err error) *starlark.EvalError {
+		var result *starlark.EvalError
+		for {
+			if evalErr, ok := err.(*starlark.EvalError); ok {
+				result = evalErr
+			}
+
+			// TODO: use errors.Unwrap when go >=1.13 is everywhere.
+			wrapper, isWrapper := err.(Wrapper)
+			if !isWrapper {
+				break
+			}
+			err = wrapper.Unwrap()
+		}
+		return result
+	}
+
+	unwrappedErr := unwrapEvalError(err)
+	const wantUnwrapped = `Traceback (most recent call last):
+  crash.star:5:2: in <toplevel>
+  crash.star:3:12: in f
+Error: floored division by zero`
+	if got := backtrace(t, unwrappedErr); got != wantUnwrapped {
+		t.Errorf("error was %s, want %s", got, wantUnwrapped)
+	}
+
+}
+
 // TestRepeatedExec parses and resolves a file syntax tree once then
 // executes it repeatedly with different values of its predeclared variables.
 func TestRepeatedExec(t *testing.T) {
@@ -703,3 +763,28 @@ func TestUnpackErrorBadType(t *testing.T) {
 		}
 	}
 }
+
+// Regression test for github.com/google/starlark-go/issues/233.
+func TestREPLChunk(t *testing.T) {
+	thread := new(starlark.Thread)
+	globals := make(starlark.StringDict)
+	exec := func(src string) {
+		f, err := syntax.Parse("<repl>", src, 0)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := starlark.ExecREPLChunk(f, thread, globals); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	exec("x = 0; y = 0")
+	if got, want := fmt.Sprintf("%v %v", globals["x"], globals["y"]), "0 0"; got != want {
+		t.Fatalf("chunk1: got %s, want %s", got, want)
+	}
+
+	exec("x += 1; y = y + 1")
+	if got, want := fmt.Sprintf("%v %v", globals["x"], globals["y"]), "1 1"; got != want {
+		t.Fatalf("chunk2: got %s, want %s", got, want)
+	}
+}
diff --git a/starlarktest/starlarktest.go b/starlarktest/starlarktest.go
index 5868ef0..0005412 100644
--- a/starlarktest/starlarktest.go
+++ b/starlarktest/starlarktest.go
@@ -15,6 +15,7 @@ package starlarktest // import "go.starlark.net/starlarktest"
 import (
 	"fmt"
 	"go/build"
+	"os"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -134,5 +135,13 @@ func freeze(thread *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, k
 // 'go build', under which a test runs in its package directory,
 // and Blaze, under which a test runs in the root of the tree.
 var DataFile = func(pkgdir, filename string) string {
+	// Check if we're being run by Bazel and change directories if so.
+	// TEST_SRCDIR and TEST_WORKSPACE are set by the Bazel test runner, so that makes a decent check
+	testSrcdir := os.Getenv("TEST_SRCDIR")
+	testWorkspace := os.Getenv("TEST_WORKSPACE")
+	if testSrcdir != "" && testWorkspace != "" {
+		return filepath.Join(testSrcdir, "net_starlark_go", pkgdir, filename)
+	}
+
 	return filepath.Join(build.Default.GOPATH, "src/go.starlark.net", pkgdir, filename)
 }