[CI] Improve automated checks (#2702)

* same stuff, but with some more spurious error supression

* fix issue in permissions found in this

* fix a few more spurious errors

* fix another issue

* semi-spurious error fixes

* .

* formatting

* move this to properly log

* distutils import + virtualenv

* more fixes

.pylintrc

@@ -0,0 +1,148 @@
+[MASTER]
+
+# Specify a configuration file.
+#rcfile=
+
+# Add files or directories to the blacklist. They should be base names, not
+# paths.
+ignore=pytest
+
+# Pickle collected data for later comparisons.
+persistent=no
+
+# List of plugins (as comma separated values of python modules names) to load,
+# usually to register additional checkers.
+load-plugins=
+
+# DO NOT CHANGE THIS VALUE # Use multiple processes to speed up Pylint.
+jobs=1
+
+# Allow loading of arbitrary C extensions. Extensions are imported into the
+# active Python interpreter and may run arbitrary code.
+unsafe-load-any-extension=no
+
+# A comma-separated list of package or module names from where C extensions may
+# be loaded. Extensions are loading into the active Python interpreter and may
+# run arbitrary code
+extension-pkg-whitelist=
+
+# Allow optimization of some AST trees. This will activate a peephole AST
+# optimizer, which will apply various small optimizations. For instance, it can
+# be used to obtain the result of joining multiple strings with the addition
+# operator. Joining a lot of strings can lead to a maximum recursion error in
+# Pylint and this flag can prevent that. It has one side effect, the resulting
+# AST will be different than the one from reality.
+optimize-ast=no
+
+
+[MESSAGES CONTROL]
+
+# Only show warnings with the listed confidence levels. Leave empty to show
+# all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED
+confidence=
+
+# Enable the message, report, category or checker with the given id(s). You can
+# either give multiple identifier separated by comma (,) or put this option
+# multiple time. See also the "--disable" option for examples.
+
+
+enable=all
+
+disable=C, # black is enforcing this for us already, incompatibly
+    W, # unbroaden this to the below specifics later on.
+    W0107, # uneccessary pass is stylisitc in most places
+    W0212, # Should likely refactor around protected access warnings later
+    W1203, # fstrings are too fast to care about enforcing this.
+    W0612, # unused vars can sometimes indicate an issue, but ...
+    W1401, # Should probably fix the reason this is disabled (start up screen)
+    W0511, # Nope, todos are fine for future people to see things to do.
+    W0613, # Too many places where we need to take unused args do to d.py ... also menus
+    W0221, # Overriden converters.
+    W0223, # abstractmethod not defined in mixins is expected
+    I, # ...
+    R # While some of these have merit, It's too large a burden to enable this right now.
+
+
+[REPORTS]
+
+output-format=parseable
+files-output=no
+reports=no
+
+
+[LOGGING]
+
+# Logging modules to check that the string format arguments are in logging
+# function parameter format
+logging-modules=logging
+
+
+[TYPECHECK]
+
+# Tells whether missing members accessed in mixin class should be ignored. A
+# mixin class is detected if its name ends with "mixin" (case insensitive).
+ignore-mixin-members=yes
+
+# TODO: Write a plyint plugin to allow this with these mixin classes
+# To use the abstractmethod we know will be defined in the final class.
+ignored-classes=redbot.cogs.mod.movetocore.MoveToCore,
+    redbot.cogs.mod.kickban.KickBanMixin,
+    redbot.cogs.mod.mutes.MuteMixin,
+    redbot.cogs.mod.names.ModInfo,
+    redbot.cogs.mod.settings.ModSettings,
+    redbot.cogs.mod.events.Events
+
+ignored-modules=distutils  # https://github.com/PyCQA/pylint/issues/73
+
+
+[VARIABLES]
+
+# Tells whether we should check for unused import in __init__ files.
+init-import=no
+
+# A regular expression matching the name of dummy variables (i.e. expectedly
+# not used).
+dummy-variables-rgx=_$|dummy
+
+
+[SIMILARITIES]
+
+# Minimum lines number of a similarity.
+min-similarity-lines=4
+
+# Ignore comments when computing similarities.
+ignore-comments=yes
+
+# Ignore docstrings when computing similarities.
+ignore-docstrings=yes
+
+# Ignore imports when computing similarities.
+ignore-imports=no
+
+
+[MISCELLANEOUS]
+
+# List of note tags to take in consideration, separated by a comma.
+notes=FIXME,XXX,TODO
+
+
+[CLASSES]
+
+# List of method names used to declare (i.e. assign) instance attributes.
+defining-attr-methods=__init__,__new__,__call__
+
+# List of valid names for the first argument in a class method.
+valid-classmethod-first-arg=cls
+
+# List of valid names for the first argument in a metaclass class method.
+valid-metaclass-classmethod-first-arg=mcs
+
+# List of member names, which should be excluded from the protected access
+# warning.
+exclude-protected=
+
+[EXCEPTIONS]
+
+# Exceptions that will emit a warning when being caught. Defaults to
+# "Exception"
+overgeneral-exceptions=Exception,discord.DiscordException

redbot/cogs/audio/manager.py

@@ -3,7 +3,7 @@ import pathlib
 import platform
 import shutil
 import asyncio
-import asyncio.subprocess
+import asyncio.subprocess  # disables for # https://github.com/PyCQA/pylint/issues/1469
 import logging
 import re
 import tempfile
@@ -42,7 +42,7 @@ class ServerManager:
     def __init__(self) -> None:
         self.ready = asyncio.Event()
 
-        self._proc: Optional[asyncio.subprocess.Process] = None
+        self._proc: Optional[asyncio.subprocess.Process] = None  # pylint:disable=no-member
         self._monitor_task: Optional[asyncio.Task] = None
         self._shutdown: bool = False
 
@@ -67,7 +67,7 @@ class ServerManager:
         shutil.copyfile(BUNDLED_APP_YML, LAVALINK_APP_YML)
 
         args = await self._get_jar_args()
-        self._proc = await asyncio.subprocess.create_subprocess_exec(
+        self._proc = await asyncio.subprocess.create_subprocess_exec(  # pylint:disable=no-member
             *args,
             cwd=str(LAVALINK_DOWNLOAD_DIR),
             stdout=asyncio.subprocess.PIPE,
@@ -117,7 +117,7 @@ class ServerManager:
         """
         This assumes we've already checked that java exists.
         """
-        _proc: asyncio.subprocess.Process = await asyncio.create_subprocess_exec(
+        _proc: asyncio.subprocess.Process = await asyncio.create_subprocess_exec(  # pylint:disable=no-member
             "java", "-version", stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE
         )
         # java -version outputs to stderr
@@ -173,7 +173,7 @@ class ServerManager:
             await self.start()
         else:
             log.critical(
-                "Your Java is borked. Please find the hs_err_pid{}.log file"
+                "Your Java is borked. Please find the hs_err_pid%d.log file"
                 " in the Audio data folder and report this issue.",
                 self._proc.pid,
             )
@@ -222,7 +222,7 @@ class ServerManager:
             return True
         args = await cls._get_jar_args()
         args.append("--version")
-        _proc = await asyncio.subprocess.create_subprocess_exec(
+        _proc = await asyncio.subprocess.create_subprocess_exec(  # pylint:disable=no-member
             *args,
             cwd=str(LAVALINK_DOWNLOAD_DIR),
             stdout=asyncio.subprocess.PIPE,

redbot/cogs/cleanup/cleanup.py

@@ -1,3 +1,4 @@
+import logging
 import re
 from datetime import datetime, timedelta
 from typing import Union, List, Callable, Set
@@ -8,12 +9,13 @@ from redbot.core import checks, commands
 from redbot.core.bot import Red
 from redbot.core.i18n import Translator, cog_i18n
 from redbot.core.utils.mod import slow_deletion, mass_purge
-from redbot.cogs.mod.log import log
 from redbot.core.utils.predicates import MessagePredicate
 from .converters import RawMessageIds
 
 _ = Translator("Cleanup", __file__)
 
+log = logging.getLogger("red.cleanup")
+
 
 @cog_i18n(_)
 class Cleanup(commands.Cog):
@@ -302,13 +304,13 @@ class Cleanup(commands.Cog):
         author = ctx.author
         try:
             mone = await channel.fetch_message(one)
-        except discord.errors.Notfound:
+        except discord.errors.NotFound:
             return await ctx.send(
                 _("Could not find a message with the ID of {id}.".format(id=one))
             )
         try:
             mtwo = await channel.fetch_message(two)
-        except discord.errors.Notfound:
+        except discord.errors.NotFound:
             return await ctx.send(
                 _("Could not find a message with the ID of {id}.".format(id=two))
             )

redbot/cogs/economy/economy.py

@@ -467,7 +467,9 @@ class Economy(commands.Cog):
             sign = "  "
             if i == 1:
                 sign = ">"
-            slot += "{}{} {} {}\n".format(sign, *[c.value for c in row])
+            slot += "{}{} {} {}\n".format(
+                sign, *[c.value for c in row]  # pylint: disable=no-member
+            )
 
         payout = PAYOUTS.get(rows[1])
         if not payout:

redbot/cogs/mod/events.py

@@ -1,13 +1,14 @@
+import logging
 from datetime import datetime
 from collections import defaultdict, deque
 
 import discord
 from redbot.core import i18n, modlog, commands
 from redbot.core.utils.mod import is_mod_or_superior
-from . import log
 from .abc import MixinMeta
 
 _ = i18n.Translator("Mod", __file__)
+log = logging.getLogger("red.mod")
 
 
 class Events(MixinMeta):

redbot/cogs/mod/kickban.py

@@ -1,5 +1,6 @@
 import asyncio
 import contextlib
+import logging
 from collections import namedtuple
 from datetime import datetime, timedelta
 from typing import cast, Optional, Union
@@ -10,8 +11,8 @@ from redbot.core.utils.chat_formatting import pagify
 from redbot.core.utils.mod import is_allowed_by_hierarchy, get_audit_reason
 from .abc import MixinMeta
 from .converters import RawUserIds
-from .log import log
 
+log = logging.getLogger("red.mod")
 _ = i18n.Translator("Mod", __file__)
 
 

redbot/cogs/mod/log.py

@@ -1,4 +0,0 @@
-import logging
-
-
-log = logging.getLogger("red.mod")

redbot/cogs/mod/movetocore.py

@@ -1,3 +1,4 @@
+import logging
 import asyncio
 import contextlib
 
@@ -5,8 +6,8 @@ import discord
 from redbot.core import commands, checks, i18n
 from redbot.core.utils.chat_formatting import box
 from .abc import MixinMeta
-from .log import log
 
+log = logging.getLogger("red.mod")
 _ = i18n.Translator("Mod", __file__)
 
 

redbot/cogs/permissions/permissions.py

@@ -28,6 +28,9 @@ COG = "COG"
 COMMAND = "COMMAND"
 GLOBAL = 0
 
+_OldConfigSchema = Dict[int, Dict[str, Dict[str, Dict[str, Dict[str, List[int]]]]]]
+_NewConfigSchema = Dict[str, Dict[int, Dict[str, Dict[int, bool]]]]
+
 # The strings in the schema are constants and should get extracted, but not translated until
 # runtime.
 translate = _
@@ -625,9 +628,6 @@ class Permissions(commands.Cog):
         await self.config.custom(COMMAND).set(new_cmd_rules)
         await self.config.version.set(__version__)
 
-    _OldConfigSchema = Dict[int, Dict[str, Dict[str, Dict[str, Dict[str, List[int]]]]]]
-    _NewConfigSchema = Dict[str, Dict[int, Dict[str, Dict[int, bool]]]]
-
     @staticmethod
     def _get_updated_schema(
         old_config: _OldConfigSchema

redbot/cogs/streams/streamtypes.py

@@ -215,7 +215,7 @@ class TwitchStream(Stream):
             status = "Untitled broadcast"
         if is_rerun:
             status += " - Rerun"
-        embed = discord.Embed(title=status, url=url)
+        embed = discord.Embed(title=status, url=url, color=0x6441A4)
         embed.set_author(name=channel["display_name"])
         embed.add_field(name="Followers", value=channel["followers"])
         embed.add_field(name="Total views", value=channel["views"])
@@ -224,7 +224,6 @@ class TwitchStream(Stream):
             embed.set_image(url=rnd(data["stream"]["preview"]["medium"]))
         if channel["game"]:
             embed.set_footer(text="Playing: " + channel["game"])
-        embed.color = 0x6441A4
 
         return embed
 
@@ -260,14 +259,13 @@ class HitboxStream(Stream):
         livestream = data["livestream"][0]
         channel = livestream["channel"]
         url = channel["channel_link"]
-        embed = discord.Embed(title=livestream["media_status"], url=url)
+        embed = discord.Embed(title=livestream["media_status"], url=url, color=0x98CB00)
         embed.set_author(name=livestream["media_name"])
         embed.add_field(name="Followers", value=channel["followers"])
         embed.set_thumbnail(url=base_url + channel["user_logo"])
         if livestream["media_thumbnail"]:
             embed.set_image(url=rnd(base_url + livestream["media_thumbnail"]))
         embed.set_footer(text="Playing: " + livestream["category_name"])
-        embed.color = 0x98CB00
 
         return embed
 
@@ -310,7 +308,7 @@ class MixerStream(Stream):
             embed.set_thumbnail(url=default_avatar)
         if data["thumbnail"]:
             embed.set_image(url=rnd(data["thumbnail"]["url"]))
-        embed.color = 0x4C90F3
+        embed.color = 0x4C90F3  # pylint: disable=assigning-non-slot
         if data["type"] is not None:
             embed.set_footer(text="Playing: " + data["type"]["name"])
         return embed
@@ -345,13 +343,12 @@ class PicartoStream(Stream):
         )
         url = "https://picarto.tv/" + data["name"]
         thumbnail = data["thumbnails"]["web"]
-        embed = discord.Embed(title=data["title"], url=url)
+        embed = discord.Embed(title=data["title"], url=url, color=0x4C90F3)
         embed.set_author(name=data["name"])
         embed.set_image(url=rnd(thumbnail))
         embed.add_field(name="Followers", value=data["followers"])
         embed.add_field(name="Total views", value=data["viewers_total"])
         embed.set_thumbnail(url=avatar)
-        embed.color = 0x132332
         data["tags"] = ", ".join(data["tags"])
 
         if not data["tags"]:
@@ -362,6 +359,5 @@ class PicartoStream(Stream):
         else:
             data["adult"] = ""
 
-        embed.color = 0x4C90F3
         embed.set_footer(text="{adult}Category: {category} | Tags: {tags}".format(**data))
         return embed

redbot/core/bot.py

@@ -27,7 +27,8 @@ def _is_submodule(parent, child):
     return parent == child or child.startswith(parent + ".")
 
 
-class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin):
+# barely spurious warning caused by our intentional shadowing
+class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin):  # pylint: disable=no-member
     """Mixin for the main bot class.
 
     This exists because `Red` inherits from `discord.AutoShardedClient`, which

redbot/core/commands/requires.py

@@ -704,29 +704,35 @@ def mod():
 class _IntKeyDict(Dict[int, _T]):
     """Dict subclass which throws KeyError when a non-int key is used."""
 
+    get: Callable
+    setdefault: Callable
+
     def __getitem__(self, key: Any) -> _T:
         if not isinstance(key, int):
             raise TypeError("Keys must be of type `int`")
-        return super().__getitem__(key)
+        return super().__getitem__(key)  # pylint: disable=no-member
 
     def __setitem__(self, key: Any, value: _T) -> None:
         if not isinstance(key, int):
             raise TypeError("Keys must be of type `int`")
-        return super().__setitem__(key, value)
+        return super().__setitem__(key, value)  # pylint: disable=no-member
 
 
 class _RulesDict(Dict[Union[int, str], PermState]):
     """Dict subclass which throws a KeyError when an invalid key is used."""
 
+    get: Callable
+    setdefault: Callable
+
     def __getitem__(self, key: Any) -> PermState:
         if key != Requires.DEFAULT and not isinstance(key, int):
             raise TypeError(f'Expected "{Requires.DEFAULT}" or int key, not "{key}"')
-        return super().__getitem__(key)
+        return super().__getitem__(key)  # pylint: disable=no-member
 
     def __setitem__(self, key: Any, value: PermState) -> None:
         if key != Requires.DEFAULT and not isinstance(key, int):
             raise TypeError(f'Expected "{Requires.DEFAULT}" or int key, not "{key}"')
-        return super().__setitem__(key, value)
+        return super().__setitem__(key, value)  # pylint: disable=no-member
 
 
 def _validate_perms_dict(perms: Dict[str, bool]) -> None:

redbot/core/config.py

@@ -30,7 +30,7 @@ def get_latest_confs() -> Tuple["Config"]:
     return tuple(ret)
 
 
-class _ValueCtxManager(Awaitable[_T], AsyncContextManager[_T]):
+class _ValueCtxManager(Awaitable[_T], AsyncContextManager[_T]):  # pylint: disable=duplicate-bases
     """Context manager implementation of config values.
 
     This class allows mutable config values to be both "get" and "set" from
@@ -1135,7 +1135,8 @@ class Config:
             )
             group = Group(identifier_data, defaults={}, driver=self.driver)
         else:
-            group = self._get_base_group(*scopes)
+            cat, *scopes = scopes
+            group = self._get_base_group(cat, *scopes)
         await group.clear()
 
     async def clear_all(self):

redbot/core/core_commands.py

@@ -963,13 +963,13 @@ class Core(commands.Cog, CoreLogic):
                 "message", check=MessagePredicate.same_context(ctx), timeout=60
             )
         except asyncio.TimeoutError:
-            self.owner.reset_cooldown(ctx)
+            ctx.command.reset_cooldown(ctx)
             await ctx.send(
                 _("The `{prefix}set owner` request has timed out.").format(prefix=ctx.prefix)
             )
         else:
             if message.content.strip() == token:
-                self.owner.reset_cooldown(ctx)
+                ctx.command.reset_cooldown(ctx)
                 await ctx.bot.db.owner.set(ctx.author.id)
                 ctx.bot.owner_id = ctx.author.id
                 await ctx.send(_("You have been set as owner."))

redbot/core/data_manager.py

@@ -35,7 +35,7 @@ basic_config_default = {"DATA_PATH": None, "COG_PATH_APPEND": "cogs", "CORE_PATH
 config_dir = None
 appdir = appdirs.AppDirs("Red-DiscordBot")
 if sys.platform == "linux":
-    if 0 < os.getuid() < 1000:
+    if 0 < os.getuid() < 1000:  # pylint: disable=no-member
         config_dir = Path(appdir.site_data_dir)
 if not config_dir:
     config_dir = Path(appdir.user_config_dir)

redbot/core/json_io.py

@@ -57,7 +57,7 @@ class JsonIO:
 
         tmp_path.replace(self.path)
 
-        # pylint: disable=E1101
+        # pylint: disable=no-member
         try:
             fd = os.open(self.path.parent, os.O_DIRECTORY)
             os.fsync(fd)

redbot/core/utils/__init__.py

@@ -69,7 +69,8 @@ def safe_delete(pth: Path):
         shutil.rmtree(str(pth), ignore_errors=True)
 
 
-class AsyncFilter(AsyncIterator[_T], Awaitable[List[_T]]):
+# https://github.com/PyCQA/pylint/issues/2717
+class AsyncFilter(AsyncIterator[_T], Awaitable[List[_T]]):  # pylint: disable=duplicate-bases
     """Class returned by `async_filter`. See that function for details.
 
     We don't recommend instantiating this class directly.
@@ -112,6 +113,9 @@ class AsyncFilter(AsyncIterator[_T], Awaitable[List[_T]]):
     async def __flatten(self) -> List[_T]:
         return [item async for item in self]
 
+    def __aiter__(self):
+        return self
+
     def __await__(self):
         # Simply return the generator filled into a list
         return self.__flatten().__await__()

redbot/setup.py

@@ -34,7 +34,7 @@ conversion_log = logging.getLogger("red.converter")
 config_dir = None
 appdir = appdirs.AppDirs("Red-DiscordBot")
 if sys.platform == "linux":
-    if 0 < os.getuid() < 1000:
+    if 0 < os.getuid() < 1000:  # pylint: disable=no-member  # Non-exist on win
         config_dir = Path(appdir.site_data_dir)
 if not config_dir:
     config_dir = Path(appdir.user_config_dir)
@@ -524,7 +524,7 @@ def convert(instance, backend):
 
 if __name__ == "__main__":
     try:
-        cli()
+        cli()  # pylint: disable=no-value-for-parameter  # click
     except KeyboardInterrupt:
         print("Exiting...")
     else:

setup.cfg

@@ -84,6 +84,7 @@ test =
     pytest==4.2.0
     pytest-asyncio==0.10.0
     six==1.12.0
+    pylint==2.3.1
 
 [options.entry_points]
 console_scripts =

tox.ini

@@ -11,13 +11,15 @@ envlist =
 skip_missing_interpreters = True
 
 [testenv]
-description = Run unit tests with pytest
+description = Run tests and basic automatic issue checking.
 whitelist_externals =
     pytest
+    pylint
 extras = voice, test, mongo
 commands =
     python -m compileall ./redbot/cogs
     pytest
+    pylint ./redbot
 
 [testenv:docs]
 description = Attempt to build docs with sphinx-build