[oil-language] Disallow any nesting of 'proc'

For compatibility, shell functions can be nested.  But procs are always
at the top level.

core/runtime.asdl

@@ -67,8 +67,8 @@ module runtime
   -- Used for ${a[i]=x}.  TODO: also use for lvalue/place.
   a_index = Str(string s) | Int(int i)
 
-  -- A cell is a wrapper for a value.  16 bytes in C++.
-  -- TODO: add spid for last-assigned location
+  -- A cell is a wrapper for a value.
+  -- TODO: add spid for declaration for 'assigning const' error
 
   -- Invariant: if exported or nameref is set, the val should be Str or Undef.
   -- This is enforced in mem.SetValue but isn't expressed in the schema.
@@ -87,8 +87,8 @@ module runtime
   --   GetValue: N/A
   --   SetValue: internal use in COMPREPLY, and Oil's 'setglobal' keyword
 
-  -- NOTE: scope_e.Dynamic should generally not be used by callers.  Most
-  -- use scope_e.Shopt to allow turning it off.
+  -- NOTE: Most callers use scope_e.Shopt rather than scope_e.Dynamic 
+  -- to respect shopt -u dynamic_scope.
   scope = Dynamic | Shopt | LocalOrGlobal | LocalOnly | GlobalOnly 
 
   -- For OSH assignment, evaluated from osh_ast.lhs_expr

core/state.py

@@ -767,7 +767,7 @@ def _InitVarsFromEnv(mem, environ):
   # variable.  Dash has a loop through environ in init.c
   for n, v in iteritems(environ):
     mem.SetValue(lvalue.Named(n), value.Str(v), scope_e.GlobalOnly,
-               flags=SetExport)
+                 flags=SetExport)
 
   # If it's not in the environment, initialize it.  This makes it easier to
   # update later in MutableOpts.

osh/cmd_parse.py

@@ -48,7 +48,7 @@
 from osh import bool_parse
 from osh import word_
 
-from typing import Optional, List, Tuple, cast, TYPE_CHECKING
+from typing import Optional, List, Dict, Any, Tuple, cast, TYPE_CHECKING
 if TYPE_CHECKING:
   from core.alloc import Arena
   from frontend.lexer import Lexer
@@ -345,6 +345,72 @@ def _MakeSimpleCommand(preparsed_list, suffix_words, redirects, block):
   return node
 
 
+class Declarations(object):
+  """Ensure that var and const can only appear once per function.
+
+  Bash allows this:
+
+  f() {
+    g() {
+      echo 'top level function defined in another one'
+    }
+  }
+
+  So we have a stack... And then
+  """
+  def __init__(self):
+    # type: () -> None
+    """
+    Args:
+      oil_proc: Whether to disallow nested proc/function declarations
+    """
+    # tokens has 'proc' or some other token
+    self.tokens = []  # type: List[Token]
+    self.names = []  # type: List[Dict[str, bool]]
+
+  def Push(self, blame_tok):
+    # type: (Token) -> None
+    if len(self.tokens) != 0:
+      if self.tokens[0].id == Id.KW_Proc or blame_tok.id == Id.KW_Proc:
+        p_die("procs can't contain other procs or functions", token=blame_tok)
+
+    self.tokens.append(blame_tok)
+    entry = {}  # type: Dict[str, bool]
+    self.names.append(entry)
+
+  def Pop(self):
+    # type: () -> None
+    self.names.pop()
+    self.tokens.pop()
+
+  def Register(self, name_tok):
+    # type: (Token) -> None
+    """
+    Register a variable or param declaration.
+    """
+    top = self.names[-1] 
+    name = name_tok.val
+    if name in top:
+      p_die('%r was already declared', name, token=name_tok)
+    else:
+      top[name] = True
+
+
+class ctx_Declarations(object):
+  def __init__(self, declarations, blame_tok):
+    # type: (Declarations, Token) -> None
+    declarations.Push(blame_tok)
+    self.declarations = declarations
+
+  def __enter__(self):
+    # type: () -> None
+    pass
+
+  def __exit__(self, type, value, traceback):
+    # type: (Any, Any, Any) -> None
+    self.declarations.Pop()
+
+
 SECONDARY_KEYWORDS = [
     Id.KW_Do, Id.KW_Done, Id.KW_Then, Id.KW_Fi, Id.KW_Elif, Id.KW_Else, Id.KW_Esac
 ]
@@ -372,6 +438,7 @@ def __init__(self, parse_ctx, w_parser, lexer, line_reader):
     # A hacky boolean to remove 'if cd / {' ambiguity.
     self.allow_block = True
     self.parse_opts = parse_ctx.parse_opts
+    self.declarations = Declarations()
 
     self.Reset()
 
@@ -1546,10 +1613,9 @@ def ParseFunctionDef(self):
     # type: () -> command__ShFunction
     """
     function_header : fname '(' ')'
-    function_def     : function_header newline_ok function_body ;
+    function_def    : function_header newline_ok function_body ;
 
     Precondition: Looking at the function name.
-    Post condition:
 
     NOTE: There is an ambiguity with:
 
@@ -1562,6 +1628,12 @@ def ParseFunctionDef(self):
 
     cur_word = cast(compound_word, self.cur_word)  # caller ensures validity
     name = word_.ShFunctionName(cur_word)
+
+    # If it passed ShFunctionName, this should be true.
+    part0 = cur_word.parts[0]
+    assert part0.tag_() == word_part_e.Literal
+    blame_tok = cast(Token, part0)
+
     if len(name) == 0:
       p_die('Invalid function name', word=cur_word)
 
@@ -1582,7 +1654,8 @@ def ParseFunctionDef(self):
 
     func = command.ShFunction()
     func.name = name
-    func.body = self.ParseCompoundCommand()
+    with ctx_Declarations(self.declarations, blame_tok):
+      func.body = self.ParseCompoundCommand()
 
     # matches ParseKshFunctionDef below
     func.spids.append(left_spid)
@@ -1595,6 +1668,7 @@ def ParseKshFunctionDef(self):
     """
     ksh_function_def : 'function' fname ( '(' ')' )? newline_ok function_body
     """
+    keyword_tok = _KeywordToken(self.cur_word)
     left_spid = word_.LeftMostSpanForWord(self.cur_word)
 
     self._Next()  # skip past 'function'
@@ -1621,7 +1695,8 @@ def ParseKshFunctionDef(self):
 
     func = command.ShFunction()
     func.name = name
-    func.body = self.ParseCompoundCommand()
+    with ctx_Declarations(self.declarations, keyword_tok):
+      func.body = self.ParseCompoundCommand()
 
     # matches ParseFunctionDef above
     func.spids.append(left_spid)
@@ -1632,11 +1707,16 @@ def ParseKshFunctionDef(self):
   def ParseOilProc(self):
     # type: () -> command__Proc
     node = command.Proc()
-    self.w_parser.ParseProc(node)
 
-    self._Next()
-    node.body = self.ParseBraceGroup()
-    # No redirects for Oil procs (only at call site)
+    keyword_tok = _KeywordToken(self.cur_word)
+    with ctx_Declarations(self.declarations, keyword_tok):
+      self.w_parser.ParseProc(node)
+
+      # TODO: self.declarations.Register(params ...)
+
+      self._Next()
+      node.body = self.ParseBraceGroup()
+      # No redirects for Oil procs (only at call site)
 
     return node
 

spec/blog1.test.sh

@@ -58,3 +58,38 @@ echo ${#1#'###'}
 ## OK dash stdout: 4
 ## OK zsh stdout: 1
 ## N-I bash/mksh status: 1
+
+#### Julia example from spec/oil-user-feedback
+
+case $SH in (dash|mksh|zsh) exit ;; esac
+
+git-branch-merged() {
+  cat <<EOF
+  foo
+* bar
+  baz
+  master
+EOF
+}
+
+shopt -s lastpipe  # required for bash, not OSH
+
+branches=()  # dangerous when set -e is on
+git-branch-merged | while read -r line; do
+  line=${line# *}  # strip leading spaces
+  if [[ $line != 'master' && ! ${line:0:1} == '*' ]]; then
+    branches+=("$line")
+  fi
+done
+
+if [[ ${#branches[@]} -eq 0 ]]; then
+  echo "No merged branches"
+else
+  echo git branch -D "${branches[@]}"
+fi
+
+## STDOUT:
+git branch -D foo baz
+## END
+## N-I dash/mksh/zsh STDOUT:
+## END

spec/oil-proc.test.sh

@@ -211,3 +211,18 @@ declare -F
 declare -f myfunc
 declare -f myproc
 ## END
+
+
+#### Nested proc is disallowed at parse time
+
+# NOTE: we can disallow this in Oil statically ...
+proc f {
+  proc g {
+    echo 'G'
+  }
+  g
+}
+f
+g
+## status: 2
+## stdout-json: ""

spec/oil-user-feedback.test.sh

@@ -57,6 +57,8 @@ variant=foo
 #### Julia port
 
 # https://lobste.rs/s/ritbgc/what_glue_languages_do_you_use_like#c_nhikri
+#
+# See bash counterpart in spec/blog1.test.sh
 
 git-branch-merged() {
   cat <<EOF
@@ -71,7 +73,7 @@ EOF
 git-branch-merged | while read --line {
   # BUG: var or const messes up in al oop.
   setvar line = _line.strip()  # removing leading space
-  if (line != "master" and not line.startswith('*')) {
+  if (line != 'master' and not line.startswith('*')) {
     echo $line
   }
 } | readarray -t :branches
@@ -85,9 +87,9 @@ if (len(branches) == 0) {
 # With "push".  Hm read --lines isn't bad.
 var branches2 = %()
 git-branch-merged | while read --line {
-  # BUG: var or const messes up in al oop.
+  # TODO: Should be 'const' once we fix it?  Or 'var'?
   setvar line = _line.strip()  # removing leading space
-  if (line != "master" and not line.startswith('*')) {
+  if (line != 'master' and not line.startswith('*')) {
     push :branches2 $line
   }
 }

test/parse-errors.sh

@@ -842,6 +842,21 @@ parse_at() {
   _oil-parse-error 'echo @"foo"'
 }
 
+oil_nested_proc() {
+  set +o errexit
+
+  _oil-parse-error 'proc p { echo 1; proc f { echo f }; echo 2 }'
+  _oil-parse-error 'proc p { echo 1; +weird() { echo f; }; echo 2 }'
+
+  # ksh function
+  _oil-parse-error 'proc p { echo 1; function f { echo f; }; echo 2 }'
+
+  _oil-parse-error 'f() { echo 1; proc inner { echo inner; }; echo 2; }'
+
+  # shell nesting is still allowed
+  _should-parse 'f() { echo 1; g() { echo g; }; echo 2; }'
+}
+
 cases-in-strings() {
   set +o errexit
 
@@ -888,6 +903,7 @@ cases-in-strings() {
   parse_dollar
   parse_backslash
   oil_to_make_nicer
+  oil_nested_proc
   parse_at
 }