Skip to content

Commit

Permalink
[errors] Improve location in: for x in (y)
Browse files Browse the repository at this point in the history
It used to point to right )

Now it points to 'in'

Arguably a lateral move ... but the real problem is I ran into this bug
again:

    for x in(y)   # doesn't work without space, but should
  • Loading branch information
Andy C committed Aug 24, 2023
1 parent 6e9e55e commit 70cedea
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 20 deletions.
14 changes: 0 additions & 14 deletions frontend/parse_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,20 +361,6 @@ def ParseYshExpr(self, lx, start_symbol):

return ast_node, last_token

def ParseYshForExpr(self, lexer, start_symbol):
# type: (Lexer, int) -> Tuple[List[NameType], expr_t, Token]
"""for (x Int, y Int in foo)"""
e_parser = self._YshParser()
with ctx_PNodeAllocator(e_parser):
pnode, last_token = e_parser.Parse(lexer, start_symbol)

if 0:
self.p_printer.Print(pnode)

lvalue, iterable = self.tr.OilForExpr(pnode)

return lvalue, iterable, last_token

def ParseYshCasePattern(self, lexer):
# type: (Lexer) -> Tuple[pat_t, Token, Token]
"""(6) | (7), / dot* '.py' /, (else), etc.
Expand Down
6 changes: 3 additions & 3 deletions osh/cmd_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -1318,7 +1318,7 @@ def _Dispatch(self, node, cmd_st):

# for YSH loop
iter_expr = None # type: expr_t
#iter_blame = None # type: Token
expr_blame = None # type: loc_t

iterable = node.iterable
UP_iterable = iterable
Expand All @@ -1335,7 +1335,7 @@ def _Dispatch(self, node, cmd_st):
elif case(for_iter_e.YshExpr):
iterable = cast(for_iter.YshExpr, UP_iterable)
iter_expr = iterable.e
iter_expr_blame = iterable.blame
expr_blame = iterable.blame

n = len(node.iter_names)
assert n > 0
Expand All @@ -1347,7 +1347,7 @@ def _Dispatch(self, node, cmd_st):

it2 = None # type: val_ops._ContainerIter
if iter_list is None: # for_expr.YshExpr
val = self.expr_ev.EvalExpr(iter_expr, loc.Missing)
val = self.expr_ev.EvalExpr(iter_expr, expr_blame)

UP_val = val
with tagswitch(val) as case:
Expand Down
7 changes: 6 additions & 1 deletion osh/cmd_parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -1314,12 +1314,17 @@ def _ParseForEachLoop(self, for_kw):

self._GetWord()
if self.c_id == Id.KW_In:
# Ideally we would want ( not 'in'. But we still have to fix the bug
# where we require a SPACE between in and (
# for x in(y) # should be accepted, but isn't

expr_blame = word_.AsKeywordToken(self.cur_word)

self._SetNext() # skip in
if self.w_parser.LookPastSpace() == Id.Op_LParen:
enode, last_token = self.parse_ctx.ParseYshExpr(
self.lexer, grammar_nt.oil_expr)
node.iterable = for_iter.YshExpr(enode, last_token)
node.iterable = for_iter.YshExpr(enode, expr_blame)

# For simplicity, we don't accept for x in (obj); do ...
self._GetWord()
Expand Down
11 changes: 9 additions & 2 deletions test/ysh-runtime-errors.sh
Original file line number Diff line number Diff line change
Expand Up @@ -195,18 +195,25 @@ test-fallback-locations() {
# From Aidan's PR -- redefinition
_error-case 'const f = 42; func f() { echo hi }'

# ForEach eval
# ForEach shell
_expr-error-case 'for x in $[2 + len(42)] { echo hi }'

# ForEach YSH
_expr-error-case 'for x in (len(42)) { echo hi }'

_expr-error-case 'while (len(42)) { echo hi }'

_expr-error-case 'case (len(42)) { pat { echo hi } }'
_expr-error-case 'case (len(42)) { pat { echo argument } }'
_expr-error-case 'case (42) { (len(42)) { echo arm } }'

_expr-error-case 'case "$[len(42)]" in pat) echo hi ;; esac'

_expr-error-case 'var x = 3 + len(42)'
_expr-error-case 'const x = 3 + len(42)'
_expr-error-case 'setvar x = 3 + len(42)'

_expr-error-case 'setvar x = "s" + 5'
_expr-error-case 'while ("s" + 5) { echo yes } '
}

test-EvalExpr-calls() {
Expand Down

0 comments on commit 70cedea

Please sign in to comment.