Skip to content

Commit

Permalink
[errors] Add location info to Oil language var references.
Browse files Browse the repository at this point in the history
- And egg expression splices.
- Also update docs about read --line --with-eol.
  • Loading branch information
Andy Chu committed Jan 9, 2021
1 parent 4589c46 commit d4c45d7
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 14 deletions.
9 changes: 5 additions & 4 deletions doc/oil-help.md
Expand Up @@ -402,10 +402,11 @@ The `.cell` action starts with `.` to indicate that its format is unstable.

#### oil-read

read --line # default var is $_line
read --line --qsn # decode QSN too
read --all # whole file including newline; var is $_all
read -0 # read until NUL, synonym for read -r -d ''
read --line # default var is $_line
read --line --with-eol # keep the \n
read --line --qsn # decode QSN too
read --all # whole file including newline; var is $_all
read -0 # read until NUL, synonym for read -r -d ''

#### run

Expand Down
4 changes: 4 additions & 0 deletions doc/osh-help.md
Expand Up @@ -556,6 +556,10 @@ Read a line from stdin, split it into tokens with the `$IFS` algorithm,
and assign the tokens to the given variables. When no VARs are given,
assign to `$REPLY`.

Note: When writing Oil, prefer the extensions documented in
[oil-read]($oil-help). The `read` builtin is confusing because `-r` needs to
be explicitly enabled.

Flags:

-a ARRAY assign the tokens to elements of this array
Expand Down
9 changes: 5 additions & 4 deletions oil_lang/expr_eval.py
Expand Up @@ -12,6 +12,7 @@
from _devbuild.gen.runtime_asdl import (
lvalue, value, value_e, scope_e,
)
from asdl import runtime
from core.pyerror import e_die, log
from frontend import consts
from oil_lang import objects
Expand Down Expand Up @@ -65,14 +66,14 @@ def CheckCircularDeps(self):
assert self.shell_ex is not None
assert self.word_ev is not None

def LookupVar(self, var_name):
def LookupVar(self, var_name, span_id=runtime.NO_SPID):
"""Convert to a Python object so we can calculate on it natively."""

# Lookup WITHOUT dynamic scope.
val = self.mem.GetValue(var_name, which_scopes=scope_e.LocalOrGlobal)
if val.tag == value_e.Undef:
# TODO: Location info
e_die('Undefined variable %r', var_name)
e_die('Undefined variable %r', var_name, span_id=span_id)

if val.tag == value_e.Str:
return val.s
Expand Down Expand Up @@ -226,7 +227,7 @@ def EvalExpr(self, node):
raise AssertionError(id_)

if node.tag == expr_e.Var:
return self.LookupVar(node.name.val)
return self.LookupVar(node.name.val, span_id=node.name.span_id)

if node.tag == expr_e.CommandSub:
id_ = node.left_token.id
Expand Down Expand Up @@ -631,7 +632,7 @@ def _MaybeReplaceLeaf(self, node):
new_leaf = re.LiteralChars(s, node.token.span_id)

elif node.tag == re_e.Splice:
obj = self.LookupVar(node.name.val)
obj = self.LookupVar(node.name.val, span_id=node.name.span_id)
if not isinstance(obj, objects.Regex):
e_die("Can't splice object of type %r into regex", obj.__class__,
token=node.name)
Expand Down
14 changes: 8 additions & 6 deletions spec/oil-user-feedback.test.sh
Expand Up @@ -92,15 +92,17 @@ foo
baz
## END

#### Location info undefined var in expression mode
#### readonly in loop: explains why const doesn't work

# TODO: Move this to runtime-errors?
# TODO: Might want to change const in Oil...
# bash actually prevents assignment and prints a warning, DOH.

if (x) {
echo x
}
seq 3 | while read -r line; do
readonly stripped=${line//1/x}
#declare stripped=${line//1/x}
echo $stripped
done
## status: 1
## STDOUT:
x
## END

24 changes: 24 additions & 0 deletions test/oil-runtime-errors.sh
Expand Up @@ -7,6 +7,18 @@

source test/common.sh

OIL=${OIL:-bin/oil}

_error-case() {
$OIL -c "$@"

# NOTE: This works with osh, not others.
local status=$?
if test $status != 1; then
die "Expected status 1, got $status"
fi
}

regex_literals() {
var sq = / 'foo'+ /
var dq = / "foo"+ /
Expand All @@ -25,6 +37,17 @@ regex_literals() {
echo $bvs
}

undefined_vars() {
set +o errexit

_error-case 'echo hi; y = 2 + x + 3'
_error-case 'if (x) { echo hello }'
_error-case 'if ($x) { echo hi }'
_error-case 'if (${x}) { echo hi }'

_error-case 'x = / @yo /'
}

_run-test() {
local name=$1

Expand All @@ -37,6 +60,7 @@ _run-test() {

run-all-with-osh() {
_run-test regex_literals
_run-test undefined_vars

return 0 # success
}
Expand Down

0 comments on commit d4c45d7

Please sign in to comment.