[ysh breaking] Change proc p(@rest) { to proc p(...rest) {

This makes it more similar to upcoming pure functions. We won't distinguish as strongly between untyped and typed args. The main axis will be positional vs. named args. Also don't run the docformatter with yapf, because it tends to mangle comments. We can run it as a one-off.
oilshell · Jun 4, 2023 · 11f14e1 · 11f14e1
1 parent 3dc612a
commit 11f14e1
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 46 deletions.
diff --git a/doc/ysh-tour.md b/doc/ysh-tour.md
@@ -1134,7 +1134,7 @@ source $_this_dir/lib/util.oil  # defines 'log' helper
 
 const DEST = '/tmp'
 
-proc my-sync(@files) {
+proc my-sync(...files) {
   ### Sync files and show which ones
 
   cp --verbose @files $DEST

diff --git a/spec/ysh-proc.test.sh b/spec/ysh-proc.test.sh
@@ -100,7 +100,7 @@ status=42
 
 shopt -s oil:all
 
-proc f(@names) {
+proc f(...names) {
   write names: @names
 }
 # this gets called with 3 args then?
@@ -117,7 +117,7 @@ status=0
 #### varargs 2
 shopt -s oil:all
 
-proc f(first, @rest) {  # @ means "the rest of the arguments"
+proc f(first, ...rest) {  # @ means "the rest of the arguments"
   write --sep ' ' -- $first
   write --sep ' ' -- @rest        # @ means "splice this array"
 }
@@ -150,7 +150,7 @@ f a b
 # With varargs and block
 shopt --set parse_proc
 
-proc g(x, y, @rest, block Block) {
+proc g(x, y, ...rest, block Block) {
   echo G
 }
 g a b c d

diff --git a/test/lint.sh b/test/lint.sh
@@ -213,7 +213,13 @@ run-yapf-2() {
   # These files originally had 4 space indentation, but it got inconsistent
   time py2-files-to-format \
     | xargs --verbose -- python3 -m yapf -i --style='{based_on_style: google: indent_width: 4}'
+}
 
+run-docformatter() {
+  ### Format docstrings
+  # Only done as a ONE OFF to indent docstrings after yapf-2
+  # Because it tends to mangle comments, e.g. grammar comments in
+  # ysh/expr_to_ast.py
   time py2-files-to-format \
     | xargs --verbose -- python3 -m docformatter --in-place
 }

diff --git a/ysh/expr_to_ast.py b/ysh/expr_to_ast.py
@@ -230,8 +230,9 @@ def _DictPair(self, p_node):
 
     def _Dict(self, p_node):
         # type: (PNode) -> expr.Dict
-        """Dict: dict_pair (',' dict_pair)* [',']
-
+        """Parse tree to LST
+        
+        dict: dict_pair (',' dict_pair)* [',']
         dict2: dict_pair (comma_newline dict_pair)* [comma_newline]
         """
         if not ISNONTERMINAL(p_node.typ):
@@ -274,8 +275,11 @@ def _Tuple(self, parent):
 
     def _TestlistComp(self, p_node, id0):
         # type: (PNode, Id_t) -> expr_t
-        """testlist_comp: (test|star_expr) ( comp_for | (',' (test|star_expr))*
-        [','] )"""
+        """Parse tree to LST
+        
+        testlist_comp:
+          (test|star_expr) ( comp_for | (',' (test|star_expr))* [','] )
+        """
         assert p_node.typ == grammar_nt.testlist_comp
         n = p_node.NumChildren()
         if n > 1 and p_node.GetChild(1).typ == grammar_nt.comp_for:
@@ -309,8 +313,7 @@ def _TestlistComp(self, p_node, id0):
 
     def _Atom(self, parent):
         # type: (PNode) -> expr_t
-        """Handles alternatives of 'atom' where there is more than one
-        child."""
+        """Handle alternatives of 'atom' where there's more than one child."""
 
         tok = parent.GetChild(0).tok
         id_ = tok.id
@@ -391,7 +394,7 @@ def _CompFor(self, p_node):
 
     def _CompareChain(self, parent):
         # type: (PNode) -> expr_t
-        """Comparison: expr (comp_op expr)*"""
+        """comparison: expr (comp_op expr)*"""
         cmp_ops = []  # type: List[Token]
         comparators = []  # type: List[expr_t]
         left = self.Expr(parent.GetChild(0))
@@ -421,7 +424,7 @@ def _CompareChain(self, parent):
 
     def _Subscript(self, parent):
         # type: (PNode) -> expr_t
-        """Subscript: expr | [expr] ':' [expr]"""
+        """subscript: expr | [expr] ':' [expr]"""
         typ0 = parent.GetChild(0).typ
 
         n = parent.NumChildren()
@@ -708,8 +711,10 @@ def MakeVarDecl(self, p_node):
 
     def MakePlaceMutation(self, p_node):
         # type: (PNode) -> command.PlaceMutation
-        """oil_place_mutation: place_list (augassign | '=') testlist
-        end_stmt."""
+        """Parse tree to LST
+        
+        oil_place_mutation: place_list (augassign | '=') testlist end_stmt
+        """
         typ = p_node.typ
         assert typ == grammar_nt.oil_place_mutation
 
@@ -762,9 +767,13 @@ def YshCasePattern(self, pnode):
 
     def _Argument(self, p_node, do_named, arglist):
         # type: (PNode, bool, ArgList) -> None
-        """Argument: ( test [comp_for] # named arg | test '=' test # var args.
+        """Parse tree to LST
 
-        | '...' test )
+        argument: (
+          test [comp_for]
+        | test '=' test  # named arg
+        | '...' test  # var args
+        )
         """
         positional = arglist.positional
         named = arglist.named
@@ -807,8 +816,12 @@ def _Argument(self, p_node, do_named, arglist):
 
     def _Arglist(self, parent, arglist):
         # type: (PNode, ArgList) -> None
-        """Arglist: argument (',' argument)* [','] [';' argument (','
-        argument)* [',']]"""
+        """Parse tree to LST
+
+        arglist:
+               argument (',' argument)* [',']
+          [';' argument (',' argument)* [','] ]
+        """
         do_named = False
         for i in xrange(parent.NumChildren()):
             p_child = parent.GetChild(i)
@@ -844,10 +857,14 @@ def _TypeExprList(self, pnode):
 
     def _ProcParam(self, pnode):
         # type: (PNode) -> Tuple[Optional[Token], Token, Optional[Token], Optional[expr_t]]
-        """proc_param: ( ':' Expr_Name | '@' Expr_Name |
+        """
+        proc_param:
+          # old
+          ':' Expr_Name |
 
-        Expr_Name [ Expr_Name ] ['=' expr]  # type is either Expr or
-        Block )
+          '...' Expr_Name
+          # type is either Expr or Block 
+          Expr_Name [ Expr_Name ] ['=' expr]  
         """
         assert pnode.typ == grammar_nt.proc_param
 
@@ -858,7 +875,7 @@ def _ProcParam(self, pnode):
         name = None  # type: Optional[Token]
         typ = None  # type: Optional[Token]
         default_val = None  # type: Optional[expr_t]
-        if tok0.id in (Id.Arith_Colon, Id.Expr_At):
+        if tok0.id in (Id.Arith_Colon, Id.Expr_At, Id.Expr_Ellipsis):
             prefix = tok0
             name = pnode.GetChild(1).tok
             return prefix, name, typ, default_val
@@ -895,7 +912,7 @@ def _ProcParams(self, p_node):
 
             # TODO: enforce the order of params.  It should look like
             # untyped* rest? typed*, which is while / if / while!
-            if prefix and prefix.id == Id.Expr_At:
+            if prefix and prefix.id in (Id.Expr_At, Id.Expr_Ellipsis):
                 rest = name
             elif typ is None:
                 untyped.append(UntypedParam(prefix, name, default_val))
@@ -966,17 +983,26 @@ def Proc(self, pnode):
 
     def func_item(self, node):
         # type: (PNode) -> command_t
-        """func_item: ( ('var' | 'const') name_type_list '=' testlist  #
-        oil_var_decl.
-
-        # TODO: for, if/switch, with, break/continue/return,
-        try/throw, etc. | 'while' test suite | 'for' name_type_list 'in'
-        test suite | flow_stmt | 'set' place_list (augassign | '=')
-        testlist  # oil_place_mutation   # x  f(x)  etc.   #   # And x =
-        1.  Python uses the same "hack" to fit within pgen2.  It also
-        # supports a = b = 1, which we don't want.   #   # And echo 'hi'
-        'there'   #   # TODO: expr_to_ast needs to validate this |
-        testlist (['=' testlist] | tea_word*) )
+        """Parse tree to LST
+
+        func_item: (
+          ('var' | 'const') name_type_list '=' testlist  # oil_var_decl.
+
+          # TODO: for, if/switch, with, break/continue/return, try/throw, etc.
+        | 'while' test suite
+        | 'for' name_type_list 'in' test suite
+        | flow_stmt
+        | 'set' place_list (augassign | '=') testlist  # oil_place_mutation   
+          # x  f(x)  etc.
+          #
+          # And x = 1.  Python uses the same "hack" to fit within pgen2.  It
+          # also supports a = b = 1, which we don't want.
+          #
+          # And echo 'hi' 'there'   
+          #
+          # TODO: expr_to_ast needs to validate this
+        | testlist (['=' testlist] | tea_word*)
+        )
         """
         if node.tok.id == Id.Expr_While:
             return command.While(self.Expr(node.GetChild(1)),
@@ -1016,7 +1042,10 @@ def func_items(self, pnode):
 
     def _Suite(self, pnode):
         # type: (PNode) -> command.CommandList
-        """Suite: '{' [Op_Newline] [func_items] '}'."""
+        """Parse tree to LST
+
+        suite: '{' [Op_Newline] [func_items] '}'
+        """
         n = pnode.NumChildren()
 
         if n == 2:  # {}
@@ -1035,9 +1064,10 @@ def _Suite(self, pnode):
 
     def TeaFunc(self, pnode, out):
         # type: (PNode, command.Func) -> None
-        """tea_func: ( '(' [func_params] [';' func_params] ')' [type_expr_list]
+        """Parse tree to LST
 
-        suite )
+        tea_func:
+          '(' [func_params] [';' func_params] ')' [type_expr_list] suite 
         """
         assert pnode.typ == grammar_nt.tea_func
         assert pnode.GetChild(0).tok.id == Id.Op_LParen  # proc foo(
@@ -1118,8 +1148,13 @@ def _Variant(self, pnode):
 
     def Enum(self, pnode, out):
         # type: (PNode, command.Enum) -> None
-        """tea_enum: Expr_Name '{' [Op_Newline] (variant variant_end)* [
-        variant [variant_end] ] '}'."""
+        """Parse tree to LST
+
+        tea_enum:
+          Expr_Name '{' [Op_Newline]
+          (variant variant_end)* [ variant [variant_end] ]
+          '}'
+        """
         assert pnode.typ == grammar_nt.tea_enum
 
         out.name = pnode.GetChild(0).tok
@@ -1151,9 +1186,11 @@ def Class(self, pnode, out):
 
     def Import(self, pnode, out):
         # type: (PNode, command.Import) -> None
-        """tea_import: ( sq_string ['as' Expr_Name]
-
-        (import_name ',')* [ import_name [','] ] )
+        """Parse tree to LST
+        
+        tea_import: (
+          sq_string ['as' Expr_Name] (import_name ',')* [ import_name [','] ]
+        )
         """
         assert pnode.typ == grammar_nt.tea_import
 
@@ -1224,9 +1261,14 @@ def _NonRangeChars(self, p_node):
 
     def _ClassLiteralTerm(self, p_node):
         # type: (PNode) -> class_literal_term_t
-        """class_literal_term: ( range_char ['-' range_char ] | '~' Expr_Name #
-        $mychars or ${mymodule.mychars} | simple_var_sub | braced_var_sub #
-        e.g. 'abc' or "abc$mychars" | dq_string ..."""
+        """Parse tree to LST
+
+        class_literal_term:
+          range_char ['-' range_char ] 
+        | '@' Expr_Name  # splice
+        | '!' Expr_Name  # negate char class
+          ...
+        """
         assert p_node.typ == grammar_nt.class_literal_term, p_node
 
         first = p_node.GetChild(0)

diff --git a/ysh/grammar.pgen2 b/ysh/grammar.pgen2
@@ -361,8 +361,16 @@ oil_expr_sub: testlist ']'
 #
 # Note: might want typed splat ... later
 
+# TODO:
+# - change @rest to ...rest
+# - change :out to 'out Ref'
+# should be compatible
+
 proc_param: (
-    ':' Expr_Name | '@' Expr_Name |
+    # OLD SYNTAX
+    ':' Expr_Name | #'@' Expr_Name |
+
+    '...' Expr_Name |
     Expr_Name [Expr_Name] ['=' expr]  # type is either Expr or Block
 )