From 7b66e9f104f114accb067150c8708d072c36a612 Mon Sep 17 00:00:00 2001
From: Qi Xiao <xiaqqaix@gmail.com>
Date: Sun, 2 May 2021 00:13:22 +0100
Subject: [PATCH] Minor fixups for #1273.

Also fix a test broken by bad merge.
---
 pkg/eval/compile_effect_test.go | 2 +-
 pkg/eval/errs/errs.go           | 7 ++++++-
 pkg/eval/frame.go               | 7 +++----
 pkg/eval/ns_test.go             | 2 +-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/pkg/eval/compile_effect_test.go b/pkg/eval/compile_effect_test.go
index cd7c42ea..f0db4423 100644
--- a/pkg/eval/compile_effect_test.go
+++ b/pkg/eval/compile_effect_test.go
@@ -134,7 +134,7 @@ func TestCommand_Assignment(t *testing.T) {
 		That("a true b = 1 2 3").Throws(errs.SetReadOnlyVar{VarName: "true"}, "true"),
 		That("@true = 1").Throws(errs.SetReadOnlyVar{VarName: "@true"}, "@true"),
 		// A readonly var as a target for the `except` clause should error.
-		That("try { fail reason } except nil { }").Throws(vars.ErrSetReadOnlyVar, "nil"),
+		That("try { fail reason } except nil { }").Throws(errs.SetReadOnlyVar{VarName: "nil"}, "nil"),
 		That("try { fail reason } except x { }").DoesNothing(),
 		// Evaluation of the assignability occurs at run-time so, if no exception is raised, this
 		// otherwise invalid use of `nil` is okay.
diff --git a/pkg/eval/errs/errs.go b/pkg/eval/errs/errs.go
index 367ab10f..0373a3c0 100644
--- a/pkg/eval/errs/errs.go
+++ b/pkg/eval/errs/errs.go
@@ -1,4 +1,4 @@
-// Package errs declares Elvish error types that are not simple strings (i.e., the Go `error` type).
+// Package errs declares error types used as exception causes.
 package errs
 
 import (
@@ -14,6 +14,7 @@ type OutOfRange struct {
 	Actual    string
 }
 
+// Error implements the error interface.
 func (e OutOfRange) Error() string {
 	if e.ValidHigh < e.ValidLow {
 		return fmt.Sprintf(
@@ -32,6 +33,7 @@ type BadValue struct {
 	Actual string
 }
 
+// Error implements the error interface.
 func (e BadValue) Error() string {
 	return fmt.Sprintf(
 		"bad value: %v must be %v, but is %v", e.What, e.Valid, e.Actual)
@@ -69,9 +71,12 @@ func nValues(n int) string {
 
 // SetReadOnlyVar is returned by the Set method of a read-only variable.
 type SetReadOnlyVar struct {
+	// Name of the read-only variable. This fiels is initially empty, and
+	// populated later when context information is available.
 	VarName string
 }
 
+// Error implements the error interface.
 func (e SetReadOnlyVar) Error() string {
 	return fmt.Sprintf("cannot set read-only variable %q", e.VarName)
 }
diff --git a/pkg/eval/frame.go b/pkg/eval/frame.go
index c5ff1b6b..2691a95d 100644
--- a/pkg/eval/frame.go
+++ b/pkg/eval/frame.go
@@ -211,12 +211,11 @@ func (fm *Frame) errorp(r diag.Ranger, e error) Exception {
 		return nil
 	case Exception:
 		return e
-	case errs.SetReadOnlyVar:
-		ctx := diag.NewContext(fm.srcMeta.Name, fm.srcMeta.Code, r)
-		e.VarName = ctx.RelevantString()
-		return &exception{e, &StackTrace{Head: ctx, Next: fm.traceback}}
 	default:
 		ctx := diag.NewContext(fm.srcMeta.Name, fm.srcMeta.Code, r)
+		if _, ok := e.(errs.SetReadOnlyVar); ok {
+			e = errs.SetReadOnlyVar{VarName: ctx.RelevantString()}
+		}
 		return &exception{e, &StackTrace{Head: ctx, Next: fm.traceback}}
 	}
 }
diff --git a/pkg/eval/ns_test.go b/pkg/eval/ns_test.go
index 5f99fbee..dc4c2b6f 100644
--- a/pkg/eval/ns_test.go
+++ b/pkg/eval/ns_test.go
@@ -26,6 +26,6 @@ func TestNs(t *testing.T) {
 
 func TestBuiltinFunctionsReadOnly(t *testing.T) {
 	Test(t,
-		That("return~ = { }").Throws(ErrorWithType(errs.SetReadOnlyVar{}), "return~"),
+		That("return~ = { }").Throws(errs.SetReadOnlyVar{VarName: "return~"}, "return~"),
 	)
 }