From c47c2123fb65cbc0da3b61769d0744603a794198 Mon Sep 17 00:00:00 2001
From: skshetry <18718008+skshetry@users.noreply.github.com>
Date: Tue, 17 Jan 2023 08:35:19 +0545
Subject: [PATCH] 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                       |  1 +
 dvc/testing/api_tests.py                      |  2 +
 dvc/testing/fixtures.py                       |  2 +
 dvc/testing/remote_tests.py                   |  2 +
 pyproject.toml                                | 21 +--------
 tests/__init__.py                             |  4 +-
 tests/conftest.py                             |  1 -
 tests/dir_helpers.py                          |  1 -
 tests/func/experiments/test_set_params.py     |  1 -
 tests/func/test_add.py                        |  3 +-
 tests/func/test_checkout.py                   |  3 +-
 tests/func/test_dvcfile.py                    |  1 -
 tests/func/test_repro.py                      |  2 +-
 tests/pylint_plugin_disable.py                | 45 -------------------
 tests/remotes/git_server.py                   |  2 +-
 tests/unit/command/test_experiments.py        |  1 -
 tests/unit/command/test_repro.py              |  2 -
 tests/unit/fs/test_azure.py                   |  1 -
 tests/unit/output/test_load.py                |  2 -
 tests/unit/remote/test_webhdfs.py             |  2 +-
 .../repo/experiments/queue/test_celery.py     |  2 +-
 tests/unit/repo/test_scm_context.py           |  2 +-
 tests/unit/stage/test_stage.py                |  3 +-
 tests/unit/test_logger.py                     |  5 ---
 tests/unit/ui/test_prompt.py                  |  2 -
 tests/unit/utils/test_collections.py          |  6 +--
 26 files changed, 20 insertions(+), 99 deletions(-)
 delete mode 100644 tests/pylint_plugin_disable.py

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index a6c79f04d..ec843f6c1 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -73,6 +73,7 @@ repos:
     - id: pylint
       name: pylint
       entry: pylint
+      files: ^dvc/
       language: system
       types: [python]
       require_serial: true
diff --git a/dvc/testing/api_tests.py b/dvc/testing/api_tests.py
index 76f5017fa..072e2e388 100644
--- a/dvc/testing/api_tests.py
+++ b/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):
diff --git a/dvc/testing/fixtures.py b/dvc/testing/fixtures.py
index d2cce3489..ce84fb749 100644
--- a/dvc/testing/fixtures.py
+++ b/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",
diff --git a/dvc/testing/remote_tests.py b/dvc/testing/remote_tests.py
index 3c556f4a9..016d269fe 100644
--- a/dvc/testing/remote_tests.py
+++ b/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
diff --git a/pyproject.toml b/pyproject.toml
index be5082d3b..3872450d3 100644
--- a/pyproject.toml
+++ b/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"
diff --git a/tests/__init__.py b/tests/__init__.py
index 7825456be..4b30ef2d2 100644
--- a/tests/__init__.py
+++ b/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))
 
diff --git a/tests/conftest.py b/tests/conftest.py
index 5337d4ff0..2c49d011d 100644
--- a/tests/conftest.py
+++ b/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()
diff --git a/tests/dir_helpers.py b/tests/dir_helpers.py
index b010d29a5..8cc4d977b 100644
--- a/tests/dir_helpers.py
+++ b/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
diff --git a/tests/func/experiments/test_set_params.py b/tests/func/experiments/test_set_params.py
index 926e4cb9e..dc79e8e36 100644
--- a/tests/func/experiments/test_set_params.py
+++ b/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
 
diff --git a/tests/func/test_add.py b/tests/func/test_add.py
index 0cff482a6..bfb8c405d 100644
--- a/tests/func/test_add.py
+++ b/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"}})
 
diff --git a/tests/func/test_checkout.py b/tests/func/test_checkout.py
index d64b82a35..be28cec87 100644
--- a/tests/func/test_checkout.py
+++ b/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)
diff --git a/tests/func/test_dvcfile.py b/tests/func/test_dvcfile.py
index 75ff86b31..42cb781ba 100644
--- a/tests/func/test_dvcfile.py
+++ b/tests/func/test_dvcfile.py
@@ -1,4 +1,3 @@
-# pylint: disable=no-member
 import textwrap
 
 import pytest
diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py
index 76770bb4d..b8b271d4c 100644
--- a/tests/func/test_repro.py
+++ b/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=[],
diff --git a/tests/pylint_plugin_disable.py b/tests/pylint_plugin_disable.py
deleted file mode 100644
index ab50080ca..000000000
--- a/tests/pylint_plugin_disable.py
+++ /dev/null
@@ -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)
diff --git a/tests/remotes/git_server.py b/tests/remotes/git_server.py
index a8596d6f8..d57377de1 100644
--- a/tests/remotes/git_server.py
+++ b/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"
 
 
diff --git a/tests/unit/command/test_experiments.py b/tests/unit/command/test_experiments.py
index 36ff5c352..12294bd41 100644
--- a/tests/unit/command/test_experiments.py
+++ b/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)
 
 
diff --git a/tests/unit/command/test_repro.py b/tests/unit/command/test_repro.py
index fb213db73..162c9add2 100644
--- a/tests/unit/command/test_repro.py
+++ b/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)
diff --git a/tests/unit/fs/test_azure.py b/tests/unit/fs/test_azure.py
index 2f3328355..3626ea6af 100644
--- a/tests/unit/fs/test_azure.py
+++ b/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):
diff --git a/tests/unit/output/test_load.py b/tests/unit/output/test_load.py
index 8ca221d9f..613c09271 100644
--- a/tests/unit/output/test_load.py
+++ b/tests/unit/output/test_load.py
@@ -1,5 +1,3 @@
-# pylint: disable=unsubscriptable-object
-
 import pytest
 from dvc_s3 import S3FileSystem
 
diff --git a/tests/unit/remote/test_webhdfs.py b/tests/unit/remote/test_webhdfs.py
index a9a508874..a14bcdc88 100644
--- a/tests/unit/remote/test_webhdfs.py
+++ b/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
diff --git a/tests/unit/repo/experiments/queue/test_celery.py b/tests/unit/repo/experiments/queue/test_celery.py
index b0502af43..14734ed0b 100644
--- a/tests/unit/repo/experiments/queue/test_celery.py
+++ b/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"
 
 
diff --git a/tests/unit/repo/test_scm_context.py b/tests/unit/repo/test_scm_context.py
index d11846663..8e53ddffd 100644
--- a/tests/unit/repo/test_scm_context.py
+++ b/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)
diff --git a/tests/unit/stage/test_stage.py b/tests/unit/stage/test_stage.py
index 51c923a08..2da313322 100644
--- a/tests/unit/stage/test_stage.py
+++ b/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
 
 
diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py
index 20bae7bd2..67f06b259 100644
--- a/tests/unit/test_logger.py
+++ b/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
 
diff --git a/tests/unit/ui/test_prompt.py b/tests/unit/ui/test_prompt.py
index e2cca07b4..ad9bc7514 100644
--- a/tests/unit/ui/test_prompt.py
+++ b/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])
diff --git a/tests/unit/utils/test_collections.py b/tests/unit/utils/test_collections.py
index e065ba3fb..dab4d5269 100644
--- a/tests/unit/utils/test_collections.py
+++ b/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