From 727e91c3f8a74eaa99d1d8e3e2cf04be96c2c5c3 Mon Sep 17 00:00:00 2001 From: Christian Clauss Date: Mon, 24 Feb 2025 19:20:45 +0100 Subject: [PATCH] Add ruff rules A,B,C4,EM,ISC,PERF,PGH,RET,RUF,S,SIM,SLF, --- .pre-commit-config.yaml | 2 +- docs/conf.py | 2 +- pyproject.toml | 34 ++++++++++++--------- test_app/models.py | 2 ++ test_settings.py | 2 +- waffle/__init__.py | 12 ++++---- waffle/admin.py | 10 +++--- waffle/apps.py | 2 +- waffle/decorators.py | 6 ++-- waffle/management/commands/waffle_flag.py | 22 +++++++------ waffle/management/commands/waffle_sample.py | 15 ++++----- waffle/management/commands/waffle_switch.py | 14 +++++---- waffle/managers.py | 2 +- waffle/middleware.py | 7 ++--- waffle/mixins.py | 9 +++--- waffle/models.py | 11 +++---- waffle/templatetags/waffle_tags.py | 3 +- waffle/tests/test_middleware.py | 4 +-- waffle/tests/test_testutils.py | 6 ++-- waffle/tests/test_views.py | 12 ++++---- waffle/tests/test_waffle.py | 6 ++-- waffle/utils.py | 6 ++-- 22 files changed, 99 insertions(+), 90 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4a39d196..009384e2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,6 +6,6 @@ repos: - id: trailing-whitespace - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.8.4 + rev: v0.9.7 hooks: - id: ruff diff --git a/docs/conf.py b/docs/conf.py index 9f180506..f1663be9 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -40,7 +40,7 @@ # General information about the project. project = 'django-waffle' -copyright = '2012-2018, James Socol' +copyright = '2012-2018, James Socol' # noqa: A001 # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the diff --git a/pyproject.toml b/pyproject.toml index 61246a80..d32277c6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,13 +71,17 @@ target-version = "py38" [tool.ruff.lint] select = [ + "A", # flake8-builtins "AIR", # Airflow "ASYNC", # flake8-async + "B", # flake8-bugbear "BLE", # flake8-blind-except + "C4", # flake8-comprehensions "C90", # McCabe cyclomatic complexity "DJ", # flake8-django "DTZ", # flake8-datetimez "E", # pycodestyle errors + "EM", # flake8-errmsg "F", # Pyflakes "FIX", # flake8-fixme "FLY", # flynt @@ -85,12 +89,20 @@ select = [ "ICN", # flake8-import-conventions "INP", # flake8-no-pep420 "INT", # flake8-gettext + "ISC", # flake8-implicit-str-concat "NPY", # NumPy-specific rules "PD", # pandas-vet + "PERF", # Perflint + "PGH", # pygrep-hooks "PIE", # flake8-pie "PL", # Pylint "PYI", # flake8-pyi + "RET", # flake8-return "RSE", # flake8-raise + "RUF", # Ruff-specific rules + "S", # flake8-bandit + "SIM", # flake8-simplify + "SLF", # flake8-self "SLOT", # flake8-slots "T10", # flake8-debugger "T20", # flake8-print @@ -99,32 +111,20 @@ select = [ "UP", # pyupgrade "W", # pycodestyle warnings "YTT", # flake8-2020 - # "A", # flake8-builtins # "ANN", # flake8-annotations # "ARG", # flake8-unused-arguments - # "B", # flake8-bugbear - # "C4", # flake8-comprehensions # "COM", # flake8-commas - # "CPY", # flake8-copyright + # "CPY", # flake8-copyright # "D", # pydocstyle - # "EM", # flake8-errmsg # "ERA", # eradicate # "EXE", # flake8-executable # "FA", # flake8-future-annotations # "FBT", # flake8-boolean-trap # "I", # isort - # "ISC", # flake8-implicit-str-concat # "N", # pep8-naming - # "PERF", # Perflint - # "PGH", # pygrep-hooks # "PT", # flake8-pytest-style # "PTH", # flake8-use-pathlib # "Q", # flake8-quotes - # "RET", # flake8-return - # "RUF", # Ruff-specific rules - # "S", # flake8-bandit - # "SIM", # flake8-simplify - # "SLF", # flake8-self # "TCH", # flake8-type-checking # "TRY", # tryceratops ] @@ -135,7 +135,11 @@ exclude = [ "*/migrations/*", "docs", ] -ignore = ["F401"] +ignore = [ + "F401", # Imported but unused + "ISC002", # Implicit string concatenation may conflict with black and ruff format + "S101", # Use of assert detected +] [tool.ruff.lint.mccabe] max-complexity = 23 @@ -143,6 +147,8 @@ max-complexity = 23 [tool.ruff.lint.per-file-ignores] "docs/conf.py" = ["INP001"] "test_app/models.py" = ["DJ008"] # FIXME +"waffle/admin.py" = ["SLF001"] # FIXME +"waffle/jinja.py" = [ "PGH003" ] # FIXME "waffle/models.py" = ["DJ012", "PYI019"] # FIXME [tool.ruff.lint.pylint] diff --git a/test_app/models.py b/test_app/models.py index 884dce4a..43e47de4 100644 --- a/test_app/models.py +++ b/test_app/models.py @@ -60,6 +60,8 @@ def is_active_for_user(self, user): company_ids = self._get_company_ids() if user.company_id in company_ids: return True + return None + return None def _get_company_ids(self): cache_key = keyfmt( diff --git a/test_settings.py b/test_settings.py index d19ee4de..4fb32ef4 100644 --- a/test_settings.py +++ b/test_settings.py @@ -20,7 +20,7 @@ SITE_ID = 1 USE_I18N = False -SECRET_KEY = 'foobar' +SECRET_KEY = 'foobar' # noqa: S105 DATABASES = { 'default': { diff --git a/waffle/__init__.py b/waffle/__init__.py index 6c706d8a..603fdbff 100755 --- a/waffle/__init__.py +++ b/waffle/__init__.py @@ -62,9 +62,9 @@ def get_waffle_model(setting_name: str) -> ( try: return django_apps.get_model(flag_model_name) - except ValueError: - raise ImproperlyConfigured(f"WAFFLE_{setting_name} must be of the form 'app_label.model_name'") - except LookupError: - raise ImproperlyConfigured( - f"WAFFLE_{setting_name} refers to model '{flag_model_name}' that has not been installed" - ) + except ValueError as ve: + msg = f"WAFFLE_{setting_name} must be of the form 'app_label.model_name'" + raise ImproperlyConfigured(msg) from ve + except LookupError as le: + msg = f"WAFFLE_{setting_name} refers to model '{flag_model_name}' that has not been installed" + raise ImproperlyConfigured(msg) from le diff --git a/waffle/admin.py b/waffle/admin.py index 1ec9592d..83e2c107 100755 --- a/waffle/admin.py +++ b/waffle/admin.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Any +from typing import Any, ClassVar from django.contrib import admin from django.contrib.admin.models import LogEntry, CHANGE, DELETION @@ -84,13 +84,13 @@ def label_and_url_for_value(self, values: Any) -> tuple[str, str]: .using(self.db) \ .get(**{key: value}) names.append(escape(str(name))) - except self.rel.model.DoesNotExist: + except self.rel.model.DoesNotExist: # noqa: PERF203 names.append('') return "(" + ", ".join(names) + ")", "" class FlagAdmin(BaseAdmin): - actions = [enable_for_all, disable_for_all, delete_individually] + actions: ClassVar = [enable_for_all, disable_for_all, delete_individually] list_display = ('name', 'note', 'everyone', 'percent', 'superusers', 'staff', 'authenticated', 'languages') list_filter = ('everyone', 'superusers', 'staff', 'authenticated') @@ -131,14 +131,14 @@ def disable_switches(ma, request, qs): class SwitchAdmin(BaseAdmin): - actions = [enable_switches, disable_switches, delete_individually] + actions: ClassVar = [enable_switches, disable_switches, delete_individually] list_display = ('name', 'active', 'note', 'created', 'modified') list_filter = ('active',) ordering = ('-id',) class SampleAdmin(BaseAdmin): - actions = [delete_individually] + actions: ClassVar = [delete_individually] list_display = ('name', 'percent', 'note', 'created', 'modified') ordering = ('-id',) diff --git a/waffle/apps.py b/waffle/apps.py index 2dfbae6e..c3fa38d1 100644 --- a/waffle/apps.py +++ b/waffle/apps.py @@ -7,4 +7,4 @@ class WaffleConfig(AppConfig): default_auto_field = 'django.db.models.AutoField' def ready(self) -> None: - import waffle.signals # noqa: F401 + import waffle.signals diff --git a/waffle/decorators.py b/waffle/decorators.py index 664dd52a..8f736bf9 100644 --- a/waffle/decorators.py +++ b/waffle/decorators.py @@ -25,8 +25,7 @@ def _wrapped_view(request, *args, **kwargs): response_to_redirect_to = get_response_to_redirect(redirect_to, *args, **kwargs) if response_to_redirect_to: return response_to_redirect_to - else: - raise Http404 + raise Http404 return view(request, *args, **kwargs) return _wrapped_view @@ -48,8 +47,7 @@ def _wrapped_view(request, *args, **kwargs): response_to_redirect_to = get_response_to_redirect(redirect_to, *args, **kwargs) if response_to_redirect_to: return response_to_redirect_to - else: - raise Http404 + raise Http404 return view(request, *args, **kwargs) return _wrapped_view diff --git a/waffle/management/commands/waffle_flag.py b/waffle/management/commands/waffle_flag.py index 12d53b2e..438d6052 100644 --- a/waffle/management/commands/waffle_flag.py +++ b/waffle/management/commands/waffle_flag.py @@ -61,14 +61,14 @@ def add_arguments(self, parser: CommandParser) -> None: parser.add_argument( '--group', '-g', action='append', - default=list(), + default=[], help='Turn on the flag for listed group names (use flag more ' 'than once for multiple groups). WARNING: This will remove ' 'any currently associated groups unless --append is used!') parser.add_argument( '--user', '-u', action='append', - default=list(), + default=[], help='Turn on the flag for listed usernames (use flag more ' 'than once for multiple users). WARNING: This will remove ' 'any currently associated users unless --append is used!') @@ -111,7 +111,8 @@ def handle(self, *args: Any, **options: Any) -> None: flag_name = options['name'] if not flag_name: - raise CommandError('You need to specify a flag name.') + msg = 'You need to specify a flag name.' + raise CommandError(msg) if options['create']: flag, created = get_waffle_flag_model().objects.get_or_create(name=flag_name) @@ -120,8 +121,9 @@ def handle(self, *args: Any, **options: Any) -> None: else: try: flag = get_waffle_flag_model().objects.get(name=flag_name) - except get_waffle_flag_model().DoesNotExist: - raise CommandError('This flag does not exist.') + except get_waffle_flag_model().DoesNotExist as dne: + msg = 'This flag does not exist.' + raise CommandError(msg) from dne # Group isn't an attribute on the Flag, but a related Many-to-Many # field, so we handle it a bit differently by looking up groups and @@ -133,8 +135,9 @@ def handle(self, *args: Any, **options: Any) -> None: try: group_instance = Group.objects.get(name=group) group_hash[group_instance.name] = group_instance.id - except Group.DoesNotExist: - raise CommandError(f'Group {group} does not exist') + except Group.DoesNotExist as dne: # noqa: PERF203 + msg = f'Group {group} does not exist' + raise CommandError(msg) from dne # If 'append' was not passed, we clear related groups if not options_append: flag.groups.clear() @@ -152,8 +155,9 @@ def handle(self, *args: Any, **options: Any) -> None: | Q(**{UserModel.EMAIL_FIELD: username}) ) user_hash.add(user_instance) - except UserModel.DoesNotExist: - raise CommandError(f'User {username} does not exist') + except UserModel.DoesNotExist as dne: # noqa: PERF203 + msg = f'User {username} does not exist' + raise CommandError(msg) from dne # If 'append' was not passed, we clear related users if not options_append: flag.users.clear() diff --git a/waffle/management/commands/waffle_sample.py b/waffle/management/commands/waffle_sample.py index e931917b..1ee80e0f 100644 --- a/waffle/management/commands/waffle_sample.py +++ b/waffle/management/commands/waffle_sample.py @@ -41,16 +41,16 @@ def handle(self, *args: Any, **options: Any) -> None: percent = options['percent'] if not (sample_name and percent): - raise CommandError( - 'You need to specify a sample name and percentage.' - ) + msg = 'You need to specify a sample name and percentage.' + raise CommandError(msg) try: percent = float(percent) if not (0.0 <= percent <= 100.0): raise ValueError - except ValueError: - raise CommandError('You need to enter a valid percentage value.') + except ValueError as ve: + msg = 'You need to enter a valid percentage value.' + raise CommandError(msg) from ve if options['create']: sample, created = get_waffle_sample_model().objects.get_or_create( @@ -60,8 +60,9 @@ def handle(self, *args: Any, **options: Any) -> None: else: try: sample = get_waffle_sample_model().objects.get(name=sample_name) - except get_waffle_sample_model().DoesNotExist: - raise CommandError('This sample does not exist.') + except get_waffle_sample_model().DoesNotExist as dne: + msg = 'This sample does not exist.' + raise CommandError(msg) from dne sample.percent = percent sample.save() diff --git a/waffle/management/commands/waffle_switch.py b/waffle/management/commands/waffle_switch.py index e9d26f45..124fdd79 100644 --- a/waffle/management/commands/waffle_switch.py +++ b/waffle/management/commands/waffle_switch.py @@ -7,9 +7,9 @@ def on_off_bool(string: str) -> bool: - if string not in ['on', 'off']: - raise ArgumentTypeError(f"invalid choice: {string!r} (choose from 'on', " - "'off')") + if string not in {'on', 'off'}: + msg = f"invalid choice: {string!r} (choose from 'on', 'off')" + raise ArgumentTypeError(msg) return string == 'on' @@ -53,7 +53,8 @@ def handle(self, *args: Any, **options: Any) -> None: state = options['state'] if not (switch_name and state is not None): - raise CommandError('You need to specify a switch name and state.') + msg = 'You need to specify a switch name and state.' + raise CommandError(msg) if options["create"]: switch, created = get_waffle_switch_model().objects.get_or_create( @@ -64,8 +65,9 @@ def handle(self, *args: Any, **options: Any) -> None: else: try: switch = get_waffle_switch_model().objects.get(name=switch_name) - except get_waffle_switch_model().DoesNotExist: - raise CommandError('This switch does not exist.') + except get_waffle_switch_model().DoesNotExist as dne: + msg = 'This switch does not exist.' + raise CommandError(msg) from dne switch.active = state switch.save() diff --git a/waffle/managers.py b/waffle/managers.py index 4f9216d3..4d8925f6 100644 --- a/waffle/managers.py +++ b/waffle/managers.py @@ -6,7 +6,7 @@ if TYPE_CHECKING: - from waffle.models import _BaseModelType, AbstractBaseFlag, AbstractBaseSample, AbstractBaseSwitch # noqa: F401 + from waffle.models import _BaseModelType, AbstractBaseFlag, AbstractBaseSample, AbstractBaseSwitch else: _BaseModelType = TypeVar("_BaseModelType") diff --git a/waffle/middleware.py b/waffle/middleware.py index ec31b1a7..a11b0a69 100644 --- a/waffle/middleware.py +++ b/waffle/middleware.py @@ -14,11 +14,8 @@ def process_response(self, request: HttpRequest, response: HttpResponse) -> Http for k in request.waffles: name = smart_str(get_setting('COOKIE') % k) active, rollout = request.waffles[k] - if rollout and not active: - # "Inactive" is a session cookie during rollout mode. - age = None - else: - age = max_age + # "Inactive" is a session cookie during rollout mode. + age = None if rollout and not active else max_age response.set_cookie(name, value=active, max_age=age, secure=secure) if hasattr(request, 'waffle_tests'): diff --git a/waffle/mixins.py b/waffle/mixins.py index 1896ca3e..bb1c25a8 100644 --- a/waffle/mixins.py +++ b/waffle/mixins.py @@ -11,13 +11,12 @@ class BaseWaffleMixin: def validate_waffle(self, waffle, func): if waffle.startswith('!'): - active = not func(waffle[1:]) - else: - active = func(waffle) - return active + return not func(waffle[1:]) + return func(waffle) def invalid_waffle(self): - raise Http404('Inactive waffle') + msg = 'Inactive waffle' + raise Http404(msg) class WaffleFlagMixin(BaseWaffleMixin): diff --git a/waffle/models.py b/waffle/models.py index fa1c4a55..8e883bdd 100644 --- a/waffle/models.py +++ b/waffle/models.py @@ -275,13 +275,12 @@ def is_active(self, request: HttpRequest, read_only: bool = False) -> bool | Non return get_setting('FLAG_DEFAULT') - if get_setting('OVERRIDE'): - if self.name in request.GET: - return request.GET[self.name] == '1' + if get_setting('OVERRIDE') and self.name in request.GET: + return request.GET[self.name] == '1' if self.everyone: return True - elif self.everyone is False: + if self.everyone is False: return False if self.testing: # Testing mode is on. @@ -322,7 +321,7 @@ def is_active(self, request: HttpRequest, read_only: bool = False) -> bool | Non return flag_active if not read_only: - if Decimal(str(random.uniform(0, 100))) <= self.percent: + if Decimal(str(random.uniform(0, 100))) <= self.percent: # noqa: S311 set_flag(request, self.name, True, self.rollout) return True set_flag(request, self.name, False, self.rollout) @@ -551,7 +550,7 @@ def is_active(self) -> bool: cache.set(self._cache_key(self.name), sample) return get_setting('SAMPLE_DEFAULT') - return Decimal(str(random.uniform(0, 100))) <= self.percent + return Decimal(str(random.uniform(0, 100))) <= self.percent # noqa: S311 class Switch(AbstractBaseSwitch): diff --git a/waffle/templatetags/waffle_tags.py b/waffle/templatetags/waffle_tags.py index 62f5bb7a..7e7f2527 100644 --- a/waffle/templatetags/waffle_tags.py +++ b/waffle/templatetags/waffle_tags.py @@ -41,7 +41,8 @@ def render(self, context): def handle_token(cls, parser, token, kind, condition): bits = token.split_contents() if len(bits) < 2: - raise template.TemplateSyntaxError(f"{bits[0]!r} tag requires an argument") + msg = f"{bits[0]!r} tag requires an argument" + raise template.TemplateSyntaxError(msg) name = bits[1] compiled_name = parser.compile_filter(name) diff --git a/waffle/tests/test_middleware.py b/waffle/tests/test_middleware.py index 11af7c91..8ef95994 100644 --- a/waffle/tests/test_middleware.py +++ b/waffle/tests/test_middleware.py @@ -17,8 +17,8 @@ def test_set_cookies(): assert 'dwf_foo' in resp.cookies assert 'dwf_bar' in resp.cookies - assert 'True' == resp.cookies['dwf_foo'].value - assert 'False' == resp.cookies['dwf_bar'].value + assert resp.cookies['dwf_foo'].value == 'True' + assert resp.cookies['dwf_bar'].value == 'False' def test_rollout_cookies(): diff --git a/waffle/tests/test_testutils.py b/waffle/tests/test_testutils.py index 3872a041..57d6db8d 100644 --- a/waffle/tests/test_testutils.py +++ b/waffle/tests/test_testutils.py @@ -66,7 +66,8 @@ def test_restores_after_exception(self): def inner(): with override_switch('foo', active=False): - raise RuntimeError("Trying to break") + msg = "Trying to break" + raise RuntimeError(msg) with self.assertRaises(RuntimeError): inner() @@ -78,7 +79,8 @@ def test_restores_after_exception_in_decorator(self): @override_switch('foo', active=False) def inner(): - raise RuntimeError("Trying to break") + msg = "Trying to break" + raise RuntimeError(msg) with self.assertRaises(RuntimeError): inner() diff --git a/waffle/tests/test_views.py b/waffle/tests/test_views.py index 8259dcc7..305c8b24 100644 --- a/waffle/tests/test_views.py +++ b/waffle/tests/test_views.py @@ -37,19 +37,19 @@ def test_waffle_status_response(self): self.assertEqual(200, response.status_code) content = response.json() - assert 'test_flag_active' in content['flags'].keys() + assert 'test_flag_active' in content['flags'] assert content['flags']['test_flag_active']['is_active'] - assert 'test_flag_inactive' in content['flags'].keys() + assert 'test_flag_inactive' in content['flags'] assert not content['flags']['test_flag_inactive']['is_active'] - assert 'test_switch_active' in content['switches'].keys() + assert 'test_switch_active' in content['switches'] assert content['switches']['test_switch_active']['is_active'] - assert 'test_switch_inactive' in content['switches'].keys() + assert 'test_switch_inactive' in content['switches'] assert not content['switches']['test_switch_inactive']['is_active'] - assert 'test_sample_active' in content['samples'].keys() + assert 'test_sample_active' in content['samples'] assert content['samples']['test_sample_active']['is_active'] - assert 'test_sample_inactive' in content['samples'].keys() + assert 'test_sample_inactive' in content['samples'] assert not content['samples']['test_sample_inactive']['is_active'] def test_flush_all_flags(self): diff --git a/waffle/tests/test_waffle.py b/waffle/tests/test_waffle.py index 87f3e69d..5fb600e4 100644 --- a/waffle/tests/test_waffle.py +++ b/waffle/tests/test_waffle.py @@ -455,7 +455,7 @@ def test_flag_created_dynamically_default_true(self): def test_no_logging_missing_flag_by_default(self, mock_logger): request = get() waffle.flag_is_active(request, 'foo') - mock_logger.log.call_count == 0 + assert mock_logger.log.call_count == 0 @override_settings(WAFFLE_LOG_MISSING_FLAGS=logging.WARNING) @mock.patch('waffle.models.logger') @@ -578,7 +578,7 @@ def test_switch_created_dynamically_true(self): @mock.patch('waffle.models.logger') def test_no_logging_missing_switch_by_default(self, mock_logger): waffle.switch_is_active('foo') - mock_logger.log.call_count == 0 + assert mock_logger.log.call_count == 0 @override_settings(WAFFLE_LOG_MISSING_SWITCHES=logging.WARNING) @mock.patch('waffle.models.logger') @@ -659,7 +659,7 @@ def test_sample_created_dynamically_default_true(self): @mock.patch('waffle.models.logger') def test_no_logging_missing_sample_by_default(self, mock_logger): waffle.switch_is_active('foo') - mock_logger.log.call_count == 0 + assert mock_logger.log.call_count == 0 @override_settings(WAFFLE_LOG_MISSING_SAMPLES=logging.WARNING) @mock.patch('waffle.models.logger') diff --git a/waffle/utils.py b/waffle/utils.py index 9e709420..a7fa3ed6 100644 --- a/waffle/utils.py +++ b/waffle/utils.py @@ -20,10 +20,8 @@ def get_setting(name: str, default: Any = None) -> Any: def keyfmt(k: str, v: str | None = None) -> str: prefix = get_setting('CACHE_PREFIX') + waffle.__version__ if v is None: - key = prefix + k - else: - key = prefix + hashlib.md5((k % v).encode('utf-8')).hexdigest() - return key + return prefix + k + return prefix + hashlib.md5((k % v).encode('utf-8')).hexdigest() # noqa: S324 def get_cache() -> BaseCache: