Update flake8 config and run Flake8 over the whole project. (#2379)

Also changes skip command permission checks for looped songs.

.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
+select = B,C,E,F,W,T4,B9,TC,TC1
+

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):

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

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:

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"
                 )

musicbot/constants.py

@@ -1,4 +1,3 @@
-import os.path
 import subprocess
 
 try:

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

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

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):