pddg / uroboros

Simple framework for building scalable CLI tool.
Apache License 2.0
14 stars 1 forks source link

Operations on `options` in the inheritance destination class also affect the inheritance source. #36

Closed pddg closed 5 years ago

pddg commented 5 years ago

Sample code is as follows.

class Opt(uroboros.Option):
    def build_option(self, parser):
        parser.add_argument("--test", type=str)
        return parser

class RootCommand(uroboros.Command):
    name = 'root'
    def run(self, args):
        self.print_help()
        return 0

class BaseCommand(uroboros.Command):
    options = []
    def run(self, args):
        print(args.test)
        return 0

class FirstCommand(BaseCommand):
    name = 'first'
    def get_options(self):
        options = super(FirstCommand, self).get_options()
        options.append(Opt())
        return options

class SecondCommand(BaseCommand):
    name = 'second'

root = RootCommand()
root.add_command(
    FirstCommand(),
    SecondCommand(),
)
root.execute()

Expected

$ pipenv run python sample.py -h
usage: root [-h] {first,second} ...

optional arguments:
  -h, --help      show this help message and exit

Sub commands:
  {first,second}
    first
    second
$pipenv run python sample.py first -h
usage: root first [-h] [--test TEST]

optional arguments:
  -h, --help   show this help message and exit
  --test TEST
$pipenv run python sample.py second -h
usage: root second [-h]

optional arguments:
  -h, --help  show this help message and exit

Actual

$ pipenv run python sample.py -h
Traceback (most recent call last):
  File "/home/pudding/.local/share/virtualenvs/swat-analyzer-EWaBsfe_/lib/python3.7/site-packages/uroboros/command.py", line 57, in execute
    self._check_initialized()
  File "/home/pudding/.local/share/virtualenvs/swat-analyzer-EWaBsfe_/lib/python3.7/site-packages/uroboros/command.py", line 322, in _check_initialized
    raise errors.CommandNotRegisteredError(self.name)
uroboros.errors.CommandNotRegisteredError: Command 'root' has not been registered yet.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "test.py", line 36, in <module>
    root.execute()
  File "/home/pudding/.local/share/virtualenvs/swat-analyzer-EWaBsfe_/lib/python3.7/site-packages/uroboros/command.py", line 59, in execute
    self.initialize()
  File "/home/pudding/.local/share/virtualenvs/swat-analyzer-EWaBsfe_/lib/python3.7/site-packages/uroboros/command.py", line 110, in initialize
    self.initialize_sub_parsers(self._parser)
  File "/home/pudding/.local/share/virtualenvs/swat-analyzer-EWaBsfe_/lib/python3.7/site-packages/uroboros/command.py", line 124, in initialize_sub_parsers
    parents=[o.get_parser() for o in cmd.get_options()],
  File "/home/pudding/.local/share/virtualenvs/swat-analyzer-EWaBsfe_/lib/python3.7/site-packages/uroboros/command.py", line 124, in <listcomp>
    parents=[o.get_parser() for o in cmd.get_options()],
  File "/home/s-kokuryo/.local/share/virtualenvs/swat-analyzer-EWaBsfe_/lib/python3.7/site-packages/uroboros/option.py", line 15, in get_parser
    return self.build_option(self.parser)
  File "test.py", line 5, in build_option
    parser.add_argument("--test", type=str)
  File "/home/pudding/.anyenv/envs/pyenv/versions/3.7.4/lib/python3.7/argparse.py", line 1367, in add_argument
    return self._add_action(action)
  File "/home/pudding/.anyenv/envs/pyenv/versions/3.7.4/lib/python3.7/argparse.py", line 1730, in _add_action
    self._optionals._add_action(action)
  File "/home/pudding/.anyenv/envs/pyenv/versions/3.7.4/lib/python3.7/argparse.py", line 1571, in _add_action
    action = super(_ArgumentGroup, self)._add_action(action)
  File "/home/pudding/.anyenv/envs/pyenv/versions/3.7.4/lib/python3.7/argparse.py", line 1381, in _add_action
    self._check_conflict(action)
  File "/home/pudding/.anyenv/envs/pyenv/versions/3.7.4/lib/python3.7/argparse.py", line 1520, in _check_conflict
    conflict_handler(action, confl_optionals)
  File "/home/pudding/.anyenv/envs/pyenv/versions/3.7.4/lib/python3.7/argparse.py", line 1529, in _handle_conflict_error
    raise ArgumentError(action, message % conflict_string)
argparse.ArgumentError: argument --test: conflicting option string: --test

Why this happened?

Python's list object is mutable. uroboros.Option's get_options() function return the pointer of options attribute itself. So the operation on it in the inheritance class also affect its source class.

How to fix

Change the implementation of uroboros.Command.get_options like permission mechanism of django-restframework's view class. uroboros.Command.options accepts both of uroboros.Option class and its instance for backward compatibility. Check the type of the attribute of uroboros.Command.options and instantiate the class if need, and generate new list and return.

pddg commented 5 years ago

Workaround for v0.2.1 or earlier.

Do not use append.

class FirstCommand(BaseCommand):
    name = 'first'
    def get_options(self):
        options = super(FirstCommand, self).get_options()
        return [Opt()] + options