Enable pylint checks (#4092)

* pylint: enable checks for imports

* ci: add github action for pylint

* enable pylint on pre-commit

* Install requirements

* use local pylint installation rather than of pre-commit's

* fix import errors

* more checks

* Silence unused imports

* Make a copy of dict

.github/workflows/check-patch.yaml

@@ -0,0 +1,19 @@
+name: Lint Python
+
+on: [push, pull_request]
+
+jobs:
+  build:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v1
+    - name: Set up Python 3.8
+      uses: actions/setup-python@v1
+      with:
+        python-version: 3.8
+    - name: Install requirements
+      run: bash ./scripts/ci/install.sh
+    - name: Check Patch
+      run: ./scripts/ci/check_patch.sh

.pre-commit-config.yaml

@@ -23,6 +23,13 @@ repos:
           - flake8-string-format
     repo: https://gitlab.com/pycqa/flake8
     rev: 3.7.9
+  - repo: local
+    hooks:
+    - id: pylint
+      name: pylint
+      entry: pylint
+      language: system
+      types: [python]
   - hooks:
       - args:
           - -i

.pylintrc

@@ -60,94 +60,41 @@ confidence=
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use "--disable=all --enable=classes
 # --disable=W".
-disable=print-statement,
-        parameter-unpacking,
-        unpacking-in-except,
-        old-raise-syntax,
-        backtick,
-        long-suffix,
-        old-ne-operator,
-        old-octal-literal,
-        import-star-module-level,
-        non-ascii-bytes-literal,
-        raw-checker-failed,
-        bad-inline-option,
-        locally-disabled,
-        file-ignored,
-        suppressed-message,
-        useless-suppression,
-        deprecated-pragma,
-        use-symbolic-message-instead,
-        apply-builtin,
-        basestring-builtin,
-        buffer-builtin,
-        cmp-builtin,
-        coerce-builtin,
-        execfile-builtin,
-        file-builtin,
-        long-builtin,
-        raw_input-builtin,
-        reduce-builtin,
-        standarderror-builtin,
-        unicode-builtin,
-        xrange-builtin,
-        coerce-method,
-        delslice-method,
-        getslice-method,
-        setslice-method,
-        no-absolute-import,
-        old-division,
-        dict-iter-method,
-        dict-view-method,
-        next-method-called,
-        metaclass-assignment,
-        indexing-exception,
-        raising-string,
-        reload-builtin,
-        oct-method,
-        hex-method,
-        nonzero-method,
-        cmp-method,
-        input-builtin,
-        round-builtin,
-        intern-builtin,
-        unichr-builtin,
-        map-builtin-not-iterating,
-        zip-builtin-not-iterating,
-        range-builtin-not-iterating,
-        filter-builtin-not-iterating,
-        using-cmp-argument,
-        eq-without-hash,
-        div-method,
-        idiv-method,
-        rdiv-method,
-        exception-message-attribute,
-        invalid-str-codec,
-        sys-max-int,
-        bad-python3-import,
-        deprecated-string-function,
-        deprecated-str-translate-call,
-        deprecated-itertools-function,
-        deprecated-types-field,
-        next-method-defined,
-        dict-items-not-iterating,
-        dict-keys-not-iterating,
-        dict-values-not-iterating,
-        deprecated-operator-function,
-        deprecated-urllib-function,
-        xreadlines-attribute,
-        deprecated-sys-function,
-        exception-escape,
-        comprehension-escape,
-        useless-object-inheritance,
-        missing-docstring,
-        bad-continuation
+disable=all
 
 # 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 (only on the command line, not in the configuration file where
 # it should appear only once). See also the "--disable" option for examples.
-enable=c-extension-no-member
+enable=print-statement,
+       c-extension-no-member,
+       import-error,
+       import-self,
+       reimported,
+       deprecated-module,
+       wildcard-import,
+       useless-import-alias,
+       unpacking-in-except,
+       parameter-unpacking,
+       unpacking-non-sequence,
+       invalid-all-object,
+       no-name-in-module,
+       undefined-variable,
+       undefined-all-variable,
+       used-before-assignment,
+       cell-var-from-loop,
+       global-variable-undefined,
+       self-cls-assignment,
+       unbalanced-tuple-unpacking,
+       possibly-unused-variable,
+       redefined-builtin,
+       redefine-in-handler,
+       unused-import,
+
+# Needs Cleanup
+# unused-argument,
+# redefined-outer-name,
+
 
 
 [REPORTS]

.travis.yml

@@ -29,7 +29,7 @@ jobs:
     os: linux
     language: python
     python: 3.7
-    before_install:
+    before_install: bash ./scripts/ci/install.sh
     install:
     script: ./scripts/ci/check_patch.sh
   # test jobs

dvc/api.py

@@ -33,7 +33,9 @@ def get_url(path, repo=None, rev=None, remote=None):
         return str(remote_obj.hash_to_path_info(out.checksum))
 
 
-def open(path, repo=None, rev=None, remote=None, mode="r", encoding=None):
+def open(  # noqa, pylint: disable=redefined-builtin
+    path, repo=None, rev=None, remote=None, mode="r", encoding=None
+):
     """
     Open file in the supplied path tracked in a repo (both DVC projects and
     plain Git repos are supported). For Git repos, HEAD is used unless a rev

dvc/dependency/__init__.py

@@ -81,11 +81,8 @@ def loadd_from(stage, d_list):
 
 def loads_from(stage, s_list, erepo=None):
     assert isinstance(s_list, list)
-    ret = []
-    for s in s_list:
-        info = {RepoDependency.PARAM_REPO: erepo} if erepo else {}
-        ret.append(_get(stage, s, info))
-    return ret
+    info = {RepoDependency.PARAM_REPO: erepo} if erepo else {}
+    return [_get(stage, s, info.copy()) for s in s_list]
 
 
 def _merge_params(s_list):

dvc/exceptions.py

@@ -330,7 +330,7 @@ class RemoteCacheRequiredError(DvcException):
         )
 
 
-class IsADirectoryError(DvcException):
+class IsADirectoryError(DvcException):  # noqa,pylint:disable=redefined-builtin
     """Raised when a file operation is requested on a directory."""
 
 

dvc/path_info.py

@@ -30,7 +30,10 @@ class PathInfo(pathlib.PurePath, _BasePath):
     def __new__(cls, *args):
         # Construct a proper subclass depending on current os
         if cls is PathInfo:
-            cls = WindowsPathInfo if os.name == "nt" else PosixPathInfo
+            cls = (  # pylint: disable=self-cls-assignment
+                WindowsPathInfo if os.name == "nt" else PosixPathInfo
+            )
+
         return cls._from_parts(args)
 
     def as_posix(self):

dvc/remote/azure.py

@@ -38,6 +38,7 @@ class AzureRemoteTree(BaseRemoteTree):
     @wrap_prop(threading.Lock())
     @cached_property
     def blob_service(self):
+        # pylint: disable=no-name-in-module
         from azure.storage.blob import BlockBlobService
         from azure.common import AzureMissingResourceHttpError
 
@@ -62,7 +63,9 @@ class AzureRemoteTree(BaseRemoteTree):
         return etag.strip('"')
 
     def _generate_download_url(self, path_info, expires=3600):
-        from azure.storage.blob import BlobPermissions
+        from azure.storage.blob import (  # pylint:disable=no-name-in-module
+            BlobPermissions,
+        )
 
         expires_at = datetime.utcnow() + timedelta(seconds=expires)
 

dvc/system.py

@@ -82,7 +82,6 @@ class System:
 
     @staticmethod
     def _reflink_linux(src, dst):
-        import os
         import fcntl
 
         FICLONE = 0x40049409
@@ -120,7 +119,7 @@ class System:
     @staticmethod
     def _getdirinfo(path):
         from collections import namedtuple
-        from win32file import (
+        from win32file import (  # pylint: disable=import-error
             CreateFileW,
             GetFileInformationByHandle,
             FILE_FLAG_BACKUP_SEMANTICS,
@@ -194,7 +193,9 @@ class System:
 
         # https://docs.microsoft.com/en-us/windows/desktop/fileio/
         # file-attribute-constants
-        from winnt import FILE_ATTRIBUTE_REPARSE_POINT
+        from winnt import (  # pylint: disable=import-error
+            FILE_ATTRIBUTE_REPARSE_POINT,
+        )
 
         if os.path.lexists(path):
             info = System._getdirinfo(path)

dvc/utils/fs.py

@@ -170,7 +170,6 @@ def makedirs(path, exist_ok=False, mode=None):
 
 def copyfile(src, dest, no_progress_bar=False, name=None):
     """Copy file with progress bar"""
-    from dvc.exceptions import DvcException
     from dvc.progress import Tqdm
 
     name = name if name else os.path.basename(dest)

dvc/utils/pkg.py

@@ -1,4 +1,5 @@
 try:
-    from .build import PKG  # file created during dvc build
+    # file is created during dvc build
+    from .build import PKG  # noqa, pylint:disable=unused-import
 except ImportError:
     PKG = None

fastentrypoints.py

@@ -91,7 +91,6 @@ easy_install.ScriptWriter.get_args = get_args
 
 def main():
     import os
-    import re
     import shutil
     import sys
 

scripts/hooks/hook-google.cloud.storage.py

@@ -1,4 +1,6 @@
-from PyInstaller.utils.hooks import copy_metadata
+from PyInstaller.utils.hooks import (  # pylint:disable=import-error
+    copy_metadata,
+)
 
 datas = copy_metadata("google-cloud-storage")
 

scripts/hooks/hook-google_compute_engine.logger.py

@@ -1,4 +1,6 @@
-from PyInstaller.utils.hooks import copy_metadata
+from PyInstaller.utils.hooks import (  # pylint:disable=import-error
+    copy_metadata,
+)
 
 datas = copy_metadata("google-compute-engine")
 hiddenimports = ["google_cloud_engine"]

scripts/hooks/hook-pydrive2.py

@@ -1,4 +1,6 @@
-from PyInstaller.utils.hooks import copy_metadata
+from PyInstaller.utils.hooks import (  # pylint:disable=import-error
+    copy_metadata,
+)
 
 datas = copy_metadata("pydrive2")
 datas += copy_metadata("google-api-python-client")

setup.py

@@ -9,7 +9,7 @@ from setuptools.command.build_py import build_py as _build_py
 # see https://github.com/ninjaaron/fast-entry_points.
 # This saves about 200 ms on startup time for non-wheel installs.
 try:
-    import fastentrypoints  # noqa: F401
+    import fastentrypoints  # noqa: F401, pylint: disable=unused-import
 except ImportError:
     pass  # not able to import when installing through pre-commit
 
@@ -127,6 +127,7 @@ tests_requirements = [
     "flake8-bugbear",
     "flake8-comprehensions",
     "flake8-string-format",
+    "pylint",
 ]
 
 if (sys.version_info) >= (3, 6):

tests/__init__.py

@@ -2,7 +2,7 @@ import os
 
 # Increasing fd ulimit for tests
 if os.name == "nt":
-    import win32file
+    import win32file  # pylint: disable=import-error
     import subprocess
 
     win32file._setmaxstdio(2048)

tests/conftest.py

@@ -2,8 +2,8 @@ import os
 
 import pytest
 
-from .dir_helpers import *  # noqa
-from .remotes import *  # noqa
+from .dir_helpers import *  # noqa, pylint:disable=wildcard-import
+from .remotes import *  # noqa, pylint:disable=wildcard-import
 
 # Prevent updater and analytics from running their processes
 os.environ["DVC_TEST"] = "true"

tests/dir_helpers.py

@@ -74,7 +74,9 @@ disable_other_loggers()
 class TmpDir(pathlib.Path):
     def __new__(cls, *args, **kwargs):
         if cls is TmpDir:
-            cls = WindowsTmpDir if os.name == "nt" else PosixTmpDir
+            cls = (  # pylint: disable=self-cls-assignment
+                WindowsTmpDir if os.name == "nt" else PosixTmpDir
+            )
         self = cls._from_parts(args, init=False)
         if not self._flavour.is_supported:
             raise NotImplementedError(

tests/func/test_add.py

@@ -526,9 +526,9 @@ class TestAddUnprotected(TestDvc):
 @pytest.fixture
 def temporary_windows_drive(tmp_path_factory):
     import string
-    import win32api
+    import win32api  # pylint: disable=import-error
     from ctypes import windll
-    from win32con import DDD_REMOVE_DEFINITION
+    from win32con import DDD_REMOVE_DEFINITION  # pylint: disable=import-error
 
     drives = [
         s[0].upper()