fix
This commit is contained in:
@@ -0,0 +1,73 @@
|
||||
# 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
|
||||
Reference in New Issue
Block a user