Install and configure ruff

This commit includes some trivial changes to make `ruff` rules pass, but
includes an enumeration of other rules which will require some effort to
enable.

This commit includes adding `noqa` directives in places where we're fine
with the violation, as well as in places we'd really like to fix it. For
an example of each category:

- I occasionally make functions to replace callable classes, and use
  class-like naming syntax to make the replacement clear. Naming rules
  can be bypassed.
- I seriously regret embracing Django signals and should move away from
  them. Their resulting no-op imports should be preserved.

`ruff .` will now pass!

.github/workflows/ci.yml

@@ -11,6 +11,7 @@ jobs:
           - "black --diff --check ws"
           - "isort --diff --check ws"
           - "mypy ws"
+          - "ruff ."
           - "pylint --jobs=0 ws"
           - "pytest"
     runs-on: ubuntu-22.04

pyproject.toml

@@ -4,6 +4,139 @@ include = '\.py$'
 # (Preserves the convention of "single quotes for data, double quotes for humans")
 skip-string-normalization = true
 
+[tool.ruff]
+select = [
+  "A",     # flake8-builtins
+  "ANN",   # flake8-annotations
+  "ARG",   # flake8-unused-arguments
+  "ASYNC", # flake8-async
+  "B",     # flake8-bugbear
+  "BLE",   # flake8-blind-except
+  "C4",    # flake8-comprehensions
+  "C90",   # mccabe
+  "COM",   # flake8-commas
+  "DJ",    # flake8-django
+  "DTZ",   # flake8-datetimez
+  "E",     # pycodestyle error
+  "TD",    # flake8-todos
+  "EXE",   # flake8-executable
+  "F",     # Pyflakes
+  "FA",    # flake8-future-annotations
+  "G",     # flake8-logging-format
+  "ICN",   # flake8-import-conventions
+  "INP",   # flake8-no-pep420
+  "INT",   # flake8-gettext
+  "ISC",   # flake8-implicit-str-concat
+  "N",     # pep8-naming
+  "PGH",   # pygrep-hooks
+  "PIE",   # flake8-pie
+  "PLC",   # pylint convention
+  "PLE",   # pylint error
+  "PLR",   # pylint refactor
+  "PLW",   # pylint warning
+  "PT",    # flake8-pytest-style
+  "PYI",   # flake8-pyi
+  "RET",   # flake8-return
+  "RSE",   # flake8-raise
+  "RUF",   # Ruff-specific rules
+  "S",     # flake8-bandit
+  "SIM",   # flake8-simplify
+  "SLF",   # flake8-self
+  "T10",   # flake8-debugger
+  "T20",   # flake8-print
+  "TCH",   # flake8-type-checking
+  "TID",   # flake8-tidy-imports
+  "TRY",   # tryceratops
+  "UP",    # pyupgrade
+  "W",     # pycodestyle warning
+  "YTT",   # flake8-2020
+
+  # Interesting, should perhaps turn on later:
+  # "D",    # pydocstyle
+  # "I",    # isort
+  # "PTH",  # flake8-use-pathlib
+  # "FLY",  # flynt
+
+  # Intentionally omitted:
+  # "Q",     # flake8-quotes        -- we let black handle quotes.
+  # "FBT",   # flake8-boolean-trap  -- I'm fine with boolean traps
+  # "ERA",   # eradicate            -- has tons of false-positives on commented "code"
+  # "EM",    # flake8-errmsg        -- I'm fine with strings directly in exceptions
+  # "PD",    # pandas-vet           -- I don't use pandas
+  # "NPY",   # NumPy-specific rules -- I don't use numpy
+]
+
+ignore = [
+  # Specific rules ignored from checkers I otherwise view as valuable:
+  "ANN001",   # Type annotations are knowingly incomplete.
+  "ANN002",   # Type annotations are knowingly incomplete.
+  "ANN003",   # Type annotations are knowingly incomplete.
+  "ANN101",   # Type annotations are knowingly incomplete.
+  "ANN102",   # Type annotations are knowingly incomplete.
+  "ANN201",   # Type annotations are knowingly incomplete.
+  "ANN202",   # Type annotations are knowingly incomplete.
+  "ANN204",   # Type annotations are knowingly incomplete.
+  "ANN205",   # Type annotations are knowingly incomplete.
+  "ANN206",   # Type annotations are knowingly incomplete.
+  "ANN401",   # Type annotations are knowingly incomplete.
+  "ARG001",   # Unused arguments are common when inheriting with `*args, **kwargs`.
+  "ARG002",   # Unused arguments are common when inheriting with `*args, **kwargs`.
+  "COM812",   # Black manages commas.
+  "D100",     # Not all public classes need docstrings.
+  "D101",     # Not all public classes need docstrings.
+  "D102",     # Not all public methods need docstrings.
+  "D107",     # Not all `__init__` methods need docstrings.
+  "E501",     # Black manages line length.
+  "N806",     # Function arguments are frequently uppercase in migrations.
+  "PT009",    # I intentionally use `unittest`-style tests.
+  "PT019",    # I don't use pytest fixtures.
+  "S101",     # Using `assert` is fine (I run with them enabled).
+  "S311",     # I use RNGs but they're not for cryptography purposes.
+  "TD002",    # This is a solo project -- I authored all the TODOs.
+  "TD003",    # This is a solo project -- I don't track issues.
+  "TRY003",   # I don't mind specifying messages in the exception raise.
+
+  # Errors which I'll plan to fix soon:
+  "B904",     # Use `raise from` to preserve context (also see TRY200)
+  "PLR0913",  # Refactor functions with too many args
+  "B905",     # Use `strict()` on `zip()`
+  "DJ001",    # Avoid nullable charfields
+  "DJ006",    # Use fields instead of exclude with `ModelForm`
+  "DJ008",    # Each model should declare `__str__`
+  "DJ012",    # Put `Meta` below other definitions
+  "DTZ001",   # Avoid naive datetimes
+  "DTZ003",   # Always use timezone-aware UTC
+  "G004",     # Logging should use lazy interpolation
+  "N818",     # Use the Error suffix on error instances
+  "PGH003",   # Avoid blanket type ignores
+  "PLR2004",  # Do not use magic values in comparisons
+  "SIM102",   # Use just one `if` statement
+  "SIM105",   # Use `contextlib.suppress`
+  "SIM117",   # Avoid repeated `with` statements
+  "T201",     # Logging should be used, not `print` statements
+  "TRY200",   # Use `raise from` to preserve context (also see B904)
+  "TRY400",   # Use logging.exception, not logging.error
+]
+
+# By default, `ruff` will respect `.gitignore`
+# However, we can/should be explicit about excluded directories for CI
+exclude = [
+  ".venv/",
+  "node_modules/",
+  "static/",
+]
+
+fixable = ["ALL"]
+unfixable = []
+
+[tool.ruff.flake8-self]
+ignore-names = [
+  "_asdict",
+  "_replace",
+  "_fields",
+  "_meta",
+]
+
 [tool.isort]
 profile = "black"
 
@@ -39,6 +172,7 @@ psycopg2 = "*"
 pwned-passwords-django = "^1.6"  # Awaiting new release to fix ValueError
 requests = "*"
 sentry-sdk = "^1.3.1"
+ruff = "^0.0.270"
 
 [tool.poetry.dev-dependencies]
 black = "*"
@@ -150,6 +284,11 @@ disable = [
   ############################
   "missing-docstring",
   "unused-argument",  # Currently quite common in method strings
+
+  ####################################
+  # We let ruff handle these instead #
+  ####################################
+  "protected-access",
 ]
 
 [tool.pylint.REPORTS]

ws/__init__.py

@@ -1,2 +1,2 @@
 # Import this eagerly when Django starts so tasks will use the initialized app
-from .celery_config import app as celery_app
+from .celery_config import app as celery_app  # noqa: F401

ws/apps.py

@@ -8,7 +8,7 @@ class TripsConfig(AppConfig):
     @staticmethod
     def load_signals():
         # pylint: disable=unused-import, unused-variable, import-outside-toplevel
-        from . import signals
+        from . import signals  # noqa: F401
 
     def ready(self):
         self.load_signals()

ws/cleanup.py

@@ -82,7 +82,7 @@ def purge_old_medical_data():
         # If some other query already holds a lock, just skip for now!
         # (likely means that the participant is being updated, and shouldn't be purged)
         # We can always try to purge again on the next scheduled run.
-        # TODO (Django 2), exclusive join *only* on Participant + e. info, not Membership
+        # TODO (Django 2): exclusive join *only* on Participant + e. info, not Membership
         # .select_for_update(skip_locked=True, of=('self', 'emergency_info'))
     )
 

ws/enums.py

@@ -142,7 +142,7 @@ def how_to_fix_for(self, trip):
             self.WAIVER_MISSING: f'<a href="{initiate_waiver}">sign a waiver</a>',
             self.WAIVER_NEEDS_RENEWAL: f'''have a <a href="{initiate_waiver}">waiver that's valid until at least {trip_date}</a>''',
         }
-        return mark_safe(mapping[self])
+        return mark_safe(mapping[self])  # noqa: S308
 
 
 @enum.unique

ws/forms.py

@@ -641,7 +641,7 @@ class Meta:
 
 
 # TODO: This should be a class, not a method.
-def LeaderApplicationForm(*args, **kwargs):
+def LeaderApplicationForm(*args, **kwargs):  # noqa: N802
     """Factory form for applying to be a leader in any activity."""
     activity_enum: enums.Activity = kwargs.pop('activity_enum')
 
@@ -651,7 +651,7 @@ class Meta:
             model = models.LeaderApplication.model_from_activity(activity_enum)
             widgets = {
                 field.name: forms.Textarea(attrs={'rows': 4})
-                for field in model._meta.fields  # pylint: disable=protected-access
+                for field in model._meta.fields
                 if isinstance(field, TextField)
             }
 
@@ -721,14 +721,14 @@ class DuesForm(forms.Form):
         widget=forms.HiddenInput(), initial='membership fees.'
     )
 
-    merchantDefinedData1 = forms.CharField(
+    merchantDefinedData1 = forms.CharField(  # noqa: N815
         widget=forms.HiddenInput(), initial=PAYMENT_TYPE
     )
-    merchantDefinedData2 = forms.ChoiceField(
+    merchantDefinedData2 = forms.ChoiceField(  # noqa: N815
         required=True, label='Affiliation', choices=list(amount_choices())
     )
-    merchantDefinedData3 = forms.EmailField(required=True, label='Email')
-    merchantDefinedData4 = forms.CharField(
+    merchantDefinedData3 = forms.EmailField(required=True, label='Email')  # noqa: N815
+    merchantDefinedData4 = forms.CharField(  # noqa: N815
         required=True,
         label="Member name",
         widget=forms.TextInput(

ws/icons.py

@@ -75,8 +75,8 @@ def for_trip(trip: models.Trip) -> SafeString:
     """Return (safe) HTML with an icon that describes the trip."""
     icon = fa_icon_for_trip(trip)
     if not icon:
-        return mark_safe('')  # (For consistent return type)
+        return mark_safe('')  # (For consistent return type)  # noqa: S308
     solid_regular = 'far' if icon == 'hand-rock' else 'fa'
     title = escape(_describe(trip))
     html = f'<i class="{solid_regular} fa-fw fa-{icon}" title="{title}"></i>'
-    return mark_safe(html)
+    return mark_safe(html)  # noqa: S308

ws/lottery/rank.py

@@ -233,7 +233,9 @@ def trips_flaked(participant: models.Participant) -> set[int]:
         # This is a lousy hack to work around mypy.
         # Locally, `participant.feedback_set` raises `attr-defined`, which I ignore
         # But then in CI, `# type: ignore[attr-defined` is flagged as unused
-        feedback: QuerySet[models.Feedback] = getattr(participant, 'feedback_set')
+        feedback: QuerySet[models.Feedback] = getattr(  # noqa: B009
+            participant, 'feedback_set'
+        )
         return set(
             feedback.filter(
                 showed_up=False, trip__program=enums.Program.WINTER_SCHOOL.value

ws/merge.py

@@ -68,7 +68,7 @@ def simple_fk_update(cursor, table, col, old_pk, new_pk):
         update {table}
            set {col} = %(new_pk)s
          where {col} = %(old_pk)s
-        """,
+        """,  # noqa: S608
         {'new_pk': new_pk, 'old_pk': old_pk},
     )
 

ws/models.py

@@ -1042,7 +1042,7 @@ class Trip(models.Model):
     )
     name = models.CharField(max_length=127)
     description = models.TextField(
-        help_text=mark_safe(
+        help_text=mark_safe(  # noqa: S308
             '<a href="https://github.com/adam-p/markdown-here/wiki/Markdown-Cheatsheet">'
             "Markdown</a> supported! "
             'Please use HTTPS images sparingly, and only if properly licensed.'
@@ -1072,7 +1072,7 @@ class Trip(models.Model):
             ('B', 'B: >1 hour to intensive care, below treeline'),
             ('C', 'C: above treeline'),
         ],
-        help_text=mark_safe(
+        help_text=mark_safe(  # noqa: S308
             'Trip leaders must meet <a href="/help/participants/ws_ratings/">requirements for terrain & activity ratings</a>.',
         ),
         db_index=True,

ws/settings.py

@@ -42,7 +42,7 @@
 GEARDB_SECRET_KEY = os.getenv(
     'GEARDB_SECRET_KEY', 'secret shared with the mitoc-gear repo'
 )
-WS_LOTTERY_LOG_DIR = os.getenv('WS_LOTTERY_LOG_DIR', '/tmp/')
+WS_LOTTERY_LOG_DIR = os.getenv('WS_LOTTERY_LOG_DIR', '/tmp/')  # noqa: S108
 
 # URL to an avatar image that is self-hosted
 # (Users who opt out of Gravatar would prefer to not have requests made to
@@ -105,7 +105,7 @@
 ]
 try:
     # The debug toolbar is always absent in prod, but optional for local development!
-    import debug_toolbar  # pylint: disable=unused-import
+    import debug_toolbar  # pylint: disable=unused-import  # noqa: F401
 except ImportError:
     pass
 else:
@@ -174,11 +174,11 @@
 WHITELISTED_BAD_PASSWORDS: list[str] = []
 
 if os.environ.get('WS_DJANGO_TEST'):
-    from .conf.test_settings import *  # pylint: disable=wildcard-import,unused-wildcard-import
+    from .conf.test_settings import *  # pylint: disable=wildcard-import,unused-wildcard-import  # noqa: F403
 elif os.environ.get('WS_DJANGO_LOCAL'):
-    from .conf.local_settings import *  # pylint: disable=wildcard-import,unused-wildcard-import
+    from .conf.local_settings import *  # pylint: disable=wildcard-import,unused-wildcard-import  # noqa: F403
 else:
-    from .conf.production_settings import *  # pylint: disable=wildcard-import,unused-wildcard-import
+    from .conf.production_settings import *  # pylint: disable=wildcard-import,unused-wildcard-import  # noqa: F403
 
 # 32 bit signed integers (default before Django 3.2) are plenty big enough for our purposes.
 DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'
@@ -234,7 +234,7 @@
                 "ws.context_processors.participant_and_groups",
                 "ws.context_processors.angular_templates",
             ],
-            'debug': DEBUG,
+            'debug': DEBUG,  # noqa: F405
         },
     }
 ]
@@ -410,7 +410,7 @@
         'file': {
             'level': 'INFO',
             'class': 'logging.FileHandler',
-            'filename': os.getenv('DJANGO_LOG_FILE', '/tmp/django.log'),
+            'filename': os.getenv('DJANGO_LOG_FILE', '/tmp/django.log'),  # noqa: S108
             'formatter': 'verbose',
         }
     },
@@ -423,7 +423,7 @@
 FRONTEND_DIR = os.path.join(BASE_DIR, 'frontend')
 WEBPACK_LOADER = {
     'DEFAULT': {
-        'CACHE': DEBUG,
+        'CACHE': DEBUG,  # noqa: F405
         'BUNDLE_DIR_NAME': '/static/frontend/',
         'STATS_FILE': os.path.join(FRONTEND_DIR, 'webpack-stats.json'),
     }

ws/signals/__init__.py

@@ -1 +1 @@
-from . import auth_signals, signup_signals
+from . import auth_signals, signup_signals  # noqa: F401

ws/templatetags/application_tags.py

@@ -19,7 +19,7 @@ def application_summary(application):
 
 @register.inclusion_tag('for_templatetags/application_details.html')
 def application_details(application):
-    all_fields = application._meta.fields  # pylint:disable=protected-access
+    all_fields = application._meta.fields
     text_fields = [
         (field, getattr(application, field.name))
         for field in all_fields

ws/templatetags/avatar_tags.py

@@ -18,7 +18,7 @@ def avatar(participant, size=40, img_rounded=True):
         'size': size,
         'class': 'class="img-rounded"' if img_rounded else '',
     }
-    return mark_safe(
+    return mark_safe(  # noqa: S308
         '<img {class} src="{url}" alt="User avatar"'
         ' height="{size}" width="{size}">'.format(**kwargs)
     )

ws/templatetags/markdown_tags.py

@@ -7,4 +7,4 @@
 
 @register.filter(name="markdown")
 def markdown_filter(text):
-    return mark_safe(markdown2.markdown(text, safe_mode="escape"))
+    return mark_safe(markdown2.markdown(text, safe_mode="escape"))  # noqa: S308

ws/templatetags/misc_tags.py

@@ -23,4 +23,4 @@ def format_phone_number(number):
 @register.filter
 def redact(content: str, should_redact: bool) -> str:
     """Optionally replace content that should be redacted."""
-    return mark_safe("<em>redacted</em>") if should_redact else content
+    return mark_safe("<em>redacted</em>") if should_redact else content  # noqa: S308

ws/tests/email/test_renew.py

@@ -70,7 +70,7 @@ def test_normal_renewal_no_discounts(self):
         )
 
         with mock.patch.object(mail.EmailMultiAlternatives, 'send') as send:
-            with self.settings(UNSUBSCRIBE_SECRET_KEY='sooper-secret'):
+            with self.settings(UNSUBSCRIBE_SECRET_KEY='sooper-secret'):  # noqa: S106
                 msg = renew.send_email_reminding_to_renew(par)
         send.assert_called_once()
 
@@ -117,7 +117,7 @@ def test_participant_with_discounts(self):
         par.discounts.add(DiscountFactory.create(name='Acme Corp'))
 
         with mock.patch.object(mail.EmailMultiAlternatives, 'send') as send:
-            with self.settings(UNSUBSCRIBE_SECRET_KEY='sooper-secret'):
+            with self.settings(UNSUBSCRIBE_SECRET_KEY='sooper-secret'):  # noqa: S106
                 msg = renew.send_email_reminding_to_renew(par)
         send.assert_called_once()