Select and comply with all Ruff rules, minus a curated ignore list

.pre-commit-config.yaml

@@ -9,7 +9,7 @@ repos:
         language: system
         require_serial: true
         pass_filenames: false
-        files: "^pyproject\\.toml|^src/.*|^tests/.*|^docs/conf.py$"
+        files: "^pyproject\\.toml|ruff\\.toml|^src/.*|^tests/.*|^docs/conf.py$"
 
       - id: format-rust
         name: Format Rust
@@ -24,7 +24,7 @@ repos:
         entry: just lint-python
         language: system
         pass_filenames: false
-        files: "^pyproject\\.toml|^src/.*|^tests/.*|^docs/conf.py$"
+        files: "^pyproject\\.toml|ruff\\.toml|^src/.*|^tests/.*|^docs/conf.py$"
 
       - id: lint-rust
         name: Lint Rust

pyproject.toml

@@ -77,13 +77,3 @@ grimp = ["py.typed"]
 [tool.setuptools.packages.find]
 where = ["src"]
 namespaces = false
-
-[tool.ruff]
-line-length = 99
-exclude = [
-    'tests/assets', # These are sample packages for tests to run under - we don't want ruff to mess with them.
-    'docs/', # Documentation configuration scripts
-]
-lint.select = ["A", "C4", "E4", "E7", "E9", "F", "FLY", "FURB", "INP", "PLE", "PLR", "PT", "RUF", "SIM", "UP"]
-lint.ignore = ["C408", "PLR2004", "PLR0913", "PT006", "PT007", "SIM108", "SIM118", "SIM300"]
-lint.per-file-ignores."tests/**/*.py" = [ "RUF012", "RUF059" ]

ruff.toml

@@ -0,0 +1,58 @@
+line-length = 99
+exclude = [
+  'tests/assets', # These are sample packages for tests to run under - we don't want ruff to mess with them.
+  'docs/',        # Documentation configuration scripts
+]
+lint.select = [ "ALL" ]
+lint.ignore = [
+  "A005",    # [stdlib-module-shadowing]                Backward Compat.    Violated by `modulefinder.py`.
+  "ANN401",  # [any-type]                               Controversial.      We use `Any` type annotations in several places intentionally.
+  "ARG002",  # [unused-method-argument]                 Backward Compat.
+  "C408",    # [unnecessary-collection-call]            Controversial.      A micro-optimization like `dict(a=1)` -> `{"a": 1}` that hurts readability.
+  "COM812",  # [missing-trailing-comma]                 TODO enable.        Has a large diff to fix all occurrences.
+  "D1",      # [pydocstyle : Undocumented]              TODO enable.        Not a priority to have every public interface documented yet.
+  "D2",      # [pydocstyle : Whitespace issues]         TODO enable.        Has a large diff to fix all occurrences.
+  "E501",    # [line-too-long]                          Controversial.      Long comments and docstrings are sometimes clearer. The formatter already addresses most violations.
+  "FBT",     # [flake8-boolean-trap]                    Backward Compat.
+  "FIX",     # [flake8-fixme]                           Controversial.      Different projects use different conventions for FIXME/TODO comments.
+  "G004",    # [logging-f-string]                       Controversial.      A micro-optimization but hurts readability.
+  "N818",    # [error-suffix-on-exception-name]         Backward Compat.
+  "PERF203", # [try-except-in-loop]                     Controversial.      A micro-optimization but increases code complexity.
+  "PLR0913", # [too-many-arguments]                     Backward Compat.
+  "PLW1641", # [eq-without-hash]                        TODO enable.        This is low-priority, and basically an enhancement to make classes more ergonomic.
+  "PTH",     # [flake8-use-pathlib]                     Controversial.      `pathlib` adoption is subtle and might be too disruptive for this codebase. e.g. https://github.com/python-grimp/grimp/pull/289#discussion_r2636012008
+  "PT007",   # [pytest-parametrize-values-wrong-type]   TODO enable.        Inconsistent use of `@pytest.mark.parametrize("value", ("a", "b", "c"))` versus `...["a", "b", "c"])` in parameterizations.
+  "PYI025",  # [unaliased-collections-abc-set-import]   Controversial.      `collections.abc.Set` is not necessarily confused with the built-in `set`.
+  "RET504",  # [unnecessary-assign]                     Controversial.      A micro-optimization but hurts readability.
+  "RET505",  # [superfluous-else-return]                Controversial.      Explicit `else` blocks can improve readability.
+  "S101",    # [assert]                                 Controversial.       `assert` statements are sometimes appropriate for type-checking purposes.
+  "SIM108",  # [if-else-block-instead-of-if-exp]        Controversial.      Ternary/Binary in-line expressions often hurt readability.
+  "SIM118",  # [in-dict-keys]                           Controversial.      `if key in dict.keys()` sometimes has clearer intent than `if key in dict`.
+  "TC001",   # [typing-only-first-party-import]         Controversial.      This package has no/few dependencies so TYPE_CHECKING guards offer no material benefit and hurt readability.
+  "TC002",   # [typing-only-third-party-import]         Controversial.      This package has no/few dependencies so TYPE_CHECKING guards offer no material benefit and hurt readability.
+  "TC003",   # [typing-only-standard-library-import]    Controversial.      This package has no/few dependencies so TYPE_CHECKING guards offer no material benefit and hurt readability.
+  "TD",      # [missing-space-after-todo-colon]         Controversial.      Different projects use different conventions for FIXME/TODO comments.  
+  "TID252",  # [relative-imports]                       Controversial.      There are pros and cons to both relative and absolute imports in a packaging context.   
+]
+lint.per-file-ignores."tests/**/*.py" = [
+  "ANN",     # [flake8-annotations]                 Type annotations not needed in a test suite
+  "ARG",     # [flake8-unused-arguments]            Interface design doesn't need attention in a test suite
+  "B007",    # [unused-loop-control-variable]       Unused variables are common in tests    
+  "B018",    # [useless-expression]                 Unused variables are common in tests 
+  "B033",    # [duplicate-value]                    Can be deliberate in a test suite
+  "D",       # [pydocstyle]                         Docstrings not needed in a test suite
+  "EM",      # [flake8-errmsg]                      Error message style doesn't need attention in a test suite
+  "N806",    # [non-lowercase-variable-in-function] Variable naming conventions don't need attention in a test suite
+  "PLR2004", # [magic-value-comparison]             Magic values are common in tests
+  "RUF012",  # [mutable-class-default]              Type annotations not needed in a test suite    
+  "RUF059",  # [unused-unpacked-variable]           Unused variables are common in tests
+  "S",       # [flake8-bandit]                      Security patterns don't need attention in a test suite
+  "SIM300",  # [yoda-conditions]                    TODO enable.   
+  "TRY",     # [tryceratops]                        Exception handling patterns don't need attention in a test suite
+]
+lint.pydocstyle.convention = "google"
+lint.future-annotations = true
+lint.flake8-builtins.strict-checking = true
+lint.flake8-pytest-style.parametrize-names-type = "csv"
+lint.flake8-type-checking.quote-annotations = true
+lint.typing-extensions = false

src/grimp/__init__.py

@@ -1,8 +1,8 @@
 __version__ = "3.14"
 
-from .application.graph import DetailedImport, ImportGraph, Import
+from .application.graph import DetailedImport, Import, ImportGraph
 from .domain.analysis import PackageDependency, Route
-from .domain.valueobjects import DirectImport, Module, Layer
+from .domain.valueobjects import DirectImport, Layer, Module
 from .main import build_graph
 
 __all__ = [

src/grimp/adaptors/caching.py

@@ -1,15 +1,17 @@
-import hashlib
+from __future__ import annotations
 
+import hashlib
 import json
 import logging
+from typing import Any
 
+from grimp import _rustgrimp as rust  # type: ignore[attr-defined]
 from grimp.application.ports.filesystem import BasicFileSystem
 from grimp.application.ports.modulefinder import FoundPackage, ModuleFile
 from grimp.domain.valueobjects import DirectImport, Module
 
 from ..application.ports.caching import Cache as AbstractCache
 from ..application.ports.caching import CacheMiss
-from grimp import _rustgrimp as rust  # type: ignore[attr-defined]
 
 logger = logging.getLogger(__name__)
 PrimitiveFormat = dict[str, list[tuple[str, int | None, str]]]
@@ -64,7 +66,7 @@ def make_data_file_unique_string(
 class Cache(AbstractCache):
     DEFAULT_CACHE_DIR = ".grimp_cache"
 
-    def __init__(self, *args, namer: type[CacheFileNamer], **kwargs) -> None:
+    def __init__(self, *args: Any, namer: type[CacheFileNamer], **kwargs: Any) -> None:
         """
         Don't instantiate Cache directly; use Cache.setup().
         """
@@ -82,7 +84,7 @@ def setup(
         exclude_type_checking_imports: bool = False,
         cache_dir: str | None = None,
         namer: type[CacheFileNamer] = CacheFileNamer,
-    ) -> "Cache":
+    ) -> Cache:
         cache = cls(
             file_system=file_system,
             found_packages=found_packages,
@@ -103,17 +105,17 @@ def cache_dir_or_default(cls, cache_dir: str | None) -> str:
     def read_imports(self, module_file: ModuleFile) -> set[DirectImport]:
         try:
             cached_mtime = self._mtime_map[module_file.module.name]
-        except KeyError:
-            raise CacheMiss
+        except KeyError as exc:
+            raise CacheMiss from exc
         if cached_mtime != module_file.mtime:
             raise CacheMiss
 
         try:
             return self._data_map[module_file.module]
-        except KeyError:
+        except KeyError as exc:
             # While we would expect the module to be in here,
             # there's no point in crashing if, for some reason, it's not.
-            raise CacheMiss
+            raise CacheMiss from exc
 
     def write(
         self,
@@ -170,12 +172,13 @@ def _read_mtime_map_file(self, found_package: FoundPackage) -> dict[str, float]:
             return {}
         try:
             deserialized = json.loads(serialized)
-            logger.info(f"Used cache meta file {meta_cache_filename}.")
-            return deserialized
         except json.JSONDecodeError:
             logger.warning(f"Could not use corrupt cache file {meta_cache_filename}.")
             return {}
 
+        logger.info(f"Used cache meta file {meta_cache_filename}.")
+        return deserialized
+
     def _build_data_map(self) -> None:
         self._data_map = self._read_data_map_file()
 

src/grimp/adaptors/filesystem.py

@@ -1,9 +1,11 @@
+from __future__ import annotations
+
 import os
 import tokenize
 from collections.abc import Iterator
 
-from grimp.application.ports.filesystem import AbstractFileSystem, BasicFileSystem
 from grimp import _rustgrimp as rust  # type: ignore[attr-defined]
+from grimp.application.ports.filesystem import AbstractFileSystem, BasicFileSystem
 
 
 class FileSystem(AbstractFileSystem):

src/grimp/adaptors/modulefinder.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import logging
 from collections.abc import Iterable, Set
 
@@ -79,9 +81,11 @@ def _get_python_files_and_namespace_dirs_inside_package(
             for d in dirs_to_remove:
                 dirs.remove(d)
 
-            for filename in files:
-                if self._is_python_file(filename, dirpath):
-                    python_files.append(self.file_system.join(dirpath, filename))
+            python_files.extend(
+                self.file_system.join(dirpath, filename)
+                for filename in files
+                if self._is_python_file(filename, dirpath)
+            )
 
         namespace_dirs = self._determine_namespace_dirs(candidate_namespace_dirs, python_files)
         return python_files, namespace_dirs
@@ -112,9 +116,11 @@ def _is_python_file(self, filename: str, dirpath: str) -> bool:
         Files with extra dots in the name won't be treated as Python files.
 
         Args:
-            filename (str): the filename, excluding the path.
+            filename: the filename, excluding the path.
+            dirpath: the directory path containing the file.
+
         Returns:
-            bool: whether it's a Python file.
+            Whether it's a Python file.
         """
         # Ignore hidden files.
         if filename.startswith("."):
@@ -138,12 +144,13 @@ def _module_name_from_filename(
     ) -> str:
         """
         Args:
-            package_name (string) - the importable name of the top level package. Could
+            package_name: the importable name of the top level package. Could
                 be namespaced.
-            filename_and_path (string) - the full name of the Python file.
-            package_directory (string) - the full path of the top level Python package directory.
-         Returns:
-            Absolute module name for importing (string).
+            filename_and_path: the full name of the Python file.
+            package_directory: the full path of the top level Python package directory.
+
+        Returns:
+            Absolute module name for importing.
         """
         internal_filename_and_path = filename_and_path[len(package_directory) :]
         internal_filename_and_path_without_extension = internal_filename_and_path[1:-3]
@@ -160,11 +167,12 @@ def _namespace_from_dir(
     ) -> str:
         """
         Args:
-            package_name (string) - the importable name of the top level package. Could
+            package_name: the importable name of the top level package. Could
                 be namespaced.
-            namespace_dir (string) - the full name of the namespace directory.
-            package_directory (string) - the full path of the top level Python package directory.
-         Returns:
+            namespace_dir: the full name of the namespace directory.
+            package_directory: the full path of the top level Python package directory.
+
+        Returns:
             Absolute module name for importing (string).
         """
         parent_of_package_directory = package_directory[: -len(package_name)]

src/grimp/adaptors/packagefinder.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import importlib.util
 import logging
 import sys
@@ -19,7 +21,8 @@ def determine_package_directories(
         spec = importlib.util.find_spec(package_name)
         if not spec:
             logger.debug(f"sys.path: {sys.path}")
-            raise ValueError(f"Could not find package '{package_name}' in your Python path.")
+            msg = f"Could not find package '{package_name}' in your Python path."
+            raise ValueError(msg)
 
         if (
             spec.has_location

src/grimp/application/config.py

@@ -1,19 +1,21 @@
+from __future__ import annotations
+
 from typing import Any
 
 
 class Settings:
-    def __init__(self):
-        self._config = {}
+    def __init__(self) -> None:
+        self._config: dict[str, Any] = {}
 
-    def configure(self, **config_dict: Any):
+    def configure(self, **config_dict: Any) -> None:
         self._config.update(config_dict)
 
-    def __getattr__(self, name):
+    def __getattr__(self, name: str) -> Any:
         if name[:2] != "__":
             return self._config[name]
-        return super().__getattr__(name)
+        return super().__getattribute__(name)
 
-    def copy(self) -> "Settings":
+    def copy(self) -> Settings:
         new_instance = self.__class__()
         new_instance.configure(**self._config)
         return new_instance