Skip to content
Merged
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
9 changes: 7 additions & 2 deletions sopel/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,13 @@ def clean_callable(func, config):

if any(hasattr(func, attr) for attr in ['commands', 'nickname_commands', 'action_commands']):
if hasattr(func, 'example'):
# If no examples are flagged as user-facing, just show the first one like Sopel<7.0 did
examples = [rec["example"] for rec in func.example if rec["help"]] or [func.example[0]["example"]]
# If no examples are flagged as user-facing,
# just show the first one like Sopel<7.0 did
examples = [
rec["example"]
for rec in func.example
if rec["is_help"]
] or [func.example[0]["example"]]
for i, example in enumerate(examples):
example = example.replace('$nickname', nick)
if example[0] != help_prefix and not example.startswith(nick):
Expand Down
56 changes: 28 additions & 28 deletions sopel/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -582,17 +582,16 @@ def command(*command_list: str) -> Callable:

.. note::

You can use a regular expression for the command name(s), but this is
**not recommended** since version 7.1. For backward compatibility,
this behavior will be kept until version 8.0.
The command name will be escaped for use in a regular expression.
As such it is not possible to use something like ``.command\\d+`` to
catch something like ``.command1`` or ``.command2``.

Regex patterns are confusing for your users; please don't use them in
command names!
You have several options at your disposal to replace a regex in the
command name:

If you still want to use a regex pattern, please use the :func:`rule`
decorator instead. For extra arguments and subcommands based on a regex
pattern, you should handle these inside your decorated function, by
using the ``trigger`` object.
* use a command alias
* parse the arguments with your own regex within your plugin callable
* use a :func:`rule` instead

"""
def add_attribute(function):
Expand Down Expand Up @@ -629,15 +628,18 @@ def nickname_command(*command_list: str) -> Callable:

.. note::

You can use a regular expression for the command name(s), but this is
**not recommended** since version 7.1. For backward compatibility,
this behavior will be kept until version 8.0.
The command name will be escaped to be used in a regex command. As such
it is not possible to use something like ``command\\d+`` to catch
something like ``Bot: command1`` or ``Bot: command2``.

Regex patterns are confusing for your users; please don't use them in
command names!
You have several options at your disposal to replace a regex in the
command name:

If you need to use a regex pattern, please use the :func:`rule`
decorator instead, with the ``$nick`` variable::
* use a command alias
* parse the arguments with your own regex within your plugin callable
* use a :func:`rule`

The :func:`rule` can be used with a ``$nick`` variable::

@rule(r'$nick .*')
# Would trigger on anything starting with "$nickname[:,]? ",
Expand Down Expand Up @@ -679,15 +681,18 @@ def action_command(*command_list: str) -> Callable:

.. note::

You can use a regular expression for the command name(s), but this is
**not recommended** since version 7.1. For backward compatibility,
this behavior will be kept until version 8.0.
The command name will be escaped for use in a regular expression.
As such it is not possible to use something like ``/me command\\d+``
to catch something like ``/me command1`` or ``/me command2``.

You have several options at your disposal to replace a regex in the
command name:

Regex patterns are confusing for your users; please don't use them in
command names!
* use a command alias
* parse the arguments with your own regex within your plugin callable
* use a :func:`rule`

If you need to use a regex pattern, please use the :func:`rule`
decorator instead, with the :func:`ctcp` decorator::
The :func:`rule` must be used with the :func:`ctcp` decorator::

@rule(r'hello!?')
@ctcp('ACTION')
Expand Down Expand Up @@ -1586,11 +1591,6 @@ def __call__(self, func):
"is_pattern": self.use_re,
"is_admin": self.admin,
"is_owner": self.owner,
# old-style flags
# TODO: to be removed in Sopel 8.0
"privmsg": self.privmsg,
"admin": self.admin,
"help": self.user_help,
}
func.example.append(record)
return func
Expand Down
47 changes: 6 additions & 41 deletions sopel/plugins/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -1249,41 +1249,6 @@ def has_alias(self, name):
"""
return name in self._aliases

def escape_name(self, name):
"""Escape the provided name if needed.

.. note::

Until now, Sopel has allowed command name to be regex pattern.
It was mentioned in the documentation without much details, and
there were no tests for it.

In order to ensure backward compatibility with previous versions of
Sopel, we make sure to escape command name only when it's needed.

**It is not recommended to use a regex pattern for your command
name. This feature will be removed in Sopel 8.0.**

"""
if set('.^$*+?{}[]\\|()') & set(name):
# the name contains a regex pattern special character
# we assume the user knows what they are doing
try:
# make sure it compiles properly
# (nobody knows what they are doing)
re.compile(name)
except re.error as error:
original_name = name
name = re.escape(name)
LOGGER.warning(
'Command name "%s" is an invalid regular expression '
'and will be replaced by "%s": %s',
original_name, name, error)
else:
name = re.escape(name)

return name

@abc.abstractmethod
def get_rule_regex(self):
"""Make the rule regex for this named rule.
Expand Down Expand Up @@ -1403,8 +1368,8 @@ def get_rule_regex(self):

to create a compiled regex to return.
"""
name = [self.escape_name(self._name)]
aliases = [self.escape_name(alias) for alias in self._aliases]
name = [re.escape(self._name)]
aliases = [re.escape(alias) for alias in self._aliases]
pattern = r'|'.join(name + aliases)

# Escape all whitespace with a single backslash.
Expand Down Expand Up @@ -1525,8 +1490,8 @@ def get_rule_regex(self):

to create a compiled regex to return.
"""
name = [self.escape_name(self._name)]
aliases = [self.escape_name(alias) for alias in self._aliases]
name = [re.escape(self._name)]
aliases = [re.escape(alias) for alias in self._aliases]
pattern = r'|'.join(name + aliases)

return _compile_pattern(
Expand Down Expand Up @@ -1610,8 +1575,8 @@ def get_rule_regex(self):

to create a compiled regex to return.
"""
name = [self.escape_name(self._name)]
aliases = [self.escape_name(alias) for alias in self._aliases]
name = [re.escape(self._name)]
aliases = [re.escape(alias) for alias in self._aliases]
pattern = r'|'.join(name + aliases)
pattern = self.PATTERN_TEMPLATE.format(command=pattern)
return re.compile(pattern, re.IGNORECASE | re.VERBOSE)
Expand Down
78 changes: 33 additions & 45 deletions test/plugins/test_plugins_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -2125,10 +2125,7 @@ def handler(wrapped, trigger):
assert result.group(1) == 'reverse'


def test_command_from_callable_regex_pattern(mockbot):
# TODO: this test must FAIL for Sopel 8.0
# Command name as regex pattern will be removed in Sopel 8.0

def test_command_from_callable_escaped_regex_pattern(mockbot):
# prepare callable
@plugin.commands('main .*')
def handler(wrapped, trigger):
Expand All @@ -2139,21 +2136,23 @@ def handler(wrapped, trigger):
# create rule from a cleaned callable
rule = rules.Command.from_callable(mockbot.settings, handler)

# match on ".main anything"
# does not match on ".main anything"
line = ':[email protected] PRIVMSG #sopel :.main anything'
pretrigger = trigger.PreTrigger(mockbot.nick, line)
results = list(rule.match(mockbot, pretrigger))

assert not results, 'Regex commands are not allowed since Sopel 8.0'

# match on ".main .*"
line = ':[email protected] PRIVMSG #sopel :.main .*'
pretrigger = trigger.PreTrigger(mockbot.nick, line)
results = list(rule.match(mockbot, pretrigger))

assert len(results) == 1, (
'Exactly 1 command must match; MUST fail for Sopel 8.0')
'Command name must be escaped to get an exact match')
result = results[0]
assert result.group(0) == '.main anything'
assert result.group(1) == 'main anything'
assert result.group(2) is None
assert result.group(3) is None
assert result.group(4) is None
assert result.group(5) is None
assert result.group(6) is None
assert result.group(0) == '.main .*'
assert result.group(1) == 'main .*'


def test_command_from_callable_invalid(mockbot):
Expand All @@ -2169,21 +2168,6 @@ def handler(wrapped, trigger):
rules.Command.from_callable(mockbot.settings, handler)


def test_command_escape_name():
rule = rules.Command('hello', r'\.', plugin='testplugin')

assert rule.escape_name('hello') == 'hello'
assert rule.escape_name('hello world') == r'hello\ world'
assert rule.escape_name(r'hello\ world') == r'hello\ world', (
'Valid pattern must not be escaped')
assert rule.escape_name(r'.*') == r'.*', (
'Valid pattern must not be escaped')
assert rule.escape_name(r'a[bc]d') == r'a[bc]d', (
'Valid pattern must not be escaped')
assert rule.escape_name(r'hello(') == r'hello\(', (
'Invalid pattern must be escaped')


# -----------------------------------------------------------------------------
# tests for :class:`sopel.plugins.rules.NickCommand`

Expand Down Expand Up @@ -2444,10 +2428,6 @@ def handler(wrapped, trigger):


def test_nick_command_from_callable_regex_pattern(mockbot):
# TODO: this test must FAIL for Sopel 8.0
# Command name as regex pattern will be removed in Sopel 8.0

# prepare callable
@plugin.nickname_commands('do .*')
def handler(wrapped, trigger):
wrapped.reply('Hi!')
Expand All @@ -2457,16 +2437,21 @@ def handler(wrapped, trigger):
# create rule from a cleaned callable
rule = rules.NickCommand.from_callable(mockbot.settings, handler)

# match on ".main anything"
# does not match on ".do anything"
line = ':[email protected] PRIVMSG #sopel :TestBot: do anything'
pretrigger = trigger.PreTrigger(mockbot.nick, line)
results = list(rule.match(mockbot, pretrigger))

assert len(results) == 1, (
'Exactly 1 command must match; MUST fail for Sopel 8.0')
assert not results, 'Regex commands are not allowed since Sopel 8.0'

# match on ".do .*"
line = ':[email protected] PRIVMSG #sopel :TestBot: do .*'
pretrigger = trigger.PreTrigger(mockbot.nick, line)
results = list(rule.match(mockbot, pretrigger))
assert len(results) == 1, 'Exactly 1 command must match'
result = results[0]
assert result.group(0) == 'TestBot: do anything'
assert result.group(1) == 'do anything'
assert result.group(0) == 'TestBot: do .*'
assert result.group(1) == 'do .*'
assert result.group(2) is None
assert result.group(3) is None
assert result.group(4) is None
Expand Down Expand Up @@ -2649,9 +2634,6 @@ def handler(wrapped, trigger):


def test_action_command_from_callable_regex_pattern(mockbot):
# TODO: this test must FAIL for Sopel 8.0
# Command name as regex pattern will be removed in Sopel 8.0

# prepare callable
@plugin.action_commands('do .*')
def handler(wrapped, trigger):
Expand All @@ -2662,16 +2644,22 @@ def handler(wrapped, trigger):
# create rule from a cleaned callable
rule = rules.ActionCommand.from_callable(mockbot.settings, handler)

# match on ".main anything"
# does not match on ".do anything"
line = ':[email protected] PRIVMSG #sopel :\x01ACTION do anything\x01'
pretrigger = trigger.PreTrigger(mockbot.nick, line)
results = list(rule.match(mockbot, pretrigger))

assert len(results) == 1, (
'Exactly 1 command must match; MUST fail for Sopel 8.0')
assert not results, 'Regex commands are not allowed since Sopel 8.0'

# match on ".do .*"
line = ':[email protected] PRIVMSG #sopel :\x01ACTION do .*\x01'
pretrigger = trigger.PreTrigger(mockbot.nick, line)
results = list(rule.match(mockbot, pretrigger))

assert len(results) == 1, 'Exactly 1 command must match'
result = results[0]
assert result.group(0) == 'do anything'
assert result.group(1) == 'do anything'
assert result.group(0) == 'do .*'
assert result.group(1) == 'do .*'
assert result.group(2) is None
assert result.group(3) is None
assert result.group(4) is None
Expand Down