Improve timedelta conversions (#6349)

Co-authored-by: zephyrkul <zephyrkul@users.noreply.github.com>

redbot/cogs/mutes/converters.py

@@ -1,63 +1,76 @@
+from __future__ import annotations
+
 import logging
 import re
-from typing import Union, Dict
+from typing import Optional, TypedDict
 from datetime import timedelta
+from typing_extensions import Annotated
 
 from discord.ext.commands.converter import Converter
 from redbot.core import commands
 from redbot.core import i18n
-
+from redbot.core.commands.converter import TIME_RE
-log = logging.getLogger("red.cogs.mutes")
-
-# the following regex is slightly modified from Red
-# it's changed to be slightly more strict on matching with finditer
-# this is to prevent "empty" matches when parsing the full reason
-# This is also designed more to allow time interval at the beginning or the end of the mute
-# to account for those times when you think of adding time *after* already typing out the reason
-# https://github.com/Cog-Creators/Red-DiscordBot/blob/V3/develop/redbot/core/commands/converter.py#L55
-TIME_RE_STRING = r"|".join(
-    [
-        r"((?P<weeks>\d+?)\s?(weeks?|w))",
-        r"((?P<days>\d+?)\s?(days?|d))",
-        r"((?P<hours>\d+?)\s?(hours?|hrs|hr?))",
-        r"((?P<minutes>\d+?)\s?(minutes?|mins?|m(?!o)))",  # prevent matching "months"
-        r"((?P<seconds>\d+?)\s?(seconds?|secs?|s))",
-    ]
-)
-TIME_RE = re.compile(TIME_RE_STRING, re.I)
-TIME_SPLIT = re.compile(r"t(?:ime)?=")
 
 _ = i18n.Translator("Mutes", __file__)
+log = logging.getLogger("red.cogs.mutes")
+
+TIME_SPLIT = re.compile(r"t(?:ime\s?)?=\s*")
 
 
-class MuteTime(Converter):
+def _edgematch(pattern: re.Pattern[str], argument: str) -> Optional[re.Match[str]]:
+    """Internal utility to match at either end of the argument string"""
+    # precondition: pattern does not end in $
+    # precondition: argument does not end in whitespace
+    return pattern.match(argument) or re.search(
+        pattern.pattern + "$", argument, flags=pattern.flags
+    )
+
+
+class _MuteTime(TypedDict, total=False):
+    duration: timedelta
+    reason: str
+
+
+class _MuteTimeConverter(Converter):
     """
     This will parse my defined multi response pattern and provide usable formats
     to be used in multiple responses
     """
 
-    async def convert(
+    async def convert(self, ctx: commands.Context, argument: str) -> _MuteTime:
-        self, ctx: commands.Context, argument: str
+        time_split = TIME_SPLIT.search(argument)
-    ) -> Dict[str, Union[timedelta, str, None]]:
+        result: _MuteTime = {}
-        time_split = TIME_SPLIT.split(argument)
-        result: Dict[str, Union[timedelta, str, None]] = {}
         if time_split:
-            maybe_time = time_split[-1]
+            maybe_time = argument[time_split.end() :]
+            strategy = re.match
         else:
             maybe_time = argument
+            strategy = _edgematch
 
-        time_data = {}
+        match = strategy(TIME_RE, maybe_time)
-        for time in TIME_RE.finditer(maybe_time):
+        if match:
-            argument = argument.replace(time[0], "")
+            time_data = {k: int(v) for k, v in match.groupdict().items() if v is not None}
-            for k, v in time.groupdict().items():
+            for k in time_data:
-                if v:
+                if k in ("years", "months"):
-                    time_data[k] = int(v)
+                    raise commands.BadArgument(
-        if time_data:
+                        _("`{unit}` is not a valid unit of time for this command").format(unit=k)
+                    )
             try:
-                result["duration"] = timedelta(**time_data)
+                result["duration"] = duration = timedelta(**time_data)
             except OverflowError:
                 raise commands.BadArgument(
                     _("The time provided is too long; use a more reasonable time.")
                 )
+            if duration <= timedelta(seconds=0):
+                raise commands.BadArgument(_("The time provided must not be in the past."))
+            if time_split:
+                start, end = time_split.span()
+                end += match.end()
+            else:
+                start, end = match.span()
+            argument = argument[:start] + argument[end:]
         result["reason"] = argument.strip()
         return result
+
+
+MuteTime = Annotated[_MuteTime, _MuteTimeConverter]

redbot/core/commands/converter.py

@@ -57,20 +57,22 @@ USER_MENTION_REGEX = re.compile(r"<@!?([0-9]{15,21})>$")
 
 # Taken with permission from
 # https://github.com/mikeshardmind/SinbadCogs/blob/816f3bc2ba860243f75112904b82009a8a9e1f99/scheduler/time_utils.py#L9-L19
-TIME_RE_STRING = r"\s?".join(
+# with modifications
-    [
+TIME_RE = re.compile(
-        r"((?P<years>\d+?)\s?(years?|y))?",
+    r"""
-        r"((?P<months>\d+?)\s?(months?|mo))?",
+        (\s?(  # match deliminators here to make word border below unambiguous
-        r"((?P<weeks>\d+?)\s?(weeks?|w))?",
+            (?P<years>[\+-]?\d+)\s?(years?|y)
-        r"((?P<days>\d+?)\s?(days?|d))?",
+          | (?P<months>[\+-]?\d+)\s?(months?|mo)
-        r"((?P<hours>\d+?)\s?(hours?|hrs|hr?))?",
+          | (?P<weeks>[\+-]?\d+)\s?(weeks?|w)
-        r"((?P<minutes>\d+?)\s?(minutes?|mins?|m(?!o)))?",  # prevent matching "months"
+          | (?P<days>[\+-]?\d+)\s?(days?|d)
-        r"((?P<seconds>\d+?)\s?(seconds?|secs?|s))?",
+          | (?P<hours>[\+-]?\d+)\s?(hours?|hrs|hr?)
-    ]
+          | (?P<minutes>[\+-]?\d+)\s?(minutes?|mins?|m)
+          | (?P<seconds>[\+-]?\d+)\s?(seconds?|secs?|s)
+        ))+\b
+    """,
+    flags=re.IGNORECASE | re.VERBOSE,
 )
 
-TIME_RE = re.compile(TIME_RE_STRING, re.I)
-
 
 def _parse_and_match(string_to_match: str, allowed_units: List[str]) -> Optional[Dict[str, int]]:
     """
@@ -92,13 +94,13 @@ def parse_timedelta(
     argument: str,
     *,
     maximum: Optional[timedelta] = None,
-    minimum: Optional[timedelta] = None,
+    minimum: Optional[timedelta] = timedelta(seconds=0),
     allowed_units: Optional[List[str]] = None,
 ) -> Optional[timedelta]:
     """
     This converts a user provided string into a timedelta
 
-    The units should be in order from largest to smallest.
+    If a unit is specified multiple times, only the last is considered.
     This works with or without whitespace.
 
     Parameters
@@ -109,6 +111,7 @@ def parse_timedelta(
         If provided, any parsed value higher than this will raise an exception
     minimum : Optional[datetime.timedelta]
         If provided, any parsed value lower than this will raise an exception
+        Defaults to 0 seconds, pass None explicitly to allow negative values
     allowed_units : Optional[List[str]]
         If provided, you can constrain a user to expressing the amount of time
         in specific units. The units you can chose to provide are the same as the
@@ -162,7 +165,7 @@ def parse_relativedelta(
     """
     This converts a user provided string into a datetime with offset from NOW
 
-    The units should be in order from largest to smallest.
+    If a unit is specified multiple times, only the last is considered.
     This works with or without whitespace.
 
     Parameters