diff --git a/bert_e/reactor.py b/bert_e/reactor.py index e28ebb6e..92e15c2e 100644 --- a/bert_e/reactor.py +++ b/bert_e/reactor.py @@ -160,6 +160,13 @@ def __init__(self, keyword: str): self.keyword = keyword +class IncorrectSyntax(Error): + """An option was called with arguments using the command syntax.""" + def __init__(self, keyword: str): + super().__init__() + self.keyword = keyword + + LOG = logging.getLogger(__name__) Command = namedtuple('Command', ['handler', 'help', 'privileged', 'authored']) @@ -412,6 +419,8 @@ def handle_commands(self, job, text, prefix, privileged=False): registered command to be considered a typo). NotPrivileged: when a privileged command call is found and the method is called with privileged=False. + IncorrectSyntax: if an option is used like a command, with + positional arguments. """ raw = text.strip() @@ -445,6 +454,10 @@ def handle_commands(self, job, text, prefix, privileged=False): return raise NotFound(key) if not isinstance(command, Command): + if args: + # An option followed by arguments (e.g. ``/after_pull_request + # 1509``) is not valid: options take ``key=value``. + raise IncorrectSyntax(key) return if command.privileged and not privileged: diff --git a/bert_e/templates/incorrect_command_syntax.md b/bert_e/templates/incorrect_command_syntax.md index d4c1b5e6..b3754e09 100644 --- a/bert_e/templates/incorrect_command_syntax.md +++ b/bert_e/templates/incorrect_command_syntax.md @@ -5,7 +5,7 @@ Incorrect command syntax {% endblock %} {% block message %} -It seems that your command syntax is incorrect. The correct usage is: +It seems that your command syntax is incorrect.{% if extra_message %} {{ extra_message }}{% endif %} The correct usage is: ``` @{{ robot }} option[=argument] diff --git a/bert_e/tests/unit/test_reactor.py b/bert_e/tests/unit/test_reactor.py index dd55dc0b..a7ea59f2 100644 --- a/bert_e/tests/unit/test_reactor.py +++ b/bert_e/tests/unit/test_reactor.py @@ -15,7 +15,8 @@ import pytest -from bert_e.reactor import Command, NotFound, NotPrivileged, Option, Reactor +from bert_e.reactor import (Command, IncorrectSyntax, NotFound, NotPrivileged, + Option, Reactor) # All tests are run on a Reactor subclass to avoid sharing state. @@ -391,3 +392,20 @@ def help(job, *args): assert reactor._has_close_match('gemini') is False assert reactor._has_close_match('copilot') is False assert reactor._has_close_match('other-bot-name') is False + + +def test_handle_commands_option_with_positional_args_raises(reactor_cls, job): + """``/after_pull_request 1509`` must not be silently ignored.""" + + @reactor_cls.option(default=set()) + def after_pull_request(job, pr_id=None): + job.settings['after_pull_request'].add(pr_id) + + reactor = reactor_cls() + + with pytest.raises(IncorrectSyntax) as err: + reactor.handle_commands(job, '/after_pull_request 1509', '@bert-e') + assert err.value.keyword == 'after_pull_request' + + # an option alone is still ignored by handle_commands + reactor.handle_commands(job, '/after_pull_request', '@bert-e') diff --git a/bert_e/workflow/gitwaterflow/__init__.py b/bert_e/workflow/gitwaterflow/__init__.py index 3ad5c435..75c461fb 100644 --- a/bert_e/workflow/gitwaterflow/__init__.py +++ b/bert_e/workflow/gitwaterflow/__init__.py @@ -23,7 +23,8 @@ from bert_e.job import handler, CommitJob, PullRequestJob, QueuesJob from bert_e.lib.cli import confirm from bert_e.lib.simplecmd import CommandError -from bert_e.reactor import Reactor, NotFound, NotPrivileged, NotAuthored +from bert_e.reactor import (Reactor, NotFound, NotPrivileged, NotAuthored, + IncorrectSyntax) from ..git_utils import push, clone_git_repo from ..pr_utils import find_comment, notify_user from .branches import ( @@ -360,6 +361,13 @@ def handle_comments(job): active_options=job.active_options, command=err.keyword, author=author, self_pr=(author == pr_author), comment=text ) from err + except IncorrectSyntax as err: + raise messages.IncorrectCommandSyntax( + extra_message="'%s' is an option, not a command; use " + "'%s=' instead." % ( + err.keyword, err.keyword), + active_options=job.active_options + ) from err def check_commit_diff(job):