Make respect_role_hierarchy a decorator

* Move respect_role_hierarchy to the decorators modules
* Get the command name from the context instead of an argument

2 files changed, 44 insertions, 59 deletions

diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py
index b8003da1d..7e1a96036 100644
--- a/bot/cogs/moderation.py
+++ b/bot/cogs/moderation.py
@@ -15,7 +15,7 @@ from bot import constants
 from bot.cogs.modlog import ModLog
 from bot.constants import Colours, Event, Icons, MODERATION_ROLES
 from bot.converters import Duration, InfractionSearchQuery
-from bot.decorators import with_role
+from bot.decorators import respect_role_hierarchy, with_role
 from bot.pagination import LinePaginator
 from bot.utils.moderation import already_has_active_infraction, post_infraction
 from bot.utils.scheduling import Scheduler
@@ -120,13 +120,9 @@ class Moderation(Scheduler, Cog):
 
     @with_role(*MODERATION_ROLES)
     @command()
+    @respect_role_hierarchy()
     async def kick(self, ctx: Context, user: Member, *, reason: str = None) -> None:
         """Kicks a user with the provided reason."""
-        if not await self.respect_role_hierarchy(ctx, user, 'kick'):
-            # Ensure ctx author has a higher top role than the target user
-            # Warning is sent to ctx by the helper method
-            return
-
         infraction = await post_infraction(ctx, user, type="kick", reason=reason)
         if infraction is None:
             return
@@ -166,13 +162,9 @@ class Moderation(Scheduler, Cog):
 
     @with_role(*MODERATION_ROLES)
     @command()
+    @respect_role_hierarchy()
     async def ban(self, ctx: Context, user: UserTypes, *, reason: str = None) -> None:
         """Create a permanent ban infraction for a user with the provided reason."""
-        if not await self.respect_role_hierarchy(ctx, user, 'ban'):
-            # Ensure ctx author has a higher top role than the target user
-            # Warning is sent to ctx by the helper method
-            return
-
         if await already_has_active_infraction(ctx=ctx, user=user, type="ban"):
             return
 
@@ -283,6 +275,7 @@ class Moderation(Scheduler, Cog):
 
     @with_role(*MODERATION_ROLES)
     @command()
+    @respect_role_hierarchy()
     async def tempban(self, ctx: Context, user: UserTypes, duration: Duration, *, reason: str = None) -> None:
         """
         Create a temporary ban infraction for a user with the provided expiration and reason.
@@ -291,11 +284,6 @@ class Moderation(Scheduler, Cog):
         """
         expiration = duration
 
-        if not await self.respect_role_hierarchy(ctx, user, 'tempban'):
-            # Ensure ctx author has a higher top role than the target user
-            # Warning is sent to ctx by the helper method
-            return
-
         if await already_has_active_infraction(ctx=ctx, user=user, type="ban"):
             return
 
@@ -381,17 +369,13 @@ class Moderation(Scheduler, Cog):
 
     @with_role(*MODERATION_ROLES)
     @command(hidden=True, aliases=['shadowkick', 'skick'])
+    @respect_role_hierarchy()
     async def shadow_kick(self, ctx: Context, user: Member, *, reason: str = None) -> None:
         """
         Kick a user for the provided reason.
 
         This does not send the user a notification.
         """
-        if not await self.respect_role_hierarchy(ctx, user, 'shadowkick'):
-            # Ensure ctx author has a higher top role than the target user
-            # Warning is sent to ctx by the helper method
-            return
-
         infraction = await post_infraction(ctx, user, type="kick", reason=reason, hidden=True)
         if infraction is None:
             return
@@ -429,17 +413,13 @@ class Moderation(Scheduler, Cog):
 
     @with_role(*MODERATION_ROLES)
     @command(hidden=True, aliases=['shadowban', 'sban'])
+    @respect_role_hierarchy()
     async def shadow_ban(self, ctx: Context, user: UserTypes, *, reason: str = None) -> None:
         """
         Create a permanent ban infraction for a user with the provided reason.
 
         This does not send the user a notification.
         """
-        if not await self.respect_role_hierarchy(ctx, user, 'shadowban'):
-            # Ensure ctx author has a higher top role than the target user
-            # Warning is sent to ctx by the helper method
-            return
-
         if await already_has_active_infraction(ctx=ctx, user=user, type="ban"):
             return
 
@@ -526,6 +506,7 @@ class Moderation(Scheduler, Cog):
 
     @with_role(*MODERATION_ROLES)
     @command(hidden=True, aliases=["shadowtempban, stempban"])
+    @respect_role_hierarchy()
     async def shadow_tempban(
         self, ctx: Context, user: UserTypes, duration: Duration, *, reason: str = None
     ) -> None:
@@ -538,11 +519,6 @@ class Moderation(Scheduler, Cog):
         """
         expiration = duration
 
-        if not await self.respect_role_hierarchy(ctx, user, 'shadowtempban'):
-            # Ensure ctx author has a higher top role than the target user
-            # Warning is sent to ctx by the helper method
-            return
-
         if await already_has_active_infraction(ctx=ctx, user=user, type="ban"):
             return
 
@@ -1074,32 +1050,6 @@ class Moderation(Scheduler, Cog):
                 await ctx.send(str(error.errors[0]))
                 error.handled = True
 
-    @staticmethod
-    async def respect_role_hierarchy(ctx: Context, target: UserTypes, infr_type: str) -> bool:
-        """
-        Check if the highest role of the invoking member is greater than that of the target member.
-
-        If this check fails, a warning is sent to the invoking ctx.
-
-        Returns True always if target is not a discord.Member instance.
-        """
-        if not isinstance(target, Member):
-            return True
-
-        actor = ctx.author
-        target_is_lower = target.top_role < actor.top_role
-        if not target_is_lower:
-            log.info(
-                f"{actor} ({actor.id}) attempted to {infr_type} "
-                f"{target} ({target.id}), who has an equal or higher top role."
-            )
-            await ctx.send(
-                f":x: {actor.mention}, you may not {infr_type} "
-                "someone with an equal or higher top role."
-            )
-
-        return target_is_lower
-
 
 def setup(bot: Bot) -> None:
     """Moderation cog load."""
diff --git a/bot/decorators.py b/bot/decorators.py
index 33a6bcadd..a44d62afa 100644
--- a/bot/decorators.py
+++ b/bot/decorators.py
@@ -6,7 +6,7 @@ from functools import wraps
 from typing import Any, Callable, Container, Optional
 from weakref import WeakValueDictionary
 
-from discord import Colour, Embed
+from discord import Colour, Embed, Member
 from discord.errors import NotFound
 from discord.ext import commands
 from discord.ext.commands import CheckFailure, Context
@@ -72,7 +72,7 @@ def locked() -> Callable:
 
     Subsequent calls to the command from the same author are ignored until the command has completed invocation.
 
-    This decorator has to go before (below) the `command` decorator.
+    This decorator must go before (below) the `command` decorator.
     """
     def wrap(func: Callable) -> Callable:
         func.__locks = WeakValueDictionary()
@@ -103,6 +103,8 @@ def redirect_output(destination_channel: int, bypass_roles: Container[int] = Non
     Changes the channel in the context of the command to redirect the output to a certain channel.
 
     Redirect is bypassed if the author has a role to bypass redirection.
+
+    This decorator must go before (below) the `command` decorator.
     """
     def wrap(func: Callable) -> Callable:
         @wraps(func)
@@ -140,3 +142,36 @@ def redirect_output(destination_channel: int, bypass_roles: Container[int] = Non
                     log.trace("Redirect output: Deleted invocation message")
         return inner
     return wrap
+
+
+def respect_role_hierarchy(target_arg_name: str = "user") -> Callable:
+    """
+    Ensure the highest role of the invoking member is greater than that of the target member.
+
+    If the condition fails, a warning is sent to the invoking context. A target which is not an
+    instance of discord.Member will always pass.
+
+    This decorator must go before (below) the `command` decorator.
+    """
+    def wrap(func: Callable) -> Callable:
+        @wraps(func)
+        async def inner(self: Callable, ctx: Context, *args, **kwargs) -> Any:
+            target = kwargs[target_arg_name]
+            if not isinstance(target, Member):
+                return await func(self, ctx, *args, **kwargs)
+
+            cmd = ctx.command.name
+            actor = ctx.author
+            if target.top_role >= actor.top_role:
+                log.info(
+                    f"{actor} ({actor.id}) attempted to {cmd} "
+                    f"{target} ({target.id}), who has an equal or higher top role."
+                )
+                await ctx.send(
+                    f":x: {actor.mention}, you may not {cmd} "
+                    "someone with an equal or higher top role."
+                )
+            else:
+                return await func(self, ctx, *args, **kwargs)
+        return inner
+    return wrap