[tools/osh2oil] Simplify and improve ShFunction translation

Also fix semi_tok compile error.

frontend/syntax.asdl

@@ -331,7 +331,7 @@ module syntax
          Token? esac_kw, List[Redir] redirects)
     # The keyword is optional in the case of bash-style functions
     # (ie. "foo() { ... }") which do not have one.
-  | ShFunction(Token? keyword, loc name_loc, str name, command body)
+  | ShFunction(Token? keyword, Token name_tok, str name, command body)
   | TimeBlock(Token keyword, command pipeline)
     # Some nodes optimize it out as List[command], but we use CommandList for
     # 1. the top level

osh/cmd_eval.py

@@ -1341,9 +1341,9 @@ def _Dispatch(self, node, cmd_st):
         node = cast(command.ShFunction, UP_node)
         if node.name in self.procs and not self.exec_opts.redefine_proc():
           e_die("Function %s was already defined (redefine_proc)" % node.name,
-                node.name_loc)
+                node.name_tok)
         self.procs[node.name] = Proc(
-            node.name, node.name_loc, proc_sig.Open(), node.body, [], True)
+            node.name, node.name_tok, proc_sig.Open(), node.body, [], True)
 
         status = 0
 

osh/cmd_parse.py

@@ -1232,8 +1232,6 @@ def _ParseForEachLoop(self, for_kw):
 
     self._NewlineOk()
 
-    semi_tok = None
-
     self._Peek()
     if self.c_id == Id.KW_In:
 
@@ -1248,6 +1246,7 @@ def _ParseForEachLoop(self, for_kw):
           p_die('Expected { after iterable expression',
                 loc.Word(self.cur_word))
       else:
+        semi_tok = None  # type: Optional[Token]
         iter_words, semi_tok = self.ParseForWords()
         node.semi_tok = semi_tok
 
@@ -1733,8 +1732,6 @@ def ParseFunctionDef(self):
 
     Bash only accepts the latter, though it doesn't really follow a grammar.
     """
-    left_spid = _KeywordSpid(self.cur_word)
-
     word0 = cast(CompoundWord, self.cur_word)  # caller ensures validity
     name = word_.ShFunctionName(word0)
     if len(name) == 0:  # example: foo$x is invalid
@@ -1760,21 +1757,16 @@ def ParseFunctionDef(self):
       # would just be 'f'
       self._Next()
 
-      after_name_spid = location.OfWordLeft(self.cur_word) + 1
-
       self._NewlineOk()
 
       func = command.ShFunction.CreateNull()
       func.name = name
       with ctx_VarChecker(self.var_checker, blame_tok):
         func.body = self.ParseCompoundCommand()
 
-      func.name_loc = loc.Word(word0)
+      name_spid = location.OfWordLeft(word0)
+      func.name_tok = self.arena.GetToken(name_spid)
 
-      # matches ParseKshFunctionDef below
-      func.spids.append(left_spid)
-      func.spids.append(left_spid)  # name span id is same as left_spid in this case
-      func.spids.append(after_name_spid)
       return func
     else:
       p_die('Expected ) in function definition', loc.Word(self.cur_word))
@@ -1786,7 +1778,6 @@ def ParseKshFunctionDef(self):
     ksh_function_def : 'function' fname ( '(' ')' )? newline_ok function_body
     """
     keyword_tok = word_.AsKeywordToken(self.cur_word)
-    left_spid = location.OfWordLeft(self.cur_word)
 
     self._Next()  # skip past 'function'
     self._Peek()
@@ -1796,18 +1787,14 @@ def ParseKshFunctionDef(self):
     if len(name) == 0:  # example: foo$x is invalid
       p_die('Invalid KSH-style function name', loc.Word(cur_word))
 
-    name_loc = loc.Word(self.cur_word)
-    name_spid = location.OfWordLeft(self.cur_word)
-    after_name_spid = name_spid + 1
+    name_word = self.cur_word
     self._Next()  # skip past 'function name
 
     self._Peek()
     if self.c_id == Id.Op_LParen:
       self.lexer.PushHint(Id.Op_RParen, Id.Right_ShFunction)
       self._Next()
       self._Eat(Id.Right_ShFunction)
-      # Change it: after )
-      after_name_spid = location.OfWordLeft(self.cur_word) + 1
 
     self._NewlineOk()
 
@@ -1817,12 +1804,10 @@ def ParseKshFunctionDef(self):
       func.body = self.ParseCompoundCommand()
 
     func.keyword = keyword_tok
-    func.name_loc = name_loc
 
-    # matches ParseFunctionDef above
-    func.spids.append(left_spid)
-    func.spids.append(name_spid)
-    func.spids.append(after_name_spid)
+    name_spid = location.OfWordLeft(name_word)
+    func.name_tok = self.arena.GetToken(name_spid)
+
     return func
 
   def ParseOilProc(self):

test/ysh-prettify.sh

@@ -421,33 +421,19 @@ test-posix-func() {
     echo "hi"
   }'
 
-  # Non-brace function bodies
-  # TODO: Bail in this case
-  check-osh2ysh '
-  f() (
-    echo hi
-  )' \
-  '
-  proc f (
-    echo hi
-  )' \
-  INVALID
-
-  return
-
-  # TODO: Move the brace
+  # The brace is moved
   check-osh2ysh '
   f()
   {
     echo "hi"
   }' '
-  proc f
-  {
+  proc f {
     echo "hi"
   }'
 
-  # No nested functinos
   return
+
+  # Nested functinos
   check-osh2ysh '
 func1() {
   echo func1
@@ -464,7 +450,19 @@ proc func1 {
     echo func2
   }
 }'
+  return
 
+  # Non-brace function bodies
+  # TODO: Bail in this case
+  check-osh2ysh '
+  f() (
+    echo hi
+  )' \
+  '
+  proc f (
+    echo hi
+  )' \
+  INVALID
 }
 
 test-ksh-func() {

tools/osh2oil.py

@@ -568,23 +568,25 @@ def DoCommand(self, node, local_symbols, at_top_level=False):
         new_local_symbols = {}  # type: Dict[str, bool]
 
         # Should be the left most span, including 'function'
-        self.cursor.PrintUntil(node.spids[0])
-
-        self.f.write('proc ')
-        self.f.write(node.name)
-        self.cursor.SkipUntil(node.spids[2])
-
-        if node.body.tag() == command_e.BraceGroup:
-          # Don't add "do" like a standalone brace group.  Just use {}.
-          for child in cast(BraceGroup, node.body).children:
-            self.DoCommand(child, new_local_symbols)
-        else:
-          pass
-          # Add {}.
-          # proc foo {
-          #   shell {echo hi; echo bye}
-          # }
-          #self.DoCommand(node.body)
+        if node.keyword:  # function foo { ...
+          self.cursor.PrintUntil(node.keyword.span_id)
+        else:  # foo() { ...
+          self.cursor.PrintUntil(node.name_tok.span_id)
+
+        self.f.write('proc %s ' % node.name)
+
+        UP_body = node.body
+        with tagswitch(UP_body) as case:
+          if case(command_e.BraceGroup):
+            body = cast(BraceGroup, UP_body)
+            self.cursor.SkipUntil(body.left.span_id)
+
+            # Don't add "do" like a standalone brace group.  Just use {}.
+            for child in body.children:
+              self.DoCommand(child, new_local_symbols)
+          else:
+            # very rare cases like f() ( subshell )
+            pass
 
       elif case(command_e.DoGroup):
         node = cast(command.DoGroup, UP_node)