Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/next-release/enhancement-arguments-756.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "enhancement",
"category": "arguments",
"description": "Suggest similar option names when an unrecognized option is passed to a command"
}
44 changes: 44 additions & 0 deletions awscli/argparser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 8 additions & 1 deletion awscli/clidriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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(
Expand Down
14 changes: 12 additions & 2 deletions awscli/customizations/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
29 changes: 29 additions & 0 deletions tests/blackbox/test_ls_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions tests/functional/s3/test_ls_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
76 changes: 74 additions & 2 deletions tests/unit/test_argparser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)
28 changes: 28 additions & 0 deletions tests/unit/test_clidriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading