From 033d0113a510e4604d37c602b1b662d47f62fed2 Mon Sep 17 00:00:00 2001
From: Tobotimus <Tobotimus@users.noreply.github.com>
Date: Sat, 9 Jun 2018 11:20:40 +1000
Subject: [PATCH] [V3] Send meaningful responses on conversion failure (#1817)

* [V3] Send meaningful responses on conversion failures

* Replace existing `discord.ext.commands` imports

Just to be sure

* Better Permissions converter response
---
 redbot/cogs/admin/admin.py             |  3 +-
 redbot/cogs/admin/announcer.py         |  2 +-
 redbot/cogs/admin/converters.py        |  2 +-
 redbot/cogs/alias/__init__.py          |  4 +--
 redbot/cogs/audio/__init__.py          |  2 +-
 redbot/cogs/downloader/converters.py   |  3 +-
 redbot/cogs/downloader/repo_manager.py | 14 ++++-----
 redbot/cogs/mod/checks.py              |  2 +-
 redbot/cogs/permissions/converters.py  |  9 ++++--
 redbot/cogs/trivia/trivia.py           |  2 +-
 redbot/core/checks.py                  |  2 +-
 redbot/core/commands/__init__.py       |  1 +
 redbot/core/commands/commands.py       | 41 +++++++++++++++++++++++++-
 redbot/core/commands/errors.py         | 13 ++++++++
 redbot/core/events.py                  |  8 +++--
 15 files changed, 82 insertions(+), 26 deletions(-)
 create mode 100644 redbot/core/commands/errors.py

diff --git a/redbot/cogs/admin/admin.py b/redbot/cogs/admin/admin.py
index 2b5c45e44..a513027cd 100644
--- a/redbot/cogs/admin/admin.py
+++ b/redbot/cogs/admin/admin.py
@@ -1,9 +1,8 @@
 from typing import Tuple
 
 import discord
-from discord.ext import commands
 
-from redbot.core import Config, checks
+from redbot.core import Config, checks, commands
 
 import logging
 
diff --git a/redbot/cogs/admin/announcer.py b/redbot/cogs/admin/announcer.py
index 9ee9cffbc..8c02b6e10 100644
--- a/redbot/cogs/admin/announcer.py
+++ b/redbot/cogs/admin/announcer.py
@@ -1,7 +1,7 @@
 import asyncio
 
 import discord
-from discord.ext import commands
+from redbot.core import commands
 
 
 class Announcer:
diff --git a/redbot/cogs/admin/converters.py b/redbot/cogs/admin/converters.py
index 875927f22..f507d975f 100644
--- a/redbot/cogs/admin/converters.py
+++ b/redbot/cogs/admin/converters.py
@@ -1,5 +1,5 @@
 import discord
-from discord.ext import commands
+from redbot.core import commands
 
 
 class MemberDefaultAuthor(commands.Converter):
diff --git a/redbot/cogs/alias/__init__.py b/redbot/cogs/alias/__init__.py
index b1b8587ca..df7b4900e 100644
--- a/redbot/cogs/alias/__init__.py
+++ b/redbot/cogs/alias/__init__.py
@@ -1,6 +1,6 @@
 from .alias import Alias
-from discord.ext import commands
+from redbot.core.bot import Red
 
 
-def setup(bot: commands.Bot):
+def setup(bot: Red):
     bot.add_cog(Alias(bot))
diff --git a/redbot/cogs/audio/__init__.py b/redbot/cogs/audio/__init__.py
index bdf94a1c3..34968fd36 100644
--- a/redbot/cogs/audio/__init__.py
+++ b/redbot/cogs/audio/__init__.py
@@ -5,7 +5,7 @@ import logging
 
 from .audio import Audio
 from .manager import start_lavalink_server
-from discord.ext import commands
+from redbot.core import commands
 from redbot.core.data_manager import cog_data_path
 import redbot.core
 
diff --git a/redbot/cogs/downloader/converters.py b/redbot/cogs/downloader/converters.py
index 801db62fb..d6522c658 100644
--- a/redbot/cogs/downloader/converters.py
+++ b/redbot/cogs/downloader/converters.py
@@ -1,6 +1,5 @@
 import discord
-from discord.ext import commands
-from .repo_manager import RepoManager
+from redbot.core import commands
 from .installable import Installable
 
 
diff --git a/redbot/cogs/downloader/repo_manager.py b/redbot/cogs/downloader/repo_manager.py
index 3d9ab7c3a..32126ce66 100644
--- a/redbot/cogs/downloader/repo_manager.py
+++ b/redbot/cogs/downloader/repo_manager.py
@@ -2,17 +2,13 @@ import asyncio
 import functools
 import os
 import pkgutil
-import shutil
 from concurrent.futures import ThreadPoolExecutor
 from pathlib import Path
 from subprocess import run as sp_run, PIPE
 from sys import executable
 from typing import Tuple, MutableMapping, Union
 
-from discord.ext import commands
-
-from redbot.core import Config
-from redbot.core import data_manager
+from redbot.core import data_manager, commands
 from redbot.core.utils import safe_delete
 from .errors import *
 from .installable import Installable, InstallableType
@@ -232,7 +228,7 @@ class Repo(RepoJSONMixin):
         ----------
         branch : `str`, optional
             Override for repo's branch attribute.
-            
+
         Returns
         -------
         str
@@ -381,7 +377,7 @@ class Repo(RepoJSONMixin):
             Directory to install shared libraries to.
         libraries : `tuple` of `Installable`
             A subset of available libraries.
- 
+
         Returns
         -------
         bool
@@ -403,7 +399,7 @@ class Repo(RepoJSONMixin):
 
     async def install_requirements(self, cog: Installable, target_dir: Path) -> bool:
         """Install a cog's requirements.
-        
+
         Requirements will be installed via pip directly into
         :code:`target_dir`.
 
@@ -465,7 +461,7 @@ class Repo(RepoJSONMixin):
     @property
     def available_cogs(self) -> Tuple[Installable]:
         """`tuple` of `installable` : All available cogs in this Repo.
-        
+
         This excludes hidden or shared packages.
         """
         # noinspection PyTypeChecker
diff --git a/redbot/cogs/mod/checks.py b/redbot/cogs/mod/checks.py
index 5f8de3663..91ed9d334 100644
--- a/redbot/cogs/mod/checks.py
+++ b/redbot/cogs/mod/checks.py
@@ -1,4 +1,4 @@
-from discord.ext import commands
+from redbot.core import commands
 import discord
 
 
diff --git a/redbot/cogs/permissions/converters.py b/redbot/cogs/permissions/converters.py
index de4526b26..8d2a8faff 100644
--- a/redbot/cogs/permissions/converters.py
+++ b/redbot/cogs/permissions/converters.py
@@ -11,7 +11,10 @@ class CogOrCommand(commands.Converter):
         if ret:
             return "commands", ret.qualified_name
 
-        raise commands.BadArgument()
+        raise commands.BadArgument(
+            'Cog or command "{arg}" not found. Please note that this is case sensitive.'
+            "".format(arg=arg)
+        )
 
 
 class RuleType(commands.Converter):
@@ -21,4 +24,6 @@ class RuleType(commands.Converter):
         if arg.lower() in ("deny", "blacklist", "denied"):
             return "deny"
 
-        raise commands.BadArgument()
+        raise commands.BadArgument(
+            '"{arg}" is not a valid rule. Valid rules are "allow" or "deny"'.format(arg=arg)
+        )
diff --git a/redbot/cogs/trivia/trivia.py b/redbot/cogs/trivia/trivia.py
index b78beaa9c..582bb75c2 100644
--- a/redbot/cogs/trivia/trivia.py
+++ b/redbot/cogs/trivia/trivia.py
@@ -2,7 +2,7 @@
 from collections import Counter
 import yaml
 import discord
-from discord.ext import commands
+from redbot.core import commands
 from redbot.ext import trivia as ext_trivia
 from redbot.core import Config, checks
 from redbot.core.data_manager import cog_data_path
diff --git a/redbot/core/checks.py b/redbot/core/checks.py
index 68ebb7be2..9a4a0869d 100644
--- a/redbot/core/checks.py
+++ b/redbot/core/checks.py
@@ -1,5 +1,5 @@
 import discord
-from discord.ext import commands
+from redbot.core import commands
 
 
 async def check_overrides(ctx, *, level):
diff --git a/redbot/core/commands/__init__.py b/redbot/core/commands/__init__.py
index 77ab1b024..ec2a4dc45 100644
--- a/redbot/core/commands/__init__.py
+++ b/redbot/core/commands/__init__.py
@@ -2,3 +2,4 @@
 from discord.ext.commands import *
 from .commands import *
 from .context import *
+from .errors import *
diff --git a/redbot/core/commands/commands.py b/redbot/core/commands/commands.py
index 92bc6ff67..de6bb77a5 100644
--- a/redbot/core/commands/commands.py
+++ b/redbot/core/commands/commands.py
@@ -4,12 +4,20 @@ This module contains extended classes and functions which are intended to
 replace those from the `discord.ext.commands` module.
 """
 import inspect
+from typing import TYPE_CHECKING
 
 from discord.ext import commands
 
+from .errors import ConversionFailure
+from ..i18n import Translator
+
+if TYPE_CHECKING:
+    from .context import Context
 
 __all__ = ["Command", "Group", "command", "group"]
 
+_ = Translator("commands.commands", __file__)
+
 
 class Command(commands.Command):
     """Command class for Red.
@@ -53,7 +61,7 @@ class Command(commands.Command):
         """
         Returns all parent commands of this command.
 
-        This is a list, sorted by the length of :attr:`.qualified_name` from highest to lowest. 
+        This is a list, sorted by the length of :attr:`.qualified_name` from highest to lowest.
         If the command has no parents, this will be an empty list.
         """
         cmd = self.parent
@@ -63,6 +71,37 @@ class Command(commands.Command):
             cmd = cmd.parent
         return sorted(entries, key=lambda x: len(x.qualified_name), reverse=True)
 
+    async def do_conversion(self, ctx: "Context", converter, argument: str):
+        """Convert an argument according to its type annotation.
+
+        Raises
+        ------
+        ConversionFailure
+            If doing the conversion failed.
+
+        Returns
+        -------
+        Any
+            The converted argument.
+
+        """
+        # Let's not worry about all of this junk if it's just a str converter
+        if converter is str:
+            return argument
+
+        try:
+            return await super().do_conversion(ctx, converter, argument)
+        except commands.BadArgument as exc:
+            raise ConversionFailure(converter, argument, *exc.args) from exc
+        except ValueError as exc:
+            # Some common converters need special treatment...
+            if converter in (int, float):
+                message = _('"{argument}" is not a number.').format(argument=argument)
+                raise ConversionFailure(converter, argument, message) from exc
+
+            # We should expose anything which might be a bug in the converter
+            raise exc
+
     def command(self, cls=None, *args, **kwargs):
         """A shortcut decorator that invokes :func:`.command` and adds it to
         the internal command list via :meth:`~.GroupMixin.add_command`.
diff --git a/redbot/core/commands/errors.py b/redbot/core/commands/errors.py
new file mode 100644
index 000000000..7435b544f
--- /dev/null
+++ b/redbot/core/commands/errors.py
@@ -0,0 +1,13 @@
+"""Errors module for the commands package."""
+from discord.ext import commands
+
+__all__ = ["ConversionFailure"]
+
+
+class ConversionFailure(commands.BadArgument):
+    """Raised when converting an argument fails."""
+
+    def __init__(self, converter, argument: str, *args):
+        self.converter = converter
+        self.argument = argument
+        super().__init__(*args)
diff --git a/redbot/core/events.py b/redbot/core/events.py
index bf5274b21..84d350b68 100644
--- a/redbot/core/events.py
+++ b/redbot/core/events.py
@@ -11,9 +11,8 @@ from pkg_resources import DistributionNotFound
 
 
 import discord
-from discord.ext import commands
 
-from . import __version__
+from . import __version__, commands
 from .data_manager import storage_type
 from .utils.chat_formatting import inline, bordered, pagify, box
 from .utils import fuzzy_command_search
@@ -185,6 +184,11 @@ def init_events(bot, cli_flags):
     async def on_command_error(ctx, error):
         if isinstance(error, commands.MissingRequiredArgument):
             await ctx.send_help()
+        elif isinstance(error, commands.ConversionFailure):
+            if error.args:
+                await ctx.send(error.args[0])
+            else:
+                await ctx.send_help()
         elif isinstance(error, commands.BadArgument):
             await ctx.send_help()
         elif isinstance(error, commands.DisabledCommand):