Implement role checks using has_any_role

Use `has_any_role` to reduce redundancy.

Because discord.py always makes a check's predicate a coroutine, the
checks now have to be awaited.

4 files changed, 35 insertions, 34 deletions

diff --git a/bot/cogs/information.py b/bot/cogs/information.py
index abfbcb84e..5b132a469 100644
--- a/bot/cogs/information.py
+++ b/bot/cogs/information.py
@@ -15,11 +15,12 @@ from bot import constants
 from bot.bot import Bot
 from bot.decorators import in_whitelist
 from bot.pagination import LinePaginator
-from bot.utils.checks import InWhitelistCheckFailure, cooldown_with_role_bypass, with_role_check
+from bot.utils.checks import InWhitelistCheckFailure, cooldown_with_role_bypass, without_role_check
 from bot.utils.time import time_since
 
 log = logging.getLogger(__name__)
 
+
 STATUS_EMOTES = {
     Status.offline: constants.Emojis.status_offline,
     Status.dnd: constants.Emojis.status_dnd,
@@ -197,12 +198,12 @@ class Information(Cog):
             user = ctx.author
 
         # Do a role check if this is being executed on someone other than the caller
-        elif user != ctx.author and not with_role_check(ctx, *constants.MODERATION_ROLES):
+        elif user != ctx.author and await without_role_check(ctx, *constants.MODERATION_ROLES):
             await ctx.send("You may not use this command on users other than yourself.")
             return
 
         # Non-staff may only do this in #bot-commands
-        if not with_role_check(ctx, *constants.STAFF_ROLES):
+        if await without_role_check(ctx, *constants.STAFF_ROLES):
             if not ctx.channel.id == constants.Channels.bot_commands:
                 raise InWhitelistCheckFailure(constants.Channels.bot_commands)
 
diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py
index 08bce2153..d7357e3fa 100644
--- a/bot/cogs/reminders.py
+++ b/bot/cogs/reminders.py
@@ -117,9 +117,9 @@ class Reminders(Cog):
 
         If mentions aren't allowed, also return the type of mention(s) disallowed.
         """
-        if without_role_check(ctx, *STAFF_ROLES):
+        if await without_role_check(ctx, *STAFF_ROLES):
             return False, "members/roles"
-        elif without_role_check(ctx, *MODERATION_ROLES):
+        elif await without_role_check(ctx, *MODERATION_ROLES):
             return all(isinstance(mention, discord.Member) for mention in mentions), "roles"
         else:
             return True, ""
@@ -240,7 +240,7 @@ class Reminders(Cog):
         Expiration is parsed per: http://strftime.org/
         """
         # If the user is not staff, we need to verify whether or not to make a reminder at all.
-        if without_role_check(ctx, *STAFF_ROLES):
+        if await without_role_check(ctx, *STAFF_ROLES):
 
             # If they don't have permission to set a reminder in this channel
             if ctx.channel.id not in WHITELISTED_CHANNELS:
@@ -431,7 +431,7 @@ class Reminders(Cog):
 
         The check passes when the user is an admin, or if they created the reminder.
         """
-        if with_role_check(ctx, Roles.admins):
+        if await with_role_check(ctx, Roles.admins):
             return True
 
         api_response = await self.bot.api_client.get(f"bot/reminders/{reminder_id}")
diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py
index ae156cf70..afbe1d3b8 100644
--- a/bot/cogs/verification.py
+++ b/bot/cogs/verification.py
@@ -178,9 +178,10 @@ class Verification(Cog):
             error.handled = True
 
     @staticmethod
-    def bot_check(ctx: Context) -> bool:
+    async def bot_check(ctx: Context) -> bool:
         """Block any command within the verification channel that is not !accept."""
-        if ctx.channel.id == constants.Channels.verification and without_role_check(ctx, *constants.MODERATION_ROLES):
+        is_verification = ctx.channel.id == constants.Channels.verification
+        if is_verification and await without_role_check(ctx, *constants.MODERATION_ROLES):
             return ctx.command.name == "accept"
         else:
             return True
diff --git a/bot/utils/checks.py b/bot/utils/checks.py
index f0ef36302..c2e41efb3 100644
--- a/bot/utils/checks.py
+++ b/bot/utils/checks.py
@@ -1,6 +1,6 @@
 import datetime
 import logging
-from typing import Callable, Container, Iterable, Optional
+from typing import Callable, Container, Iterable, Optional, Union
 
 from discord.ext.commands import (
     BucketType,
@@ -11,6 +11,8 @@ from discord.ext.commands import (
     Context,
     Cooldown,
     CooldownMapping,
+    NoPrivateMessage,
+    has_any_role,
 )
 
 from bot import constants
@@ -89,35 +91,32 @@ def in_whitelist_check(
     return False
 
 
-def with_role_check(ctx: Context, *role_ids: int) -> bool:
-    """Returns True if the user has any one of the roles in role_ids."""
-    if not ctx.guild:  # Return False in a DM
-        log.trace(f"{ctx.author} tried to use the '{ctx.command.name}'command from a DM. "
-                  "This command is restricted by the with_role decorator. Rejecting request.")
-        return False
+async def with_role_check(ctx: Context, *roles: Union[str, int]) -> bool:
+    """
+    Returns True if the context's author has any of the specified roles.
 
-    for role in ctx.author.roles:
-        if role.id in role_ids:
-            log.trace(f"{ctx.author} has the '{role.name}' role, and passes the check.")
-            return True
+    `roles` are the names or IDs of the roles for which to check.
+    False is always returns if the context is outside a guild.
+    """
+    try:
+        return await has_any_role(*roles).predicate(ctx)
+    except CheckFailure:
+        return False
 
-    log.trace(f"{ctx.author} does not have the required role to use "
-              f"the '{ctx.command.name}' command, so the request is rejected.")
-    return False
 
+async def without_role_check(ctx: Context, *roles: Union[str, int]) -> bool:
+    """
+    Returns True if the context's author doesn't have any of the specified roles.
 
-def without_role_check(ctx: Context, *role_ids: int) -> bool:
-    """Returns True if the user does not have any of the roles in role_ids."""
-    if not ctx.guild:  # Return False in a DM
-        log.trace(f"{ctx.author} tried to use the '{ctx.command.name}' command from a DM. "
-                  "This command is restricted by the without_role decorator. Rejecting request.")
+    `roles` are the names or IDs of the roles for which to check.
+    False is always returns if the context is outside a guild.
+    """
+    try:
+        return not await has_any_role(*roles).predicate(ctx)
+    except NoPrivateMessage:
         return False
-
-    author_roles = [role.id for role in ctx.author.roles]
-    check = all(role not in author_roles for role in role_ids)
-    log.trace(f"{ctx.author} tried to call the '{ctx.command.name}' command. "
-              f"The result of the without_role check was {check}.")
-    return check
+    except CheckFailure:
+        return True
 
 
 def cooldown_with_role_bypass(rate: int, per: float, type: BucketType = BucketType.default, *,