Fix keyword-only arguments (google#18)

Adds support for keyword-only arguments to Fire.

Author: jgarst

.travis.yml

@@ -8,4 +8,4 @@ before_install:
   - pip install --upgrade setuptools pip
 install:
   - python setup.py develop
-script: nosetests --ignore-files=parser_fuzz_test.py
+script: nosetests --ignore-files=parser_fuzz_test.py --exclude=test_components_py3.py

fire/completion.py

@@ -157,8 +157,8 @@ def Completions(component, verbose=False):
     A list of completions for a command that would so far return the component.
   """
   if inspect.isroutine(component) or inspect.isclass(component):
-    fn_args = inspectutils.GetArgSpec(component).args
-    return _CompletionsFromArgs(fn_args)
+    spec = inspectutils.GetFullArgSpec(component)
+    return _CompletionsFromArgs(spec.args + spec.kwonlyargs)
 
   elif isinstance(component, (tuple, list)):
     return [str(index) for index in range(len(component))]

fire/core.py

@@ -543,31 +543,38 @@ def _MakeParseFn(fn):
     can then be called with fn(*varargs, **kwargs). The remaining_args are
     the leftover args from the arguments to the parse function.
   """
-  fn_args, fn_varargs, fn_keywords, fn_defaults = inspectutils.GetArgSpec(fn)
+  fn_spec = inspectutils.GetFullArgSpec(fn)
+  all_args = fn_spec.args + fn_spec.kwonlyargs
   metadata = decorators.GetMetadata(fn)
 
-  # Note: num_required_args is the number of arguments without default values.
-  # All of these arguments are required.
-  num_required_args = len(fn_args) - len(fn_defaults)
+  # Note: num_required_args is the number of positional arguments without
+  # default values. All of these arguments are required.
+  num_required_args = len(fn_spec.args) - len(fn_spec.defaults)
+  required_kwonly = set(fn_spec.kwonlyargs) - set(fn_spec.kwonlydefaults)
 
   def _ParseFn(args):
     """Parses the list of `args` into (varargs, kwargs), remaining_args."""
-    kwargs, remaining_args = _ParseKeywordArgs(args, fn_args, fn_keywords)
+    kwargs, remaining_args = _ParseKeywordArgs(args, all_args, fn_spec.varkw)
 
     # Note: _ParseArgs modifies kwargs.
     parsed_args, kwargs, remaining_args, capacity = _ParseArgs(
-        fn_args, fn_defaults, num_required_args, kwargs, remaining_args,
+        fn_spec.args, fn_spec.defaults, num_required_args, kwargs, remaining_args,
         metadata)
 
-    if fn_varargs or fn_keywords:
+    if fn_spec.varargs or fn_spec.varkw:
       # If we're allowed *varargs or **kwargs, there's always capacity.
       capacity = True
 
-    if fn_keywords is None and kwargs:
-      raise FireError('Unexpected kwargs present', kwargs)
+    extra_kw = set(kwargs) - set(fn_spec.kwonlyargs)
+    if fn_spec.varkw is None and extra_kw:
+      raise FireError('Unexpected kwargs present:', extra_kw)
+
+    missing_kwonly = set(required_kwonly) - set(kwargs)
+    if missing_kwonly:
+      raise FireError('Missing required flags:', missing_kwonly)
 
     # If we accept *varargs, then use all remaining arguments for *varargs.
-    if fn_varargs is not None:
+    if fn_spec.varargs is not None:
       varargs, remaining_args = remaining_args, []
     else:
       varargs = []
@@ -577,7 +584,7 @@ def _ParseFn(args):
 
     varargs = parsed_args + varargs
 
-    consumed_args = args[:len(args)-len(remaining_args)]
+    consumed_args = args[:len(args) - len(remaining_args)]
     return (varargs, kwargs), consumed_args, remaining_args, capacity
 
   return _ParseFn

fire/fire_test.py

@@ -20,6 +20,7 @@
 from fire import test_components as tc
 from fire import trace
 
+import six
 import unittest
 
 
@@ -102,6 +103,13 @@ def testFireAnnotatedArgs(self):
     self.assertEqual(fire.Fire(tc.Annotations, 'double 5'), 10)
     self.assertEqual(fire.Fire(tc.Annotations, 'triple 5'), 15)
 
+  @unittest.skipIf(six.PY2, 'Keyword-only arguments not supported in Python 2')
+  def testFireKeywordOnlyArgs(self):
+    self.assertIsNone(fire.Fire(tc.py3.KeywordOnly, 'double 5'))
+
+    self.assertEqual(fire.Fire(tc.py3.KeywordOnly, 'double --count 5'), 10)
+    self.assertEqual(fire.Fire(tc.py3.KeywordOnly, 'triple --count 5'), 15)
+
   def testFireProperties(self):
     self.assertEqual(fire.Fire(tc.TypedProperties, 'alpha'), True)
     self.assertEqual(fire.Fire(tc.TypedProperties, 'beta'), (1, 2, 3))

fire/helputils.py

@@ -126,26 +126,23 @@ def HelpString(component, trace=None, verbose=False):
   return '\n'.join(lines)
 
 
-def _UsageStringFromFnDetails(command, args, varargs, keywords, defaults):
-  """Get a usage string from the function details for the given command.
+def _UsageStringFromFullArgSpec(command, spec):
+  """Get a usage string from the FullArgSpec for the given command.
 
   The strings look like:
   command --arg ARG [--opt OPT] [VAR ...] [--KWARGS ...]
 
   Args:
     command: The command leading up to the function.
-    args: The args accepted by the function.
-    varargs: If not None, a string naming the *varargs variable used by the fn.
-    keywords: If not None, a string naming the **kwargs varargs used by the fn.
-    defaults: The default values for args accepted by the function.
+    spec: a FullArgSpec object describing the function.
   Returns:
     The usage string for the function.
   """
-  num_required_args = len(args) - len(defaults)
+  num_required_args = len(spec.args) - len(spec.defaults)
 
   help_flags = []
   help_positional = []
-  for index, arg in enumerate(args):
+  for index, arg in enumerate(spec.args):
     flag = arg.replace('_', '-')
     if index < num_required_args:
       help_flags.append('--{flag} {value}'.format(flag=flag, value=arg.upper()))
@@ -155,13 +152,21 @@ def _UsageStringFromFnDetails(command, args, varargs, keywords, defaults):
           flag=flag, value=arg.upper()))
       help_positional.append('[{value}]'.format(value=arg.upper()))
 
-  if varargs:
-    help_flags.append('[{var} ...]'.format(var=varargs.upper()))
-    help_positional.append('[{var} ...]'.format(var=varargs.upper()))
+  if spec.varargs:
+    help_flags.append('[{var} ...]'.format(var=spec.varargs.upper()))
+    help_positional.append('[{var} ...]'.format(var=spec.varargs.upper()))
 
-  if keywords:
-    help_flags.append('[--{kwarg} ...]'.format(kwarg=keywords.upper()))
-    help_positional.append('[--{kwarg} ...]'.format(kwarg=keywords.upper()))
+  for arg in spec.kwonlyargs:
+    if arg in spec.kwonlydefaults:
+      arg_str = '[--{flag} {value}]'.format(flag=arg, value=arg.upper())
+    else:
+      arg_str = '--{flag} {value}'.format(flag=arg, value=arg.upper())
+    help_flags.append(arg_str)
+    help_positional.append(arg_str)
+
+  if spec.varkw:
+    help_flags.append('[--{kwarg} ...]'.format(kwarg=spec.varkw.upper()))
+    help_positional.append('[--{kwarg} ...]'.format(kwarg=spec.varkw.upper()))
 
   commands_flags = command + ' '.join(help_flags)
   commands_positional = command + ' '.join(help_positional)
@@ -178,8 +183,8 @@ def UsageString(component, trace=None, verbose=False):
   command = trace.GetCommand() + ' ' if trace else ''
 
   if inspect.isroutine(component) or inspect.isclass(component):
-    args, varargs, keywords, defaults = inspectutils.GetArgSpec(component)
-    return _UsageStringFromFnDetails(command, args, varargs, keywords, defaults)
+    spec = inspectutils.GetFullArgSpec(component)
+    return _UsageStringFromFullArgSpec(command, spec)
 
   elif isinstance(component, (list, tuple)):
     length = len(component)

fire/inspectutils.py

@@ -24,7 +24,32 @@
 import six
 
 
-def _GetArgSpecFnInfo(fn):
+class FullArgSpec(object):
+  """The arguments of a function, as in Python 3's inspect.FullArgSpec."""
+
+  def __init__(self, args=None, varargs=None, varkw=None, defaults=None,
+               kwonlyargs=None, kwonlydefaults=None, annotations=None):
+    """Constructs a FullArgSpec with each provided attribute, or the default.
+
+    Args:
+      args: A list of the argument names accepted by the function.
+      varargs: The name of the *varargs argument or None if there isn't one.
+      varkw: The name of the **kwargs argument or None if there isn't one.
+      defaults: A tuple of the defaults for the arguments that accept defaults.
+      kwonlyargs: A list of argument names that must be passed with a keyword.
+      kwonlydefaults: A dictionary of keyword only arguments and their defaults.
+      annotations: A dictionary of arguments and their annotated types.
+    """
+    self.args = args or []
+    self.varargs = varargs
+    self.varkw = varkw
+    self.defaults = defaults or ()
+    self.kwonlyargs = kwonlyargs or []
+    self.kwonlydefaults = kwonlydefaults or {}
+    self.annotations = annotations or {}
+
+
+def _GetArgSpecInfo(fn):
   """Gives information pertaining to computing the ArgSpec of fn.
 
   Determines if the first arg is supplied automatically when fn is called.
@@ -57,51 +82,35 @@ class with an __init__ method.
   return fn, skip_arg
 
 
-def GetArgSpec(fn):
-  """Returns information about the function signature.
+def GetFullArgSpec(fn):
+  """Returns a FullArgSpec describing the given callable."""
 
-  Args:
-    fn: The function to analyze.
-  Returns:
-    A named tuple of type inspect.ArgSpec with the following fields:
-      args: A list of the argument names accepted by the function.
-      varargs: The name of the *varargs argument or None if there isn't one.
-      keywords: The name of the **kwargs argument or None if there isn't one.
-      defaults: A tuple of the defaults for the arguments that accept defaults.
-  """
-  fn, skip_arg = _GetArgSpecFnInfo(fn)
+  fn, skip_arg = _GetArgSpecInfo(fn)
 
   try:
-
     if six.PY2:
-      argspec = inspect.getargspec(fn)
-      keywords = argspec.keywords
+      args, varargs, varkw, defaults = inspect.getargspec(fn)  # pylint: disable=deprecated-method
+      kwonlyargs = kwonlydefaults = None
+      annotations = getattr(fn, '__annotations__', None)
     else:
-      argspec = inspect.getfullargspec(fn)
-      keywords = argspec.varkw
-
-    args = argspec.args
-    defaults = argspec.defaults or ()
-    varargs = argspec.varargs
+      (args, varargs, varkw, defaults,
+       kwonlyargs, kwonlydefaults, annotations) = inspect.getfullargspec(fn)
 
   except TypeError:
-    args = []
-    defaults = ()
     # If we can't get the argspec, how do we know if the fn should take args?
     # 1. If it's a builtin, it can take args.
     # 2. If it's an implicit __init__ function (a 'slot wrapper'), take no args.
     # Are there other cases?
-    varargs = 'vars' if inspect.isbuiltin(fn) else None
-    keywords = 'kwargs' if inspect.isbuiltin(fn) else None
+    if inspect.isbuiltin(fn):
+      return FullArgSpec(varargs='vars', varkw='kwargs')
+    else:
+      return FullArgSpec()
 
-  if skip_arg:
-    args = args[1:]  # Remove self.
+  if skip_arg and args:
+    args.pop(0)  # Remove 'self' or 'cls' from the list of arguments.
 
-  return inspect.ArgSpec(
-      args=args,
-      varargs=varargs,
-      keywords=keywords,
-      defaults=defaults)
+  return FullArgSpec(args, varargs, varkw, defaults,
+                     kwonlyargs, kwonlydefaults, annotations)
 
 
 def Info(component):

fire/inspectutils_test.py

@@ -16,61 +16,76 @@
 from __future__ import division
 from __future__ import print_function
 
-from fire import inspectutils
-from fire import test_components as tc
+import unittest
 import six
 
-import unittest
+from fire import inspectutils
+from fire import test_components as tc
 
 
 class InspectUtilsTest(unittest.TestCase):
 
-  def testGetArgSpecReturnType(self):
-    # Asserts that the named tuple returned by GetArgSpec has the appropriate
-    # fields.
-    argspec = inspectutils.GetArgSpec(tc.identity)
-    args, varargs, keywords, defaults = argspec
-    self.assertEqual(argspec.args, args)
-    self.assertEqual(argspec.defaults, defaults)
-    self.assertEqual(argspec.varargs, varargs)
-    self.assertEqual(argspec.keywords, keywords)
-
-  def testGetArgSpec(self):
-    args, varargs, keywords, defaults = inspectutils.GetArgSpec(tc.identity)
-    self.assertEqual(args, ['arg1', 'arg2'])
-    self.assertEqual(defaults, (10,))
-    self.assertEqual(varargs, 'arg3')
-    self.assertEqual(keywords, 'arg4')
-
-  def testGetArgSpecBuiltin(self):
-    args, varargs, keywords, defaults = inspectutils.GetArgSpec('test'.upper)
-    self.assertEqual(args, [])
-    self.assertEqual(defaults, ())
-    self.assertEqual(varargs, 'vars')
-    self.assertEqual(keywords, 'kwargs')
-
-  def testGetArgSpecSlotWrapper(self):
-    args, varargs, keywords, defaults = inspectutils.GetArgSpec(tc.NoDefaults)
-    self.assertEqual(args, [])
-    self.assertEqual(defaults, ())
-    self.assertEqual(varargs, None)
-    self.assertEqual(keywords, None)
-
-  def testGetArgSpecClassNoInit(self):
-    args, varargs, keywords, defaults = inspectutils.GetArgSpec(
-        tc.OldStyleEmpty)
-    self.assertEqual(args, [])
-    self.assertEqual(defaults, ())
-    self.assertEqual(varargs, None)
-    self.assertEqual(keywords, None)
-
-  def testGetArgSpecMethod(self):
-    args, varargs, keywords, defaults = inspectutils.GetArgSpec(
-        tc.NoDefaults().double)
-    self.assertEqual(args, ['count'])
-    self.assertEqual(defaults, ())
-    self.assertEqual(varargs, None)
-    self.assertEqual(keywords, None)
+  def testGetFullArgSpec(self):
+    spec = inspectutils.GetFullArgSpec(tc.identity)
+    self.assertEqual(spec.args, ['arg1', 'arg2', 'arg3', 'arg4'])
+    self.assertEqual(spec.defaults, (10, 20))
+    self.assertEqual(spec.varargs, 'arg5')
+    self.assertEqual(spec.varkw, 'arg6')
+    self.assertEqual(spec.kwonlyargs, [])
+    self.assertEqual(spec.kwonlydefaults, {})
+    self.assertEqual(spec.annotations, {'arg2': int, 'arg4': int})
+
+  @unittest.skipIf(six.PY2, 'No keyword arguments in python 2')
+  def testGetFullArgSpecPy3(self):
+    spec = inspectutils.GetFullArgSpec(tc.py3.identity)
+    self.assertEqual(spec.args, ['arg1', 'arg2', 'arg3', 'arg4'])
+    self.assertEqual(spec.defaults, (10, 20))
+    self.assertEqual(spec.varargs, 'arg5')
+    self.assertEqual(spec.varkw, 'arg10')
+    self.assertEqual(spec.kwonlyargs, ['arg6', 'arg7', 'arg8', 'arg9'])
+    self.assertEqual(spec.kwonlydefaults, {'arg8': 30, 'arg9': 40})
+    self.assertEqual(spec.annotations,
+                     {'arg2': int, 'arg4': int, 'arg7': int, 'arg9': int})
+
+  def testGetFullArgSpecFromBuiltin(self):
+    spec = inspectutils.GetFullArgSpec('test'.upper)
+    self.assertEqual(spec.args, [])
+    self.assertEqual(spec.defaults, ())
+    self.assertEqual(spec.varargs, 'vars')
+    self.assertEqual(spec.varkw, 'kwargs')
+    self.assertEqual(spec.kwonlyargs, [])
+    self.assertEqual(spec.kwonlydefaults, {})
+    self.assertEqual(spec.annotations, {})
+
+  def testGetFullArgSpecFromSlotWrapper(self):
+    spec = inspectutils.GetFullArgSpec(tc.NoDefaults)
+    self.assertEqual(spec.args, [])
+    self.assertEqual(spec.defaults, ())
+    self.assertEqual(spec.varargs, None)
+    self.assertEqual(spec.varkw, None)
+    self.assertEqual(spec.kwonlyargs, [])
+    self.assertEqual(spec.kwonlydefaults, {})
+    self.assertEqual(spec.annotations, {})
+
+  def testGetFullArgSpecFromClassNoInit(self):
+    spec = inspectutils.GetFullArgSpec(tc.OldStyleEmpty)
+    self.assertEqual(spec.args, [])
+    self.assertEqual(spec.defaults, ())
+    self.assertEqual(spec.varargs, None)
+    self.assertEqual(spec.varkw, None)
+    self.assertEqual(spec.kwonlyargs, [])
+    self.assertEqual(spec.kwonlydefaults, {})
+    self.assertEqual(spec.annotations, {})
+
+  def testGetFullArgSpecFromMethod(self):
+    spec = inspectutils.GetFullArgSpec(tc.NoDefaults().double)
+    self.assertEqual(spec.args, ['count'])
+    self.assertEqual(spec.defaults, ())
+    self.assertEqual(spec.varargs, None)
+    self.assertEqual(spec.varkw, None)
+    self.assertEqual(spec.kwonlyargs, [])
+    self.assertEqual(spec.kwonlydefaults, {})
+    self.assertEqual(spec.annotations, {})
 
   def testInfoOne(self):
     info = inspectutils.Info(1)