fix: stricter ruff config and code adjust to match

chore(linting): ruff configuration that selects nearly all rules
refactor: code adjustments for things that ruff can't fix itself
refactor: all lists in e.g. serializers and views were replaced with tuples

pyproject.toml

@@ -94,12 +94,14 @@ exclude_lines = [
     "def __str__",
     "def __unicode__",
     "def __repr__",
+    "if TYPE_CHECKING",
 ]
 omit = [
     "*/migrations/*",
     "*/apps.py",
     "*/admin.py",
     "manage.py",
+    "timed/redmine/management/commands/import_project_data.py",
     "timed/settings_*.py",
     "timed/wsgi.py",
     "timed/forms.py",

ruff.toml

@@ -11,27 +11,106 @@ docstring-code-format = true
 docstring-code-line-length = 88
 
 [lint]
-select = ["E", "F", "W", "I", "BLE", "T10"]
+select = [
+	"F",       # pyflakes
+	"E",       # pycodestyle errors
+	"I",       # isort
+	"C90",     # mccabe
+	"N",       # pep8-naming
+	"D",       # pydocstyle
+	"UP",      # pyupgrade
+	"ANN",     # flake8-annotations
+	"ASYNC",   # flake8-async
+	"S",       # flake8-bandit
+	"BLE",     # flake8-blind-exception
+	"FBT",     # flake8-boolean-trap
+	"B",       # flake8-bugbear
+	"A",       # flake8-builtins
+	"COM",     # flake8-commas
+	"C4",      # flake8-comprehensions
+	"T10",     # flake8-debugger
+	"DJ",      # flake8-django
+	"EM",      # flake8-errmsg
+	"EXE",     # flake8-executable
+	"FA",      # flake8-future-annotations
+	"ISC",     # flake8-implicit-str-concat
+	"ICN",     # flake8-import-conventions
+	"G",       # flake8-logging-format
+	"INP",     # flake8-no-pep420
+	"PIE",     # flake8-pie
+	"T20",     # flake8-print
+	"PYI",     # flake8-pyi
+	"PT",      # flake8-pytest-style
+	"Q",       # flake8-quotes
+	"RSE",     # flake8-raise
+	"RET",     # flake8-return
+	"SLF",     # flake8-self
+	"SLOT",    # flake8-slots
+	"SIM",     # flake8-simplify
+	"TID",     # flake8-tidy-imports
+	"TCH",     # flake8-type-checking
+	"INT",     # flake8-gettext
+	"ARG",     # flake8-unused-arguments
+	"PTH",     # flake8-use-pathlib
+	"TD",      # flake8-todos
+	"ERA",     # eradicate
+	"PGH",     # pygrep-hooks
+	"PL",      # pylint
+	"TRY",     # tryceratops
+	"PERF",    # perflint
+	"RUF",     # ruff specific rules
+	"W605",    # invalid escape sequence
+]
 ignore = [
-    # https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules
-    "W191",
-    "E111",
-    "E114",
-    "E117",
-    "E501",
-    "D206",
-    "D300",
-    "Q000",
-    "Q001",
-    "Q002",
-    "Q003",
-    "COM812",
-    "COM819",
-    "ISC001",
-    "ISC002"
+	"ANN101",  # this is deprecated and annotating self is unnecessary
+	"D203",    # we prefer blank-line-before-class (D211) for black compat
+	"D213",    # we prefer multi-line-summary-first-line (D212)
+	"COM812",  # ignore due to conflict with formatter
+	"ISC001",  # ignore due to conflict with formatter
+	"E501",    # managed by formatter
+	"TD002",   # don't require author of TODO
+	"TD003",   # don't require link to TODO
+	"D100",    # don't enforce existance of docstrings
+	"D101",    # don't enforce existance of docstrings
+	"D102",    # don't enforce existance of docstrings
+	"D103",    # don't enforce existance of docstrings
+	"D104",    # don't enforce existance of docstrings
+	"D105",    # don't enforce existance of docstrings
+	"D106",    # don't enforce existance of docstrings
+	"D107",    # don't enforce existance of docstrings
+	"DJ001",   # null=True on string-based fields such as CharField (#1052) 
+]
+
+[lint.per-file-ignores]
+"**/{conftest.py,tests/*.py}" = [
+	"D",       # pydocstyle is optional for tests
+	"ANN",     # flake8-annotations are optional for tests
+	"S101",    # assert is allow in tests
+	"S105",    # tests may have hardcoded secrets
+	"S106",    # tests may have hardcoded passwords
+	"S311",    # tests may use pseudo-random generators
+	"S108",    # /tmp is allowed in tests since it's expected to be mocked
+	"DTZ00",   # tests often run in UTC
+	"INP001",  # tests do not need a dunder init
+	"PLR0913", # tests can have a lot of arguments (fixtures)
+	"PLR2004", # tests can use magic values
+]
+"**/*/factories.py" = [
+	"S311",    # factories may use pseudo-random generators
 ]
 
 [lint.isort]
 known-first-party = ["timed"]
 known-third-party = ["pytest_factoryboy"]
 combine-as-imports = true
+
+[lint.flake8-annotations]
+# TODO: drop this, its temporary
+# https://github.com/adfinis/timed-backend/issues/1054
+ignore-fully-untyped = true
+
+[lint.flake8-unused-arguments]
+ignore-variadic-names = true
+
+[lint.flake8-pytest-style]
+fixture-parentheses = false

timed/authentication.py

@@ -12,13 +12,10 @@
 
 
 class TimedOIDCAuthenticationBackend(OIDCAuthenticationBackend):
-    def get_introspection(self, access_token, id_token, payload):
+    def get_introspection(self, access_token, _id_token, _payload):
         """Return user details dictionary."""
-
         basic = base64.b64encode(
-            f"{settings.OIDC_RP_INTROSPECT_CLIENT_ID}:{settings.OIDC_RP_INTROSPECT_CLIENT_SECRET}".encode(
-                "utf-8"
-            )
+            f"{settings.OIDC_RP_INTROSPECT_CLIENT_ID}:{settings.OIDC_RP_INTROSPECT_CLIENT_SECRET}".encode()
         ).decode()
         headers = {
             "Authorization": f"Basic {basic}",
@@ -29,42 +26,39 @@ def get_introspection(self, access_token, id_token, payload):
             verify=settings.OIDC_VERIFY_SSL,
             headers=headers,
             data={"token": access_token},
+            timeout=10,
         )
         response.raise_for_status()
         return response.json()
 
     def get_userinfo_or_introspection(self, access_token):
         try:
-            claims = self.cached_request(
-                self.get_userinfo, access_token, "auth.userinfo"
-            )
-            return claims
-        except requests.HTTPError as e:
-            if e.response.status_code not in [401, 403]:
-                raise e
+            return self.cached_request(self.get_userinfo, access_token, "auth.userinfo")
+        except requests.HTTPError as exc:
+            if exc.response.status_code not in [401, 403]:
+                raise
             if settings.OIDC_CHECK_INTROSPECT:
                 try:
                     # check introspection if userinfo fails (confidential client)
                     claims = self.cached_request(
                         self.get_introspection, access_token, "auth.introspection"
                     )
                     if "client_id" not in claims:
-                        raise SuspiciousOperation(
-                            "client_id not present in introspection"
-                        )
-                    return claims
+                        msg = "client_id not present in introspection"
+                        raise SuspiciousOperation(msg)
                 except requests.HTTPError as e:
                     # if the authorization fails it's not a valid client or
                     # the token is expired and permission is denied.
                     # Handing on the 401 Client Error would be transformed into
                     # a 500 by Django's exception handling. But that's not what we want.
                     if e.response.status_code not in [401, 403]:  # pragma: no cover
-                        raise e
-            raise AuthenticationFailed()
+                        raise
+                else:
+                    return claims
+            raise AuthenticationFailed from exc
 
-    def get_or_create_user(self, access_token, id_token, payload):
+    def get_or_create_user(self, access_token, _id_token, _payload):
         """Verify claims and return user, otherwise raise an Exception."""
-
         claims = self.get_userinfo_or_introspection(access_token)
 
         users = self.filter_users_by_claims(claims)
@@ -73,15 +67,14 @@ def get_or_create_user(self, access_token, id_token, payload):
             user = users.get()
             self.update_user_from_claims(user, claims)
             return user
-        elif settings.OIDC_CREATE_USER:
+        if settings.OIDC_CREATE_USER:
             return self.create_user(claims)
-        else:
-            LOGGER.debug(
-                "Login failed: No user with username %s found, and "
-                "OIDC_CREATE_USER is False",
-                self.get_username(claims),
-            )
-            return None
+        LOGGER.debug(
+            "Login failed: No user with username %s found, and "
+            "OIDC_CREATE_USER is False",
+            self.get_username(claims),
+        )
+        return None
 
     def update_user_from_claims(self, user, claims):
         user.email = claims.get(settings.OIDC_EMAIL_CLAIM, "")
@@ -106,7 +99,6 @@ def cached_request(self, method, token, cache_prefix):
 
     def create_user(self, claims):
         """Return object for a newly created user account."""
-
         username = self.get_username(claims)
         email = claims.get(settings.OIDC_EMAIL_CLAIM, "")
         first_name = claims.get(settings.OIDC_FIRSTNAME_CLAIM, "")
@@ -119,5 +111,6 @@ def create_user(self, claims):
     def get_username(self, claims):
         try:
             return claims[settings.OIDC_USERNAME_CLAIM]
-        except KeyError:
-            raise SuspiciousOperation("Couldn't find username claim")
+        except KeyError as exc:
+            msg = "Couldn't find username claim"
+            raise SuspiciousOperation(msg) from exc

timed/conftest.py

@@ -14,8 +14,8 @@
 
 
 def register_module(module):
-    for name, obj in inspect.getmembers(module):
-        if isinstance(obj, FactoryMetaClass) and not obj._meta.abstract:
+    for _name, obj in inspect.getmembers(module):
+        if isinstance(obj, FactoryMetaClass) and not obj._meta.abstract:  # noqa: SLF001
             register(obj)
 
 
@@ -26,7 +26,7 @@ def register_module(module):
 
 
 @pytest.fixture
-def auth_user(db):
+def auth_user(db):  # noqa: ARG001
     return get_user_model().objects.create_user(
         username="user",
         password="123qweasd",
@@ -38,7 +38,7 @@ def auth_user(db):
 
 
 @pytest.fixture
-def admin_user(db):
+def admin_user(db):  # noqa: ARG001
     return get_user_model().objects.create_user(
         username="admin",
         password="123qweasd",
@@ -50,7 +50,7 @@ def admin_user(db):
 
 
 @pytest.fixture
-def superadmin_user(db):
+def superadmin_user(db):  # noqa: ARG001
     return get_user_model().objects.create_user(
         username="superadmin",
         password="123qweasd",
@@ -62,7 +62,7 @@ def superadmin_user(db):
 
 
 @pytest.fixture
-def external_employee(db):
+def external_employee(db):  # noqa: ARG001
     user = get_user_model().objects.create_user(
         username="user",
         password="123qweasd",
@@ -76,7 +76,7 @@ def external_employee(db):
 
 
 @pytest.fixture
-def internal_employee(db):
+def internal_employee(db):  # noqa: ARG001
     user = get_user_model().objects.create_user(
         username="user",
         password="123qweasd",
@@ -140,16 +140,15 @@ def internal_employee_client(internal_employee):
     return client
 
 
-@pytest.fixture(scope="function", autouse=True)
+@pytest.fixture(autouse=True)
 def _autoclear_cache():
     cache.clear()
 
 
 def setup_customer_and_employment_status(
     user, is_assignee, is_customer, is_employed, is_external
 ):
-    """
-    Set up customer and employment status.
+    """Set up customer and employment status.
 
     Return a 2-tuple of assignee and employment, if they
     were created