style: pylint (#3720)

* fix: add pylint and fix issue

* chore: add pylint to pre-commit

* fix: local variable warning surfaced mistake in intree

.gitattributes

@@ -0,0 +1 @@
+docs/*.svg binary

.github/matchers/pylint.json

@@ -0,0 +1,32 @@
+{
+  "problemMatcher": [
+    {
+      "severity": "warning",
+      "pattern": [
+        {
+          "regexp": "^([^:]+):(\\d+):(\\d+): ([A-DF-Z]\\d+): \\033\\[[\\d;]+m([^\\033]+).*$",
+          "file": 1,
+          "line": 2,
+          "column": 3,
+          "code": 4,
+          "message": 5
+        }
+      ],
+      "owner": "pylint-warning"
+    },
+    {
+      "severity": "error",
+      "pattern": [
+        {
+          "regexp": "^([^:]+):(\\d+):(\\d+): (E\\d+): \\033\\[[\\d;]+m([^\\033]+).*$",
+          "file": 1,
+          "line": 2,
+          "column": 3,
+          "code": 4,
+          "message": 5
+        }
+      ],
+      "owner": "pylint-error"
+    }
+  ]
+}

.github/workflows/format.yml

@@ -12,6 +12,9 @@ on:
     - stable
     - "v*"
 
+env:
+  FORCE_COLOR: 3
+
 jobs:
   pre-commit:
     name: Format
@@ -19,6 +22,8 @@ jobs:
     steps:
     - uses: actions/checkout@v2
     - uses: actions/setup-python@v2
+    - name: Add matchers
+      run: echo "::add-matcher::$GITHUB_WORKSPACE/.github/matchers/pylint.json"
     - uses: pre-commit/action@v2.0.3
       with:
         # Slow hooks are marked with manual - slow is okay here, run them too

.pre-commit-config.yaml

@@ -87,23 +87,30 @@ repos:
   - id: rst-directive-colons
   - id: rst-inline-touching-normal
 
-# Flake8 also supports pre-commit natively (same author)
-- repo: https://github.com/PyCQA/flake8
-  rev: "4.0.1"
-  hooks:
-  - id: flake8
-    additional_dependencies: &flake8_dependencies
-      - flake8-bugbear
-      - pep8-naming
-    exclude: ^(docs/.*|tools/.*)$
-
 # Automatically remove noqa that are not used
 - repo: https://github.com/asottile/yesqa
   rev: "v1.3.0"
   hooks:
   - id: yesqa
+    additional_dependencies: &flake8_dependencies
+      - flake8-bugbear
+      - pep8-naming
+
+# Flake8 also supports pre-commit natively (same author)
+- repo: https://github.com/PyCQA/flake8
+  rev: "4.0.1"
+  hooks:
+  - id: flake8
+    exclude: ^(docs/.*|tools/.*)$
     additional_dependencies: *flake8_dependencies
 
+# PyLint has native support - not always usable, but works for us
+- repo: https://github.com/PyCQA/pylint
+  rev: "v2.12.2"
+  hooks:
+  - id: pylint
+    files: ^pybind11
+
 # CMake formatting
 - repo: https://github.com/cheshirekow/cmake-format-precommit
   rev: "v0.6.13"

noxfile.py

@@ -1,8 +1,13 @@
+import os
+
 import nox
 
 nox.options.sessions = ["lint", "tests", "tests_packaging"]
 
-PYTHON_VERSIONS = ["3.6", "3.7", "3.8", "3.9", "3.10", "3.11"]
+PYTHON_VERISONS = ["3.6", "3.7", "3.8", "3.9", "3.10", "3.11", "pypy3.7", "pypy3.8"]
+
+if os.environ.get("CI", None):
+    nox.options.error_on_missing_interpreters = True
 
 
 @nox.session(reuse_venv=True)
@@ -14,7 +19,7 @@ def lint(session: nox.Session) -> None:
     session.run("pre-commit", "run", "-a")
 
 
-@nox.session(python=PYTHON_VERSIONS)
+@nox.session(python=PYTHON_VERISONS)
 def tests(session: nox.Session) -> None:
     """
     Run the tests (requires a compiler).

pybind11/__main__.py

@@ -1,3 +1,5 @@
+# pylint: disable=missing-function-docstring
+
 import argparse
 import sys
 import sysconfig

pybind11/commands.py

@@ -3,16 +3,23 @@ import os
 DIR = os.path.abspath(os.path.dirname(__file__))
 
 
-def get_include(user: bool = False) -> str:
+def get_include(user: bool = False) -> str:  # pylint: disable=unused-argument
+    """
+    Return the path to the pybind11 include directory. The historical "user"
+    argument is unused, and may be removed.
+    """
     installed_path = os.path.join(DIR, "include")
     source_path = os.path.join(os.path.dirname(DIR), "include")
     return installed_path if os.path.exists(installed_path) else source_path
 
 
 def get_cmake_dir() -> str:
+    """
+    Return the path to the pybind11 CMake module directory.
+    """
     cmake_installed_path = os.path.join(DIR, "share", "cmake", "pybind11")
     if os.path.exists(cmake_installed_path):
         return cmake_installed_path
-    else:
-        msg = "pybind11 not installed, installation required to access the CMake files"
-        raise ImportError(msg)
+
+    msg = "pybind11 not installed, installation required to access the CMake files"
+    raise ImportError(msg)

pybind11/setup_helpers.py

@@ -239,7 +239,7 @@ def has_flag(compiler: Any, flag: str) -> bool:
     with tmp_chdir():
         fname = Path("flagcheck.cpp")
         # Don't trigger -Wunused-parameter.
-        fname.write_text("int main (int, char **) { return 0; }")
+        fname.write_text("int main (int, char **) { return 0; }", encoding="utf-8")
 
         try:
             compiler.compile([str(fname)], extra_postargs=[flag])
@@ -303,29 +303,31 @@ def intree_extensions(
     """
     exts = []
 
-    for path in paths:
-        if package_dir is None:
+    if package_dir is None:
+        for path in paths:
             parent, _ = os.path.split(path)
             while os.path.exists(os.path.join(parent, "__init__.py")):
                 parent, _ = os.path.split(parent)
             relname, _ = os.path.splitext(os.path.relpath(path, parent))
             qualified_name = relname.replace(os.path.sep, ".")
             exts.append(Pybind11Extension(qualified_name, [path]))
-        else:
-            for prefix, parent in package_dir.items():
-                if path.startswith(parent):
-                    relname, _ = os.path.splitext(os.path.relpath(path, parent))
-                    qualified_name = relname.replace(os.path.sep, ".")
-                    if prefix:
-                        qualified_name = prefix + "." + qualified_name
-                    exts.append(Pybind11Extension(qualified_name, [path]))
+        return exts
 
-    if not exts:
-        msg = (
-            f"path {path} is not a child of any of the directories listed "
-            f"in 'package_dir' ({package_dir})"
-        )
-        raise ValueError(msg)
+    for path in paths:
+        for prefix, parent in package_dir.items():
+            if path.startswith(parent):
+                relname, _ = os.path.splitext(os.path.relpath(path, parent))
+                qualified_name = relname.replace(os.path.sep, ".")
+                if prefix:
+                    qualified_name = prefix + "." + qualified_name
+                exts.append(Pybind11Extension(qualified_name, [path]))
+                break
+        else:
+            msg = (
+                f"path {path} is not a child of any of the directories listed "
+                f"in 'package_dir' ({package_dir})"
+            )
+            raise ValueError(msg)
 
     return exts
 
@@ -339,7 +341,7 @@ def naive_recompile(obj: str, src: str) -> bool:
     return os.stat(obj).st_mtime < os.stat(src).st_mtime
 
 
-def no_recompile(obg: str, src: str) -> bool:
+def no_recompile(obg: str, src: str) -> bool:  # pylint: disable=unused-argument
     """
     This is the safest but slowest choice (and is the default) - will always
     recompile sources.
@@ -412,7 +414,7 @@ class ParallelCompile:
         self,
         envvar: Optional[str] = None,
         default: int = 0,
-        max: int = 0,
+        max: int = 0,  # pylint: disable=redefined-builtin
         needs_recompile: Callable[[str, str], bool] = no_recompile,
     ) -> None:
         self.envvar = envvar
@@ -488,6 +490,9 @@ class ParallelCompile:
         return compile_function
 
     def install(self: S) -> S:
+        """
+        Installs the compile function into distutils.ccompiler.CCompiler.compile.
+        """
         distutils.ccompiler.CCompiler.compile = self.function()  # type: ignore[assignment]
         return self
 

pyproject.toml

@@ -30,3 +30,30 @@ strict = true
 [[tool.mypy.overrides]]
 module = ["ghapi.*", "setuptools.*"]
 ignore_missing_imports = true
+
+
+[tool.pytest.ini_options]
+minversion = "6.0"
+addopts = ["-ra", "--showlocals", "--strict-markers", "--strict-config"]
+xfail_strict = true
+filterwarnings = ["error"]
+log_cli_level = "info"
+testpaths = [
+    "tests",
+]
+timeout=300
+
+
+[tool.pylint]
+master.py-version = "3.6"
+reports.output-format = "colorized"
+messages_control.disable = [
+  "design",
+  "fixme",
+  "imports",
+  "line-too-long",
+  "imports",
+  "invalid-name",
+  "protected-access",
+  "missing-module-docstring",
+]