diff --git a/.changes/next-release/feature-help-57237.json b/.changes/next-release/feature-help-57237.json new file mode 100644 index 000000000000..db6b96f47ae7 --- /dev/null +++ b/.changes/next-release/feature-help-57237.json @@ -0,0 +1,5 @@ +{ + "type": "feature", + "category": "help", + "description": "Add a ``--help`` parameter that renders the same help as the ``help`` subcommand on every command (provider, service, operation, and custom commands)." +} diff --git a/awscli/argparser.py b/awscli/argparser.py index 8ddb5f228550..8495e43b4056 100644 --- a/awscli/argparser.py +++ b/awscli/argparser.py @@ -21,6 +21,7 @@ " aws help\n" " aws help\n" " aws help\n" + " aws [ ...] --help\n" ) USAGE = ( "aws [options] [ ...] [parameters]\n" @@ -60,6 +61,37 @@ def choices(self, val): pass +def _is_help_option_token(arg): + """Return True only for the literal ``--help`` token. + + Abbreviations (``--he``/``--hel``/``--h``) and ``=``-bearing forms + (``--help=x``, ``--instance-ids=--help``) are not help; they fall through to + the normal parser. + """ + return arg == '--help' + + +def first_help_option_index(args): + """Return the index of the first ``--help`` token, or ``None`` if absent.""" + for index, arg in enumerate(args): + if _is_help_option_token(arg): + return index + return None + + +def is_help_option_present(args): + """Return True if the literal ``--help`` token is present in ``args``.""" + return first_help_option_index(args) is not None + + +def strip_help_options(args): + """Return ``args`` with every ``--help`` token removed. + + All other tokens keep their original position and value. + """ + return [arg for arg in args if not _is_help_option_token(arg)] + + class CLIArgParser(argparse.ArgumentParser): Formatter = argparse.RawTextHelpFormatter diff --git a/awscli/clidriver.py b/awscli/clidriver.py index bc860a4a8bf2..0de5bba991c2 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -36,11 +36,15 @@ from awscli import __version__ from awscli.alias import AliasCommandInjector, AliasLoader from awscli.argparser import ( + ArgParseException, ArgTableArgParser, FirstPassGlobalArgParser, MainArgParser, ServiceArgParser, SubCommandArgParser, + first_help_option_index, + is_help_option_present, + strip_help_options, ) from awscli.argprocess import unpack_argument from awscli.arguments import ( @@ -94,7 +98,7 @@ HISTORY_RECORDER = get_global_history_recorder() METADATA_FILENAME = 'metadata.json' INSTALL_FILENAME = 'install.json' -_NO_AUTO_PROMPT_ARGS = ['help', '--version'] +_NO_AUTO_PROMPT_ARGS = ['help', '--help', '--version'] _CLI_AUTO_PROMPT_OPTION = '--cli-auto-prompt' _NO_CLI_AUTO_PROMPT_OPTION = '--no-cli-auto-prompt' # Don't remove this line. The idna encoding @@ -591,6 +595,16 @@ def main(self, args=None): self._add_aliases(command_table, parser) parsed_args = None try: + # When --help is present but no command is named, render provider + # help instead of failing on the required ``command`` positional + # argument. When a command is named, fall through so its own + # __call__ renders the more specific help. Kept inside the try so a + # ``--version`` SystemExit(0) from the pre-help slice parse still + # exits 0. + if is_help_option_present(args) and not self._names_a_command( + args, parser + ): + args = self._route_to_provider_help(args) # Because _handle_top_level_args emits events, it's possible # that exceptions can be raised, which should have the same # general exception handling logic as calling into the @@ -615,6 +629,44 @@ def main(self, args=None): parsed_globals=parsed_args, ) + def _names_a_command(self, args, parser): + # ``parser`` is the provider-level ``MainArgParser`` (the one built in + # ``main()`` and passed in). Return True if a real command is named + # before the first --help token. Parsing the pre-help slice with that + # parser lets argparse consume the VALUES of value-taking global options + # instead of mistaking them for a command: in ``aws --region ec2 + # --help`` the ``ec2`` is --region's value, so no command is named and + # provider help renders. + head = self._tokens_before_help(args) + if head == list(args): + # No --help token present; nothing to decide here. + return True + try: + parsed, _ = parser.parse_known_args(head) + except ArgParseException: + # Only ArgParseException (raised by CLIArgParser.error) means "no + # parseable command", e.g. ``--region ec2`` -> "required: command". + # We deliberately do not catch broader exceptions so a --version + # SystemExit and a user KeyboardInterrupt propagate. + return False + return getattr(parsed, 'command', None) is not None + + def _tokens_before_help(self, args): + # The tokens that precede the first --help token. Tokens at or after + # the first help token are ignored for routing, exactly as the + # positional ``help`` token ignores everything after it. + help_index = first_help_option_index(args) + if help_index is None: + return list(args) + return list(args[:help_index]) + + def _route_to_provider_help(self, args): + # Render provider help: keep the tokens before the first --help and + # append the ``help`` positional argument, so a trailing command is + # ignored. + head = self._tokens_before_help(args) + return head + ['help'] + def _emit_session_event(self, parsed_args): # This event is guaranteed to run after the session has been # initialized and a profile has been set. This was previously @@ -737,10 +789,33 @@ def __call__(self, args, parsed_globals): # we can go ahead and create the parser for it. We # can also grab the Service object from botocore. service_parser = self.create_parser() - parsed_args, remaining = service_parser.parse_known_args(args) command_table = self._get_command_table() + # Resolve help intent before binding. If --help is present we still + # want to route to a specific operation's help when an operation was + # named (e.g. ``aws ec2 describe-instances --help``); the operation's + # own __call__ renders it. Only when no operation token is present + # (e.g. ``aws ec2 --help``) do we render this service's help here. + if is_help_option_present(args): + operation = self._find_operation_in_args(args, command_table) + if operation is None: + return self.create_help_command()( + strip_help_options(args), parsed_globals + ) + parsed_args, remaining = service_parser.parse_known_args(args) return command_table[parsed_args.operation](remaining, parsed_globals) + def _find_operation_in_args(self, args, command_table): + # Return the first token before the first --help that names an + # operation, or None. An operation named after --help is ignored: + # ``aws ec2 --help describe-instances`` renders EC2 (service) help, the + # same as ``aws ec2 help describe-instances``. + help_index = first_help_option_index(args) + candidates = args if help_index is None else args[:help_index] + for token in candidates: + if not token.startswith('-') and token in command_table: + return token + return None + def _create_command_table(self): command_table = OrderedDict() service_model = self._get_service_model() @@ -890,6 +965,21 @@ def _parse_potential_subcommand(self, args, subcommand_table): return parser.parse_known_args(args) return None + def _subcommand_precedes_help(self, args, maybe_parsed_subcommand): + # True if the subcommand is named before the first --help token, + # decided from what the subcommand parser binds on the pre-help slice + # rather than a value-blind string index. Mirrors + # ``BasicCommand._subcommand_precedes_help``. + help_index = first_help_option_index(args) + if help_index is None: + return True + subcommand_name = maybe_parsed_subcommand[1] + head = args[:help_index] + parsed_head = self._parse_potential_subcommand( + head, self.subcommand_table + ) + return parsed_head is not None and parsed_head[1] == subcommand_name + def __call__(self, args, parsed_globals): # Once we know we're trying to call a particular operation # of a service we can go ahead and load the parameters. @@ -907,9 +997,26 @@ def __call__(self, args, parsed_globals): maybe_parsed_subcommand = self._parse_potential_subcommand( args, subcommand_table ) - if maybe_parsed_subcommand is not None: + # Descend into a parsed subcommand only when it is named before the + # first --help token, mirroring the guard in ``BasicCommand.__call__``. + # Otherwise ``--help`` before a subcommand (e.g. a hypothetical + # ``aws myservice myoperation --help mysubcommand``) would render the + # subcommand's help instead of this operation's. No operation + # currently has a subcommand table, so this is a forward-looking + # robustness check that keeps the two dispatch layers symmetric. + if ( + maybe_parsed_subcommand is not None + and self._subcommand_precedes_help(args, maybe_parsed_subcommand) + ): new_args, subcommand_name = maybe_parsed_subcommand return subcommand_table[subcommand_name](new_args, parsed_globals) + # Resolve --help before binding so a preceding value option (e.g. + # ``--instance-ids i-123 --help``) cannot swallow it. The positional + # ``help`` path below still works; this is an additional path. + if is_help_option_present(args): + return self.create_help_command()( + strip_help_options(args), parsed_globals + ) operation_parser = self._create_operation_parser( self.arg_table, subcommand_table ) diff --git a/awscli/customizations/commands.py b/awscli/customizations/commands.py index 5c8afefd7105..259ee2c67d61 100644 --- a/awscli/customizations/commands.py +++ b/awscli/customizations/commands.py @@ -1,3 +1,4 @@ +import argparse import copy import logging import os @@ -7,7 +8,12 @@ from botocore.validate import validate_parameters import awscli -from awscli.argparser import ArgTableArgParser, SubCommandArgParser +from awscli.argparser import ( + ArgTableArgParser, + SubCommandArgParser, + first_help_option_index, + is_help_option_present, +) from awscli.argprocess import unpack_argument, unpack_cli_arg from awscli.arguments import CustomArgument, create_argument_model_from_schema from awscli.bcdoc import docevents @@ -148,11 +154,24 @@ def __call__(self, args, parsed_globals): maybe_parsed_subcommand = self._parse_potential_subcommand( args, self._subcommand_table ) - if maybe_parsed_subcommand is not None: + # Descend into a subcommand only when it is named before the first + # --help token: ``aws configure get --help`` renders ``get`` help, but + # ``aws configure --help get`` renders configure help (help resolves at + # the depth reached when --help appears; later tokens are ignored). + if ( + maybe_parsed_subcommand is not None + and self._subcommand_precedes_help(args, maybe_parsed_subcommand) + ): new_args, subcommand_name = maybe_parsed_subcommand return self._subcommand_table[subcommand_name]( new_args, parsed_globals ) + # Resolve --help before binding so a preceding positional value (e.g. + # ``aws configure get region --help``) cannot hide it. The positional + # ``help`` path below still works; this is an additional path. + if is_help_option_present(args): + self._display_help(self._build_help_parsed_args(), parsed_globals) + return 0 parser = ArgTableArgParser(self.arg_table, self.subcommand_table) parsed_args, remaining = parser.parse_known_args(args) @@ -249,6 +268,37 @@ def _display_help(self, parsed_args, parsed_globals): help_command = self.create_help_command() help_command(parsed_args, parsed_globals) + def _subcommand_precedes_help(self, args, maybe_parsed_subcommand): + # True if the subcommand is named before the first --help token: when + # --help comes first, render this command's help instead of descending + # into the subcommand. + # + # Decide by re-parsing the pre-help slice via + # ``_parse_potential_subcommand`` rather than by + # ``args.index(subcommand_name)``. A naive string index returns the + # first literal occurrence of the name. If an option value equals the + # subcommand name, that occurrence can be the value rather than the + # actual subcommand token. Parsing the slice consumes option values + # correctly, mirroring ``CLIDriver._names_a_command``. + help_index = first_help_option_index(args) + if help_index is None: + return True + subcommand_name = maybe_parsed_subcommand[1] + head = args[:help_index] + parsed_head = self._parse_potential_subcommand( + head, self._subcommand_table + ) + return parsed_head is not None and parsed_head[1] == subcommand_name + + def _build_help_parsed_args(self): + # BasicHelp.__call__ does not read the parsed args namespace (it + # generates doc events from the command object), so a minimal + # namespace with help='help' is sufficient and mirrors what the + # positional ``help`` path produced. + namespace = argparse.Namespace() + namespace.help = 'help' + return namespace + def create_help_command(self): command_help_table = {} if self.SUBCOMMANDS: diff --git a/tests/functional/docs/test_help_flag.py b/tests/functional/docs/test_help_flag.py new file mode 100644 index 000000000000..8a4526eb3c47 --- /dev/null +++ b/tests/functional/docs/test_help_flag.py @@ -0,0 +1,161 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Functional tests for the ``--help`` parameter. + +Cover ``--help`` on every command surface and confirm it renders the same help +as the ``help`` subcommand. +""" + +from awscli.testutils import BaseAWSHelpOutputTest + + +class TestHelpFlagResolvedBeforeBinding(BaseAWSHelpOutputTest): + """``--help`` renders help everywhere, before argument binding.""" + + def test_help_flag_simple_operation(self): + self.driver.main(['ec2', 'describe-instances', '--help']) + self.assert_contains('describe-instances') + + def test_help_flag_provider(self): + self.driver.main(['--help']) + self.assert_contains('aws') + + def test_help_flag_service(self): + self.driver.main(['ec2', '--help']) + self.assert_contains('ec2') + + def test_help_flag_after_value_taking_option(self): + # A space-separated value option precedes --help. Resolving help before + # binding shows the operation help rather than running the operation. + self.driver.main( + ['ec2', 'describe-instances', '--instance-ids', 'i-123', '--help'] + ) + self.assert_contains('describe-instances') + + def test_help_flag_before_value_taking_option(self): + # --help before a value option still shows help, mirroring + # `help --instance-ids i-123`. + self.driver.main( + ['ec2', 'describe-instances', '--help', '--instance-ids', 'i-123'] + ) + self.assert_contains('describe-instances') + + def test_help_flag_custom_command_positional(self): + # `configure get --help`: the BasicCommand positional + # `varname` consumes the real value, and --help still shows `get` help. + self.driver.main(['configure', 'get', 'region', '--help']) + self.assert_contains('get') + + def test_help_flag_optional_value_option(self): + # `--generate-cli-skeleton --help`: an optional-value option precedes + # --help, and the operation help still renders. + self.driver.main( + ['ec2', 'describe-instances', '--generate-cli-skeleton', '--help'] + ) + self.assert_contains('describe-instances') + + def test_abbreviation_is_not_help(self): + # Exact-token-only: only the literal --help renders help. An + # abbreviation such as --hel is NOT help; it falls through to the + # operation parser as an unknown option (exit 252, no help output). + rc = self.driver.main(['ec2', 'describe-instances', '--hel']) + self.assertEqual(rc, 252) + self.assert_not_contains('describe-instances') + + def test_help_flag_waiter_state(self): + self.driver.main(['ec2', 'wait', 'instance-running', '--help']) + self.assert_contains('instance-running') + + def test_help_flag_custom_nested_command(self): + self.driver.main(['cloudformation', 'package', '--help']) + self.assert_contains('package') + + +class TestHelpFlagValueNotMisinterpreted(BaseAWSHelpOutputTest): + """A genuine option *value* of ``--help`` must not trigger help.""" + + def test_attached_value_help_is_not_help_request(self): + # `--instance-ids=--help` supplies the string '--help' as the value of + # --instance-ids; it is NOT a help request. Help must NOT render + # (the command proceeds and fails later on missing region/creds). + rc = self.driver.main( + ['ec2', 'describe-instances', '--instance-ids=--help'] + ) + self.assertNotEqual(rc, 0) + self.assert_not_contains('describe-instances\n*****') + + +class TestHelpFlagPositionSelectsHelpLevel(BaseAWSHelpOutputTest): + """Where ``--help`` sits selects which command level's help renders, since + routing considers only the command tokens before the first ``--help``. + Every token after ``--help`` is ignored. + """ + + def test_flag_before_operation_renders_service_help(self): + # aws ec2 --help describe-instances ... == aws ec2 help describe-instances ... + # ``Available Commands`` appears in service and provider help but not in + # operation help, so it marks that service help rendered here. + self.driver.main( + ['ec2', '--help', 'describe-instances', '--instance-ids', 'i-123'] + ) + self.assert_contains('Available Commands') # service help + + def test_flag_after_operation_renders_operation_help(self): + # The operation token precedes --help, so help resolves at operation + # level. Assert a positive operation-only marker (a describe-instances + # parameter) so the test fails if nothing renders, not merely the + # absence of the service-help "Available Commands" section. + self.driver.main(['ec2', 'describe-instances', '--help']) + self.assert_contains('--instance-ids') # operation help rendered + self.assert_not_contains('Available Commands') # and NOT service help + + def test_flag_before_command_at_provider_renders_provider_help(self): + # aws --help ec2 -> provider help, not ec2 help. The provider help + # lists services under "Available Commands"; ec2 (service) help lists + # operations. Both contain the string, so we assert the provider-only + # synopsis line instead. + self.driver.main(['--help', 'ec2']) + self.assert_contains('The AWS Command Line Interface') + + def test_flag_before_subcommand_renders_parent_help(self): + # aws configure --help get -> configure help (parent), not get help. + self.driver.main(['configure', '--help', 'get']) + self.assert_contains('Available Commands') + + def test_flag_after_subcommand_renders_subcommand_help(self): + # aws configure get --help -> `get` help. Assert a positive + # subcommand-only marker (the `get` command's own description) so the + # test fails if nothing renders, not merely the absence of the + # "Available Commands" section (`get` has no subcommands). + self.driver.main(['configure', 'get', '--help']) + self.assert_contains( + 'Get a configuration value' + ) # `get` help rendered + self.assert_not_contains( + 'Available Commands' + ) # and not a parent listing + + +class TestHelpFlagSkipsOptionValues(BaseAWSHelpOutputTest): + """A value that happens to name a command/operation must not be mistaken for + one. Routing parses the pre-help slice, so the values of value-taking + global options are consumed by argparse rather than read as a command. + These mirror the positional ``help`` token: ``aws --region ec2 --help`` + renders provider help because ``ec2`` is --region's value, not the command. + """ + + def test_option_value_naming_a_command_renders_provider_help(self): + # aws --region ec2 --help == aws --region ec2 help -> provider help. + # `ec2` is --region's VALUE, not the command. + self.driver.main(['--region', 'ec2', '--help']) + self.assert_contains('The AWS Command Line Interface') + + def test_valid_global_value_then_flag_renders_provider_help(self): + self.driver.main(['--region', 'us-west-2', '--help']) + self.assert_contains('The AWS Command Line Interface') + + def test_global_value_at_service_renders_service_help(self): + # aws ec2 --output json --help -> EC2 service help (option json is a + # valid --output value; no operation is named before --help). + self.driver.main(['ec2', '--output', 'json', '--help']) + self.assert_contains('Available Commands') # service help diff --git a/tests/unit/customizations/test_commands.py b/tests/unit/customizations/test_commands.py index 790871c07f8b..e256555262e8 100644 --- a/tests/unit/customizations/test_commands.py +++ b/tests/unit/customizations/test_commands.py @@ -240,3 +240,66 @@ def test_custom_service_operation_in_user_agent(self): self._assert_customization_in_user_agent( ' md/command#rds.add-option-to-option-group' ) + + +class _CmdWithValueOptionAndSubcommand(BasicCommand): + """A command that has BOTH a value-taking option and a subcommand whose + name can equal that option's value. This is the arrangement that exposes + the `_subcommand_precedes_help` `args.index` edge. + + No command currently has this shape, so the edge is not reachable + through the real CLI today; this synthetic command exercises the routing + helper's logic directly so the fix is testable. + """ + + NAME = 'valsubcmd' + ARG_TABLE = [ + { + 'name': 'opt', + 'help_text': 'A value-taking option.', + 'action': 'store', + 'cli_type_name': 'string', + } + ] + SUBCOMMANDS = [{'name': 'get', 'command_class': BasicCommand}] + + +class TestSubcommandPrecedesHelp(unittest.TestCase): + """`_subcommand_precedes_help` must decide from the subcommand's real + parsed position, not from `args.index()` (first literal occurrence). + + With an `args.index` implementation, an option value that equals the + subcommand name and sits before `--help` is mistaken for the subcommand, so + `['--opt', 'get', '--help', 'get']` (real subcommand after `--help`) wrongly + returns True. The parser-derived implementation returns False. + """ + + def setUp(self): + self.cmd = _CmdWithValueOptionAndSubcommand(FakeSession()) + # Build the tables the routing helper relies on. + self.cmd._subcommand_table = self.cmd._build_subcommand_table() + self.cmd._arg_table = self.cmd._build_arg_table() + + def _decide(self, args): + mps = self.cmd._parse_potential_subcommand( + args, self.cmd._subcommand_table + ) + assert mps is not None, f'no subcommand parsed for {args!r}' + return self.cmd._subcommand_precedes_help(args, mps) + + def test_option_value_equal_to_subcommand_name_before_help(self): + # 'get' appears first as --opt's VALUE (before --help); the real 'get' + # subcommand is AFTER --help. Must be False (render parent help). + self.assertFalse( + self._decide(['--opt', 'get', '--help', 'get']), + "the option value 'get' before --help must not be mistaken for " + "the subcommand that appears after --help", + ) + + def test_real_subcommand_before_help(self): + # Real subcommand 'get' before --help -> descend into it (True). + self.assertTrue(self._decide(['get', '--help'])) + + def test_real_subcommand_after_help(self): + # 'get' only after --help -> parent help (False). + self.assertFalse(self._decide(['--help', 'get'])) diff --git a/tests/unit/test_argparser.py b/tests/unit/test_argparser.py index 7cc19dedeb58..2dc58e5c54a5 100644 --- a/tests/unit/test_argparser.py +++ b/tests/unit/test_argparser.py @@ -12,7 +12,12 @@ # language governing permissions and limitations under the License. from argparse import ArgumentParser -from awscli.argparser import CommandAction, FirstPassGlobalArgParser +from awscli.argparser import ( + CommandAction, + FirstPassGlobalArgParser, + is_help_option_present, + strip_help_options, +) from awscli.testutils import unittest @@ -67,3 +72,51 @@ def test_not_parse_unknown_args(self): ) self.assertEqual(parsed_args.debug, True) self.assertEqual(remains, ['--foo', 'bar']) + + +class TestHelpOptionExactTokenOnly(unittest.TestCase): + """Only the literal ``--help`` token counts as help. + + Abbreviations ``--he``/``--hel`` (and ``--h``) are NOT help requests. They + fall through to the normal parser as unknown options. + ``is_help_option_present`` and ``strip_help_options`` share the single + :func:`_is_help_option_token` predicate, so they can never disagree: + detection returns True exactly for the tokens stripping removes. + """ + + def _detects_and_strips_consistently(self, token): + args = ['ec2', 'describe-instances', token] + detected = is_help_option_present(args) + stripped = strip_help_options(args) + removed = token not in stripped + # Invariant: a token detected as help is removed by strip, and a token + # not detected as help is left in place. + self.assertEqual( + detected, + removed, + f"detection/stripping disagree for {token!r}: " + f"detected={detected} removed={removed} stripped={stripped}", + ) + return detected + + def test_exact_help_token_is_recognized(self): + self.assertTrue( + self._detects_and_strips_consistently('--help'), + "the literal '--help' token must be recognized as help", + ) + + def test_abbreviations_are_not_help(self): + # Exact-token-only: no prefix of --help is accepted, not even --hel. + for token in ['--he', '--hel', '--h', '--he', '--']: + self.assertFalse( + self._detects_and_strips_consistently(token), + f"{token!r} must NOT be treated as help (exact-token-only)", + ) + + def test_help_with_attached_value_is_not_a_bare_flag(self): + # '--instance-ids=--help' carries '--help' as a value, not a help token, + # and '--help=x' is malformed for a store_true flag; neither is help. + for token in ['--instance-ids=--help', '--help=x']: + args = ['ec2', 'describe-instances', token] + self.assertFalse(is_help_option_present(args)) + self.assertEqual(strip_help_options(args), args) diff --git a/tests/unit/test_clidriver.py b/tests/unit/test_clidriver.py index 7635c302e7aa..258c7b8e3fdd 100644 --- a/tests/unit/test_clidriver.py +++ b/tests/unit/test_clidriver.py @@ -217,6 +217,7 @@ def _generate_auto_prompt_resolve_cases(): Case(['--no-cli-auto-prompt'], 'on-partial', 'off'), Case(['--version'], 'on', 'off'), Case(['help'], 'on', 'off'), + Case(['--help'], 'on', 'off'), ] @@ -834,7 +835,7 @@ def test_help_blurb_in_operation_error_message(self): self.assertIn(HELP_BLURB, self.stderr.getvalue()) def test_help_blurb_in_unknown_argument_error_message(self): - args = ['s3api', 'list-objects', '--help'] + args = ['s3api', 'list-objects', '--unknown-flag-xyz'] driver = create_clidriver(args) rc = driver.main(args) self.assertEqual(rc, 252) @@ -1130,6 +1131,39 @@ def test_idempotency_token_is_not_required(self): 'Idempotency tokens should not be required', ) + def test_help_flag_before_subcommand_renders_operation_help(self): + # With --help BEFORE a subcommand token, help must resolve at this + # operation's depth, not the subcommand's. No operation has a + # subcommand table, so inject a fake one: a fake operation with a single + # fake "fake-subcommand". For `` --help fake-subcommand`` the + # operation's help must render and the subcommand must not be dispatched. + # (Without the guard the dispatch would descend into the subcommand.) + fake_subcommand = mock.Mock() + self.cmd._subcommand_table = {'fake-subcommand': fake_subcommand} + operation_help = mock.Mock() + self.cmd.create_help_command = mock.Mock(return_value=operation_help) + parsed_globals = mock.Mock() + + self.cmd(['--help', 'fake-subcommand'], parsed_globals) + + operation_help.assert_called_once() + fake_subcommand.assert_not_called() + + def test_subcommand_before_help_flag_still_dispatches_subcommand(self): + # Symmetric check: for `` fake-subcommand --help`` the + # subcommand is named before --help, so the dispatch still descends into + # it (and the subcommand renders its own help). + fake_subcommand = mock.Mock() + self.cmd._subcommand_table = {'fake-subcommand': fake_subcommand} + operation_help = mock.Mock() + self.cmd.create_help_command = mock.Mock(return_value=operation_help) + parsed_globals = mock.Mock() + + self.cmd(['fake-subcommand', '--help'], parsed_globals) + + fake_subcommand.assert_called_once() + operation_help.assert_not_called() + class TestAWSCLIEntryPoint(unittest.TestCase): def setUp(self): @@ -1308,5 +1342,52 @@ def test_falls_back_to_other_when_no_source_field_anywhere(self, data_dir): assert get_distribution_source() == 'other' +class TestHelpFlagPreHelpSliceParse(unittest.TestCase): + """The pre-help-slice parse in ``CLIDriver._names_a_command`` must catch + only argparse errors (``ArgParseException``), not ``BaseException``. + + A broad ``except BaseException`` swallows ``SystemExit`` (raised by the + ``--version`` action) and ``KeyboardInterrupt`` during the slice parse. + Swallowing ``--version``'s ``SystemExit`` misroutes ``aws --version --help`` + as "no command named" instead of mirroring the positional ``help`` token. + Swallowing ``KeyboardInterrupt`` hides a user interrupt. Narrowing to + ``ArgParseException`` lets both propagate. + """ + + def test_version_flag_before_help_agrees_with_positional_help(self): + # `aws --version --help` must behave like `aws --version help`: the + # --version action fires and the process exits 0 in both cases. + driver = create_clidriver() + rc_flag = driver.main(['--version', '--help']) + driver2 = create_clidriver() + rc_positional = driver2.main(['--version', 'help']) + self.assertEqual(rc_flag, 0) + self.assertEqual(rc_positional, 0) + self.assertEqual(rc_flag, rc_positional) + + def test_keyboard_interrupt_during_slice_parse_propagates(self): + # A KeyboardInterrupt raised while parsing the pre-help slice must + # propagate, not be swallowed and reported as "no command named". + driver = create_clidriver() + command_table = driver._get_command_table() + parser = driver.create_parser(command_table) + with mock.patch.object( + parser, 'parse_known_args', side_effect=KeyboardInterrupt + ): + with self.assertRaises(KeyboardInterrupt): + driver._names_a_command(['ec2', '--help'], parser) + + def test_argparse_error_during_slice_parse_is_no_command(self): + # A genuine argparse failure on the slice (e.g. `--region` missing its + # value) is caught and reported as "no command named" so provider help + # renders. + driver = create_clidriver() + command_table = driver._get_command_table() + parser = driver.create_parser(command_table) + self.assertFalse( + driver._names_a_command(['--region', '--help'], parser) + ) + + if __name__ == '__main__': unittest.main() diff --git a/tests/unit/test_structured_error.py b/tests/unit/test_structured_error.py index 6b113ca1b56c..c556aa40c727 100644 --- a/tests/unit/test_structured_error.py +++ b/tests/unit/test_structured_error.py @@ -793,6 +793,7 @@ def test_unknown_argument_error_remains_plain_text(self): ' aws help\n' ' aws help\n' ' aws help\n' + ' aws [ ...] --help\n' '\n' '\n' 'aws: [ERROR]: --invalid-arg\n'