Repository: neogeny/TatSu

Issue:
all names in the successful option should be present in the  AST
related to #199

Given the grammar below, users expect if the first option in the choice for `expr_range` parses, then both `from` and `to` will be present in the resulting AST, and that is not the current behavior in TatSu.  Some matching strings are "2 .. 9", "2 ..", ".. 9", and "..", and TatSu will only include the names that matched, so the AST for ".." will be `".."`, which is incorrect.

```ebnf
start = expr_range ;

expr_range =
    | [from: expr] '..' [to: expr]
    | expr
    ;

expr =
    /[\d]+/
;
```

cc @Victorious3 

Discussion:

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 1ffced3..c39225e 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -14,8 +14,11 @@ The format of this *Change Log* is inspired by `keeapachangelog.org`_.
 ---------------
 .. _`X.Y.Z`: https://github.com/apalala/tatsu/compare/v5.6.1...master
 
+*   Drop support for Python3.8 (security and source-code only releases since 2021/0503). Code in TatSu using new language features may not compile under that version of Python.
 *   Parser configuration variables were moved into a ``config: ParserConfig`` attribute.This may impact code that accessed those variables directly. Now instead of ``model.configuration_var`` you should use ``model.config.configuration_var``
 *   Now ``config: ParserConfig`` is used in ``__init__()`` and ``parse()`` methods of ``contexts.ParseContext``, ``grammars.Grammar``, and elsewhere to avoid the very long parameter lists that abounded. ``ParseContext`` also provides clean and clear ways of overridinga group of settings with another.
+*   All names defined in the successful choice in a rule are now defined in the resulting ``AST``. Names within optionals or closures that did not match will have their values set to ``None``.
+*   Moved build configuration from ``setup.py`` in favor of ``setup.cfg``  and ``pyproject.toml`` (`@KOLANICH`_)
 
 
 `5.6.1`_ @ 2021-03-22
@@ -358,6 +361,7 @@ Added
 .. _@gegenschall: https://bitbucket.org/gegenschall
 .. _@hariedo: https://github.com/hariedo
 .. _@heronils: https://github.com/heronils
+.. _@KOLANICH: https://github.com/KOLANICH
 .. _@manueljacob: https://github.com/manueljacob
 .. _@mjdominus: https://github.com/mjdominus
 .. _@paulhoule: https://github.com/paulhoule
diff --git a/grammar/tatsu.ebnf b/grammar/tatsu.ebnf
index 0954b2d..d26fe99 100644
--- a/grammar/tatsu.ebnf
+++ b/grammar/tatsu.ebnf
@@ -139,10 +139,17 @@ expre
 
 choice::Choice
     =
-    ['|' ~] @+:sequence {'|' ~ @+:sequence}+
+    ['|' ~] @+:option {'|' ~ @+:option}+
     ;
 
 
+option::Option
+    =
+    @:sequence
+    ;
+
+
+
 sequence::Sequence
     =
     sequence:{element}+
diff --git a/tatsu/bootstrap.py b/tatsu/bootstrap.py
index a13de4d..05deb96 100644
--- a/tatsu/bootstrap.py
+++ b/tatsu/bootstrap.py
@@ -9,7 +9,6 @@
 # Any changes you make to it will be overwritten the next time
 # the file is generated.
 
-
 from __future__ import annotations
 
 import sys
@@ -34,7 +33,7 @@ class EBNFBootstrapBuffer(Buffer):
         nameguard=None,
         comments_re='\\(\\*((?:.|\\n)*?)\\*\\)',
         eol_comments_re='#([^\\n]*?)$',
-        ignorecase=None,
+        ignorecase=False,
         namechars='',
         **kwargs
     ):
@@ -57,7 +56,7 @@ class EBNFBootstrapParser(Parser):
         nameguard=None,
         comments_re='\\(\\*((?:.|\\n)*?)\\*\\)',
         eol_comments_re='#([^\\n]*?)$',
-        ignorecase=None,
+        ignorecase=False,
         left_recursion=False,
         parseinfo=True,
         keywords=None,
@@ -95,9 +94,17 @@ class EBNFBootstrapParser(Parser):
                 with self._option():
                     self._directive_()
                     self.add_last_node_to_name('directives')
+                    self._define(
+                        [],
+                        ['directives']
+                    )
                 with self._option():
                     self._keyword_()
                     self.add_last_node_to_name('keywords')
+                    self._define(
+                        [],
+                        ['keywords']
+                    )
                 self._error(
                     'expecting one of: '
                     '<directive> <keyword>'
@@ -111,9 +118,17 @@ class EBNFBootstrapParser(Parser):
                 with self._option():
                     self._rule_()
                     self.add_last_node_to_name('rules')
+                    self._define(
+                        [],
+                        ['rules']
+                    )
                 with self._option():
                     self._keyword_()
                     self.add_last_node_to_name('keywords')
+                    self._define(
+                        [],
+                        ['keywords']
+                    )
                 self._error(
                     'expecting one of: '
                     '<rule> <keyword>'
@@ -151,6 +166,10 @@ class EBNFBootstrapParser(Parser):
                     self._cut()
                     self._regex_()
                     self.name_last_node('value')
+                    self._define(
+                        ['name', 'value'],
+                        []
+                    )
                 with self._option():
                     with self._group():
                         self._token('whitespace')
@@ -174,6 +193,10 @@ class EBNFBootstrapParser(Parser):
                                 "<regex> 'None' 'False'"
                             )
                     self.name_last_node('value')
+                    self._define(
+                        ['name', 'value'],
+                        []
+                    )
                 with self._option():
                     with self._group():
                         with self._choice():
@@ -199,13 +222,25 @@ class EBNFBootstrapParser(Parser):
                                 self._cut()
                                 self._boolean_()
                                 self.name_last_node('value')
+                                self._define(
+                                    ['value'],
+                                    []
+                                )
                             with self._option():
                                 self._constant(True)
                                 self.name_last_node('value')
+                                self._define(
+                                    ['value'],
+                                    []
+                                )
                             self._error(
                                 'expecting one of: '
                                 "'::'"
                             )
+                    self._define(
+                        ['name', 'value'],
+                        []
+                    )
                 with self._option():
                     with self._group():
                         self._token('grammar')
@@ -215,6 +250,10 @@ class EBNFBootstrapParser(Parser):
                     self._cut()
                     self._word_()
                     self.name_last_node('value')
+                    self._define(
+                        ['name', 'value'],
+                        []
+                    )
                 with self._option():
                     with self._group():
                         self._token('namechars')
@@ -224,6 +263,10 @@ class EBNFBootstrapParser(Parser):
                     self._cut()
                     self._string_()
                     self.name_last_node('value')
+                    self._define(
+                        ['name', 'value'],
+                        []
+                    )
                 self._error(
                     'expecting one of: '
                     "'comments' 'eol_comments' 'whitespace'"
@@ -275,6 +318,10 @@ class EBNFBootstrapParser(Parser):
                 self._cut()
                 self._params_()
                 self.name_last_node('params')
+                self._define(
+                    ['params'],
+                    []
+                )
             with self._option():
                 self._token('(')
                 self._cut()
@@ -283,6 +330,10 @@ class EBNFBootstrapParser(Parser):
                         with self._option():
                             self._kwparams_()
                             self.name_last_node('kwparams')
+                            self._define(
+                                ['kwparams'],
+                                []
+                            )
                         with self._option():
                             self._params_()
                             self.name_last_node('params')
@@ -290,14 +341,26 @@ class EBNFBootstrapParser(Parser):
                             self._cut()
                             self._kwparams_()
                             self.name_last_node('kwparams')
+                            self._define(
+                                ['kwparams', 'params'],
+                                []
+                            )
                         with self._option():
                             self._params_()
                             self.name_last_node('params')
+                            self._define(
+                                ['params'],
+                                []
+                            )
                         self._error(
                             'expecting one of: '
                             '<kwparams> <params>'
                         )
                 self._token(')')
+                self._define(
+                    ['kwparams', 'params'],
+                    []
+                )
             self._error(
                 'expecting one of: '
                 "'::' '('"
@@ -324,6 +387,10 @@ class EBNFBootstrapParser(Parser):
                     self._cut()
                     self._params_()
                     self.name_last_node('params')
+                    self._define(
+                        ['params'],
+                        []
+                    )
                 with self._option():
                     self._token('(')
                     self._cut()
@@ -332,6 +399,10 @@ class EBNFBootstrapParser(Parser):
                             with self._option():
                                 self._kwparams_()
                                 self.name_last_node('kwparams')
+                                self._define(
+                                    ['kwparams'],
+                                    []
+                                )
                             with self._option():
                                 self._params_()
                                 self.name_last_node('params')
@@ -339,14 +410,26 @@ class EBNFBootstrapParser(Parser):
                                 self._cut()
                                 self._kwparams_()
                                 self.name_last_node('kwparams')
+                                self._define(
+                                    ['kwparams', 'params'],
+                                    []
+                                )
                             with self._option():
                                 self._params_()
                                 self.name_last_node('params')
+                                self._define(
+                                    ['params'],
+                                    []
+                                )
                             self._error(
                                 'expecting one of: '
                                 '<kwparams> <params>'
                             )
                     self._token(')')
+                    self._define(
+                        ['kwparams', 'params'],
+                        []
+                    )
                 self._error(
                     'expecting one of: '
                     "'::' '('"
@@ -443,7 +526,8 @@ class EBNFBootstrapParser(Parser):
                 self._sequence_()
             self._error(
                 'expecting one of: '
-                "<sequence> '|' <choice> <element>"
+                "<option> '|' <choice> <element>"
+                '<sequence>'
             )
 
     @tatsumasu('Choice')
@@ -451,16 +535,21 @@ class EBNFBootstrapParser(Parser):
         with self._optional():
             self._token('|')
             self._cut()
-        self._sequence_()
+        self._option_()
         self.add_last_node_to_name('@')
 
         def block1():
             self._token('|')
             self._cut()
-            self._sequence_()
+            self._option_()
             self.add_last_node_to_name('@')
         self._positive_closure(block1)
 
+    @tatsumasu('Option')
+    def _option_(self):  # noqa
+        self._sequence_()
+        self.name_last_node('@')
+
     @tatsumasu('Sequence')
     def _sequence_(self):  # noqa
 
@@ -994,9 +1083,9 @@ class EBNFBootstrapParser(Parser):
                 'expecting one of: '
                 "<STRING> <string> 'r' <raw_string>"
                 "'True' 'False' <boolean> (?!\\d)\\w+"
-                '<word> 0[xX](\\d|[a-fA-F])+ <hex> [-+]?(?'
-                ':\\d+\\.\\d*|\\d*\\.\\d+)(?:[Ee][-+]?\\d+)?'
-                '<float> [-+]?\\d+ <int>'
+                '<word> 0[xX](\\d|[a-fA-F])+ <hex> [-'
+                '+]?(?:\\d+\\.\\d*|\\d*\\.\\d+)(?:[Ee][-'
+                '+]?\\d+)? <float> [-+]?\\d+ <int>'
             )
 
     @tatsumasu()
@@ -1156,6 +1245,9 @@ class EBNFBootstrapSemantics(object):
     def choice(self, ast):  # noqa
         return ast
 
+    def option(self, ast):  # noqa
+        return ast
+
     def sequence(self, ast):  # noqa
         return ast
 
@@ -1325,7 +1417,12 @@ def main(filename, start=None, **kwargs):
         with open(filename) as f:
             text = f.read()
     parser = EBNFBootstrapParser()
-    return parser.parse(text, start=start, filename=filename, **kwargs)
+    return parser.parse(
+        text,
+        rule_name=start,
+        filename=filename,
+        **kwargs
+    )
 
 
 if __name__ == '__main__':
@@ -1333,9 +1430,5 @@ if __name__ == '__main__':
     from tatsu.util import asjson
 
     ast = generic_main(main, EBNFBootstrapParser, name='EBNFBootstrap')
-    print('AST:')
-    print(ast)
-    print()
-    print('JSON:')
-    print(json.dumps(asjson(ast), indent=2))
-    print()
+    data = asjson(ast)
+    print(json.dumps(data, indent=2))
diff --git a/tatsu/codegen/python.py b/tatsu/codegen/python.py
index eae998a..0cb38b9 100644
--- a/tatsu/codegen/python.py
+++ b/tatsu/codegen/python.py
@@ -13,6 +13,7 @@ from tatsu.util import (
     compress_seq,
     RETYPE
 )
+from tatsu import grammars
 from tatsu.exceptions import CodegenError
 from tatsu.objectmodel import Node
 from tatsu.objectmodel import BASE_CLASS_TOKEN
@@ -39,6 +40,31 @@ class Base(ModelRenderer):
     def defines(self):
         return self.node.defines()
 
+    def make_defines_declaration(self):
+        defines = compress_seq(self.defines())
+        ldefs = set(safe_name(d) for d, l in defines if l)
+        sdefs = set(safe_name(d) for d, l in defines if not l and d not in ldefs)
+
+        if not (sdefs or ldefs):
+            return ''
+        else:
+            sdefs = '[%s]' % ', '.join(repr(d) for d in sorted(sdefs))
+            ldefs = '[%s]' % ', '.join(repr(d) for d in sorted(ldefs))
+            if not ldefs:
+                return '\n\n    self._define(%s, %s)' % (sdefs, ldefs)
+            else:
+                return indent(
+                    '\n' +
+                    trim(self.define_template % (sdefs, ldefs))
+                )
+
+    define_template = '''\
+            self._define(
+                %s,
+                %s
+            )\
+        '''
+
 
 class Void(Base):
     template = 'self._void()'
@@ -126,12 +152,6 @@ class Sequence(Base):
 
 class Choice(Base):
     def render_fields(self, fields):
-        template = trim(self.option_template)
-        options = [
-            template.format(
-                option=indent(self.rend(o))) for o in self.node.options
-        ]
-        options = '\n'.join(o for o in options)
         firstset = self.node.lookahead_str()
         if firstset:
             msglines = textwrap.wrap(firstset, width=40)
@@ -139,10 +159,11 @@ class Choice(Base):
         else:
             error = ['no available options']
         error = [repr(e) for e in error]
-        fields.update(n=self.counter(),
-                      options=indent(options),
-                      error=error,
-                      )
+        fields.update(
+            n=self.counter(),
+            error=error,
+        )
+        fields.update(options='\n'.join(self.rend(o) for o in self.node.options))
 
     def render(self, **fields):
         if len(self.node.options) == 1:
@@ -150,20 +171,27 @@ class Choice(Base):
         else:
             return super().render(**fields)
 
-    option_template = '''\
-                    with self._option():
-                    {option}\
-                    '''
-
     template = '''\
                 with self._choice():
-                {options}
+                {options:1::}
                     self._error(
                 {error:2:\\n:}
                     )\
                 '''
 
 
+class Option(_Decorator):
+    def render_fields(self, fields):
+        defines = self.make_defines_declaration()
+        fields.update(defines=defines)
+
+    template = '''\
+                with self._option():
+                {exp:1::}\
+                {defines}\
+                '''
+
+
 class Closure(_Decorator):
     def render_fields(self, fields):
         fields.update(n=self.counter())
@@ -371,23 +399,9 @@ class Rule(_Decorator):
 
         fields.update(params=params)
 
-        defines = compress_seq(self.defines())
-        ldefs = set(safe_name(d) for d, l in defines if l)
-        sdefs = set(safe_name(d) for d, l in defines if not l and d not in ldefs)
-
-        if not (sdefs or ldefs):
-            sdefines = ''
-        else:
-            sdefs = '[%s]' % ', '.join(repr(d) for d in sorted(sdefs))
-            ldefs = '[%s]' % ', '.join(repr(d) for d in sorted(ldefs))
-            if not ldefs:
-                sdefines = '\n\n    self._define(%s, %s)' % (sdefs, ldefs)
-            else:
-                sdefines = indent(
-                    '\n' +
-                    trim(self.define_template % (sdefs, ldefs))
-                )
-
+        sdefines = ''
+        if not isinstance(self.node, grammars.Choice):
+            sdefines = self.make_defines_declaration()
         fields.update(defines=sdefines)
         leftrec = self.node.is_leftrec
         fields.update(leftrec='\n@leftrec' if leftrec else '')
diff --git a/tatsu/g2e/semantics.py b/tatsu/g2e/semantics.py
index e00cba9..3e47e9b 100644
--- a/tatsu/g2e/semantics.py
+++ b/tatsu/g2e/semantics.py
@@ -54,6 +54,7 @@ class ANTLRSemantics(object):
         if len(options) == 1:
             return options[0]
         else:
+            options = [model.Option(o) for o in options]
             return model.Choice(options)
 
     def elements(self, ast):
diff --git a/tatsu/grammars.py b/tatsu/grammars.py
index 402986c..718aee0 100644
--- a/tatsu/grammars.py
+++ b/tatsu/grammars.py
@@ -130,6 +130,19 @@ class Model(Node):
     def defines(self):
         return []
 
+    def _add_defined_attributes(self, ctx, ast=None):
+        if ast is None:
+            return
+        if not hasattr(ast, '_define'):
+            return
+
+        defines = dict(compress_seq(self.defines()))
+
+        keys = [k for k, list in defines.items() if not list]
+        list_keys = [k for k, list in defines.items() if list]
+        ctx._define(keys, list_keys)
+        ast._define(keys, list_keys)
+
     def lookahead(self, k=1):
         if self._lookahead is None:
             self._lookahead = kdot(self.firstset(k), self.followset(k), k)
@@ -246,7 +259,7 @@ class EOF(Model):
 class Decorator(Model):
     def __init__(self, ast=None, exp=None, **kwargs):
         if exp is not None:
-            self.exp = exp
+            self.exp = ast = exp
         elif not isinstance(ast, AST):
             # Patch to avoid bad interactions with attribute setting in Model.
             # Also a shortcut for subexpressions that are not ASTs.
@@ -541,6 +554,13 @@ class Choice(Model):
         return self.options
 
 
+class Option(Decorator):
+    def parse(self, ctx):
+        result = super().parse(ctx)
+        self._add_defined_attributes(ctx, result)
+        return result
+
+
 class Closure(Decorator):
     def parse(self, ctx):
         return ctx._closure(lambda: self.exp.parse(ctx))
@@ -672,6 +692,7 @@ class EmptyClosure(Model):
 class Optional(Decorator):
     def parse(self, ctx):
         ctx.last_node = None
+        self._add_defined_attributes(ctx, ctx.ast)
         with ctx._optional():
             return self.exp.parse(ctx)
 
@@ -841,7 +862,6 @@ class Rule(Decorator):
 
     def parse(self, ctx):
         result = self._parse_rhs(ctx, self.exp)
-        self._add_defined_attributes(result)
         return result
 
     def _parse_rhs(self, ctx, exp):
@@ -856,14 +876,6 @@ class Rule(Decorator):
         result = ctx._call(ruleinfo)
         return result
 
-    def _add_defined_attributes(self, ast):
-        defines = compress_seq(self.defines())
-        if not isinstance(ast, (AST, Node)):
-            return
-        for d, l in defines:
-            if not hasattr(ast, d):
-                setattr(ast, d, [] if l else None)
-
     # def firstset(self, k=1):
     #     return self.exp.firstset(k=k)
 


Test patch:
diff --git a/test/grammar/defines_test.py b/test/grammar/defines_test.py
new file mode 100644
index 0000000..f0be7fb
--- /dev/null
+++ b/test/grammar/defines_test.py
@@ -0,0 +1,84 @@
+from __future__ import annotations
+
+import pytest  # noqa
+
+from tatsu.tool import compile, gencode
+
+
+def test_name_in_option():
+    grammar = '''
+        start = expr_range ;
+
+        expr_range =
+            | [from: expr] '..' [to: expr]
+            | expr
+            ;
+
+        expr =
+            /[\d]+/
+        ;
+    '''
+
+    model = compile(grammar)
+
+    ast = model.parse('1 .. 10')
+    assert ast == {'from': '1', 'to': '10'}
+
+    ast = model.parse('10')
+    assert ast == '10'
+
+    ast = model.parse(' .. 10')
+    assert ast == {'from': None, 'to': '10'}
+
+    ast = model.parse('1 .. ')
+    assert ast == {'from': '1', 'to': None}
+
+    ast = model.parse(' .. ')
+    assert ast == {'from': None, 'to': None}
+
+    code = gencode(grammar=grammar)
+    print(code)
+
+
+def test_by_option():
+    grammar = '''
+        start = expr_range ;
+
+        expr_range =
+            | [from: expr] '..' [to: expr]
+            | left:expr ','  [right:expr]
+            ;
+
+        expr =
+            /[\d]+/
+        ;
+    '''
+
+    model = compile(grammar)
+
+    ast = model.parse(' .. 10')
+    assert ast == {'from': None, 'to': '10'}
+
+    ast = model.parse('1, 2')
+    assert ast == {'left': '1', 'right': '2'}
+
+    ast = model.parse('1, ')
+    assert ast == {'left': '1', 'right': None}
+
+
+def test_inner_options():
+    grammar = '''
+        start = switch;
+        switch = 'switch' [(on:'on'|off:'off')] ;
+    '''
+
+    model = compile(grammar)
+
+    ast = model.parse('switch on')
+    assert ast == {'on': 'on', 'off': None}
+
+    ast = model.parse('switch off')
+    assert ast == {'off': 'off', 'on': None}
+
+    ast = model.parse('switch')
+    assert ast == {'off': None, 'on': None}