Python parser cleanup (#62)

* Cleaned up the python parser by abstracting a little, and added a few channels as exceptions to the tags cooldown system

* Refactored to make the code more DRY

* Addressing Volcyy's requests for change

3 files changed, 152 insertions, 124 deletions

diff --git a/bot/__init__.py b/bot/__init__.py
index 02b3d5e98..429c01829 100644
--- a/bot/__init__.py
+++ b/bot/__init__.py
@@ -83,63 +83,105 @@ def _get_word(self) -> str:
     the bot command part of a message, but
     allows the command to ignore case sensitivity,
     and allows commands to have Python syntax.
-
-    Example of valid Python syntax calls:
-    ------------------------------
-    bot.tags.set("test", 'a dark, dark night')
-    bot.help(tags.delete)
-    bot.hELP(tags.delete)
     """
 
-    pos = 0
-    while not self.eof:
-        try:
-            current = self.buffer[self.index + pos]
-            if current.isspace() or current == "(" or current == "[":
-                break
-            pos += 1
-        except IndexError:
-            break
+    def parse_python(buffer_pos):
+        """
+        Takes the instance of the view and parses the buffer, if it contains valid python syntax.
+        This may fail spectacularly with a SyntaxError, which must be caught by the caller.
 
-    self.previous = self.index
-    result = self.buffer[self.index:self.index + pos]
-    self.index += pos
-    next = None
+        Example of valid Python syntax calls:
+        ------------------------------
+        bot.tags.set("test", 'a dark, dark night')
+        bot.help(tags.delete)
+        bot.hELP(tags.delete)
+        bot.tags['internet']
+        bot.tags['internet'] = "A series of tubes"
 
-    # Check what's after the '('
-    if len(self.buffer) != self.index:
-        next = self.buffer[self.index + 1]
+        :return: the parsed command
+        """
+
+        # Check what's after the '(' or '['
+        next_char = None
+        if len(self.buffer) != self.index:
+            next_char = self.buffer[self.index + 1]
 
-    # Is it possible to parse this without syntax error?
-    syntax_valid = True
-    try:
         # Catch raw channel, member or role mentions and wrap them in quotes.
+        tempbuffer = self.buffer
         tempbuffer = re.sub(r"(<(?:@|@!|[#&])\d+>)",
                             r'"\1"',
-                            self.buffer)
-        ast.literal_eval(tempbuffer[self.index:])
-        self.buffer = tempbuffer
-    except SyntaxError:
-        log.warning("The command cannot be parsed by ast.literal_eval because it raises a SyntaxError.")
-        # TODO: It would be nice if this actually made the bot return a SyntaxError. ClickUp #1b12z  # noqa: T000
-        syntax_valid = False
-
-    # Conditions for a valid, parsable command.
-    python_parse_conditions = (
-        current == "("
-        and next
-        and next != ")"
-        and syntax_valid
-    )
-
-    if python_parse_conditions:
-        log.debug(f"A python-style command was used. Attempting to parse. Buffer is {self.buffer}. "
+                            tempbuffer)
+
+        # Let's parse!
+        log.debug("A python-style command was used. Attempting to parse. "
+                  f"Buffer is '{self.buffer}'. Tempbuffer is '{tempbuffer}'. "
                   "A step-by-step can be found in the trace log.")
 
-        # Parse the args
-        log.trace("Parsing command with ast.literal_eval.")
-        args = self.buffer[self.index:]
-        args = ast.literal_eval(args)
+        if current == "(" and next_char == ")":
+            # Move the cursor to capture the ()'s
+            log.debug("User called command without providing arguments.")
+            buffer_pos += 2
+            parsed_result = self.buffer[self.previous:self.index + (buffer_pos+2)]
+            self.index += 2
+            args = ''
+
+        if current == "(" and next_char:
+
+            # Parse the args
+            log.trace(f"Parsing command with ast.literal_eval. args are {tempbuffer[self.index:]}")
+            args = tempbuffer[self.index:]
+            args = ast.literal_eval(args)
+
+            # Return what we'd return for a non-python syntax call
+            log.trace(f"Returning {self.buffer[self.previous:self.index]}")
+            parsed_result = self.buffer[self.previous:self.index]
+
+        # Check if a command in the form of `bot.tags['ask']`
+        # or alternatively `bot.tags['ask'] = 'whatever'` was used.
+        elif current == "[":
+
+            # Syntax is `bot.tags['ask']` => mimic `getattr`
+            log.trace(f"Got a command candidate for getitem / setitem parsing: {self.buffer}")
+            if self.buffer.endswith("]"):
+
+                # Key: The first argument, specified `bot.tags[here]`
+                key = tempbuffer[self.index + 1:tempbuffer.rfind("]")]
+                log.trace(f"Command mimicks getitem. Key: {key!r}")
+                args = ast.literal_eval(key)
+
+                # Use the cogs `.get` method.
+                parsed_result = self.buffer[self.previous:self.index] + ".get"
+
+            # Syntax is `bot.tags['ask'] = 'whatever'` => mimic `setattr`
+            elif "=" in self.buffer and not self.buffer.endswith("="):
+                equals_pos = tempbuffer.find("=")
+                closing_bracket_pos = tempbuffer.rfind("]", 0, equals_pos)
+
+                # Key: The first argument, specified `bot.tags[here]`
+                key_contents = tempbuffer[self.index + 1:closing_bracket_pos]
+                key = ast.literal_eval(key_contents)
+
+                # Value: The second argument, specified after the `=`
+                right_hand = tempbuffer.split("=", maxsplit=1)[1].strip()
+                value = ast.literal_eval(right_hand)
+
+                # If the value is a falsy value - mimick `bot.tags.delete(key)`
+                if not value:
+                    log.trace(f"Command mimicks delitem. Key: {key!r}.")
+                    parsed_result = self.buffer[self.previous:self.index] + ".delete"
+                    args = key
+
+                # Otherwise, assume assignment, for example `bot.tags['this'] = 'that'`
+                else:
+                    log.trace(f"Command mimicks setitem. Key: {key!r}, value: {value!r}.")
+                    parsed_result = self.buffer[self.previous:self.index] + ".set"
+                    args = (key, value)
+
+            # Syntax is god knows what, pass it along
+            else:
+                parsed_result = self.buffer
+                args = ''
+                log.trace(f"Command is of unknown syntax: {self.buffer}")
 
         # Force args into container
         if not isinstance(args, tuple):
@@ -158,82 +200,57 @@ def _get_word(self) -> str:
             arg = arg.replace('"', '\\"')
 
             # Adding double quotes to every argument
-            log.trace(f"Wrapping all args in double quotes.")
+            log.trace("Wrapping all args in double quotes.")
             new_args.append(f'"{arg}"')
 
-        # Add the result to the buffer
-        new_args = " ".join(new_args)
-        self.buffer = f"{self.buffer[:self.index]} {new_args}"
-        log.trace(f"Modified the buffer. New buffer is now {self.buffer}")
-
-        # Recalibrate the end since we've removed commas
+        # Reconstruct valid discord.py syntax
+        prefix = self.buffer[:self.previous]
+        self.buffer = f"{prefix}{parsed_result} {' '.join(new_args)}"
+        self.index = len(f"{prefix}{parsed_result}")
         self.end = len(self.buffer)
+        log.trace(f"Modified the buffer. New buffer is now '{self.buffer}'")
 
-    elif current == "(" and next == ")":
-        # Move the cursor to capture the ()'s
-        log.debug("User called command without providing arguments.")
-        pos += 2
-        result = self.buffer[self.previous:self.index + (pos+2)]
-        self.index += 2
-
-    # Check if a command in the form of `bot.tags['ask']`
-    # or alternatively `bot.tags['ask'] = 'whatever'` was used.
-    elif current == "[":
-        log.trace(f"Got a command candidate for getitem / setitem mimick: {self.buffer}")
-        # Syntax is `bot.tags['ask']` => mimic `getattr`
-        if self.buffer.endswith("]"):
-            # Key: The first argument, specified `bot.tags[here]`
-            key = self.buffer[self.index + 1:self.buffer.rfind("]")]
-            log.trace(f"Command mimicks getitem. Key: {key!r}")
-
-            # note: if not key, this corresponds to an empty argument
-            #       so this should throw / return a SyntaxError ?
-            args = ast.literal_eval(key)
-
-            # Use the cog's `get` command.
-            result = self.buffer[self.previous:self.index] + ".get"
-
-        # Syntax is `bot.tags['ask'] = 'whatever'` => mimic `setattr`
-        elif "=" in self.buffer and not self.buffer.endswith("="):
-            equals_pos = self.buffer.find("=")
-            closing_bracket_pos = self.buffer.rfind("]", 0, equals_pos)
-
-            # Key: The first argument, specified `bot.tags[here]`
-            key_contents = self.buffer[self.index + 1:closing_bracket_pos]
-            key = ast.literal_eval(key_contents)
-
-            # Value: The second argument, specified after the `=`
-            right_hand = self.buffer.split("=", maxsplit=1)[1].strip()
-            value = ast.literal_eval(right_hand)
-
-            # If the value is a falsy value - mimick `bot.tags.delete(key)`
-            if not value:
-                log.trace(f"Command mimicks delitem. Key: {key!r}.")
-                result = self.buffer[self.previous:self.index] + ".delete"
-                args = key
-
-            # Otherwise, assume assignment, for example `bot.tags['this'] = 'that'`
-            else:
-                # Allow using double quotes in triple double quote string
-                value = value.replace('"', '\\"')
-                log.trace(f"Command mimicks setitem. Key: {key!r}, value: {value!r}.")
-
-                # Use the cog's `set` command.
-                result = self.buffer[self.previous:self.index] + ".set"
-                args = f'"{key}" "{value}"'
-
-        # Syntax is god knows what, pass it along
-        # in the future, this should probably return / throw SyntaxError
-        else:
-            result = self.buffer
-            args = ''
-            log.trace(f"Command is of unknown syntax: {self.buffer}")
+        return parsed_result
 
-        # Reconstruct valid discord.py syntax
-        self.buffer = f"{result} {args}"
-        self.index = len(result)
-        self.end = len(self.buffer)
-        log.trace(f"Mimicked command: {self.buffer}")
+    # Iterate through the buffer and determine
+    pos = 0
+    while not self.eof:
+        try:
+            current = self.buffer[self.index + pos]
+            if current.isspace() or current == "(" or current == "[":
+                break
+            pos += 1
+        except IndexError:
+            break
+
+    self.previous = self.index
+    result = self.buffer[self.index:self.index + pos]
+    self.index += pos
+
+    # If the command looks like a python syntax command, try to parse it.
+    if current == "(" or current == "[":
+        try:
+            result = parse_python(pos)
+
+        except SyntaxError:
+            log.debug(
+                "A SyntaxError was encountered while parsing a python-syntaxed command:"
+                "\nTraceback (most recent call last):\n"
+                '  File "<stdin>", line 1, in <module>\n'
+                f"    {self.buffer}\n"
+                f"     {' ' * self.index}^\n"
+                "SyntaxError: invalid syntax"
+            )
+            return
+
+        except ValueError:
+            log.debug(
+                "A ValueError was encountered while parsing a python-syntaxed command:"
+                "\nTraceback (most recent call last):\n"
+                '  File "<stdin>", line 1, in <module>\n'
+                f"ValueError: could not ast.literal_eval the following: '{self.buffer}'"
+            )
+            return
 
     return result
 
diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py
index 24ace01c7..1c8f54d9f 100644
--- a/bot/cogs/tags.py
+++ b/bot/cogs/tags.py
@@ -9,8 +9,9 @@ from discord.ext.commands import (
 )
 
 from bot.constants import (
-    ADMIN_ROLE, MODERATOR_ROLE, OWNER_ROLE,
-    SITE_API_KEY, SITE_API_TAGS_URL, TAG_COOLDOWN,
+    ADMIN_ROLE, BOT_COMMANDS_CHANNEL, DEVTEST_CHANNEL,
+    HELPERS_CHANNEL, MODERATOR_ROLE, OWNER_ROLE,
+    SITE_API_KEY, SITE_API_TAGS_URL, TAG_COOLDOWN
 )
 from bot.decorators import with_role
 from bot.exceptions import CogBadArgument
@@ -18,6 +19,12 @@ from bot.pagination import LinePaginator
 
 log = logging.getLogger(__name__)
 
+TEST_CHANNELS = (
+    DEVTEST_CHANNEL,
+    BOT_COMMANDS_CHANNEL,
+    HELPERS_CHANNEL
+)
+
 
 class TagNameConverter(Converter):
     @staticmethod
@@ -209,10 +216,12 @@ class Tags:
             if tag_name:
                 log.debug(f"{ctx.author} requested the tag '{tag_name}'")
                 embed.title = tag_name
-                self.tag_cooldowns[tag_name] = {
-                    "time": time.time(),
-                    "channel": ctx.channel.id
-                }
+
+                if ctx.channel.id not in TEST_CHANNELS:
+                    self.tag_cooldowns[tag_name] = {
+                        "time": time.time(),
+                        "channel": ctx.channel.id
+                    }
 
             else:
                 embed.title = "**Current tags**"
diff --git a/bot/constants.py b/bot/constants.py
index b0a2feb2f..7b2d8214a 100644
--- a/bot/constants.py
+++ b/bot/constants.py
@@ -5,6 +5,7 @@ PYTHON_GUILD = 267624335836053506
 
 # Channels
 BOT_CHANNEL = 267659945086812160
+BOT_COMMANDS_CHANNEL = 267659945086812160
 CHECKPOINT_TEST_CHANNEL = 422077681434099723
 DEVLOG_CHANNEL = 409308876241108992
 DEVTEST_CHANNEL = 414574275865870337
@@ -12,6 +13,7 @@ HELP1_CHANNEL = 303906576991780866
 HELP2_CHANNEL = 303906556754395136
 HELP3_CHANNEL = 303906514266226689
 HELP4_CHANNEL = 439702951246692352
+HELPERS_CHANNEL = 385474242440986624
 MOD_LOG_CHANNEL = 282638479504965634
 PYTHON_CHANNEL = 267624335836053506
 VERIFICATION_CHANNEL = 352442727016693763