cleanup linters: get rid of pylint-pytest, run pylint only on dvc/ directory, etc (#8822)

* cleanup linters: get rid of pylint-pytest, run pylint only on dvc/ directory, etc.

* remove pylint direcitves

* Apply suggestions from code review

* Update pyproject.toml

.pre-commit-config.yaml

@@ -73,6 +73,7 @@ repos:
     - id: pylint
       name: pylint
       entry: pylint
+      files: ^dvc/
       language: system
       types: [python]
       require_serial: true

dvc/testing/api_tests.py

@@ -4,6 +4,8 @@ from dvc import api
 from dvc.api import DVCFileSystem
 from dvc.utils.fs import remove
 
+# pylint: disable=unused-argument
+
 
 class TestAPI:
     def test_get_url(self, tmp_dir, dvc, remote):

dvc/testing/fixtures.py

@@ -4,6 +4,8 @@ from typing import Dict, Tuple
 
 import pytest
 
+# pylint: disable=redefined-outer-name,unused-argument
+
 __all__ = [
     "make_tmp_dir",
     "tmp_dir",

dvc/testing/remote_tests.py

@@ -6,6 +6,8 @@ import pytest
 from dvc.stage.cache import RunCacheNotSupported
 from dvc.utils.fs import remove
 
+# pylint: disable=unused-argument
+
 
 class TestRemote:
     def test(self, tmp_dir, dvc, remote):  # pylint: disable=W0613

pyproject.toml

@@ -100,13 +100,7 @@ tests = [
     "filelock==3.9.0",
     "beautifulsoup4==4.11.1",
     "pywin32>=225; sys_platform == 'win32'",  # optional dependency
-    # pylint requirements
     "pylint==2.15.10",
-    # we use this to suppress pytest-related false positives in our tests.
-    "pylint-pytest==1.1.2",
-    # we use this to suppress some messages in tests, eg: foo/bar naming,
-    # and, protected method calls in our tests
-    "pylint-plugin-utils==0.7",
     # type-checking
     "mypy==0.991",
     "types-requests",
@@ -242,13 +236,10 @@ module = [
     "celery.*",
     "configobj.*",
     "dpath.*",
-    "dvc_data.*",
     "dvc_http.*",
-    "dvc_objects.*",
     "dvc_render.*",
     "dvc_ssh",
     "dvc_studio_client.*",
-    "dvc_task.*",
     "flatten_dict",
     "fsspec.*",
     "funcy.*",  # https://github.com/Suor/funcy/issues/106,
@@ -271,11 +262,6 @@ module = [
 ]
 ignore_missing_imports = true
 
-[tool.pylint.master]
-extension-pkg-whitelist = ["pygit2"]
-init-hook = "import sys; sys.path.append(str('tests'))"
-load-plugins = ["pylint_pytest", "pylint_plugin_disable"]
-
 [tool.pylint.message_control]
 disable = [
     "format", "refactoring", "spelling", "design",
@@ -285,19 +271,16 @@ disable = [
     "logging-format-interpolation", "logging-fstring-interpolation",
     "missing-function-docstring", "missing-module-docstring",
     "missing-class-docstring", "raise-missing-from", "import-outside-toplevel",
-    "cannot-enumerate-pytest-fixtures",
 ]
 enable = ["c-extension-no-member", "no-else-return"]
 
 [tool.pylint.typecheck]
 generated-members = ["pytest.lazy_fixture", "logging.TRACE", "logger.trace", "sys.getwindowsversion", "argparse.Namespace"]
-ignored-classes = ["Dvcfile"]
-ignored-modules = ["azure"]
 signature-mutators = ["funcy.decorators.decorator"]
 
 [tool.pylint.variables]
-dummy-variables-rgx = "_+$|(_[a-zA-Z0-9_]*[a-zA-Z0-9]+?$)|dummy|^ignored_|^unused_"
-ignored-argument-names = "_.*|^ignored_|^unused_|args|kwargs"
+dummy-variables-rgx = "_+$|(_[a-zA-Z0-9_]*[a-zA-Z0-9]+?$)"
+ignored-argument-names = "_.*|args|kwargs"
 
 [tool.codespell]
 ignore-words-list = "ba,datas,fo,uptodate,cachable,falsy"

tests/__init__.py

@@ -3,13 +3,13 @@ import os
 # Increasing fd ulimit for tests
 if os.name == "nt":
     try:
-        import win32file  # pylint: disable=import-error
+        import win32file
     except ImportError:
         pass
     else:
         win32file._setmaxstdio(4096)
 else:
-    import resource  # pylint: disable=import-error
+    import resource
 
     resource.setrlimit(resource.RLIMIT_NOFILE, (4096, 4096))
 

tests/conftest.py

@@ -61,7 +61,6 @@ def enable_ui():
 
 @pytest.fixture(autouse=True)
 def clean_repos():
-    # pylint: disable=redefined-outer-name
     from dvc.external_repo import clean_repos
 
     clean_repos()

tests/dir_helpers.py

@@ -43,7 +43,6 @@ from global repo template to creating everything inplace, which:
     - does not create unnecessary files
 """
 
-# pylint: disable=redefined-outer-name, attribute-defined-outside-init
 
 import os
 import pathlib

tests/func/experiments/test_set_params.py

@@ -13,7 +13,6 @@ from tests.func.utils.test_hydra import hydra_setup
 )
 def test_modify_params(params_repo, dvc, changes, expected):
     dvc.experiments.run(params=changes)
-    # pylint: disable=unspecified-encoding
     with open("params.yaml") as fobj:
         assert fobj.read().strip() == expected
 

tests/func/test_add.py

@@ -555,7 +555,6 @@ def temporary_windows_drive(tmp_path_factory):
     from ctypes import windll
 
     try:
-        # pylint: disable=import-error
         import win32api
         from win32con import DDD_REMOVE_DEFINITION
     except ImportError:
@@ -890,7 +889,7 @@ def test_add_preserve_fields(tmp_dir, dvc):
 # are the same 260 chars, which makes the test unnecessarily complex
 @pytest.mark.skipif(os.name == "nt", reason="unsupported on Windows")
 def test_add_long_fname(tmp_dir, dvc):
-    name_max = os.pathconf(tmp_dir, "PC_NAME_MAX")  # pylint: disable=no-member
+    name_max = os.pathconf(tmp_dir, "PC_NAME_MAX")
     name = "a" * name_max
     tmp_dir.gen({"data": {name: "foo"}})
 

tests/func/test_checkout.py

@@ -138,7 +138,7 @@ def test_remove_files_when_checkout(tmp_dir, dvc, scm):
 
 
 class TestCheckoutCleanWorkingDir:
-    def test(self, mocker, tmp_dir, dvc):  # pylint: disable=unused-argument
+    def test(self, mocker, tmp_dir, dvc):
         mock_prompt = mocker.patch("dvc.prompt.confirm", return_value=True)
         (stage,) = tmp_dir.dvc_gen("data", {"foo": "foo"})
 
@@ -308,7 +308,6 @@ def test_checkout_hook(mocker, tmp_dir, dvc):
 
 
 def test_checkout_suggest_git(tmp_dir, dvc, scm):
-    # pylint: disable=no-member
     with pytest.raises(CheckoutErrorSuggestGit) as e:
         dvc.checkout(targets="gitbranch")
     assert isinstance(e.value.__cause__, NoOutputOrStageError)

tests/func/test_dvcfile.py

@@ -1,4 +1,3 @@
-# pylint: disable=no-member
 import textwrap
 
 import pytest

tests/func/test_repro.py

@@ -848,7 +848,7 @@ def test_repro_no_commit(tmp_dir, dvc, copy_script, run_stage):
 
 
 class TestReproAlreadyCached:
-    def test(self, dvc, run_stage):  # pylint: disable=redefined-outer-name
+    def test(self, dvc, run_stage):
         stage = run_stage(
             always_changed=True,
             deps=[],

tests/pylint_plugin_disable.py

@@ -1,45 +0,0 @@
-"""
-Disable certain checks for the tests.
-
-To disable new checks, add to the `SUPPRESS_CHECKS` dictionary,
-with `message_id` of the check to disable as a key and a list of
-methods that check for that particular message.
-"""
-import os.path
-from typing import TYPE_CHECKING
-
-from pylint.checkers.base import NameChecker
-from pylint.checkers.classes import ClassChecker
-
-if TYPE_CHECKING:
-    from astroid import node_classes, scoped_nodes
-    from pylint.lint import PyLinter
-
-
-TESTS_FOLDER = os.path.join(os.path.abspath(os.path.dirname(__file__)), "")
-SUPPRESS_CHECKS = {
-    "protected-access": [
-        ClassChecker.visit_assign,
-        ClassChecker.visit_attribute,
-    ],
-    "blacklisted-name": [
-        NameChecker.visit_global,
-        NameChecker.visit_assignname,
-    ],
-}
-
-
-def is_node_in_tests(node: "node_classes.NodeNG"):
-    module: "scoped_nodes.Module" = node.root()
-    return module.file.startswith(TESTS_FOLDER)
-
-
-def register(linter: "PyLinter"):
-    try:
-        from pylint_plugin_utils import suppress_message
-    except ImportError:
-        return
-
-    for msg, checks in SUPPRESS_CHECKS.items():
-        for checker_method in checks:
-            suppress_message(linter, checker_method, msg, is_node_in_tests)

tests/remotes/git_server.py

@@ -4,7 +4,7 @@ from dvc_ssh.tests.cloud import SSH, TEST_SSH_KEY_PATH, TEST_SSH_USER
 
 class GitSSH(SSH):
     @staticmethod
-    def get_url(host, port):  # pylint: disable=arguments-differ
+    def get_url(host, port):
         return f"ssh://{host}:{port}/tmp/data/git"
 
 

tests/unit/command/test_experiments.py

@@ -146,7 +146,6 @@ def test_experiments_run(dvc, scm, mocker):
     mocker.patch.object(cmd.repo, "reproduce")
     mocker.patch.object(cmd.repo.experiments, "run")
     cmd.run()
-    # pylint: disable=no-member
     cmd.repo.experiments.run.assert_called_with(**default_arguments)
 
 

tests/unit/command/test_repro.py

@@ -25,7 +25,6 @@ def test_default_arguments(dvc, mocker):
     cmd = CmdRepro(parse_args(["repro"]))
     mocker.patch.object(cmd.repo, "reproduce")
     cmd.run()
-    # pylint: disable=no-member
     cmd.repo.reproduce.assert_called_with(
         **common_arguments, **repro_arguments
     )
@@ -38,5 +37,4 @@ def test_downstream(dvc, mocker):
     arguments = common_arguments.copy()
     arguments.update(repro_arguments)
     arguments.update({"downstream": True})
-    # pylint: disable=no-member
     cmd.repo.reproduce.assert_called_with(**arguments)

tests/unit/fs/test_azure.py

@@ -33,7 +33,6 @@ def test_init(dvc):
 def test_azure_login_methods():
     def get_login_method(config):
         fs = AzureFileSystem(**config)
-        # pylint: disable=pointless-statement
         return fs.login_method
 
     with pytest.raises(AzureAuthError):

tests/unit/output/test_load.py

@@ -1,5 +1,3 @@
-# pylint: disable=unsubscriptable-object
-
 import pytest
 from dvc_s3 import S3FileSystem
 

tests/unit/remote/test_webhdfs.py

@@ -49,5 +49,5 @@ def test_verify_ssl(dvc, webhdfs_config, monkeypatch, mocker):
     del webhdfs_config["token"]
     fs = WebHDFSFileSystem(**webhdfs_config)
     # ssl verify can't be set until after the file system is instantiated
-    fs.fs  # pylint: disable=pointless-statement
+    fs.fs
     assert mock_session.verify == ssl_verify

tests/unit/repo/experiments/queue/test_celery.py

@@ -17,7 +17,7 @@ def test_shutdown_no_tasks(test_queue, mocker):
 
 
 @shared_task
-def _foo(arg=None):  # pylint: disable=unused-argument
+def _foo(arg=None):
     return "foo"
 
 

tests/unit/repo/test_scm_context.py

@@ -135,7 +135,7 @@ def test_scm_context_decorator(scm_context, mocker):
 
     repo = mocker.MagicMock(scm_context=scm_context)
 
-    def test_method(repo, *args, **kwargs):  # pylint: disable=unused-argument
+    def test_method(repo, *args, **kwargs):
         scm_context.track_file("foo")
 
     method = mocker.MagicMock(wraps=test_method)

tests/unit/stage/test_stage.py

@@ -78,7 +78,7 @@ def test_stage_update(dvc, mocker):
 @pytest.mark.skipif(
     not isinstance(
         threading.current_thread(),
-        threading._MainThread,  # pylint: disable=protected-access
+        threading._MainThread,
     ),
     reason="Not running in the main thread.",
 )
@@ -94,7 +94,6 @@ def test_stage_run_ignore_sigint(dvc, mocker):
     assert popen.called_once()
     assert communicate.called_once_with()
     signal_mock.assert_any_call(signal.SIGINT, signal.SIG_IGN)
-    # pylint: disable=comparison-with-callable
     assert signal.getsignal(signal.SIGINT) == signal.default_int_handler
 
 

tests/unit/test_logger.py

@@ -29,7 +29,6 @@ def dt(mocker):
 
 
 class TestColorFormatter:
-    # pylint: disable=broad-except
     def test_debug(self, caplog, dt):
         with caplog.at_level(logging.DEBUG, logger="dvc"):
             logger.debug("message")
@@ -95,8 +94,6 @@ class TestColorFormatter:
 
             assert expected == formatter.format(caplog.records[0])
 
-    # pylint: disable=used-before-assignment
-
     def test_exception_under_verbose(self, caplog, dt):
         with caplog.at_level(logging.DEBUG, logger="dvc"):
             try:
@@ -193,8 +190,6 @@ class TestColorFormatter:
             assert "Exception: first" in stack_trace
             assert "dvc.exceptions.DvcException: second" in stack_trace
 
-    # pylint: enable=used-before-assignment
-
     def test_progress_awareness(self, mocker, capsys, caplog):
         from dvc.progress import Tqdm
 

tests/unit/ui/test_prompt.py

@@ -6,8 +6,6 @@ from rich.console import Console as RichConsole
 
 from dvc.ui.prompt import Confirm, InvalidResponse, Prompt
 
-# pylint: disable=no-member
-
 
 @pytest.mark.parametrize("side_effect", [EOFError, KeyboardInterrupt])
 @pytest.mark.parametrize("prompt_cls", [Prompt, Confirm])

tests/unit/utils/test_collections.py

@@ -1,4 +1,3 @@
-# pylint: disable=unidiomatic-typecheck
 import json
 
 import pytest
@@ -24,7 +23,7 @@ class MyInt(int):
 
 def test_apply_diff_is_inplace():
     dest = MyDict()
-    dest.attr = 42  # pylint: disable=attribute-defined-outside-init
+    dest.attr = 42
     apply_diff({}, dest)
 
     assert type(dest) is MyDict, "Preserves class"
@@ -72,9 +71,6 @@ def test_chunk_dict():
     assert chunk_dict(d, 4) == [d]
 
 
-# pylint: disable=unused-argument
-
-
 def _test_func(x, y, *args, j=3, k=5, **kwargs):
     pass