Merge pull request #7078 from pradyunsg/revamp-linting-processes

Revamp linting processes

.azure-pipelines/jobs/package.yml

@@ -15,7 +15,7 @@ jobs:
     inputs:
       versionSpec: '3'
 
-  - bash: pip install tox nox setuptools wheel
+  - bash: pip install twine nox setuptools wheel
     displayName: Install dependencies
 
   - bash: nox -s generate_authors
@@ -24,12 +24,12 @@ jobs:
   - bash: nox -s generate_news -- --yes
     displayName: Generate NEWS.rst
 
-  - bash: tox -e packaging
-    displayName: Run Tox packaging
-
   - bash: python setup.py sdist bdist_wheel
     displayName: Create sdist and wheel
 
+  - bash: twine check dist/*
+    displayName: Check distributions with twine
+
   - task: PublishBuildArtifacts@1
     displayName: 'Publish Artifact: dist'
     inputs:

.github/workflows/python-linters.yml

@@ -20,10 +20,6 @@ jobs:
         env:
         - TOXENV: docs
         - TOXENV: lint
-        - TOXENV: lint-py2
-          PYTHON_VERSION: 2.7
-        - TOXENV: mypy
-        - TOXENV: packaging
     steps:
     - uses: actions/checkout@master
     - name: Set up Python ${{ matrix.env.PYTHON_VERSION || 3.7 }}
@@ -44,17 +40,3 @@ jobs:
       run: >-
         python -m tox
       env: ${{ matrix.env }}
-
-  news_format:
-    name: Check NEWS format
-    runs-on: ubuntu-18.04
-    steps:
-    - uses: actions/checkout@master
-    - name: Set up Python
-      uses: actions/setup-python@v1
-      with:
-        version: 3.7
-    - name: Install nox
-      run: pip install nox
-    - name: Check NEWS format
-      run: nox -s validate_news

.pre-commit-config.yaml

@@ -0,0 +1,53 @@
+exclude: 'src/pip/_vendor/'
+
+repos:
+- repo: https://github.com/pre-commit/pre-commit-hooks
+  rev: v2.3.0
+  hooks:
+  - id: check-builtin-literals
+  - id: check-added-large-files
+  - id: check-case-conflict
+  - id: check-toml
+  - id: check-yaml
+  - id: debug-statements
+  - id: end-of-file-fixer
+    exclude: WHEEL
+  - id: flake8
+    exclude: tests/data
+  - id: forbid-new-submodules
+  - id: trailing-whitespace
+    exclude: .patch
+
+- repo: https://github.com/timothycrosley/isort
+  rev: 4.3.21
+  hooks:
+  - id: isort
+    files: \.py$
+
+- repo: https://github.com/pre-commit/mirrors-mypy
+  rev: v0.720
+  hooks:
+  - id: mypy
+    exclude: docs|tests
+    args: []
+  - id: mypy
+    name: mypy, for Py2
+    exclude: docs|tests
+    args: ["-2"]
+
+- repo: https://github.com/pre-commit/pygrep-hooks
+  rev: v1.4.1
+  hooks:
+  - id: python-no-log-warn
+  - id: python-no-eval
+  - id: rst-backticks
+    # Validate existing ReST files and NEWS fragments.
+    files: .*\.rst$|^news/.*
+    types: [file]
+    # The errors flagged in NEWS.rst are old.
+    exclude: NEWS.rst
+
+- repo: https://github.com/mgedmin/check-manifest
+  rev: '0.39'
+  hooks:
+  - id: check-manifest

.travis.yml

@@ -17,10 +17,6 @@ jobs:
     - stage: primary
       env: TOXENV=docs
     - env: TOXENV=lint
-    - env: TOXENV=lint-py2
-      python: 2.7
-    - env: TOXENV=mypy
-    - env: TOXENV=packaging
     # Latest CPython
     - env: GROUP=1
       python: 2.7

MANIFEST.in

@@ -16,6 +16,7 @@ exclude .mailmap
 exclude .appveyor.yml
 exclude .travis.yml
 exclude .readthedocs.yml
+exclude .pre-commit-config.yaml
 exclude tox.ini
 exclude noxfile.py
 

docs/html/development/architecture/overview.rst

@@ -140,4 +140,3 @@ files on PyPI. It’s for getting all files of Flask.)
 .. _`tracking issue`: https://github.com/pypa/pip/issues/6831
 .. _PyPI: https://pypi.org/
 .. _PyPI Simple API: https://warehouse.readthedocs.io/api-reference/legacy/#simple-project-api
-

docs/html/development/getting-started.rst

@@ -68,35 +68,15 @@ tools, you can tell pip to skip those tests:
 Running Linters
 ---------------
 
-pip uses :pypi:`flake8` and :pypi:`isort` for linting the codebase. These
-ensure that the codebase is in compliance with :pep:`8` and the imports are
-consistently ordered and styled.
+pip uses :pypi:`pre-commit` for managing linting of the codebase.
+``pre-commit`` performs various checks on all files in pip and uses tools that
+help follow a consistent code style within the codebase.
 
 To use linters locally, run:
 
 .. code-block:: console
 
     $ tox -e lint
-    $ tox -e lint-py2
-
-The above commands run the linters on Python 3 followed by Python 2.
-
-.. note::
-
-    Do not silence errors from flake8 with ``# noqa`` comments or otherwise.
-
-Running mypy
-------------
-
-pip uses :pypi:`mypy` to run static type analysis, which helps catch certain
-kinds of bugs. The codebase uses `PEP 484 type-comments`_ due to compatibility
-requirements with Python 2.7.
-
-To run the ``mypy`` type checker, run:
-
-.. code-block:: console
-
-    $ tox -e mypy
 
 Building Documentation
 ----------------------

docs/html/installing.rst

@@ -120,7 +120,7 @@ pip works on Unix/Linux, macOS, and Windows.
 ----
 
 .. [1] "Secure" in this context means using a modern browser or a
-       tool like `curl` that verifies SSL certificates when downloading from
+       tool like ``curl`` that verifies SSL certificates when downloading from
        https URLs.
 
 .. [2] Beginning with pip v1.5.1, ``get-pip.py`` stopped requiring setuptools to

docs/html/logic.rst

@@ -5,4 +5,3 @@ Internal Details
 ================
 
 This content is now covered in the :doc:`Reference Guide <reference/index>`
-

docs/html/reference/pip.rst

@@ -234,4 +234,3 @@ General Options
 ***************
 
 .. pip-general-options::
-

docs/html/reference/pip_install.rst

@@ -572,7 +572,7 @@ each sdist that wheels are built from and places the resulting wheels inside.
 
 Pip attempts to choose the best wheels from those built in preference to
 building a new wheel. Note that this means when a package has both optional
-C extensions and builds `py` tagged wheels when the C extension can't be built
+C extensions and builds ``py`` tagged wheels when the C extension can't be built
 that pip will not attempt to build a better wheel for Pythons that would have
 supported it, once any generic wheel is built. To correct this, make sure that
 the wheels are built with Python specific tags - e.g. pp on PyPy.
@@ -826,7 +826,7 @@ Options
 Examples
 ********
 
-#. Install `SomePackage` and its dependencies from `PyPI`_ using :ref:`Requirement Specifiers`
+#. Install ``SomePackage`` and its dependencies from `PyPI`_ using :ref:`Requirement Specifiers`
 
     ::
 
@@ -842,7 +842,7 @@ Examples
       $ pip install -r requirements.txt
 
 
-#. Upgrade an already installed `SomePackage` to the latest from PyPI.
+#. Upgrade an already installed ``SomePackage`` to the latest from PyPI.
 
     ::
 

docs/html/reference/pip_uninstall.rst

@@ -34,4 +34,3 @@ Examples
       /home/me/env/lib/python2.7/site-packages/simplejson-2.2.1-py2.7.egg-info
     Proceed (y/n)? y
       Successfully uninstalled simplejson
-

docs/html/user_guide.rst

@@ -90,7 +90,7 @@ In practice, there are 4 common uses of Requirements files:
 1. Requirements files are used to hold the result from :ref:`pip freeze` for the
    purpose of achieving :ref:`repeatable installations <Repeatability>`.  In
    this case, your requirement file contains a pinned version of everything that
-   was installed when `pip freeze` was run.
+   was installed when ``pip freeze`` was run.
 
    ::
 
@@ -100,11 +100,11 @@ In practice, there are 4 common uses of Requirements files:
 2. Requirements files are used to force pip to properly resolve dependencies.
    As it is now, pip `doesn't have true dependency resolution
    <https://github.com/pypa/pip/issues/988>`_, but instead simply uses the first
-   specification it finds for a project. E.g. if `pkg1` requires `pkg3>=1.0` and
-   `pkg2` requires `pkg3>=1.0,<=2.0`, and if `pkg1` is resolved first, pip will
-   only use `pkg3>=1.0`, and could easily end up installing a version of `pkg3`
-   that conflicts with the needs of `pkg2`.  To solve this problem, you can
-   place `pkg3>=1.0,<=2.0` (i.e. the correct specification) into your
+   specification it finds for a project. E.g. if ``pkg1`` requires ``pkg3>=1.0`` and
+   ``pkg2`` requires ``pkg3>=1.0,<=2.0``, and if ``pkg1`` is resolved first, pip will
+   only use ``pkg3>=1.0``, and could easily end up installing a version of ``pkg3``
+   that conflicts with the needs of ``pkg2``.  To solve this problem, you can
+   place ``pkg3>=1.0,<=2.0`` (i.e. the correct specification) into your
    requirements file directly along with the other top level requirements. Like
    so:
 
@@ -115,8 +115,8 @@ In practice, there are 4 common uses of Requirements files:
      pkg3>=1.0,<=2.0
 
 3. Requirements files are used to force pip to install an alternate version of a
-   sub-dependency.  For example, suppose `ProjectA` in your requirements file
-   requires `ProjectB`, but the latest version (v1.3) has a bug, you can force
+   sub-dependency.  For example, suppose ``ProjectA`` in your requirements file
+   requires ``ProjectB``, but the latest version (v1.3) has a bug, you can force
    pip to accept earlier versions like so:
 
    ::
@@ -126,23 +126,23 @@ In practice, there are 4 common uses of Requirements files:
 
 4. Requirements files are used to override a dependency with a local patch that
    lives in version control.  For example, suppose a dependency,
-   `SomeDependency` from PyPI has a bug, and you can't wait for an upstream fix.
+   ``SomeDependency`` from PyPI has a bug, and you can't wait for an upstream fix.
    You could clone/copy the src, make the fix, and place it in VCS with the tag
-   `sometag`.  You'd reference it in your requirements file with a line like so:
+   ``sometag``.  You'd reference it in your requirements file with a line like so:
 
    ::
 
      git+https://myvcs.com/some_dependency@sometag#egg=SomeDependency
 
-   If `SomeDependency` was previously a top-level requirement in your
+   If ``SomeDependency`` was previously a top-level requirement in your
    requirements file, then **replace** that line with the new line. If
-   `SomeDependency` is a sub-dependency, then **add** the new line.
+   ``SomeDependency`` is a sub-dependency, then **add** the new line.
 
 
 It's important to be clear that pip determines package dependencies using
 `install_requires metadata
 <https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-dependencies>`_,
-not by discovering `requirements.txt` files embedded in projects.
+not by discovering ``requirements.txt`` files embedded in projects.
 
 See also:
 
@@ -374,7 +374,7 @@ look like this:
 
 Each subcommand can be configured optionally in its own section so that every
 global setting with the same name will be overridden; e.g. decreasing the
-``timeout`` to ``10`` seconds when running the `freeze`
+``timeout`` to ``10`` seconds when running the ``freeze``
 (`Freezing Requirements <./#freezing-requirements>`_) command and using
 ``60`` seconds for all other commands is possible with:
 

news/6991.bugfix

@@ -1 +1 @@
- Ignore "require_virtualenv" in `pip config`
+Ignore "require_virtualenv" in ``pip config``

news/fix-test-pep518-forkbombs.trivial

@@ -1 +1 @@
-Fix copy-paste issue in `test_pep518_forkbombs`.
+Fix copy-paste issue in ``test_pep518_forkbombs``.

noxfile.py

@@ -1,7 +1,6 @@
 """Release time helpers, executed using nox.
 """
 
-import glob
 import io
 import subprocess
 
@@ -31,18 +30,6 @@ def get_author_list():
     return sorted(authors, key=lambda x: x.lower())
 
 
-# -----------------------------------------------------------------------------
-# Ad-hoc commands
-# -----------------------------------------------------------------------------
-@nox.session
-def validate_news(session):
-    session.install("rstcheck")
-
-    news_files = sorted(glob.glob("news/*"))
-
-    session.run("rstcheck", *news_files)
-
-
 # -----------------------------------------------------------------------------
 # Commands used during the release process
 # -----------------------------------------------------------------------------

src/pip/_internal/commands/__init__.py

@@ -4,7 +4,7 @@ Package containing all pip commands
 from __future__ import absolute_import
 
 import importlib
-from collections import namedtuple, OrderedDict
+from collections import OrderedDict, namedtuple
 
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 

src/pip/_internal/distributions/__init__.py

@@ -1,6 +1,5 @@
 from pip._internal.distributions.source.legacy import SourceDistribution
 from pip._internal.distributions.wheel import WheelDistribution
-
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 
 if MYPY_CHECK_RUNNING:

src/pip/_internal/index.py

@@ -34,7 +34,7 @@ from pip._internal.wheel import Wheel
 
 if MYPY_CHECK_RUNNING:
     from typing import (
-        Any, FrozenSet, Iterable, List, Optional, Set, Text, Tuple,
+        FrozenSet, Iterable, List, Optional, Set, Text, Tuple, Union,
     )
     from pip._vendor.packaging.version import _BaseVersion
     from pip._internal.collector import LinkCollector
@@ -43,7 +43,7 @@ if MYPY_CHECK_RUNNING:
     from pip._internal.pep425tags import Pep425Tag
     from pip._internal.utils.hashes import Hashes
 
-    BuildTag = Tuple[Any, ...]  # either empty tuple or Tuple[int, str]
+    BuildTag = Union[Tuple[()], Tuple[int, str]]
     CandidateSortingKey = (
         Tuple[int, int, int, _BaseVersion, BuildTag, Optional[int]]
     )
@@ -511,7 +511,7 @@ class CandidateEvaluator(object):
         """
         valid_tags = self._supported_tags
         support_num = len(valid_tags)
-        build_tag = tuple()  # type: BuildTag
+        build_tag = ()  # type: BuildTag
         binary_preference = 0
         link = candidate.link
         if link.is_wheel:

src/pip/_internal/operations/check.py

@@ -68,8 +68,8 @@ def check_package_set(package_set, should_ignore=None):
         def should_ignore(name):
             return False
 
-    missing = dict()
-    conflicting = dict()
+    missing = {}
+    conflicting = {}
 
     for package_name in package_set:
         # Info about dependencies of package_name

src/pip/_internal/req/__init__.py

@@ -5,12 +5,13 @@ from __future__ import absolute_import
 
 import logging
 
-from .req_install import InstallRequirement
-from .req_set import RequirementSet
-from .req_file import parse_requirements
 from pip._internal.utils.logging import indent_log
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 
+from .req_file import parse_requirements
+from .req_install import InstallRequirement
+from .req_set import RequirementSet
+
 if MYPY_CHECK_RUNNING:
     from typing import Any, List, Sequence
 

src/pip/_internal/vcs/__init__.py

@@ -2,11 +2,14 @@
 # the vcs package don't need to import deeper than `pip._internal.vcs`.
 # (The test directory and imports protected by MYPY_CHECK_RUNNING may
 # still need to import from a vcs sub-package.)
-from pip._internal.vcs.versioncontrol import (  # noqa: F401
-    RemoteNotFoundError, is_url, make_vcs_requirement_url, vcs,
-)
 # Import all vcs modules to register each VCS in the VcsSupport object.
 import pip._internal.vcs.bazaar
 import pip._internal.vcs.git
 import pip._internal.vcs.mercurial
 import pip._internal.vcs.subversion  # noqa: F401
+from pip._internal.vcs.versioncontrol import (  # noqa: F401
+    RemoteNotFoundError,
+    is_url,
+    make_vcs_requirement_url,
+    vcs,
+)

tasks/__init__.py

@@ -1,5 +1,4 @@
 import invoke
-
 from tools.automation import vendoring
 
 ns = invoke.Collection(vendoring)

tests/conftest.py

@@ -181,10 +181,10 @@ def pip_src(tmpdir_factory):
             return to_ignore
 
         # Ignore all compiled files and egg-info.
-        ignored = list()
-        for pattern in ["__pycache__", "*.pyc", "pip.egg-info"]:
-            ignored.extend(fnmatch.filter(names, pattern))
-        return set(ignored)
+        ignored = set()
+        for pattern in ("__pycache__", "*.pyc", "pip.egg-info"):
+            ignored.update(fnmatch.filter(names, pattern))
+        return ignored
 
     pip_src = Path(str(tmpdir_factory.mktemp('pip_src'))).joinpath('pip_src')
     # Copy over our source tree so that each use is self contained

tests/data/indexes/datarequire/fakepackage/index.html

@@ -5,4 +5,3 @@
     <a data-requires-python='&gt;=3.3'        href="/fakepackage-3.3.0.tar.gz#md5=00000000000000000000000000000000" rel="internal">fakepackage-3.3.0.tar.gz</a><br/>
     <a data-requires-python='&gt;&lt;X.y.z'   href="/fakepackage-9.9.9.tar.gz#md5=00000000000000000000000000000000" rel="internal">fakepackage-9.9.9.tar.gz</a><br/>
 </body></html>
-

tests/data/src/pep517_setup_and_pyproject/setup.py

@@ -1,2 +1,3 @@
 from setuptools import setup
+
 setup(name="dummy", version="0.1")

tests/data/src/pep517_setup_only/setup.py

@@ -1,2 +1,3 @@
 from setuptools import setup
+
 setup(name="dummy", version="0.1")

tests/data/src/pep518_with_namespace_package-1.0/setup.py

@@ -2,7 +2,6 @@ from setuptools import setup
 
 import simple_namespace.module
 
-
 setup(
     name='pep518_with_namespace_package',
     version='1.0',

tests/data/src/simple_namespace/setup.py

@@ -1,6 +1,5 @@
 from setuptools import setup
 
-
 setup(
     name='simple_namespace',
     version='1.0',

tests/lib/__init__.py

@@ -1,14 +1,14 @@
 from __future__ import absolute_import
 
+import os
+import re
+import shutil
+import site
+import subprocess
+import sys
+import textwrap
 from contextlib import contextmanager
 from textwrap import dedent
-import os
-import sys
-import re
-import textwrap
-import site
-import shutil
-import subprocess
 
 import pytest
 from scripttest import FoundDir, TestFileEnvironment

tools/automation/vendoring/__init__.py

@@ -1,11 +1,11 @@
 """"Vendoring script, python 3.5 with requests needed"""
 
-from pathlib import Path
 import os
 import re
 import shutil
 import tarfile
 import zipfile
+from pathlib import Path
 
 import invoke
 import requests

tools/requirements/lint.txt

@@ -1,2 +0,0 @@
-flake8 == 3.7.6
-isort == 4.3.4

tools/travis/run.sh

@@ -1,9 +1,8 @@
 #!/bin/bash
 set -e
 
-# Short circuit tests and linting jobs if there are no code changes involved.
-if [[ $TOXENV != docs ]] && [[ $TOXENV != lint-py2 ]] && [[ $TOXENV != lint ]]; then
-    # Keep lint and lint-py2, for docs/conf.py
+# Short circuit test runs if there are no code changes involved.
+if [[ $TOXENV != docs ]] || [[ $TOXENV != lint ]]; then
     if [[ "$TRAVIS_PULL_REQUEST" == "false" ]]
     then
         echo "This is not a PR -- will do a complete build."
@@ -18,7 +17,7 @@ if [[ $TOXENV != docs ]] && [[ $TOXENV != lint-py2 ]] && [[ $TOXENV != lint ]];
         echo "$changes"
         if ! echo "$changes" | grep -qvE '(\.rst$)|(^docs)|(^news)|(^\.github)'
         then
-            echo "Only Documentation was updated -- skipping build."
+            echo "Code was not changed -- skipping build."
             exit
         fi
     fi

tox.ini

@@ -1,7 +1,7 @@
 [tox]
 minversion = 3.4.0
 envlist =
-    docs, packaging, mypy, lint, lint-py2,
+    docs, packaging, lint,
     py27, py35, py36, py37, py38, pypy, pypy3
 
 [helpers]
@@ -50,33 +50,9 @@ commands =
     check-manifest
     python setup.py check -m -r -s
 
-[lint]
-deps = -r{toxinidir}/tools/requirements/lint.txt
-
 [testenv:lint]
 skip_install = True
-basepython = python3
-deps = {[lint]deps}
 commands_pre =
+deps = pre-commit
 commands =
-    flake8
-    isort --check-only --diff
-
-[testenv:lint-py2]
-skip_install = True
-basepython = python2
-deps = {[lint]deps}
-commands_pre =
-# No need to flake8 docs, tools & tasks in py2
-commands =
-    flake8 src tests
-    isort --check-only --diff
-
-[testenv:mypy]
-skip_install = True
-basepython = python3
-deps = -r{toxinidir}/tools/requirements/mypy.txt
-commands_pre =
-commands =
-    mypy src
-    mypy src -2
+   pre-commit run [] --all-files --show-diff-on-failure