Lint with pre-commit (#7900)

Following [my comment here](#7589 (comment)) and [Django's own move to pre-commit](https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/#pre-commit-checks).

* Add pre-commit config file to run flake8 and isort.
* Add extra "common sense" hooks.
* Run pre-commit on GitHub actions using the [official action](https://github.com/pre-commit/action/). This is a good way to get up-and-running but it would be better if we activated [pre-commit.ci](https://pre-commit.ci/), which is faster and will auto-update the hooks for us going forwards.
* Remove `runtests.py` code for running linting tools.
* Remove `runtests.py --fast` flag, since that would now just run `pytest -q`, which can be done with `runtests.py -q` instead.
* Remove tox configuration and requirements files for linting.
* Update the contributing guide to mention setting up pre-commit.

Author: adamchainz

.github/workflows/pre-commit.yml

@@ -0,0 +1,24 @@
+name: pre-commit
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-20.04
+
+    steps:
+      - uses: actions/checkout@v2
+        with:
+          fetch-depth: 0
+
+      - uses: actions/setup-python@v2
+        with:
+          python-version: 3.9
+
+      - uses: pre-commit/[email protected]
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}

.gitignore

@@ -15,6 +15,7 @@
 MANIFEST
 coverage.*
 
+!.github
 !.gitignore
+!.pre-commit-config.yaml
 !.travis.yml
-!.isort.cfg

.pre-commit-config.yaml

@@ -0,0 +1,20 @@
+repos:
+- repo: https://github.com/pre-commit/pre-commit-hooks
+  rev: v3.4.0
+  hooks:
+  - id: check-added-large-files
+  - id: check-case-conflict
+  - id: check-json
+  - id: check-merge-conflict
+  - id: check-symlinks
+  - id: check-toml
+- repo: https://github.com/pycqa/isort
+  rev: 5.8.0
+  hooks:
+  - id: isort
+- repo: https://gitlab.com/pycqa/flake8
+  rev: 3.9.0
+  hooks:
+  - id: flake8
+    additional_dependencies:
+    - flake8-tidy-imports

.travis.yml

@@ -27,7 +27,6 @@ matrix:
       - { python: "3.9", env: DJANGO=main }
 
       - { python: "3.8", env: TOXENV=base }
-      - { python: "3.8", env: TOXENV=lint }
       - { python: "3.8", env: TOXENV=docs }
 
       - python: "3.8"

docs/community/contributing.md

@@ -54,11 +54,19 @@ To start developing on Django REST framework, first create a Fork from the
 Then clone your fork. The clone command will look like this, with your GitHub
 username instead of YOUR-USERNAME:
 
-    git clone https://github.com/YOUR-USERNAME/Spoon-Knife
+    git clone https://github.com/YOUR-USERNAME/django-rest-framework
 
 See GitHub's [_Fork a Repo_][how-to-fork] Guide for more help.
 
 Changes should broadly follow the [PEP 8][pep-8] style conventions, and we recommend you set up your editor to automatically indicate non-conforming styles.
+You can check your contributions against these conventions each time you commit using the [pre-commit](https://pre-commit.com/) hooks, which we also run on CI.
+To set them up, first ensure you have the pre-commit tool installed, for example:
+
+    python -m pip install pre-commit
+
+Then run:
+
+    pre-commit install
 
 ## Testing
 
@@ -79,18 +87,6 @@ Run using a more concise output style.
 
     ./runtests.py -q
 
-Run the tests using a more concise output style, no coverage, no flake8.
-
-    ./runtests.py --fast
-
-Don't run the flake8 code linting.
-
-    ./runtests.py --nolint
-
-Only run the flake8 code linting, don't run the tests.
-
-    ./runtests.py --lintonly
-
 Run the tests for a given test case.
 
     ./runtests.py MyTestCase

requirements.txt

@@ -9,5 +9,4 @@
 -r requirements/requirements-optionals.txt
 -r requirements/requirements-testing.txt
 -r requirements/requirements-documentation.txt
--r requirements/requirements-codestyle.txt
 -r requirements/requirements-packaging.txt

requirements/requirements-codestyle.txt

@@ -1,42 +1,8 @@
 #! /usr/bin/env python3
-import subprocess
 import sys
 
 import pytest
 
-PYTEST_ARGS = {
-    'default': [],
-    'fast': ['-q'],
-}
-
-FLAKE8_ARGS = ['rest_framework', 'tests']
-
-ISORT_ARGS = ['--check-only', '--diff', 'rest_framework', 'tests']
-
-
-def exit_on_failure(ret, message=None):
-    if ret:
-        sys.exit(ret)
-
-
-def flake8_main(args):
-    print('Running flake8 code linting')
-    ret = subprocess.call(['flake8'] + args)
-    print('flake8 failed' if ret else 'flake8 passed')
-    return ret
-
-
-def isort_main(args):
-    print('Running isort code checking')
-    ret = subprocess.call(['isort'] + args)
-
-    if ret:
-        print('isort failed: Some modules have incorrectly ordered imports. Fix by running `isort --recursive .`')
-    else:
-        print('isort passed')
-
-    return ret
-
 
 def split_class_and_function(string):
     class_string, function_string = string.split('.', 1)
@@ -54,31 +20,6 @@ def is_class(string):
 
 
 if __name__ == "__main__":
-    try:
-        sys.argv.remove('--nolint')
-    except ValueError:
-        run_flake8 = True
-        run_isort = True
-    else:
-        run_flake8 = False
-        run_isort = False
-
-    try:
-        sys.argv.remove('--lintonly')
-    except ValueError:
-        run_tests = True
-    else:
-        run_tests = False
-
-    try:
-        sys.argv.remove('--fast')
-    except ValueError:
-        style = 'default'
-    else:
-        style = 'fast'
-        run_flake8 = False
-        run_isort = False
-
     if len(sys.argv) > 1:
         pytest_args = sys.argv[1:]
         first_arg = pytest_args[0]
@@ -104,14 +45,5 @@ def is_class(string):
             # `runtests.py TestCase [flags]`
             # `runtests.py test_function [flags]`
             pytest_args = ['tests', '-k', pytest_args[0]] + pytest_args[1:]
-    else:
-        pytest_args = PYTEST_ARGS[style]
-
-    if run_tests:
-        exit_on_failure(pytest.main(pytest_args))
-
-    if run_flake8:
-        exit_on_failure(flake8_main(FLAKE8_ARGS))
 
-    if run_isort:
-        exit_on_failure(isort_main(ISORT_ARGS))
+    sys.exit(pytest.main(pytest_args))

runtests.py

@@ -5,7 +5,7 @@ envlist =
        {py36,py37,py38,py39}-django31,
        {py36,py37,py38,py39}-django32,
        {py38,py39}-djangomain,
-       base,dist,lint,docs,
+       base,dist,docs,
 
 [travis:env]
 DJANGO =
@@ -16,7 +16,7 @@ DJANGO =
     main: djangomain
 
 [testenv]
-commands = python -W error::DeprecationWarning -W error::PendingDeprecationWarning runtests.py --fast --coverage {posargs}
+commands = python -W error::DeprecationWarning -W error::PendingDeprecationWarning runtests.py --coverage {posargs}
 envdir = {toxworkdir}/venvs/{envname}
 setenv =
        PYTHONDONTWRITEBYTECODE=1
@@ -37,18 +37,12 @@ deps =
         -rrequirements/requirements-testing.txt
 
 [testenv:dist]
-commands = ./runtests.py --fast --no-pkgroot --staticfiles {posargs}
+commands = ./runtests.py --no-pkgroot --staticfiles {posargs}
 deps =
         django
         -rrequirements/requirements-testing.txt
         -rrequirements/requirements-optionals.txt
 
-[testenv:lint]
-commands = ./runtests.py --lintonly
-deps =
-        -rrequirements/requirements-codestyle.txt
-        -rrequirements/requirements-testing.txt
-
 [testenv:docs]
 skip_install = true
 commands = mkdocs build