[Permissions] Help menu respects rules (#2339)

Resolves #2249.

Signed-off-by: Toby Harradine <tobyharradine@gmail.com>

redbot/cogs/permissions/permissions.py

@@ -148,23 +148,20 @@ class Permissions(commands.Cog):
         if not command:
             return await ctx.send_help()
 
-        message = copy(ctx.message)
-        message.author = user
-        message.content = "{}{}".format(ctx.prefix, command)
+        fake_message = copy(ctx.message)
+        fake_message.author = user
+        fake_message.content = "{}{}".format(ctx.prefix, command)
 
         com = ctx.bot.get_command(command)
         if com is None:
             out = _("No such command")
         else:
+            fake_context = await ctx.bot.get_context(fake_message)
             try:
-                testcontext = await ctx.bot.get_context(message, cls=commands.Context)
-                to_check = [*reversed(com.parents)] + [com]
-                can = False
-                for cmd in to_check:
-                    can = await cmd.can_run(testcontext)
-                    if can is False:
-                        break
-            except commands.CheckFailure:
+                can = await com.can_run(
+                    fake_context, check_all_parents=True, change_permission_state=False
+                )
+            except commands.CommandError:
                 can = False
 
             out = (

redbot/core/commands/commands.py

@@ -157,12 +157,31 @@ class Command(CogCommandMixin, commands.Command):
             cmd = cmd.parent
         return sorted(entries, key=lambda x: len(x.qualified_name), reverse=True)
 
-    async def can_run(self, ctx: "Context") -> bool:
+    # noinspection PyMethodOverriding
+    async def can_run(
+        self,
+        ctx: "Context",
+        *,
+        check_all_parents: bool = False,
+        change_permission_state: bool = False,
+    ) -> bool:
         """Check if this command can be run in the given context.
 
         This function first checks if the command can be run using
         discord.py's method `discord.ext.commands.Command.can_run`,
         then will return the result of `Requires.verify`.
+
+        Keyword Arguments
+        -----------------
+        check_all_parents : bool
+            If ``True``, this will check permissions for all of this
+            command's parents and its cog as well as the command
+            itself. Defaults to ``False``.
+        change_permission_state : bool
+            Whether or not the permission state should be changed as
+            a result of this call. For most cases this should be
+            ``False``. Defaults to ``False``.
+
         """
         ret = await super().can_run(ctx)
         if ret is False:
@@ -171,8 +190,21 @@ class Command(CogCommandMixin, commands.Command):
         # This is so contexts invoking other commands can be checked with
         # this command as well
         original_command = ctx.command
+        original_state = ctx.permission_state
         ctx.command = self
 
+        if check_all_parents is True:
+            # Since we're starting from the beginning, we should reset the state to normal
+            ctx.permission_state = PermState.NORMAL
+            for parent in reversed(self.parents):
+                try:
+                    result = await parent.can_run(ctx, change_permission_state=True)
+                except commands.CommandError:
+                    result = False
+
+                if result is False:
+                    return False
+
         if self.parent is None and self.instance is not None:
             # For top-level commands, we need to check the cog's requires too
             ret = await self.instance.requires.verify(ctx)
@@ -183,6 +215,17 @@ class Command(CogCommandMixin, commands.Command):
             return await self.requires.verify(ctx)
         finally:
             ctx.command = original_command
+            if not change_permission_state:
+                ctx.permission_state = original_state
+
+    async def _verify_checks(self, ctx):
+        if not self.enabled:
+            raise commands.DisabledCommand(f"{self.name} command is disabled")
+
+        if not (await self.can_run(ctx, change_permission_state=True)):
+            raise commands.CheckFailure(
+                f"The check functions for command {self.qualified_name} failed."
+            )
 
     async def do_conversion(
         self, ctx: "Context", converter, argument: str, param: inspect.Parameter
@@ -238,7 +281,9 @@ class Command(CogCommandMixin, commands.Command):
             if cmd.hidden:
                 return False
             try:
-                can_run = await self.can_run(ctx)
+                can_run = await self.can_run(
+                    ctx, check_all_parents=True, change_permission_state=False
+                )
             except commands.CheckFailure:
                 return False
             else:

redbot/core/help_formatter.py

@@ -23,6 +23,7 @@ discord.py 1.0.0a
 
 This help formatter contains work by Rapptz (Danny) and SirThane#1780.
 """
+import contextlib
 from collections import namedtuple
 from typing import List, Optional, Union
 
@@ -224,8 +225,8 @@ class Help(dpy_formatter.HelpFormatter):
 
         return ret
 
-    async def format_help_for(self, ctx, command_or_bot, reason: str = None):
-        """Formats the help page and handles the actual heavy lifting of how  ### WTF HAPPENED?
+    async def format_help_for(self, ctx, command_or_bot, reason: str = ""):
+        """Formats the help page and handles the actual heavy lifting of how
         the help command looks like. To change the behaviour, override the
         :meth:`~.HelpFormatter.format` method.
 
@@ -244,10 +245,24 @@ class Help(dpy_formatter.HelpFormatter):
         """
         self.context = ctx
         self.command = command_or_bot
+
+        # We want the permission state to be set as if the author had run the command he is
+        # requesting help for. This is so the subcommands shown in the help menu correctly reflect
+        # any permission rules set.
+        if isinstance(self.command, commands.Command):
+            with contextlib.suppress(commands.CommandError):
+                await self.command.can_run(
+                    self.context, check_all_parents=True, change_permission_state=True
+                )
+        elif isinstance(self.command, commands.Cog):
+            with contextlib.suppress(commands.CommandError):
+                # Cog's don't have a `can_run` method, so we use the `Requires` object directly.
+                await self.command.requires.verify(self.context)
+
         emb = await self.format()
 
         if reason:
-            emb["embed"]["title"] = "{0}".format(reason)
+            emb["embed"]["title"] = reason
 
         ret = []