fix: resolve issue #284

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:

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]

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')

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=<value>' instead." % (
+                                  err.keyword, err.keyword),
+                active_options=job.active_options
+            ) from err
 
 
 def check_commit_diff(job):