From a08f79274aeb304dd7962eb9ae0d73e396e843ec Mon Sep 17 00:00:00 2001
From: Fae <itsthefae@gmail.com>
Date: Fri, 22 Dec 2023 14:49:15 -0800
Subject: [PATCH] Update flake8 config and run Flake8 over the whole project.
 (#2379)

Also changes skip command permission checks for looped songs.
---
 .flake8                       |  5 +++--
 bootstrap.py                  | 17 ++++++++---------
 musicbot/__init__.py          | 14 +++++---------
 musicbot/bot.py               | 35 +++++++++++++++++++++--------------
 musicbot/config.py            |  8 ++++----
 musicbot/constants.py         |  1 -
 musicbot/entry.py             | 21 +++++++++++----------
 musicbot/json.py              |  2 +-
 musicbot/lib/event_emitter.py |  3 +--
 musicbot/permissions.py       | 10 +++++-----
 musicbot/player.py            |  4 ++--
 musicbot/playlist.py          |  4 ++--
 musicbot/utils.py             | 12 ++++++------
 run.py                        |  3 +--
 14 files changed, 70 insertions(+), 69 deletions(-)

diff --git a/.flake8 b/.flake8
index 3ef95b42c..aa05fce9e 100644
--- a/.flake8
+++ b/.flake8
@@ -1,5 +1,6 @@
 [flake8]
-ignore = E203, E266, E501, E731, W503, F403, F401
+ignore = E203, E266, E501, W503, C901
 max-line-length = 99
 max-complexity = 18
-select = B,C,E,F,W,T4,B9
\ No newline at end of file
+select = B,C,E,F,W,T4,B9,TC,TC1
+
diff --git a/bootstrap.py b/bootstrap.py
index 0bdfdd88c..fa0001c41 100644
--- a/bootstrap.py
+++ b/bootstrap.py
@@ -33,7 +33,6 @@
 import re
 import shutil
 import sys
-import logging
 import platform
 import tempfile
 import traceback
@@ -157,7 +156,7 @@ def __getattribute__(self, item):
         try:
             # Check for platform variant of function first
             return object.__getattribute__(self, item + "_" + SYS_PLATFORM)
-        except:
+        except Exception:
             pass
 
         if item.endswith("_dist"):
@@ -166,11 +165,11 @@ def __getattribute__(self, item):
                 return object.__getattribute__(
                     self, item.rsplit("_", 1)[0] + "_" + SYS_PLATFORM
                 )
-            except:
+            except Exception:
                 try:
                     # If there's no dist variant, try to fallback to the generic, ex: setup_dist -> setup
                     return object.__getattribute__(self, item.rsplit("_", 1)[0])
-                except:
+                except Exception:
                     pass
 
         return object.__getattribute__(self, item)
@@ -308,7 +307,7 @@ def setup(self, data):
 
 class EnsureGit(SetupTask):
     WIN_OPTS = dedent(
-        """
+        r"""
         [Setup]
         Lang=default
         Group=Git
@@ -344,7 +343,7 @@ def _get_latest_win_git_version():
 
             match = re.match(r"v(\d+\.\d+\.\d+)", full_ver)
             return match.groups()[0], full_ver
-        except:
+        except Exception:
             return version
 
     @classmethod
@@ -483,7 +482,7 @@ class EnsurePip(SetupTask):
     def check(self):
         # Check if pip is installed by importing it.
         try:
-            import pip
+            import pip  # noqa: F401
         except ImportError:
             return False
         else:
@@ -492,7 +491,7 @@ def check(self):
     def download(self):
         # Try and use ensurepip.
         try:
-            import ensurepip
+            import ensurepip  # noqa: F401
 
             return False
         except ImportError:
@@ -556,7 +555,7 @@ class SetupMusicbot(SetupTask):
     def _rm(self, f):
         try:
             return os.unlink(f)
-        except:
+        except Exception:
             pass
 
     def _rm_glob(self, p):
diff --git a/musicbot/__init__.py b/musicbot/__init__.py
index 4da7e9383..c3ac83df4 100644
--- a/musicbot/__init__.py
+++ b/musicbot/__init__.py
@@ -1,5 +1,3 @@
-import sys
-import inspect
 import logging
 from .bot import MusicBot
 from .constructs import BetterLogRecord
@@ -8,12 +6,6 @@
 
 logging.setLogRecordFactory(BetterLogRecord)
 
-_func_prototype = (
-    "def {logger_func_name}(self, message, *args, **kwargs):\n"
-    "    if self.isEnabledFor({levelname}):\n"
-    "        self._log({levelname}, message, args, **kwargs)"
-)
-
 
 def _add_logger_level(levelname, level, *, func_name=None):
     """
@@ -25,6 +17,11 @@ def _add_logger_level(levelname, level, *, func_name=None):
     :type func_name: str
         The name of the logger function to log to a level, e.g. "info" for log.info(...)
     """
+    _func_prototype = (
+        "def {logger_func_name}(self, message, *args, **kwargs):\n"
+        "    if self.isEnabledFor({levelname}):\n"
+        "        self._log({levelname}, message, args, **kwargs)"
+    )
 
     func_name = func_name or levelname.lower()
 
@@ -57,6 +54,5 @@ def _add_logger_level(levelname, level, *, func_name=None):
 )
 log.addHandler(fhandler)
 
-del _func_prototype
 del _add_logger_level
 del fhandler
diff --git a/musicbot/bot.py b/musicbot/bot.py
index f63f9961a..33d733a2a 100644
--- a/musicbot/bot.py
+++ b/musicbot/bot.py
@@ -9,7 +9,6 @@
 import random
 import re
 import shlex
-import shutil
 import ssl
 import sys
 import time
@@ -64,7 +63,7 @@ class MusicBot(discord.Client):
     def __init__(self, config_file=None, perms_file=None, aliases_file=None):
         try:
             sys.stdout.write("\x1b]2;MusicBot {}\x07".format(BOTVERSION))
-        except:
+        except Exception:
             pass
 
         print()
@@ -1210,7 +1209,7 @@ async def run(self):
                 "Bot cannot login, bad credentials.",
                 "Fix your token in the options file.  "
                 "Remember that each field should be on their own line.",
-            )  #     ^^^^ In theory self.config.auth should never have no items
+            )  # ^^^^ In theory self.config.auth should never have no items
 
         finally:
             try:
@@ -2184,6 +2183,7 @@ async def cmd_move(self, channel, command, leftover_args):
 
         Swaps the location of a song within the playlist.
         """
+        # TODO: this command needs some tlc. args renamed, better checks.
         player = self.get_player_in(channel.guild)
         if not player:
             raise exceptions.CommandError(
@@ -2209,7 +2209,8 @@ async def cmd_move(self, channel, command, leftover_args):
         try:
             indexes.append(int(command) - 1)
             indexes.append(int(leftover_args[0]) - 1)
-        except:
+        except (ValueError, IndexError):
+            # TODO: return command error instead, specific to the exception.
             return Response(
                 self.str.get(
                     "cmd-move-indexes_not_intergers", "Song indexes must be integers!"
@@ -2706,8 +2707,8 @@ async def get_info(song_url):
                     reply_text += self.str.get(
                         "cmd-play-eta-error", " - cannot estimate time until playing"
                     )
-                except:
-                    traceback.print_exc()
+                except Exception:
+                    log.exception("Unhandled exception in _cmd_play time until play.")
 
         return Response(reply_text, delete_after=30)
 
@@ -2789,7 +2790,7 @@ async def _cmd_play_playlist_async(
                         player.playlist.entries.remove(e)
                         entries_added.remove(e)
                         drop_count += 1
-                    except:
+                    except Exception:
                         pass
 
             if drop_count:
@@ -3236,7 +3237,7 @@ async def cmd_np(self, player, channel, guild, message):
             song_progress = ftimedelta(timedelta(seconds=player.progress))
             song_total = (
                 ftimedelta(timedelta(seconds=player.current_entry.duration))
-                if player.current_entry.duration != None
+                if player.current_entry.duration is not None
                 else "(no duration data)"
             )
 
@@ -3652,13 +3653,19 @@ async def cmd_skip(
 
         permission_force_skip = permissions.instaskip or (
             self.config.allow_author_skip
-            and author == player.current_entry.meta.get("author", None)
+            and author == current_entry.meta.get("author", None)
+            # TODO: Is there a case where this fails due to changes in author objects?
         )
         force_skip = param.lower() in ["force", "f"]
 
         if permission_force_skip and (force_skip or self.config.legacy_skip):
-            if not permissions.skiplooped and player.repeatsong:
-                raise errors.PermissionsError(
+            if (
+                not permission_force_skip
+                and not permissions.skiplooped
+                and player.repeatsong
+            ):
+                # TODO: permissions.skiplooped defaults to False even in Permissive. Change that maybe?
+                raise exceptions.PermissionsError(
                     self.str.get(
                         "cmd-skip-force-noperms-looped-song",
                         "You do not have permission to force skip a looped song.",
@@ -4030,7 +4037,7 @@ async def cmd_queue(self, channel, player):
             song_progress = ftimedelta(timedelta(seconds=player.progress))
             song_total = (
                 ftimedelta(timedelta(seconds=player.current_entry.duration))
-                if player.current_entry.duration != None
+                if player.current_entry.duration is not None
                 else "(no duration data)"
             )
             prog_str = "`[%s/%s]`" % (song_progress, song_total)
@@ -4307,7 +4314,7 @@ async def cmd_perms(
 
         if not user_mentions and target:
             user = guild.get_member_named(target)
-            if user == None:
+            if user is None:
                 try:
                     user = await self.fetch_user(target)
                 except discord.NotFound:
@@ -4606,7 +4613,7 @@ async def cmd_debug(self, message, _player, *, data):
 
         try:
             result = eval(code, scope)
-        except:
+        except Exception:
             try:
                 exec(code, scope)
             except Exception as e:
diff --git a/musicbot/config.py b/musicbot/config.py
index eac8d94fb..84f3d2c6f 100644
--- a/musicbot/config.py
+++ b/musicbot/config.py
@@ -394,7 +394,7 @@ def run_checks(self):
                 self.bot_exception_ids = set(
                     int(x) for x in self.bot_exception_ids.replace(",", " ").split()
                 )
-            except:
+            except ValueError:
                 log.warning("BotExceptionIDs data is invalid, will ignore all bots")
                 self.bot_exception_ids = set()
 
@@ -403,7 +403,7 @@ def run_checks(self):
                 self.bound_channels = set(
                     int(x) for x in self.bound_channels.replace(",", " ").split() if x
                 )
-            except:
+            except ValueError:
                 log.warning(
                     "BindToChannels data is invalid, will not bind to any channels"
                 )
@@ -416,7 +416,7 @@ def run_checks(self):
                     for x in self.autojoin_channels.replace(",", " ").split()
                     if x
                 )
-            except:
+            except ValueError:
                 log.warning(
                     "AutojoinChannels data is invalid, will not autojoin any channels"
                 )
@@ -429,7 +429,7 @@ def run_checks(self):
                     for x in self.nowplaying_channels.replace(",", " ").split()
                     if x
                 )
-            except:
+            except ValueError:
                 log.warning(
                     "NowPlayingChannels data is invalid, will use the default behavior for all servers"
                 )
diff --git a/musicbot/constants.py b/musicbot/constants.py
index d57e1cdb8..ff5b93925 100644
--- a/musicbot/constants.py
+++ b/musicbot/constants.py
@@ -1,4 +1,3 @@
-import os.path
 import subprocess
 
 try:
diff --git a/musicbot/entry.py b/musicbot/entry.py
index 004a9c6cb..2400f50e2 100644
--- a/musicbot/entry.py
+++ b/musicbot/entry.py
@@ -79,8 +79,8 @@ def _for_each_future(self, cb):
             try:
                 cb(future)
 
-            except:
-                traceback.print_exc()
+            except Exception:
+                log.exception("Unhandled exception in _for_each_future callback.")
 
     def __eq__(self, other):
         return self is other
@@ -248,7 +248,7 @@ async def _download(self):
                                 self.playlist.bot.aiosession, self.url, "CONTENT-LENGTH"
                             )
                         )
-                    except:
+                    except Exception:
                         rsize = 0
 
                     lfile = os.path.join(
@@ -308,7 +308,7 @@ async def _download(self):
                     try:
                         mediainfo = pymediainfo.MediaInfo.parse(self.filename)
                         self.duration = mediainfo.tracks[0].duration / 1000
-                    except:
+                    except Exception:
                         self.duration = None
 
                 else:
@@ -351,7 +351,7 @@ async def _download(self):
             if self.playlist.bot.config.use_experimental_equalization:
                 try:
                     aoptions = await self.get_mean_volume(self.filename)
-                except Exception as e:
+                except Exception:
                     log.error(
                         "There as a problem with working out EQ, likely caused by a strange installation of FFmpeg. "
                         "This has not impacted the ability for the bot to work, but will mean your tracks will not be equalised."
@@ -365,9 +365,10 @@ async def _download(self):
             # Trigger ready callbacks.
             self._for_each_future(lambda future: future.set_result(self))
 
-        except Exception as e:
+        # Flake8 thinks 'e' is never used, and later undefined. Maybe the lambda is too much.
+        except Exception as e:  # noqa: F841
             traceback.print_exc()
-            self._for_each_future(lambda future: future.set_exception(e))
+            self._for_each_future(lambda future: future.set_exception(e))  # noqa: F821
 
         finally:
             self._is_downloading = False
@@ -389,10 +390,10 @@ async def get_mean_volume(self, input_file):
         i_matches = re.findall(r'"input_i" : "(-?([0-9]*\.[0-9]+))",', output)
         if i_matches:
             log.debug("i_matches={}".format(i_matches[0][0]))
-            I = float(i_matches[0][0])
+            IVAL = float(i_matches[0][0])
         else:
             log.debug("Could not parse I in normalise json.")
-            I = float(0)
+            IVAL = float(0)
 
         lra_matches = re.findall(r'"input_lra" : "(-?([0-9]*\.[0-9]+))",', output)
         if lra_matches:
@@ -427,7 +428,7 @@ async def get_mean_volume(self, input_file):
             offset = float(0)
 
         return "-af loudnorm=I=-24.0:LRA=7.0:TP=-2.0:linear=true:measured_I={}:measured_LRA={}:measured_TP={}:measured_thresh={}:offset={}".format(
-            I, LRA, TP, thresh, offset
+            IVAL, LRA, TP, thresh, offset
         )
 
     # noinspection PyShadowingBuiltins
diff --git a/musicbot/json.py b/musicbot/json.py
index 4e197e4ab..40d87d0be 100644
--- a/musicbot/json.py
+++ b/musicbot/json.py
@@ -25,6 +25,6 @@ def get(self, item, fallback=None):
         try:
             data = self.data[item]
         except KeyError:
-            log.warning("Could not grab data from i18n key {0}.".format(item, fallback))
+            log.warning(f"Could not grab data from i18n key {item}.")
             data = fallback
         return data
diff --git a/musicbot/lib/event_emitter.py b/musicbot/lib/event_emitter.py
index 2634a0803..bcd83eb51 100644
--- a/musicbot/lib/event_emitter.py
+++ b/musicbot/lib/event_emitter.py
@@ -13,14 +13,13 @@ def emit(self, event, *args, **kwargs):
             return
 
         for cb in list(self._events[event]):
-            # noinspection PyBroadException
             try:
                 if asyncio.iscoroutinefunction(cb):
                     asyncio.ensure_future(cb(*args, **kwargs), loop=self.loop)
                 else:
                     cb(*args, **kwargs)
 
-            except:
+            except Exception:
                 traceback.print_exc()
 
     def on(self, event, cb):
diff --git a/musicbot/permissions.py b/musicbot/permissions.py
index bb8955fc0..ad3975071 100644
--- a/musicbot/permissions.py
+++ b/musicbot/permissions.py
@@ -133,7 +133,7 @@ def for_user(self, user):
                 return group
 
         # The only way I could search for roles is if I add a `server=None` param and pass that too
-        if type(user) == discord.User:
+        if isinstance(user, discord.User):
             return self.default_group
 
         # We loop again so that we don't return a role based group before we find an assigned one
@@ -225,22 +225,22 @@ def validate(self):
 
         try:
             self.max_songs = max(0, int(self.max_songs))
-        except:
+        except ValueError:
             self.max_songs = PermissionsDefaults.MaxSongs
 
         try:
             self.max_song_length = max(0, int(self.max_song_length))
-        except:
+        except ValueError:
             self.max_song_length = PermissionsDefaults.MaxSongLength
 
         try:
             self.max_playlist_length = max(0, int(self.max_playlist_length))
-        except:
+        except ValueError:
             self.max_playlist_length = PermissionsDefaults.MaxPlaylistLength
 
         try:
             self.max_search_items = max(0, int(self.max_search_items))
-        except:
+        except ValueError:
             self.max_search_items = PermissionsDefaults.MaxSearchItems
 
         if int(self.max_search_items) > 100:
diff --git a/musicbot/player.py b/musicbot/player.py
index 91bb8b22e..73fd045bf 100644
--- a/musicbot/player.py
+++ b/musicbot/player.py
@@ -194,7 +194,7 @@ async def _play(self, _continue=False):
             if self.is_stopped or _continue:
                 try:
                     entry = await self.playlist.get_next_entry()
-                except:
+                except IndexError:
                     log.warning("Failed to get entry, retrying", exc_info=True)
                     self.loop.call_later(0.1, self.play)
                     return
@@ -406,7 +406,7 @@ def filter_stderr(stderr: io.BytesIO, future: asyncio.Future):
 def check_stderr(data: bytes):
     try:
         data = data.decode("utf8")
-    except:
+    except UnicodeDecodeError:
         log.ffmpeg("Unknown error decoding message from ffmpeg", exc_info=True)
         return True  # fuck it
 
diff --git a/musicbot/playlist.py b/musicbot/playlist.py
index 1f824d04c..1bfb8ac0f 100644
--- a/musicbot/playlist.py
+++ b/musicbot/playlist.py
@@ -463,14 +463,14 @@ async def estimate_time_until(self, position, player):
         """
         (very) Roughly estimates the time till the queue will 'position'
         """
-        if any(e.duration == None for e in islice(self.entries, position - 1)):
+        if any(e.duration is None for e in islice(self.entries, position - 1)):
             raise InvalidDataError("no duration data")
         else:
             estimated_time = sum(e.duration for e in islice(self.entries, position - 1))
 
         # When the player plays a song, it eats the first playlist item, so we just have to add the time back
         if not player.is_stopped and player.current_entry:
-            if player.current_entry.duration == None:  # duration can be 0
+            if player.current_entry.duration is None:  # duration can be 0
                 raise InvalidDataError("no duration data in current entry")
             else:
                 estimated_time += player.current_entry.duration - player.progress
diff --git a/musicbot/utils.py b/musicbot/utils.py
index 4b7d30424..6999240a8 100644
--- a/musicbot/utils.py
+++ b/musicbot/utils.py
@@ -41,9 +41,9 @@ def paginate(content, *, length=DISCORD_MSG_CHAR_LIMIT, reserve=0):
     """
     Split up a large string or list of strings into chunks for sending to discord.
     """
-    if type(content) == str:
+    if isinstance(content, str):
         contentlist = content.split("\n")
-    elif type(content) == list:
+    elif isinstance(content, list):
         contentlist = content
     else:
         raise ValueError("Content must be str or list, not %s" % type(content))
@@ -100,21 +100,21 @@ def objdiff(obj1, obj2, *, access_attr=None, depth=0):
     changes = {}
 
     if access_attr is None:
-        attrdir = lambda x: x
+        attrdir = lambda x: x  # noqa: E731
 
     elif access_attr == "auto":
         if hasattr(obj1, "__slots__") and hasattr(obj2, "__slots__"):
-            attrdir = lambda x: getattr(x, "__slots__")
+            attrdir = lambda x: getattr(x, "__slots__")  # noqa: E731
 
         elif hasattr(obj1, "__dict__") and hasattr(obj2, "__dict__"):
-            attrdir = lambda x: getattr(x, "__dict__")
+            attrdir = lambda x: getattr(x, "__dict__")  # noqa: E731
 
         else:
             # log.everything("{}{} or {} has no slots or dict".format('-' * (depth+1), repr(obj1), repr(obj2)))
             attrdir = dir
 
     elif isinstance(access_attr, str):
-        attrdir = lambda x: list(getattr(x, access_attr))
+        attrdir = lambda x: list(getattr(x, access_attr))  # noqa: E731
 
     else:
         attrdir = dir
diff --git a/run.py b/run.py
index 478f796f1..ba0433ed3 100644
--- a/run.py
+++ b/run.py
@@ -92,13 +92,12 @@ def run_show(cls, cmd, check_output=False):
     @classmethod
     def works(cls):
         try:
-            import pip
+            import pip  # noqa: F401
 
             return True
         except ImportError:
             return False
 
-    # noinspection PyTypeChecker
     @classmethod
     def get_module_version(cls, mod):
         try: