feat(lint): Flag .first() on querysets with no explicit ordering

src/AGENTS.md

@@ -135,6 +135,7 @@ Logging (`LOG005`, `LOG011`), spans, metric tag cardinality, and `options.get()`
 2. Data migrations using `apps.get_model()` operate on historical model classes: custom `save()` methods are absent, and signals scoped to the live model class do not run. Identify and explicitly perform required side effects such as cache invalidation; see the **`generate-migration`** skill.
 3. Add indexes for queries on 1M+ row tables (`db_index=True` or `db_index_together`).
 4. **Composite indexes**: any query filtering multiple columns (e.g. `foreign_key_id__in=... AND id__gt=...`, or FK + timestamp range, or cursor pagination combining filters) needs an explicit `Index(fields=[...])` in `Meta.indexes`, ordered most-selective-first. A single FK auto-index does NOT cover multi-column filters. See **`django-perf-review`** for validation.
+5. **Single-row lookups**: use `.get_or_none(...)` for a unique row. `.first()` with no `.order_by(...)` adds `ORDER BY pk`, which can pick a slow query plan (S034); add an explicit `.order_by(...)` when several rows can match. Do not add new `# noqa: S034`.
 
 ### Redis TTLs
 

tests/tools/test_flake8_plugin.py

@@ -22,6 +22,7 @@
     S027_msg,
     S028_msg,
     S030_allowlist,
+    S034_msg,
     SentryCheck,
     _s015_msg,
     get_api_violations,
@@ -2478,3 +2479,43 @@ def get(self, request, drf_request):
     ]
     assert _run(src, filename="src/sentry/auth/principal.py") == []
     assert _run(src, filename="tests/sentry/example.py") == []
+
+
+def test_S034() -> None:
+    src = """\
+Model.objects.first()  # bad
+Model.objects.filter(id=1).first()  # bad
+Model.objects.exclude(status=1).select_related("owner").first()  # bad
+Model.objects.filter(id=1).values_list("id", flat=True).first()  # bad
+project.teams.all().first()  # bad
+group.groupassignee_set.first()  # bad
+qs.filter(active=True).first()  # bad
+Model.objects.order_by().first()  # bad: no fields clears ordering
+Model.objects.order_by("name").filter(a=1).order_by().first()  # bad: last order_by wins
+x = (
+    Model.objects.filter(id=1)
+    .first()  # bad: reported on this line
+)
+Model.objects.order_by("id").first()  # ok
+Model.objects.order_by("-date").filter(a=1).select_related("owner").first()  # ok
+Model.objects.order_by().filter(a=1).order_by("id").first()  # ok
+Model.objects.order_by(*fields).first()  # ok
+Model.objects.filter_by_semver(v).first()  # ok: custom method may order
+get_queryset().filter(a=1).first()  # ok: helper may order
+qs.first()  # ok: variable may already be ordered
+detector.data_sources.first()  # ok: not visibly a queryset
+Model.objects.get_or_none(id=1)  # ok
+Model.objects.filter(id=1).first  # ok: not called
+"""
+    expected = [
+        f"t.py:{line}:{col}: {S034_msg}"
+        for line, col in [(1, 13), (2, 26), (3, 55), (4, 55), (5, 19), (6, 23), (7, 22), (8, 24)]
+    ]
+    assert _run(src, filename="src/sentry/example.py") == [
+        *expected,
+        f"t.py:9:53: {S034_msg}",
+        f"t.py:12:4: {S034_msg}",
+    ]
+    assert _run(src, filename="getsentry/example.py") == _run(src, filename="src/sentry/example.py")
+    assert _run(src, filename="tests/sentry/example.py") == []
+    assert _run(src, filename="src/sentry/testutils/example.py") == []

tools/flake8_plugin.py

@@ -200,6 +200,34 @@
     "set_request_principal(request, user, auth)."
 )
 
+S034_msg = (
+    "S034 .first() on a queryset with no .order_by(...) makes Django add ORDER BY pk, "
+    "which can pick a slow query plan; use .get_or_none(...) for a single unique row, "
+    "or add an explicit .order_by(...) to choose which row is returned"
+)
+# Calls that return a QuerySet; seeing one in the chain means `.first()` is a
+# queryset's. Only chains visible in the expression are checked: `qs.first()` on a
+# variable may already be ordered elsewhere.
+_S034_queryset_methods = frozenset(
+    (
+        "alias",
+        "all",
+        "annotate",
+        "defer",
+        "distinct",
+        "exclude",
+        "extra",
+        "filter",
+        "only",
+        "prefetch_related",
+        "select_for_update",
+        "select_related",
+        "using",
+        "values",
+        "values_list",
+    )
+)
+
 # Existing callers are pinned to their exact function and occurrence count. This is a
 # shrink-only migration baseline: remove entries as callers move to named Seer one-shots.
 S030_allowlist: dict[str, dict[tuple[str, str], int]] = {}
@@ -570,6 +598,38 @@ def _name_of(node: ast.expr) -> str:
     return ast.unparse(node)
 
 
+def _is_unordered_queryset_first(node: ast.Call) -> bool:
+    """`<queryset>.first()` where the visible chain never orders the queryset.
+
+    Walks back from `.first()` through known QuerySet methods. The last `order_by`
+    in the chain decides: with fields it orders, with none it clears ordering. The
+    chain must start at a manager (`Model.objects`, `obj.foo_set`) or apply a known
+    QuerySet method to a name; any other call (a helper, a custom manager method)
+    may order the queryset itself, so it is not flagged.
+    """
+    if not (isinstance(node.func, ast.Attribute) and node.func.attr == "first"):
+        return False
+    if node.args or node.keywords:
+        return False
+
+    current = node.func.value
+    saw_queryset_method = False
+    while isinstance(current, ast.Call) and isinstance(current.func, ast.Attribute):
+        method = current.func.attr
+        if method == "order_by":
+            return not current.args
+        if method not in _S034_queryset_methods:
+            return False
+        saw_queryset_method = True
+        current = current.func.value
+
+    if isinstance(current, ast.Attribute) and (
+        current.attr == "objects" or current.attr.endswith("_set")
+    ):
+        return True
+    return saw_queryset_method and isinstance(current, (ast.Name, ast.Attribute))
+
+
 def _is_acting_user_id(node: ast.expr) -> bool:
     """A call to `acting_user_id(...)`."""
     return (
@@ -1630,6 +1690,15 @@ def visit_Call(self, node: ast.Call) -> None:
                 if _is_acting_user_id(keyword.value):
                     self.errors.append((keyword.lineno, keyword.col_offset, S032_msg))
 
+        # S034: `.first()` on a queryset that the chain never orders. Tests may rely
+        # on the implicit pk ordering.
+        if not _is_tests_path(self.filename) and _is_unordered_queryset_first(node):
+            # Point at `.first` rather than the start of a multi-line chain.
+            assert node.func.end_lineno is not None and node.func.end_col_offset is not None
+            self.errors.append(
+                (node.func.end_lineno, node.func.end_col_offset - len(".first"), S034_msg)
+            )
+
         if isinstance(node.func, ast.Attribute) and node.func.attr in S019_logging_methods:
             for keyword in node.keywords:
                 if keyword.arg == "extra" and isinstance(keyword.value, ast.Dict):