diff --git a/.changes/next-release/enhancement-arguments-756.json b/.changes/next-release/enhancement-arguments-756.json new file mode 100644 index 000000000000..9bb8b97f1270 --- /dev/null +++ b/.changes/next-release/enhancement-arguments-756.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "arguments", + "description": "Suggest similar option names when an unrecognized option is passed to a command" +} diff --git a/awscli/argparser.py b/awscli/argparser.py index 8ddb5f228550..4e6a29f6fd9f 100644 --- a/awscli/argparser.py +++ b/awscli/argparser.py @@ -32,6 +32,50 @@ class ArgParseException(Exception): pass +def get_option_suggestion_candidates(arg_table, session): + """Return the option names to consider when suggesting a correction. + + Includes the documented options in ``arg_table`` plus the global + options, without the leading ``--``. + """ + candidates = [ + arg.cli_name[2:] + for arg in arg_table.values() + if arg.cli_name.startswith('--') + and not getattr(arg, '_UNDOCUMENTED', False) + ] + # Misspelled global options (e.g. --regoin) aren't recognized by + # the main parser either, so they end up as unknown options of the + # command. + candidates.extend(session.get_data('cli')['options']) + return candidates + + +def format_unknown_options_error(unknown, candidates): + """Build the error message for unrecognized options. + + :param unknown: The unparsed args, which can include the values + that followed an unknown option (e.g. ``['--regoin', 'us-west-2']``). + :param candidates: Valid option names, without the leading ``--``. + """ + msg = f"Unknown options: {', '.join(unknown)}" + suggestions = [] + for arg in unknown: + if not arg.startswith('--'): + continue + name = arg[2:].split('=', 1)[0] + # Compare without the shared '--' prefix, which would otherwise + # inflate the similarity of every candidate. + for match in get_close_matches(name, candidates, cutoff=0.8): + suggestion = f' * --{match} (instead of --{name})' + # The same unknown option may be passed more than once. + if suggestion not in suggestions: + suggestions.append(suggestion) + if suggestions: + msg += '\n\nMaybe you meant:\n' + '\n'.join(suggestions) + return msg + + class CommandAction(argparse.Action): """Custom action for CLI command arguments diff --git a/awscli/clidriver.py b/awscli/clidriver.py index bc860a4a8bf2..a6cefc2d862c 100644 --- a/awscli/clidriver.py +++ b/awscli/clidriver.py @@ -41,6 +41,8 @@ MainArgParser, ServiceArgParser, SubCommandArgParser, + format_unknown_options_error, + get_option_suggestion_candidates, ) from awscli.argprocess import unpack_argument from awscli.arguments import ( @@ -922,7 +924,12 @@ def __call__(self, args, parsed_globals): remaining.append(parsed_args.help) if remaining: raise UnknownArgumentError( - f"Unknown options: {', '.join(remaining)}" + format_unknown_options_error( + remaining, + get_option_suggestion_candidates( + self.arg_table, self._session + ), + ) ) event = f'operation-args-parsed.{self._parent_name}.{self._name}' self._emit( diff --git a/awscli/customizations/commands.py b/awscli/customizations/commands.py index 5c8afefd7105..94753320c7e0 100644 --- a/awscli/customizations/commands.py +++ b/awscli/customizations/commands.py @@ -7,7 +7,12 @@ from botocore.validate import validate_parameters import awscli -from awscli.argparser import ArgTableArgParser, SubCommandArgParser +from awscli.argparser import ( + ArgTableArgParser, + SubCommandArgParser, + format_unknown_options_error, + get_option_suggestion_candidates, +) from awscli.argprocess import unpack_argument, unpack_cli_arg from awscli.arguments import CustomArgument, create_argument_model_from_schema from awscli.bcdoc import docevents @@ -202,7 +207,12 @@ def __call__(self, args, parsed_globals): # function for this top level command. if remaining: raise ParamValidationError( - f"Unknown options: {','.join(remaining)}" + format_unknown_options_error( + remaining, + get_option_suggestion_candidates( + self.arg_table, self._session + ), + ) ) rc = self._run_main(parsed_args, parsed_globals) if rc is None: diff --git a/tests/blackbox/test_ls_command.py b/tests/blackbox/test_ls_command.py index 848ab62c63bc..31ba488845f5 100644 --- a/tests/blackbox/test_ls_command.py +++ b/tests/blackbox/test_ls_command.py @@ -46,6 +46,35 @@ async def test_errors_out_with_extra_arguments(aws_cli: str) -> None: ) assert rc == 252 assert b"--extra-argument-foo" in stderr + assert b"Maybe you meant" not in stderr + + +@pytest.mark.asyncio +async def test_suggests_option_for_misspelled_option(aws_cli: str) -> None: + async with mock_server() as (server, proxy): + stdout, stderr, rc = await run_cli( + aws_cli, + ["s3", "ls", "s3://bucket/", "--recurisve"], + cli_env(proxy), + ) + + assert rc == 252 + assert b"* --recursive (instead of --recurisve)" in stderr + + +@pytest.mark.asyncio +async def test_suggests_global_option_for_misspelled_option( + aws_cli: str, +) -> None: + async with mock_server() as (server, proxy): + stdout, stderr, rc = await run_cli( + aws_cli, + ["s3", "ls", "s3://bucket/", "--regoin", "us-west-2"], + cli_env(proxy), + ) + + assert rc == 252 + assert b"* --region (instead of --regoin)" in stderr @pytest.mark.asyncio diff --git a/tests/functional/s3/test_ls_command.py b/tests/functional/s3/test_ls_command.py index 0220b40a3f85..d95cb25a7c06 100644 --- a/tests/functional/s3/test_ls_command.py +++ b/tests/functional/s3/test_ls_command.py @@ -53,6 +53,22 @@ def test_errors_out_with_extra_arguments(self): stderr = self.run_cmd('s3 ls --extra-argument-foo', expected_rc=252)[1] self.assertIn('Unknown options', stderr) self.assertIn('--extra-argument-foo', stderr) + self.assertNotIn('Maybe you meant', stderr) + + def test_suggests_option_for_misspelled_option(self): + stderr = self.run_cmd( + 's3 ls s3://bucket --recurisve', expected_rc=252 + )[1] + self.assertIn( + 'Maybe you meant:\n * --recursive (instead of --recurisve)', + stderr, + ) + + def test_suggests_global_option_for_misspelled_option(self): + stderr = self.run_cmd( + 's3 ls s3://bucket --regoin us-west-2', expected_rc=252 + )[1] + self.assertIn(' * --region (instead of --regoin)', stderr) def test_list_buckets_use_page_size(self): stdout, _, _ = self.run_cmd('s3 ls --page-size 8', expected_rc=0) diff --git a/tests/unit/test_argparser.py b/tests/unit/test_argparser.py index 7cc19dedeb58..25a0f0fe046b 100644 --- a/tests/unit/test_argparser.py +++ b/tests/unit/test_argparser.py @@ -12,8 +12,14 @@ # language governing permissions and limitations under the License. from argparse import ArgumentParser -from awscli.argparser import CommandAction, FirstPassGlobalArgParser -from awscli.testutils import unittest +from awscli.argparser import ( + CommandAction, + FirstPassGlobalArgParser, + format_unknown_options_error, + get_option_suggestion_candidates, +) +from awscli.arguments import CustomArgument +from awscli.testutils import mock, unittest class TestCommandAction(unittest.TestCase): @@ -67,3 +73,69 @@ def test_not_parse_unknown_args(self): ) self.assertEqual(parsed_args.debug, True) self.assertEqual(remains, ['--foo', 'bar']) + + +class TestGetOptionSuggestionCandidates(unittest.TestCase): + def test_includes_documented_options_and_globals_only(self): + # Suggest options and globals, excluding positional and hidden args. + hidden = CustomArgument('marker') + hidden._UNDOCUMENTED = True + arg_table = { + 'instance-ids': CustomArgument('instance-ids'), + 'outfile': CustomArgument('outfile', positional_arg=True), + 'marker': hidden, + } + session = mock.Mock() + session.get_data.return_value = {'options': {'region': {}, 'debug': {}}} + + candidates = get_option_suggestion_candidates(arg_table, session) + + self.assertCountEqual(candidates, ['instance-ids', 'region', 'debug']) + + +class TestFormatUnknownOptionsError(unittest.TestCase): + def setUp(self): + self.candidates = ['region', 'instance-ids', 'dry-run', 'no-dry-run'] + + def test_no_suggestions(self): + msg = format_unknown_options_error(['--foo', 'bar'], self.candidates) + self.assertEqual(msg, 'Unknown options: --foo, bar') + + def test_suggests_close_match(self): + msg = format_unknown_options_error( + ['--regoin', 'us-west-2'], self.candidates + ) + self.assertEqual( + msg, + 'Unknown options: --regoin, us-west-2\n\n' + 'Maybe you meant:\n' + ' * --region (instead of --regoin)', + ) + + def test_suggests_for_each_unknown_option(self): + msg = format_unknown_options_error( + ['--regoin', '--instance-idz'], self.candidates + ) + self.assertIn(' * --region (instead of --regoin)', msg) + self.assertIn(' * --instance-ids (instead of --instance-idz)', msg) + + def test_deduplicates_repeated_unknown_option(self): + msg = format_unknown_options_error( + ['--regoin', 'a', '--regoin', 'b'], self.candidates + ) + self.assertEqual( + msg, + 'Unknown options: --regoin, a, --regoin, b\n\n' + 'Maybe you meant:\n' + ' * --region (instead of --regoin)', + ) + + def test_ignores_value_after_equals(self): + msg = format_unknown_options_error( + ['--regoin=us-west-2'], self.candidates + ) + self.assertIn(' * --region (instead of --regoin)', msg) + + def test_does_not_match_values(self): + msg = format_unknown_options_error(['region'], self.candidates) + self.assertNotIn('Maybe you meant', msg) diff --git a/tests/unit/test_clidriver.py b/tests/unit/test_clidriver.py index 7635c302e7aa..f2fe50455817 100644 --- a/tests/unit/test_clidriver.py +++ b/tests/unit/test_clidriver.py @@ -840,6 +840,34 @@ def test_help_blurb_in_unknown_argument_error_message(self): self.assertEqual(rc, 252) self.assertIn(HELP_BLURB, self.stderr.getvalue()) + def test_unknown_option_suggests_operation_option(self): + rc = self.driver.main( + ['ec2', 'describe-instances', '--instance-idz', 'i-123'] + ) + self.assertEqual(rc, 252) + self.assertIn( + 'Maybe you meant:\n * --instance-ids (instead of --instance-idz)', + self.stderr.getvalue(), + ) + + def test_unknown_option_suggests_global_option(self): + rc = self.driver.main( + ['ec2', 'describe-instances', '--regoin', 'us-west-2'] + ) + self.assertEqual(rc, 252) + self.assertIn( + ' * --region (instead of --regoin)', self.stderr.getvalue() + ) + + def test_unknown_option_does_not_suggest_undocumented_option(self): + # --marker is hidden because list-objects is paginated. + rc = self.driver.main( + ['s3api', 'list-objects', '--bucket', 'b', '--markr', 'x'] + ) + self.assertEqual(rc, 252) + self.assertIn('Unknown options: --markr, x', self.stderr.getvalue()) + self.assertNotIn('Maybe you meant', self.stderr.getvalue()) + def test_idempotency_token_is_not_required_in_help_text(self): rc = self.driver.main(['servicecatalog', 'create-constraint']) self.assertEqual(rc, 252)