From f60333e58ab1cc8daac87e3261c43d856833c647 Mon Sep 17 00:00:00 2001 From: dgw Date: Sat, 31 Dec 2022 14:48:06 -0600 Subject: [PATCH 1/6] tools: versionadded annotations, plus a bonus `.. deprecated::` I forgot to put `.. deprecated:: 8.0` somewhere when deprecating the `OutputRedirect` class, last time I took a swing at this module. --- sopel/tools/__init__.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/sopel/tools/__init__.py b/sopel/tools/__init__.py index 186a991a1..92ff2ca80 100644 --- a/sopel/tools/__init__.py +++ b/sopel/tools/__init__.py @@ -1,6 +1,6 @@ """Useful miscellaneous tools and shortcuts for Sopel plugins -*Availability: 3+* +.. versionadded:: 3.0 """ # tools.py - Sopel misc tools @@ -68,8 +68,7 @@ def get_input(prompt): .. deprecated:: 7.1 - Use of this function will become a warning when Python 2 support is - dropped in Sopel 8.0. The function will be removed in Sopel 8.1. + This function will be removed in Sopel 8.1. """ return input(prompt) @@ -116,6 +115,11 @@ class OutputRedirect: """Redirect the output to the terminal and a log file. A simplified object used to write to both the terminal and a log file. + + .. deprecated:: 8.0 + + Vestige of old logging system. Will be removed in Sopel 8.1. + """ @deprecated( @@ -200,6 +204,8 @@ def get_hostmask_regex(mask): :param str mask: the hostmask that the pattern should match :return: a compiled regex pattern matching the given ``mask`` :rtype: :ref:`re.Pattern ` + + .. versionadded:: 4.4 """ mask = re.escape(mask) mask = mask.replace(r'\*', '.*') @@ -244,6 +250,8 @@ def chain_loaders(*lazy_loaders): together into one. It's primarily a helper for lazy rule decorators such as :func:`sopel.plugin.url_lazy`. + .. versionadded:: 7.1 + .. important:: This function doesn't check the uniqueness of regexes generated by From 6142f273f85b8920eab2717554cd6ccad4e42b85 Mon Sep 17 00:00:00 2001 From: dgw Date: Sat, 31 Dec 2022 15:14:34 -0600 Subject: [PATCH 2/6] tools.calculation, docs: version annotations & missing docstring Sopelunked into the history and found when all this stuff was originally added to the codebase. (Spoiler: A long time ago.) There was no docstring at all for `EquationEvaluator`, so I wrote one. Tweaked the docs page for `tools.calculation` to show everything instead of only what's listed in `__all__`, and noted that most of the module is considered internal machinery as we've been doing in `irc` etc. NOTE: I would like the "public function" to appear at the top, but could not find a nice way to do that in ~15 min of googling+experimenting. --- docs/source/package/tools/calculation.rst | 1 + sopel/tools/calculation.py | 53 ++++++++++++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/docs/source/package/tools/calculation.rst b/docs/source/package/tools/calculation.rst index 0bd2392a8..87ac1e085 100644 --- a/docs/source/package/tools/calculation.rst +++ b/docs/source/package/tools/calculation.rst @@ -4,3 +4,4 @@ sopel.tools.calculation .. automodule:: sopel.tools.calculation :members: + :ignore-module-all: diff --git a/sopel/tools/calculation.py b/sopel/tools/calculation.py index e32111582..0d4262697 100644 --- a/sopel/tools/calculation.py +++ b/sopel/tools/calculation.py @@ -1,4 +1,12 @@ -"""Tools to help safely do calculations from user input""" +"""Tools to help safely do calculations from user input + +.. versionadded:: 5.3 +.. note:: + + Most of this is internal machinery. :func:`eval_equation` is the "public" + part, used by Sopel's built-in ``calc`` plugin. + +""" from __future__ import annotations import ast @@ -18,6 +26,12 @@ class ExpressionEvaluator: Instances can pass ``binary_ops`` and ``unary_ops`` arguments with dicts of the form ``{ast.Node, function}``. When the :class:`ast.Node ` used as key is found, it will be evaluated using the given ``function``. + + .. versionadded:: 4.1 + .. versionchanged:: 5.3 + + Moved from :mod:`.tools` to :mod:`.tools.calculation`. + """ class Error(Exception): @@ -89,6 +103,12 @@ def guarded_mul(left, right): :param right: the right operand :type right: int or float :raise ValueError: if the inputs are too large to handle safely + + .. versionadded:: 4.5 + .. versionchanged:: 5.3 + + Moved from :mod:`.tools` to :mod:`.tools.calculation`. + """ # Only handle ints because floats will overflow anyway. if not isinstance(left, numbers.Integral): @@ -166,6 +186,12 @@ def pow_complexity(num, exp): accurate results outside these boundaries. The results derived from large ``num`` and ``exp`` were quite accurate for small ``num`` and very large ``exp`` though, except when ``num`` was a power of 2. + + .. versionadded:: 4.5 + .. versionchanged:: 5.3 + + Moved from :mod:`.tools` to :mod:`.tools.calculation`. + """ if num in (0, 1) or exp in (0, 1): return 0 @@ -184,6 +210,12 @@ def guarded_pow(num, exp): :param exp: exponent :type exp: int or float :raise ValueError: if the inputs are too large to handle safely + + .. versionadded:: 4.5 + .. versionchanged:: 5.3 + + Moved from :mod:`.tools` to :mod:`.tools.calculation`. + """ # Only handle ints because floats will overflow anyway. if not isinstance(num, numbers.Integral): @@ -201,6 +233,19 @@ def guarded_pow(num, exp): class EquationEvaluator(ExpressionEvaluator): + """Specific subclass of :class:`ExpressionEvaluator` for simple math + + This presets the allowed operators to safeguard against user input that + could try to do things that will adversely affect the running bot, while + still letting users pass arbitrary mathematical expressions using the + available (mostly arithmetic) operators. + + .. versionadded:: 4.5 + .. versionchanged:: 5.3 + + Moved from :mod:`.tools` to :mod:`.tools.calculation`. + + """ __bin_ops = { ast.Add: operator.add, ast.Sub: operator.sub, @@ -241,4 +286,10 @@ def __call__(self, expression_str): Supports addition (+), subtraction (-), multiplication (*), division (/), power (**) and modulo (%). + +.. versionadded:: 4.1 +.. versionchanged:: 5.3 + + Moved from :mod:`.tools` to :mod:`.tools.calculation`. + """ From 8bf37a6ca3bdcf895f0e87057e7203124b296b26 Mon Sep 17 00:00:00 2001 From: dgw Date: Sat, 31 Dec 2022 15:34:41 -0600 Subject: [PATCH 3/6] tools.calculation: we don't need the precise history of such old stuff Everything in this module can be considered "added" at the same time as the module itself, in v5.3. Even going back that far crosses a rename boundary (from `willie` to `sopel`) and it's likely a fool's errand to try and write a plugin that functions equally well under Sopel 8.0-dev, 7.x, and everything else back to 4.x. In the immortal words of Lisa Landry, "Let it go, Ray." (But this "undo" operation is kept as a separate commit in case other maintainers feel differently.) --- sopel/tools/calculation.py | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/sopel/tools/calculation.py b/sopel/tools/calculation.py index 0d4262697..ce9de6b58 100644 --- a/sopel/tools/calculation.py +++ b/sopel/tools/calculation.py @@ -26,12 +26,6 @@ class ExpressionEvaluator: Instances can pass ``binary_ops`` and ``unary_ops`` arguments with dicts of the form ``{ast.Node, function}``. When the :class:`ast.Node ` used as key is found, it will be evaluated using the given ``function``. - - .. versionadded:: 4.1 - .. versionchanged:: 5.3 - - Moved from :mod:`.tools` to :mod:`.tools.calculation`. - """ class Error(Exception): @@ -103,12 +97,6 @@ def guarded_mul(left, right): :param right: the right operand :type right: int or float :raise ValueError: if the inputs are too large to handle safely - - .. versionadded:: 4.5 - .. versionchanged:: 5.3 - - Moved from :mod:`.tools` to :mod:`.tools.calculation`. - """ # Only handle ints because floats will overflow anyway. if not isinstance(left, numbers.Integral): @@ -186,12 +174,6 @@ def pow_complexity(num, exp): accurate results outside these boundaries. The results derived from large ``num`` and ``exp`` were quite accurate for small ``num`` and very large ``exp`` though, except when ``num`` was a power of 2. - - .. versionadded:: 4.5 - .. versionchanged:: 5.3 - - Moved from :mod:`.tools` to :mod:`.tools.calculation`. - """ if num in (0, 1) or exp in (0, 1): return 0 @@ -210,12 +192,6 @@ def guarded_pow(num, exp): :param exp: exponent :type exp: int or float :raise ValueError: if the inputs are too large to handle safely - - .. versionadded:: 4.5 - .. versionchanged:: 5.3 - - Moved from :mod:`.tools` to :mod:`.tools.calculation`. - """ # Only handle ints because floats will overflow anyway. if not isinstance(num, numbers.Integral): @@ -239,12 +215,6 @@ class EquationEvaluator(ExpressionEvaluator): could try to do things that will adversely affect the running bot, while still letting users pass arbitrary mathematical expressions using the available (mostly arithmetic) operators. - - .. versionadded:: 4.5 - .. versionchanged:: 5.3 - - Moved from :mod:`.tools` to :mod:`.tools.calculation`. - """ __bin_ops = { ast.Add: operator.add, @@ -286,10 +256,4 @@ def __call__(self, expression_str): Supports addition (+), subtraction (-), multiplication (*), division (/), power (**) and modulo (%). - -.. versionadded:: 4.1 -.. versionchanged:: 5.3 - - Moved from :mod:`.tools` to :mod:`.tools.calculation`. - """ From e55a15a880b0fd3997e005f03073cef2360d826e Mon Sep 17 00:00:00 2001 From: dgw Date: Sat, 28 Oct 2023 09:19:43 -0500 Subject: [PATCH 4/6] tools.calculation: type annotations + minor corrections to API Contrary to its parameter descriptions, `pow_complexity()` was NOT able to handle `float` inputs. The `&` operator does not work with floats, nor do floats have a `bit_length()` method. `EquationEvaluator.__call__()` now takes an optional `timeout` argument just like its upstream counterpart in `ExpressionEvaluator`. --- sopel/tools/calculation.py | 42 +++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/sopel/tools/calculation.py b/sopel/tools/calculation.py index ce9de6b58..1d831027f 100644 --- a/sopel/tools/calculation.py +++ b/sopel/tools/calculation.py @@ -13,6 +13,10 @@ import numbers import operator import time +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Callable, Optional __all__ = ['eval_equation'] @@ -30,16 +34,20 @@ class ExpressionEvaluator: class Error(Exception): """Internal exception type for :class:`ExpressionEvaluator`\\s.""" - pass - def __init__(self, bin_ops=None, unary_ops=None): + def __init__( + self, + bin_ops: Optional[dict[type[ast.operator], Callable]] = None, + unary_ops: Optional[dict[type[ast.unaryop], Callable]] = None + ): self.binary_ops = bin_ops or {} self.unary_ops = unary_ops or {} - def __call__(self, expression_str, timeout=5.0): + def __call__(self, expression_str: str, timeout: float = 5.0): """Evaluate a Python expression and return the result. - :param str expression_str: the expression to evaluate + :param expression_str: the expression to evaluate + :param timeout: timeout for processing the expression, in seconds :raise SyntaxError: if the given ``expression_str`` is not a valid Python statement :raise ExpressionEvaluator.Error: if the instance of @@ -49,14 +57,12 @@ def __call__(self, expression_str, timeout=5.0): ast_expression = ast.parse(expression_str, mode='eval') return self._eval_node(ast_expression.body, time.time() + timeout) - def _eval_node(self, node, timeout): + def _eval_node(self, node: ast.AST, timeout: float): """Recursively evaluate the given :class:`ast.Node `. :param node: the AST node to evaluate - :type node: :class:`ast.AST` :param timeout: how long the expression is allowed to process before - timing out and failing - :type timeout: int or float + timing out and failing, in seconds :raise ExpressionEvaluator.Error: if it can't handle the ``node`` Uses :attr:`self.binary_ops` and :attr:`self.unary_ops` for the @@ -89,13 +95,11 @@ def _eval_node(self, node, timeout): "Ast.Node '%s' not implemented." % (type(node).__name__,)) -def guarded_mul(left, right): +def guarded_mul(left: float, right: float): """Multiply two values, guarding against overly large inputs. :param left: the left operand - :type left: int or float :param right: the right operand - :type right: int or float :raise ValueError: if the inputs are too large to handle safely """ # Only handle ints because floats will overflow anyway. @@ -116,13 +120,11 @@ def guarded_mul(left, right): return operator.mul(left, right) -def pow_complexity(num, exp): +def pow_complexity(num: int, exp: int): """Estimate the worst case time :func:`pow` takes to calculate. :param num: base - :type num: int or float :param exp: exponent - :type exp: int or float This function is based on experimental data from the time it takes to calculate ``num**exp`` in 32-bit CPython 2.7.6 on an Intel Core i7-2670QM @@ -184,13 +186,11 @@ def pow_complexity(num, exp): return exp ** 1.590 * num.bit_length() ** 1.73 / 36864057619.3 -def guarded_pow(num, exp): +def guarded_pow(num: float, exp: float): """Raise a number to a power, guarding against overly large inputs. :param num: base - :type num: int or float :param exp: exponent - :type exp: int or float :raise ValueError: if the inputs are too large to handle safely """ # Only handle ints because floats will overflow anyway. @@ -216,7 +216,7 @@ class EquationEvaluator(ExpressionEvaluator): still letting users pass arbitrary mathematical expressions using the available (mostly arithmetic) operators. """ - __bin_ops = { + __bin_ops: dict[type[ast.operator], Callable] = { ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: guarded_mul, @@ -226,7 +226,7 @@ class EquationEvaluator(ExpressionEvaluator): ast.FloorDiv: operator.floordiv, ast.BitXor: guarded_pow } - __unary_ops = { + __unary_ops: dict[type[ast.unaryop], Callable] = { ast.USub: operator.neg, ast.UAdd: operator.pos, } @@ -238,8 +238,8 @@ def __init__(self): unary_ops=self.__unary_ops ) - def __call__(self, expression_str): - result = ExpressionEvaluator.__call__(self, expression_str) + def __call__(self, expression_str: str, timeout: float = 5.0): + result = ExpressionEvaluator.__call__(self, expression_str, timeout) # This wrapper is here so additional sanity checks could be done # on the result of the eval, but currently none are done. From 15849f651faea42b356aebc546d57ebf003c97c8 Mon Sep 17 00:00:00 2001 From: James Gerity Date: Sat, 12 Aug 2023 21:05:00 -0400 Subject: [PATCH 5/6] test: add basic tests for `tools.calculation` module Co-authored-by: dgw --- test/tools/test_tools_calculation.py | 38 ++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 test/tools/test_tools_calculation.py diff --git a/test/tools/test_tools_calculation.py b/test/tools/test_tools_calculation.py new file mode 100644 index 000000000..5744b0bd0 --- /dev/null +++ b/test/tools/test_tools_calculation.py @@ -0,0 +1,38 @@ +"""Tests Sopel's calculation tools""" +from __future__ import annotations + +import ast +import operator + +import pytest + +from sopel.tools.calculation import EquationEvaluator, ExpressionEvaluator + + +def test_expression_eval(): + """Ensure ExpressionEvaluator respects limited operator set.""" + OPS = { + ast.Add: operator.add, + ast.Sub: operator.sub, + } + evaluator = ExpressionEvaluator(bin_ops=OPS) + + assert evaluator("1 + 1") == 2 + assert evaluator("43 - 1") == 42 + assert evaluator("1 + 1 - 2") == 0 + + with pytest.raises(ExpressionEvaluator.Error): + evaluator("2 * 2") + + +def test_equation_eval(): + """Test that EquationEvaluator correctly parses input and calculates results.""" + evaluator = EquationEvaluator() + + assert evaluator("1 + 1") == 2 + assert evaluator("43 - 1") == 42 + assert evaluator("(((1 + 1 + 2) * 3 / 5) ** 8 - 13) // 21 % 35") == 16.0 + assert evaluator("-42") == -42 + assert evaluator("-(-42)") == 42 + assert evaluator("+42") == 42 + assert evaluator("3 ^ 2") == 9 From 3c4e477530b89314fcef7128f4c93c3a365b3103 Mon Sep 17 00:00:00 2001 From: dgw Date: Thu, 2 Nov 2023 21:56:23 -0500 Subject: [PATCH 6/6] test: cover remainder of `tools.calculation` module Nasty assertions on exception args... Really need to refactor that silly ExpressionEvaluator.Error type into some sensible PUBLIC hierarchy. --- test/tools/test_tools_calculation.py | 32 +++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/test/tools/test_tools_calculation.py b/test/tools/test_tools_calculation.py index 5744b0bd0..e211d584c 100644 --- a/test/tools/test_tools_calculation.py +++ b/test/tools/test_tools_calculation.py @@ -21,8 +21,38 @@ def test_expression_eval(): assert evaluator("43 - 1") == 42 assert evaluator("1 + 1 - 2") == 0 - with pytest.raises(ExpressionEvaluator.Error): + with pytest.raises(ExpressionEvaluator.Error) as exc: evaluator("2 * 2") + assert "Unsupported binary operator" in exc.value.args[0] + + with pytest.raises(ExpressionEvaluator.Error) as exc: + evaluator("~2") + assert "Unsupported unary operator" in exc.value.args[0] + + +def test_equation_eval_invalid_constant(): + """Ensure unsupported constants are rejected.""" + evaluator = EquationEvaluator() + + with pytest.raises(ExpressionEvaluator.Error) as exc: + evaluator("2 + 'string'") + assert "values are not supported" in exc.value.args[0] + + +def test_equation_eval_timeout(): + """Ensure EquationEvaluator times out as expected.""" + # timeout is added to the current time; + # negative means the timeout is "reached" before even starting + timeout = -1.0 + evaluator = EquationEvaluator() + + with pytest.raises(ExpressionEvaluator.Error) as exc: + evaluator("1000000**100", timeout) + assert "Time for evaluating" in exc.value.args[0] + + with pytest.raises(ExpressionEvaluator.Error) as exc: + evaluator("+42", timeout) + assert "Time for evaluating" in exc.value.args[0] def test_equation_eval():