[oil-language] Detect duplicate declarations as syntax errors.

This includes proc params.

Wasn't too hard!

We should also be able to test set/setlocal statically.  But I
think setvar/setglobal don't know what globals there are because of
'source' and environment vars.  (shopt -u copy_env in Oil?)

This fixes the Julia example.

But we also have a 'const' issue.  I think that is OK for now.

core/state.py

@@ -1276,15 +1276,8 @@ def _CheckOilKeyword(self, keyword_id, name, cell):
     definition and mutation will help translate the Oil subset of OSH to static
     languages.
     """
-    if cell and keyword_id in (Id.KW_Var, Id.KW_Const):
-      # TODO:
-      # - This doesn't work for LOOPS.  Maybe you need to store the keyword
-      #   SPID with it?  (Would have to change PackFlags and so forth)
-      #   - note setvar and builtins like 'read' and 'mapfile' can create vars
-      #     with state.SetRef{String,Array}
-      # - Point at the ORIGINAL declaration!
-      e_die("%r has already been declared", name)
-
+    # TODO: Also do this at parse time.  We have some name resolution in
+    # ctx_Declarations.
     if cell is None and keyword_id in (Id.KW_Set, Id.KW_SetLocal,
                                        Id.KW_SetGlobal):
       e_die("%r hasn't been declared", name)

osh/cmd_parse.py

@@ -33,6 +33,8 @@
 
     source, parse_result, parse_result_t,
     speck, name_type,
+
+    proc_sig_e, proc_sig__Closed,
 )
 from _devbuild.gen import syntax_asdl  # token, etc.
 
@@ -366,7 +368,7 @@ def __init__(self):
     """
     # tokens has 'proc' or some other token
     self.tokens = []  # type: List[Token]
-    self.names = []  # type: List[Dict[str, bool]]
+    self.names = [{}]  # type: List[Dict[str, bool]]
 
   def Push(self, blame_tok):
     # type: (Token) -> None
@@ -651,7 +653,10 @@ def _ScanSimpleCommand(self):
           # Treat { and } more like operators
           if self.c_id == Id.Lit_LBrace:
             if self.allow_block:  # Disabled for if/while condition, etc.
-              block = self.ParseBraceGroup()
+              blame_tok = _KeywordToken(self.cur_word)
+              # Our own scope for 'var'
+              with ctx_Declarations(self.declarations, blame_tok):
+                block = self.ParseBraceGroup()
             if 0:
               print('--')
               block.PrettyPrint()
@@ -1594,7 +1599,10 @@ def ParseCompoundCommand(self):
     if self.c_id in (Id.KW_Var, Id.KW_Const):
       kw_token = word_.LiteralToken(self.cur_word)
       self._Next()
-      return self.w_parser.ParseVarDecl(kw_token)
+      n8 = self.w_parser.ParseVarDecl(kw_token)
+      for lhs in n8.lhs:
+        self.declarations.Register(lhs.name)
+      return n8
 
     if self.c_id in (Id.KW_SetVar, Id.KW_SetRef, Id.KW_SetLocal,
                      Id.KW_SetGlobal):
@@ -1630,15 +1638,14 @@ def ParseFunctionDef(self):
 
     cur_word = cast(compound_word, self.cur_word)  # caller ensures validity
     name = word_.ShFunctionName(cur_word)
+    if len(name) == 0:
+      p_die('Invalid function name', word=cur_word)
 
-    # If it passed ShFunctionName, this should be true.
+    # If we got a non-empty string from 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)
-
     self._Next()  # skip function name
 
     # Must be true because of lookahead
@@ -1713,8 +1720,10 @@ def ParseOilProc(self):
     keyword_tok = _KeywordToken(self.cur_word)
     with ctx_Declarations(self.declarations, keyword_tok):
       self.w_parser.ParseProc(node)
-
-      # TODO: self.declarations.Register(params ...)
+      if node.sig.tag_() == proc_sig_e.Closed:  # Register params
+        sig = cast(proc_sig__Closed, node.sig)
+        for param in sig.params:
+          self.declarations.Register(param.name)
 
       self._Next()
       node.body = self.ParseBraceGroup()

osh/word_parse.py

@@ -64,6 +64,7 @@
     word_part__ArithSub, word_part__ExtGlob, word_part__ExprSub,
 
     command, command_t, command__ForExpr, command__Proc, command__Import,
+    command__VarDecl,
 
     expr_t, source, arg_list,
 )
@@ -902,7 +903,7 @@ def _ReadExprSub(self, lex_mode):
     return node
 
   def ParseVarDecl(self, kw_token):
-    # type: (Token) -> command_t
+    # type: (Token) -> command__VarDecl
     """
     oil_var_decl: name_type_list '=' testlist end_stmt
 

spec/oil-assign.test.sh

@@ -252,7 +252,7 @@ f() {
 }
 f
 var x = "redeclaration is an error"
-## status: 1
+## status: 2
 ## STDOUT:
 x=local
 ## END

spec/oil-user-feedback.test.sh

@@ -80,8 +80,9 @@ EOF
 
 # With bash-style readarray.  The -t is annoying.
 git-branch-merged | while read --line {
-  # BUG: var or const messes up in al oop.
-  setvar line = _line.strip()  # removing leading space
+  # Note: this can't be 'const' because const is dynamic like 'readonly'.  And
+  # we don't have block scope.
+  var line = _line.strip()  # removing leading space
   if (line != 'master' and not line.startswith('*')) {
     echo $line
   }
@@ -96,10 +97,9 @@ if (len(branches) == 0) {
 # With "push".  Hm read --lines isn't bad.
 var branches2 = %()
 git-branch-merged | while read --line {
-  # TODO: Should be 'const' once we fix it?  Or 'var'?
-  setvar line = _line.strip()  # removing leading space
-  if (line != 'master' and not line.startswith('*')) {
-    push :branches2 $line
+  var line2 = _line.strip()  # removing leading space
+  if (line2 != 'master' and not line2.startswith('*')) {
+    push :branches2 $line2
   }
 }
 

test/parse-errors.sh

@@ -857,6 +857,45 @@ oil_nested_proc() {
   _should-parse 'f() { echo 1; g() { echo g; }; echo 2; }'
 }
 
+oil_var_decl() {
+  set +o errexit
+
+  _oil-parse-error '
+  proc p(x) {
+    echo hi
+    var x = 2
+  }
+  '
+
+  _oil-parse-error '
+  proc p {
+    var x = 1
+    echo hi
+    var x = 2
+  }
+  '
+
+  _oil-parse-error '
+  proc p {
+    var x = 1
+    echo hi
+    const x = 2
+  }
+  '
+
+  _should-parse '
+  var x = 1
+  proc p {
+    echo hi
+    var x = 2
+  }
+
+  proc p2 {
+    var x = 3
+  }
+  '
+}
+
 cases-in-strings() {
   set +o errexit
 
@@ -904,6 +943,7 @@ cases-in-strings() {
   parse_backslash
   oil_to_make_nicer
   oil_nested_proc
+  oil_var_decl
   parse_at
 }