Skip to content

Commit

Permalink
Fix the issue where keyword only arguments with default value is inco…
Browse files Browse the repository at this point in the history
…rrectly marked as required.

PiperOrigin-RevId: 301185283
Change-Id: I0d7e2df1d5411769db6250f819f776c955c788f9
  • Loading branch information
joejoevictor authored and dbieber committed Mar 19, 2020
1 parent aa72776 commit b978161
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 9 deletions.
31 changes: 22 additions & 9 deletions fire/helptext.py
Expand Up @@ -33,6 +33,8 @@
from __future__ import division
from __future__ import print_function

import itertools

from fire import completion
from fire import custom_descriptions
from fire import decorators
Expand Down Expand Up @@ -167,6 +169,11 @@ def _DescriptionSection(component, info):
return None


def _CreateKeywordOnlyFlagItem(flag, docstring_info, spec):
return _CreateFlagItem(
flag, docstring_info, required=flag not in spec.kwonlydefaults)


def _ArgsAndFlagsSections(info, spec, metadata):
"""The "Args and Flags" sections of the help string."""
args_with_no_defaults = spec.args[:len(spec.args) - len(spec.defaults)]
Expand Down Expand Up @@ -199,15 +206,15 @@ def _ArgsAndFlagsSections(info, spec, metadata):
('NOTES', 'You can also use flags syntax for POSITIONAL ARGUMENTS')
)

optional_flag_items = [
positional_flag_items = [
_CreateFlagItem(flag, docstring_info, required=False)
for flag in args_with_defaults
]
required_flag_items = [
_CreateFlagItem(flag, docstring_info, required=True)
kwonly_flag_items = [
_CreateKeywordOnlyFlagItem(flag, docstring_info, spec)
for flag in spec.kwonlyargs
]
flag_items = optional_flag_items + required_flag_items
flag_items = positional_flag_items + kwonly_flag_items

if spec.varkw:
description = _GetArgDescription(spec.varkw, docstring_info)
Expand Down Expand Up @@ -382,9 +389,7 @@ def _CreateFlagItem(flag, docstring_info, required=False):
flag: The name of the flag.
docstring_info: A docstrings.DocstringInfo namedtuple with information about
the containing function's docstring.
required: Whether the flag is required. Keyword-only arguments (only in
Python 3) become required flags, whereas normal keyword arguments become
optional flags.
required: Whether the flag is required.
Returns:
A string to be used in constructing the help screen for the function.
"""
Expand Down Expand Up @@ -570,13 +575,21 @@ def _GetCallableUsageItems(spec, metadata):
return items


def _KeywordOnlyArguments(spec, required=True):
return (flag for flag in spec.kwonlyargs
if required == (flag in spec.kwonlydefaults))


def _GetCallableAvailabilityLines(spec):
"""The list of availability lines for a callable for use in a usage string."""
args_with_defaults = spec.args[len(spec.args) - len(spec.defaults):]

# TODO(dbieber): Handle args_with_no_defaults if not accepts_positional_args.
optional_flags = [('--' + flag) for flag in args_with_defaults]
required_flags = [('--' + flag) for flag in spec.kwonlyargs]
optional_flags = [('--' + flag) for flag in itertools.chain(
args_with_defaults, _KeywordOnlyArguments(spec, required=False))]
required_flags = [
('--' + flag) for flag in _KeywordOnlyArguments(spec, required=True)
]

# Flags section:
availability_lines = []
Expand Down
22 changes: 22 additions & 0 deletions fire/helptext_test.py
Expand Up @@ -20,12 +20,14 @@

import os
import textwrap
import unittest

from fire import formatting
from fire import helptext
from fire import test_components as tc
from fire import testutils
from fire import trace
import six


class HelpTest(testutils.BaseTestCase):
Expand Down Expand Up @@ -158,6 +160,26 @@ def testHelpTextNoInit(self):
self.assertIn('NAME\n OldStyleEmpty', help_screen)
self.assertIn('SYNOPSIS\n OldStyleEmpty', help_screen)

@unittest.skipIf(
six.PY2,
'Python 2 does not support single asterisk in function definition')
def testHelpTextKeywordOnlyArgumentsWithDefault(self):
component = tc.py3.KeywordOnly.with_default
output = helptext.HelpText(
component=component, trace=trace.FireTrace(component, 'with_default'))
self.assertIn('NAME\n with_default', output)
self.assertIn('FLAGS\n --x=X', output)

@unittest.skipIf(
six.PY2,
'Python 2 does not support single asterisk in function definition')
def testHelpTextKeywordOnlyArgumentsWithoutDefault(self):
component = tc.py3.KeywordOnly.double
output = helptext.HelpText(
component=component, trace=trace.FireTrace(component, 'double'))
self.assertIn('NAME\n double', output)
self.assertIn('FLAGS\n --count=COUNT (required)', output)

def testHelpScreen(self):
component = tc.ClassWithDocstring()
t = trace.FireTrace(component, name='ClassWithDocstring')
Expand Down
3 changes: 3 additions & 0 deletions fire/test_components_py3.py
Expand Up @@ -32,6 +32,9 @@ def double(self, *, count):
def triple(self, *, count):
return count * 3

def with_default(self, *, x="x"):
print("x: " + x)


class LruCacheDecoratedMethod(object):

Expand Down

0 comments on commit b978161

Please sign in to comment.