Added initial typehints #494
Open
Claude / Claude Code Review
completed
May 15, 2026 in 43m 11s
Code review found 1 important issue
Found 5 candidates, confirmed 4. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | rollbar/test/fastapi_tests/test_routing.py:766-772 |
test_should_support_type_hints fails — PEP 563 strings vs type objects |
| 🟡 Nit | rollbar/lib/__init__.py:115-128 |
dict_merge TypeVar bound contradicts non-Mapping early return |
| 🟡 Nit | pyproject.toml:39-42 |
Stale Python 3.9 classifier after dropping 3.9 support |
Annotations
Check failure on line 772 in rollbar/test/fastapi_tests/test_routing.py
claude / Claude Code Review
test_should_support_type_hints fails — PEP 563 strings vs type objects
test_should_support_type_hints will fail because `rollbar/contrib/fastapi/routing.py` gained `from __future__ import annotations` in this PR, so under PEP 563 `add_to.__annotations__` now stores the string forms `'FastAPI | APIRouter'` and `'Type[APIRoute] | None'`, but the test rewritten at lines 766-772 compares with `assertDictEqual` to the actual type objects (`FastAPI | APIRouter`, `Type[APIRoute] | None`), which are not equal to strings. Fix by calling `typing.get_type_hints(add_to)` in th
Check warning on line 128 in rollbar/lib/__init__.py
claude / Claude Code Review
dict_merge TypeVar bound contradicts non-Mapping early return
The new TypeVar bound on `dict_merge` — `U = TypeVar('U', bound=dict | Mapping[str, Any])` (rollbar/lib/__init__.py:116) — contradicts the function body's explicit non-Mapping early return at line 127 (`if not isinstance(b, (dict, Mapping)): return b`), which is part of the tested contract (`test_dict_merge_not_dict` at rollbar/test/test_lib.py:58-63 passes `b=99`). External callers writing `dict_merge(a, 99)` with annotations are rejected by mypy with `Value of type variable 'U' of 'dict_merge'
Check warning on line 42 in pyproject.toml
claude / Claude Code Review
Stale Python 3.9 classifier after dropping 3.9 support
The classifiers list in pyproject.toml still includes `"Programming Language :: Python :: 3.9"` even though this PR bumped `requires-python` to `>=3.10` (and the CI matrix + README were updated to drop 3.9). This is metadata-only (pip uses `requires-python` as authoritative), but it makes PyPI's displayed 'Compatible Python versions' misleading. Fix: delete the `Programming Language :: Python :: 3.9` line from the classifiers array around line 15.
Loading