From 0c6d1b198977459331e77d2e15466f62ec69adeb Mon Sep 17 00:00:00 2001 From: Brian Helba Date: Tue, 3 Oct 2023 11:56:42 -0400 Subject: [PATCH] WIP: Add Ruff --- pyproject.toml | 78 +++++++++++++++++++++++++------ s3_file_field/_multipart_minio.py | 4 +- s3_file_field/_registry.py | 2 +- s3_file_field/fields.py | 2 +- s3_file_field/fixtures.py | 2 +- s3_file_field/rest_framework.py | 5 +- s3_file_field/widgets.py | 6 +-- tests/conftest.py | 1 - tests/test_forms.py | 1 - tests/test_registry.py | 3 +- tests/test_views.py | 18 +++---- tox.ini | 30 ++---------- 12 files changed, 89 insertions(+), 63 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c555f85..1e3be2c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -117,20 +117,6 @@ source = [ [tool.django-stubs] django_settings_module = "test_app.settings" -[tool.isort] -profile = "black" -line_length = 100 -# Sort by name, don't cluster "from" vs "import" -force_sort_within_sections = true -# Combines "as" imports on the same line -combine_as_imports = true - -# These test utilities are local, but are loaded as absolute imports -known_local_folder = [ - "fuzzy", - "test_app", -] - [tool.mypy] files = [ "s3_file_field", @@ -151,6 +137,18 @@ mypy_path = [ # as that allows multiple possible import paths namespace_packages = false +# be strict +disallow_untyped_calls = true +warn_return_any = true +strict_optional = true +warn_no_return = true +warn_redundant_casts = true +warn_unused_ignores = true + +disallow_untyped_defs = true +check_untyped_defs = true +no_implicit_reexport = true + [[tool.mypy.overrides]] module = [ "minio_storage.storage", @@ -185,3 +183,55 @@ filterwarnings = [ "ignore::DeprecationWarning:django" ] DJANGO_SETTINGS_MODULE = "test_app.settings" + +[tool.ruff] +src = [".", "tests"] +line-length = 100 +target-version = "py38" +select = ["ALL"] +ignore = [ + "ANN", # Annotations + "FIX", # Fixme + + "COM812", # Trailing comma missing + "D100", # Missing docstring in public module + "D101", # Missing docstring in public class + "D102", # Missing docstring in public method + "D103", # Missing docstring in public function + "D104", # Missing docstring in public package + "D105", # Missing docstring in magic method + "D106", # Missing docstring in public nested class + "D107", # Missing docstring in __init__ + "TD002", # Missing author in TODO + "TD003", # Missing issue link on the line following this TODO + "TRY003", # Avoid specifying long messages outside the exception class + "EM101", # Exception must not use a string literal, assign to variable first + "EM102", # Exception must not use an f-string literal, assign to variable first + + # Maybe + "ERA001", # Found commented-out code +] + +[tool.ruff.per-file-ignores] +"example/**" = [ + "S105", # Possible hardcoded password + "INP001", # Part of an implicit namespace package. +] +"tests/**" = [ + "S105", # Possible hardcoded password + "SLF001", # Private member accessed + "PLR0913", # Too many arguments to function call + "DJ007", # Do not use `__all__` + "DJ008", # Model does not define `__str__` method + "S101", # Use of assert detected +] +"**/migrations/**" = [ + "RUF012", # Mutable class attributes should be annotated with `typing.ClassVar` +] + +[tool.ruff.pydocstyle] +convention = "pep257" + +[tool.ruff.isort] +# Sort by name, don't cluster "from" vs "import" +force-sort-within-sections = true diff --git a/s3_file_field/_multipart_minio.py b/s3_file_field/_multipart_minio.py index efc816c..4c6ed12 100644 --- a/s3_file_field/_multipart_minio.py +++ b/s3_file_field/_multipart_minio.py @@ -27,7 +27,7 @@ def _create_upload_id( object_key: str, content_type: str, ) -> str: - return self._client._create_multipart_upload( + return self._client._create_multipart_upload( # noqa: SLF001 bucket_name=self._bucket_name, object_name=object_key, headers={ @@ -37,7 +37,7 @@ def _create_upload_id( ) def _abort_upload_id(self, object_key: str, upload_id: str) -> None: - self._client._abort_multipart_upload( + self._client._abort_multipart_upload( # noqa: SLF001 bucket_name=self._bucket_name, object_name=object_key, upload_id=upload_id, diff --git a/s3_file_field/_registry.py b/s3_file_field/_registry.py index b0c0285..a510ecf 100644 --- a/s3_file_field/_registry.py +++ b/s3_file_field/_registry.py @@ -17,7 +17,7 @@ def register_field(field: "S3FileField") -> None: field_id = field.id - if field_id in _fields and not (_fields[field_id] is field): + if field_id in _fields and _fields[field_id] is not field: # This might be called multiple times, but it should always be consistent raise RuntimeError(f"Cannot overwrite existing S3FileField declaration for {field_id}") _fields[field_id] = field diff --git a/s3_file_field/fields.py b/s3_file_field/fields.py index 3d32ea3..dcec531 100644 --- a/s3_file_field/fields.py +++ b/s3_file_field/fields.py @@ -66,7 +66,7 @@ def contribute_to_class(self, cls, name, **kwargs): register_field(self) @staticmethod - def uuid_prefix_filename(instance: models.Model, filename: str) -> str: + def uuid_prefix_filename(_instance: models.Model, filename: str) -> str: return f"{uuid4()}/{filename}" def formfield( diff --git a/s3_file_field/fixtures.py b/s3_file_field/fixtures.py index 7395822..940f52d 100644 --- a/s3_file_field/fixtures.py +++ b/s3_file_field/fixtures.py @@ -16,7 +16,7 @@ def stored_file_object() -> Generator[File[bytes], None, None]: """Return a File object, already saved directly into the default Storage.""" # Fix https://github.com/typeddjango/django-stubs/issues/1610 - assert isinstance(default_storage, Storage) + assert isinstance(default_storage, Storage) # noqa: S101 # Ensure the name is always randomized, even if the key doesn't exist already key = default_storage.get_alternative_name("test_key", "") # In theory, Storage.save can change the key, though this shouldn't happen with a randomized key diff --git a/s3_file_field/rest_framework.py b/s3_file_field/rest_framework.py index 1cb414f..0b7c7da 100644 --- a/s3_file_field/rest_framework.py +++ b/s3_file_field/rest_framework.py @@ -1,5 +1,7 @@ from __future__ import annotations +from typing import ClassVar + from django.core.files import File from rest_framework.fields import FileField as FileSerializerField @@ -7,7 +9,7 @@ class S3FileSerializerField(FileSerializerField): - default_error_messages = { + default_error_messages: ClassVar[dict[str, str]] = { "invalid": "Not a valid signed S3 upload. Ensure that the S3 upload flow is correct.", } @@ -25,7 +27,6 @@ def to_internal_value(self, data: str | File) -> str: # type: ignore[override] # This checks validity of the file name and size super().to_internal_value(file_object) - assert file_object.name # fields.S3FileField.save_form_data is not called by DRF, so the same behavior must be # implemented here diff --git a/s3_file_field/widgets.py b/s3_file_field/widgets.py index 8fbccbc..d536fc0 100644 --- a/s3_file_field/widgets.py +++ b/s3_file_field/widgets.py @@ -2,7 +2,7 @@ import functools import posixpath -from typing import TYPE_CHECKING, Any, Mapping, NoReturn +from typing import TYPE_CHECKING, Any, ClassVar, Mapping, NoReturn from django.core import signing from django.core.files import File @@ -58,8 +58,8 @@ class S3FileInput(ClearableFileInput): """Widget to render the S3 File Input.""" class Media: - js = ["s3_file_field/widget.js"] - css = {"all": ["s3_file_field/widget.css"]} + js: ClassVar[list[str]] = ["s3_file_field/widget.js"] + css: ClassVar[dict[str, list[str]]] = {"all": ["s3_file_field/widget.css"]} def get_context(self, *args, **kwargs) -> dict[str, Any]: # The base URL cannot be determined at the time the widget is instantiated diff --git a/tests/conftest.py b/tests/conftest.py index bff29c5..080d377 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,7 +8,6 @@ from s3_file_field._multipart import MultipartManager from s3_file_field._sizes import mb - from test_app.models import Resource # Explicitly load s3_file_field fixtures, late in Pytest plugin load order. diff --git a/tests/test_forms.py b/tests/test_forms.py index c095e35..5ccd46c 100644 --- a/tests/test_forms.py +++ b/tests/test_forms.py @@ -1,7 +1,6 @@ import pytest from s3_file_field.forms import S3FormFileField - from test_app.forms import ResourceForm diff --git a/tests/test_registry.py b/tests/test_registry.py index 0333840..e5e605c 100644 --- a/tests/test_registry.py +++ b/tests/test_registry.py @@ -5,7 +5,6 @@ from s3_file_field import _registry from s3_file_field.fields import S3FileField - from test_app.models import Resource @@ -22,7 +21,7 @@ def test_field_id(s3ff_field: S3FileField) -> None: def test_field_id_premature() -> None: s3ff_field = S3FileField() with pytest.raises(Exception, match=r"contribute_to_class"): - s3ff_field.id + s3ff_field.id # noqa:B018 def test_registry_get_field(s3ff_field: S3FileField) -> None: diff --git a/tests/test_views.py b/tests/test_views.py index 9c2bdb6..a269a0f 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -5,11 +5,11 @@ from django.urls import reverse import pytest import requests +from requests.status_codes import codes from rest_framework.test import APIClient -from s3_file_field._sizes import mb - from fuzzy import FUZZY_UPLOAD_ID, FUZZY_URL, Fuzzy +from s3_file_field._sizes import mb def test_prepare(api_client: APIClient) -> None: @@ -23,7 +23,7 @@ def test_prepare(api_client: APIClient) -> None: }, format="json", ) - assert resp.status_code == 200 + assert resp.status_code == codes.ok assert resp.data == { "object_key": Fuzzy( r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/test.txt" @@ -51,7 +51,7 @@ def test_prepare_two_parts(api_client: APIClient) -> None: }, format="json", ) - assert resp.status_code == 200 + assert resp.status_code == codes.ok assert resp.data == { "object_key": Fuzzy( r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/test.txt" @@ -77,7 +77,7 @@ def test_prepare_three_parts(api_client: APIClient) -> None: }, format="json", ) - assert resp.status_code == 200 + assert resp.status_code == codes.ok assert resp.data == { "object_key": Fuzzy( r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/test.txt" @@ -108,7 +108,7 @@ def test_full_upload_flow( }, format="json", ) - assert resp.status_code == 200 + assert resp.status_code == codes.ok initialization = resp.data assert isinstance(initialization, dict) upload_signature = initialization["upload_signature"] @@ -134,7 +134,7 @@ def test_full_upload_flow( }, format="json", ) - assert resp.status_code == 200 + assert resp.status_code == codes.ok assert resp.data == { "complete_url": Fuzzy(r".*"), "body": Fuzzy(r".*"), @@ -162,14 +162,14 @@ def test_full_upload_flow( }, format="json", ) - assert resp.status_code == 200 + assert resp.status_code == codes.ok assert resp.data == { "field_value": Fuzzy(r".*:.*"), } # Verify that the Content headers were stored correctly on the object object_resp = requests.get(default_storage.url(initialization["object_key"]), timeout=5) - assert resp.status_code == 200 + assert resp.status_code == codes.ok assert object_resp.headers["Content-Type"] == "text/plain" default_storage.delete(initialization["object_key"]) diff --git a/tox.ini b/tox.ini index a4ab19f..e7fccfd 100644 --- a/tox.ini +++ b/tox.ini @@ -17,22 +17,17 @@ extras = [testenv:lint] package = skip deps = - flake8 - flake8-black - flake8-bugbear - flake8-docstrings - flake8-isort - pep8-naming + ruff commands = - flake8 . + ruff check . [testenv:format] package = skip deps = black - isort + ruff commands = - isort . + ruff check --fix-only . black . [testenv:type] @@ -81,20 +76,3 @@ commands = pyproject-build --sdist --wheel --outdir {envtmpdir} twine check {envtmpdir}/* twine upload --skip-existing {envtmpdir}/* - -[flake8] -max-line-length = 100 -show-source = True -ignore = - # closing bracket does not match indentation of opening bracket’s line - E123 - # whitespace before ':' - E203, - # line break before binary operator - W503, - # Missing docstring in * - D10, -extend-exclude = - node_modules, -# Explicitly set this, so "python-client/pyproject.toml" is never used -black-config = pyproject.toml