[chore] Add no-getattr rule; refine no-dataclasses rule (#29871)
This commit is contained in:
@@ -7,12 +7,12 @@ paths:
|
||||
|
||||
Define data containers as `msgspec.Struct`. Do not add new
|
||||
`dataclasses.dataclass` (or `attrs`) — they weaken strict type checking and
|
||||
don't map onto Rust structs for the planned Rust migration.
|
||||
don't translate cleanly for multi-language support (e.g. the planned Rust migration).
|
||||
|
||||
```python
|
||||
import msgspec
|
||||
|
||||
class LoadSnapshot(msgspec.Struct): # frozen=, kw_only=, omit_defaults= as needed
|
||||
class LoadSnapshot(msgspec.Struct): # prefer frozen= and omit_defaults=; kw_only= as needed
|
||||
dp_rank: int = 0
|
||||
tokens: list[int] = [] # mutable defaults are safe
|
||||
```
|
||||
@@ -21,4 +21,3 @@ class LoadSnapshot(msgspec.Struct): # frozen=, kw_only=, omit_defaults= as nee
|
||||
`python/sglang/srt/managers/load_snapshot.py`.
|
||||
- New code only. Existing `@dataclass` is grandfathered — migrate opportunistically
|
||||
while editing the file, not in drive-by sweeps.
|
||||
- If a third-party API forces `@dataclass`, keep it at that boundary only.
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
---
|
||||
paths:
|
||||
- "**/*.py"
|
||||
---
|
||||
|
||||
# Don't use `getattr` / `hasattr` for defensive access
|
||||
|
||||
Over-defensive `getattr(obj, "field", default)` / `hasattr(obj, "field")` hide
|
||||
errors and defeat strict type checking. If a field is always present, accessing
|
||||
it defensively is confusing and masks real bugs. Prefer:
|
||||
|
||||
1. **`isinstance` for type narrowing** — check the type, then access fields directly:
|
||||
|
||||
```python
|
||||
if (
|
||||
isinstance(obj, (TokenizedGenerateReqInput, TokenizedEmbeddingReqInput))
|
||||
and obj.mm_inputs
|
||||
):
|
||||
```
|
||||
(see `python/sglang/srt/managers/mm_utils.py`)
|
||||
|
||||
2. **Always set the field (to `None` if needed), then do a `None` check** — the
|
||||
field should always exist, so a `None` / non-`None` check is enough:
|
||||
|
||||
```python
|
||||
obj.field = None # in __init__ / construction
|
||||
...
|
||||
if obj.field is not None:
|
||||
...
|
||||
```
|
||||
|
||||
Bad — `server_args` always has `revision`, so `getattr` is misleading and swallows
|
||||
a real `AttributeError` if the field is ever renamed:
|
||||
|
||||
```python
|
||||
revision=getattr(server_args, "revision", None), # BAD
|
||||
revision=server_args.revision, # GOOD
|
||||
```
|
||||
(see `python/sglang/srt/managers/template_detection.py`)
|
||||
Reference in New Issue
Block a user