74 lines
2.9 KiB
Markdown
74 lines
2.9 KiB
Markdown
# Review Checklist
|
|
|
|
Full criteria for categorizing findings in the Python review skill.
|
|
|
|
## 🔴 Must Fix — Correctness & Type Safety
|
|
|
|
These block merge. Apply by default.
|
|
|
|
### mypy strict violations
|
|
- `error:` any mypy error (all are must-fix under strict mode)
|
|
- Implicit `Any` — untyped function parameters or return values
|
|
- `type: ignore` without a specific error code (e.g. bare `# type: ignore`)
|
|
- Incompatible types in assignment or return
|
|
- Missing `--strict` compatibility: missing `py.typed`, missing stubs
|
|
|
|
### Logic & Security (manual review)
|
|
- Mutable default arguments: `def f(x: list = [])` → use `None` + guard
|
|
- Shadowing built-ins: `list`, `dict`, `id`, `type`, `input`, etc.
|
|
- Uncaught exceptions from external I/O without explicit handling
|
|
- SQL/shell injection risks (string formatting into queries/commands)
|
|
- Hardcoded secrets or credentials
|
|
- `assert` used for runtime validation (stripped with `-O`)
|
|
|
|
---
|
|
|
|
## 🟡 Should Fix — Code Quality
|
|
|
|
These hurt maintainability. Fix unless explicitly skipped.
|
|
|
|
### ruff violations (common)
|
|
- `E501` — line too long (respect `line-length` from pyproject.toml)
|
|
- `F401` — unused import
|
|
- `F841` — local variable assigned but never used
|
|
- `B006` — mutable argument default
|
|
- `B007` — loop variable unused
|
|
- `UP` prefix — pyupgrade rules (use modern Python syntax)
|
|
- `SIM` prefix — simplifiable conditions
|
|
- `RUF` prefix — Ruff-specific best practices
|
|
|
|
### Manual quality
|
|
- Missing return type annotation on public functions/methods
|
|
- Missing docstring on public API (classes, public methods, module)
|
|
- `pass` in `except` block without a comment explaining why
|
|
- `print()` left in production code (use `logging`)
|
|
- Overly broad `except Exception` without re-raise or specific handling
|
|
- Dead code: unreachable branches, commented-out code blocks
|
|
|
|
---
|
|
|
|
## 🟢 Consider — Optimization & Modern Python 3.13+
|
|
|
|
Suggestions only. Offer but don't apply without confirmation.
|
|
|
|
### Python 3.13+ idioms
|
|
- Use `type X = ...` (PEP 695) instead of `TypeAlias`
|
|
- Use `X | Y` union syntax instead of `Union[X, Y]` or `Optional[X]`
|
|
- Use `TypeVar` with `type` statement (PEP 695) instead of old `TypeVar()`
|
|
- `@override` decorator (PEP 698) for overriding methods
|
|
- `tomllib` (stdlib) instead of third-party TOML parsers
|
|
- `pathlib.Path` instead of `os.path` string operations
|
|
|
|
### Performance
|
|
- Generator expressions instead of list comprehensions when result is iterated once
|
|
- `__slots__` on data-heavy classes to reduce memory
|
|
- `functools.cache` / `functools.lru_cache` for pure deterministic functions
|
|
- Avoid repeated attribute lookup in tight loops (`local = self.attr`)
|
|
- Prefer `collections.deque` over `list` for O(1) pop from front
|
|
|
|
### Readability
|
|
- Replace long `if/elif` chains with `match/case` (structural pattern matching)
|
|
- Extract deeply nested logic into named helper functions
|
|
- Use `dataclasses.dataclass` or `pydantic.BaseModel` for plain data containers
|
|
- Named tuples or `TypedDict` instead of untyped `dict` returns
|