Use converters for tag command argument validation. (#56)

* Use converters for tag command argument validation.

* Remove now unused imports.

* Show red embed with an error message.

This also adds a `CogBadArgument` class, which is intended to be
used with command error handlers in cogs in order to handle their
own `BadArgument` exceptions without having `on_command_error` do so.

* Placate the flake8

3 files changed, 88 insertions, 78 deletions

diff --git a/bot/cogs/events.py b/bot/cogs/events.py
index 6f7a1423d..975b39ed9 100644
--- a/bot/cogs/events.py
+++ b/bot/cogs/events.py
@@ -9,6 +9,7 @@ from discord.ext.commands import (
 )
 
 from bot.constants import DEVLOG_CHANNEL, PYTHON_GUILD, SITE_API_KEY, SITE_API_USER_URL
+from bot.exceptions import CogBadArgument
 from bot.utils import chunks
 
 log = logging.getLogger(__name__)
@@ -49,7 +50,9 @@ class Events:
         else:
             help_command = (self.bot.get_command("help"),)
 
-        if isinstance(e, BadArgument):
+        if isinstance(e, CogBadArgument):
+            log.debug(f"Command {command} raised `CogBadArgument`, ignoring.")
+        elif isinstance(e, BadArgument):
             await ctx.send(f"Bad argument: {e}\n")
             await ctx.invoke(*help_command)
         elif isinstance(e, UserInputError):
diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py
index ca48f09cb..24ace01c7 100644
--- a/bot/cogs/tags.py
+++ b/bot/cogs/tags.py
@@ -1,21 +1,77 @@
 import logging
 import random
 import time
-from typing import Optional
 
-from discord import Colour, Embed, User
-from discord.ext.commands import AutoShardedBot, Context, command
+from discord import Colour, Embed
+from discord.ext.commands import (
+    AutoShardedBot, Context,
+    Converter, command
+)
 
 from bot.constants import (
-    ADMIN_ROLE, ERROR_REPLIES, MODERATOR_ROLE, OWNER_ROLE,
+    ADMIN_ROLE, MODERATOR_ROLE, OWNER_ROLE,
     SITE_API_KEY, SITE_API_TAGS_URL, TAG_COOLDOWN,
 )
 from bot.decorators import with_role
+from bot.exceptions import CogBadArgument
 from bot.pagination import LinePaginator
 
 log = logging.getLogger(__name__)
 
 
+class TagNameConverter(Converter):
+    @staticmethod
+    async def convert(ctx: Context, tag_name: str):
+        def is_number(value):
+            try:
+                float(value)
+            except ValueError:
+                return False
+            return True
+
+        tag_name = tag_name.lower().strip()
+
+        # The tag name has at least one invalid character.
+        if ascii(tag_name)[1:-1] != tag_name:
+            log.warning(f"{ctx.author} tried to put an invalid character in a tag name. "
+                        "Rejecting the request.")
+            raise CogBadArgument("Don't be ridiculous, you can't use that character!")
+
+        # The tag name is either empty, or consists of nothing but whitespace.
+        elif not tag_name:
+            log.warning(f"{ctx.author} tried to create a tag with a name consisting only of whitespace. "
+                        "Rejecting the request.")
+            raise CogBadArgument("Tag names should not be empty, or filled with whitespace.")
+
+        # The tag name is a number of some kind, we don't allow that.
+        elif is_number(tag_name):
+            log.warning(f"{ctx.author} tried to create a tag with a digit as its name. "
+                        "Rejecting the request.")
+            raise CogBadArgument("Tag names can't be numbers.")
+
+        # The tag name is longer than 127 characters.
+        elif len(tag_name) > 127:
+            log.warning(f"{ctx.author} tried to request a tag name with over 127 characters. "
+                        "Rejecting the request.")
+            raise CogBadArgument("Are you insane? That's way too long!")
+
+        return tag_name
+
+
+class TagContentConverter(Converter):
+    @staticmethod
+    async def convert(ctx: Context, tag_content: str):
+        tag_content = tag_content.strip()
+
+        # The tag contents should not be empty, or filled with whitespace.
+        if not tag_content:
+            log.warning(f"{ctx.author} tried to create a tag containing only whitespace. "
+                        "Rejecting the request.")
+            raise CogBadArgument("Tag contents should not be empty, or filled with whitespace.")
+
+        return tag_content
+
+
 class Tags:
     """
     Save new tags and fetch existing tags.
@@ -89,59 +145,6 @@ class Tags:
 
         return tag_data
 
-    @staticmethod
-    async def validate(author: User, tag_name: str, tag_content: str = None) -> Optional[Embed]:
-        """
-        Create an embed based on the validity of a tag's name and content
-
-        :param author: The user that called the command
-        :param tag_name: The name of the tag to validate.
-        :param tag_content: The tag's content, if any.
-        :return: A validation embed if invalid, otherwise None
-        """
-
-        def is_number(value):
-            try:
-                float(value)
-            except ValueError:
-                return False
-            else:
-                return True
-
-        embed = Embed()
-        embed.colour = Colour.red()
-
-        # 'tag_name' has at least one invalid character.
-        if ascii(tag_name)[1:-1] != tag_name:
-            log.warning(f"{author} tried to put an invalid character in a tag name. "
-                        "Rejecting the request.")
-            embed.description = "Don't be ridiculous, you can't use that character!"
-
-        # 'tag_content' or 'tag_name' are either empty, or consist of nothing but whitespace
-        elif (tag_content is not None and not tag_content) or not tag_name:
-            log.warning(f"{author} tried to create a tag with a name consisting only of whitespace. "
-                        "Rejecting the request.")
-            embed.description = "Tags should not be empty, or filled with whitespace."
-
-        # 'tag_name' is a number of some kind, we don't allow that.
-        elif is_number(tag_name):
-            log.error("inside the is_number")
-            log.warning(f"{author} tried to create a tag with a digit as its name. "
-                        "Rejecting the request.")
-            embed.description = "Tag names can't be numbers."
-
-        # 'tag_name' is longer than 127 characters
-        elif len(tag_name) > 127:
-            log.warning(f"{author} tried to request a tag name with over 127 characters. "
-                        "Rejecting the request.")
-            embed.description = "Are you insane? That's way too long!"
-
-        else:
-            return None
-
-        embed.title = random.choice(ERROR_REPLIES)
-        return embed
-
     @command(name="tags()", aliases=["tags"], hidden=True)
     async def info_command(self, ctx: Context):
         """
@@ -154,7 +157,7 @@ class Tags:
         return await ctx.invoke(self.bot.get_command("help"), "Tags")
 
     @command(name="tags.get()", aliases=["tags.get", "tags.show()", "tags.show", "get_tag"])
-    async def get_command(self, ctx: Context, tag_name: str=None):
+    async def get_command(self, ctx: Context, tag_name: TagNameConverter=None):
         """
         Get a list of all tags or a specified tag.
 
@@ -195,13 +198,6 @@ class Tags:
 
         tags = []
 
-        if tag_name is not None:
-            tag_name = tag_name.lower().strip()
-            validation = await self.validate(ctx.author, tag_name)
-
-            if validation is not None:
-                return await ctx.send(embed=validation)
-
         embed = Embed()
         embed.colour = Colour.red()
         tag_data = await self.get_tag_data(tag_name)
@@ -259,7 +255,7 @@ class Tags:
 
     @with_role(ADMIN_ROLE, OWNER_ROLE, MODERATOR_ROLE)
     @command(name="tags.set()", aliases=["tags.set", "tags.add", "tags.add()", "tags.edit", "tags.edit()", "add_tag"])
-    async def set_command(self, ctx: Context, tag_name: str, tag_content: str):
+    async def set_command(self, ctx: Context, tag_name: TagNameConverter, tag_content: TagContentConverter):
         """
         Create a new tag or edit an existing one.
 
@@ -268,11 +264,6 @@ class Tags:
         :param tag_content: The content of the tag.
         """
 
-        validation = await self.validate(ctx.author, tag_name, tag_content)
-
-        if validation is not None:
-            return await ctx.send(embed=validation)
-
         tag_name = tag_name.lower().strip()
         tag_content = tag_content.strip()
 
@@ -300,7 +291,7 @@ class Tags:
 
     @with_role(ADMIN_ROLE, OWNER_ROLE)
     @command(name="tags.delete()", aliases=["tags.delete", "tags.remove", "tags.remove()", "remove_tag"])
-    async def delete_command(self, ctx: Context, tag_name: str):
+    async def delete_command(self, ctx: Context, tag_name: TagNameConverter):
         """
         Remove a tag from the database.
 
@@ -308,11 +299,6 @@ class Tags:
         :param tag_name: The name of the tag to delete.
         """
 
-        validation = await self.validate(ctx.author, tag_name)
-
-        if validation is not None:
-            return await ctx.send(embed=validation)
-
         tag_name = tag_name.lower().strip()
         embed = Embed()
         embed.colour = Colour.red()
@@ -340,6 +326,17 @@ class Tags:
 
         return await ctx.send(embed=embed)
 
+    @get_command.error
+    @set_command.error
+    @delete_command.error
+    async def command_error(self, ctx, error):
+        if isinstance(error, CogBadArgument):
+            embed = Embed()
+            embed.colour = Colour.red()
+            embed.description = str(error)
+            embed.title = random.choice(self.FAIL_TITLES)
+            await ctx.send(embed=embed)
+
     @command(name="tags.keys()")
     async def keys_command(self, ctx: Context):
         """
diff --git a/bot/exceptions.py b/bot/exceptions.py
new file mode 100644
index 000000000..8f5da91e9
--- /dev/null
+++ b/bot/exceptions.py
@@ -0,0 +1,10 @@
+from discord.ext.commands import BadArgument
+
+
+class CogBadArgument(BadArgument):
+    """
+    A custom `BadArgument` subclass that can be used for
+    setting up custom error handlers on a per-command
+    basis in cogs. The standard `on_command_error` handler
+    ignores any exceptions of this type.
+    """