Skip to content

Commit 013f061

Browse files
thomas-manginclaude
andcommitted
enforce: Prohibit all # type: comments in codebase
- Add rule to ESSENTIAL_PROTOCOLS.md prohibiting # type: comments - Rewrite check_type_ignores to fail on ANY # type: comment (zero tolerance) - Convert 13 # type: comments in check.py to Python 3.12+ inline annotations - Remove obsolete type_ignore_baseline.txt (no longer needed) - Update test description in test_everything All type annotations must use inline syntax: `x: int = value` Not comment syntax: `x = value # type: int` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 3508f5e commit 013f061

File tree

6 files changed

+62
-86
lines changed

6 files changed

+62
-86
lines changed

.claude/ESSENTIAL_PROTOCOLS.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ uv run pytest tests/unit/specific_test.py::test_function -v
256256
- Python 3.12+ syntax: `int | str` NOT `Union[int, str]`
257257
- NO code requiring mypy config changes
258258
- BGP APIs: keep `negotiated` parameter (stable API, unused OK)
259-
- Fix type errors at root cause, avoid `# type: ignore`
259+
- Fix type errors at root cause, NEVER use `# type:` comments
260260
- `cast()` is acceptable ONLY when preceded by runtime type check (isinstance/hasattr)
261261
- Example: `if isinstance(x, bool): return cast(T, x)`
262262
- Example: `return cast(int, value)` without check ❌
@@ -278,6 +278,9 @@ uv run pytest tests/unit/specific_test.py::test_function -v
278278
**Prohibited:**
279279
- `Union[int, str]` instead of `int | str`
280280
- Adding mypy suppressions or relaxed settings
281+
- **ANY mypy `# type` comments** - includes `# type: ignore`, `# type: ignore[code]`, `# type: X`
282+
- These are workarounds that hide problems instead of fixing them
283+
- Fix the actual type issue at its source instead
281284
- Removing `negotiated` parameter from pack/unpack methods
282285
- Introducing asyncio (custom reactor pattern used)
283286

@@ -307,7 +310,7 @@ When fixing bugs or refactoring code:
307310

308311
**DON'T:**
309312
- Use `cast()` to silence type errors without fixing the underlying issue
310-
- Use `# type: ignore` to suppress valid warnings
313+
- Use ANY `# type:` comments (ignore, ignore[code], type annotations)
311314
- Apply workarounds that leave technical debt
312315
- Choose minimal changes over correct changes
313316
- Avoid touching other files when the fix belongs there
@@ -556,4 +559,4 @@ Before ending ANY session where you worked on a plan:
556559

557560
---
558561

559-
**Updated:** 2025-12-15
562+
**Updated:** 2025-12-18

.claude/settings.local.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,5 +139,6 @@
139139
]
140140
}
141141
]
142-
}
142+
},
143+
"outputStyle": "ExaBGP Terse"
143144
}

qa/bin/check_type_ignores

Lines changed: 40 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
#!/usr/bin/env python3
22
"""
3-
Check for type:ignore regression in the codebase.
3+
Check for prohibited # type: comments in the codebase.
44
5-
This script counts type:ignore comments and fails if the count increases
6-
beyond the baseline. Run with --update to set a new baseline.
5+
This script scans for ANY # type: comments and fails if found.
6+
All type annotations must use Python 3.12+ inline syntax.
7+
8+
Prohibited patterns:
9+
# type: ignore
10+
# type: ignore[error-code]
11+
# type: SomeType (Python 2 style annotation)
712
813
Usage:
9-
./qa/bin/check_type_ignores # Check against baseline
10-
./qa/bin/check_type_ignores --update # Update baseline
11-
./qa/bin/check_type_ignores --show # Show all type:ignore locations
14+
./qa/bin/check_type_ignores # Check and fail if any found
15+
./qa/bin/check_type_ignores --show # Show all locations
1216
"""
1317

1418
from __future__ import annotations
@@ -17,32 +21,29 @@ import argparse
1721
import re
1822
import sys
1923
from pathlib import Path
20-
from typing import Dict, List, Tuple
2124

22-
# Baseline file location
23-
BASELINE_FILE = Path(__file__).parent.parent / 'type_ignore_baseline.txt'
2425
SRC_DIR = Path(__file__).parent.parent.parent / 'src' / 'exabgp'
2526

26-
# Pattern to match type: ignore comments
27-
TYPE_IGNORE_PATTERN = re.compile(r'#\s*type:\s*ignore')
27+
# Pattern to match ANY # type: comment (ignore, type annotations, etc.)
28+
TYPE_COMMENT_PATTERN = re.compile(r'#\s*type\s*:')
2829

2930

30-
def find_type_ignores() -> Dict[str, List[Tuple[int, str]]]:
31-
"""Find all type:ignore comments in the codebase."""
32-
results: Dict[str, List[Tuple[int, str]]] = {}
31+
def find_type_comments() -> dict[str, list[tuple[int, str]]]:
32+
"""Find all # type: comments in the codebase."""
33+
results: dict[str, list[tuple[int, str]]] = {}
3334

3435
for py_file in SRC_DIR.rglob('*.py'):
3536
# Skip vendored files
3637
if 'vendoring' in str(py_file):
3738
continue
3839

3940
rel_path = str(py_file.relative_to(SRC_DIR.parent.parent))
40-
matches: List[Tuple[int, str]] = []
41+
matches: list[tuple[int, str]] = []
4142

4243
try:
4344
with open(py_file, 'r', encoding='utf-8') as f:
4445
for line_num, line in enumerate(f, 1):
45-
if TYPE_IGNORE_PATTERN.search(line):
46+
if TYPE_COMMENT_PATTERN.search(line):
4647
matches.append((line_num, line.rstrip()))
4748
except Exception as e:
4849
print(f'Error reading {py_file}: {e}', file=sys.stderr)
@@ -54,31 +55,13 @@ def find_type_ignores() -> Dict[str, List[Tuple[int, str]]]:
5455
return results
5556

5657

57-
def count_ignores(results: Dict[str, List[Tuple[int, str]]]) -> int:
58-
"""Count total number of type:ignore comments."""
58+
def count_comments(results: dict[str, list[tuple[int, str]]]) -> int:
59+
"""Count total number of # type: comments."""
5960
return sum(len(matches) for matches in results.values())
6061

6162

62-
def read_baseline() -> int:
63-
"""Read the baseline count from file."""
64-
if not BASELINE_FILE.exists():
65-
return -1
66-
try:
67-
with open(BASELINE_FILE, 'r') as f:
68-
return int(f.read().strip())
69-
except (ValueError, IOError):
70-
return -1
71-
72-
73-
def write_baseline(count: int) -> None:
74-
"""Write the baseline count to file."""
75-
with open(BASELINE_FILE, 'w') as f:
76-
f.write(f'{count}\n')
77-
print(f'Baseline updated: {count} type:ignore comments')
78-
79-
80-
def show_ignores(results: Dict[str, List[Tuple[int, str]]]) -> None:
81-
"""Display all type:ignore locations."""
63+
def show_comments(results: dict[str, list[tuple[int, str]]]) -> None:
64+
"""Display all # type: comment locations."""
8265
for filepath in sorted(results.keys()):
8366
matches = results[filepath]
8467
print(f'\n{filepath}:')
@@ -87,44 +70,34 @@ def show_ignores(results: Dict[str, List[Tuple[int, str]]]) -> None:
8770

8871

8972
def main() -> int:
90-
parser = argparse.ArgumentParser(description='Check type:ignore regression')
91-
parser.add_argument('--update', action='store_true', help='Update baseline')
73+
parser = argparse.ArgumentParser(description='Check for prohibited # type: comments')
9274
parser.add_argument('--show', action='store_true', help='Show all locations')
9375
args = parser.parse_args()
9476

95-
results = find_type_ignores()
96-
current_count = count_ignores(results)
77+
results = find_type_comments()
78+
current_count = count_comments(results)
9779

9880
if args.show:
99-
show_ignores(results)
100-
print(f'\nTotal: {current_count} type:ignore comments in {len(results)} files')
101-
return 0
102-
103-
if args.update:
104-
write_baseline(current_count)
81+
if results:
82+
show_comments(results)
83+
print(f'\nTotal: {current_count} # type: comments in {len(results)} files')
84+
else:
85+
print('No # type: comments found')
10586
return 0
10687

107-
# Check mode
108-
baseline = read_baseline()
109-
110-
if baseline < 0:
111-
print(f'No baseline found. Current count: {current_count}')
112-
print('Run with --update to create baseline')
113-
return 1
114-
115-
print(f'type:ignore count: {current_count} (baseline: {baseline})')
116-
117-
if current_count > baseline:
118-
print(f'FAIL: {current_count - baseline} new type:ignore comments added')
88+
# Check mode - fail if ANY # type: comments found
89+
if current_count > 0:
90+
print(f'FAIL: Found {current_count} prohibited # type: comments in {len(results)} files')
91+
print()
92+
print('All # type: comments are prohibited. Use Python 3.12+ inline annotations instead:')
93+
print(' Bad: x = value # type: int')
94+
print(' Good: x: int = value')
95+
print()
11996
print('Run with --show to see all locations')
12097
return 1
121-
elif current_count < baseline:
122-
print(f'IMPROVEMENT: {baseline - current_count} type:ignore comments removed')
123-
print('Consider running --update to lower the baseline')
124-
return 0
125-
else:
126-
print('OK: No regression')
127-
return 0
98+
99+
print('OK: No # type: comments found')
100+
return 0
128101

129102

130103
if __name__ == '__main__':

qa/bin/test_everything

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ TEST_DESCRIPTIONS: dict[str, str] = {
180180
'encoding': 'Functional encoding tests',
181181
'api': 'Functional API tests',
182182
'cli': 'Functional CLI tests',
183-
'type-ignore': 'Type ignore regression check',
183+
'type-ignore': 'Prohibited # type: comments check',
184184
}
185185

186186
# Ordered list of test names

qa/type_ignore_baseline.txt

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/exabgp/configuration/check.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,12 @@ def check_generation(neighbors: dict[str, Neighbor]) -> bool:
129129
)
130130
pack1 = packed[0]
131131

132-
_packed = packed # type: list[bytes]
133-
_pack1 = pack1 # type: bytes
132+
_packed: list[bytes] = packed
133+
_pack1: bytes = pack1
134134
log.debug(lazymsg('parsed route requires {count} updates', count=len(_packed)), 'parser')
135135
log.debug(lazymsg('update size is {size}', size=len(_pack1)), 'parser')
136136

137-
_str1 = str1 # type: str
137+
_str1: str = str1
138138
log.debug(lazymsg('parsed route {route}', route=_str1), 'parser')
139139
log.debug(lazymsg('parsed hex {hex}', hex=od(_pack1)), 'parser')
140140

@@ -159,8 +159,8 @@ def check_generation(neighbors: dict[str, Neighbor]) -> bool:
159159
str2 = route2.extensive()
160160
pack2 = list(UpdateCollection([routed], [], update.attributes).messages(negotiated_out))[0]
161161

162-
_str2 = str2 # type: str
163-
_pack2 = pack2 # type: bytes
162+
_str2: str = str2
163+
_pack2: bytes = pack2
164164
log.debug(lazymsg('recoded route {route}', route=_str2), 'parser')
165165
log.debug(lazymsg('recoded hex {hex}', hex=od(_pack2)), 'parser')
166166

@@ -199,8 +199,8 @@ def check_generation(neighbors: dict[str, Neighbor]) -> bool:
199199
skip = True
200200
else:
201201
log.debug(lazymsg('check.strings.different'), 'parser')
202-
_str1r = str1r # type: str
203-
_str2r = str2r # type: str
202+
_str1r: str = str1r
203+
_str2r: str = str2r
204204
log.debug(lazymsg('[{s}]', s=_str1r), 'parser')
205205
log.debug(lazymsg('[{s}]', s=_str2r), 'parser')
206206
return False
@@ -211,8 +211,8 @@ def check_generation(neighbors: dict[str, Neighbor]) -> bool:
211211
log.debug(lazymsg('check.encoding.skip reason=non_transitive_attributes'), 'parser')
212212
elif pack1 != pack2:
213213
log.debug(lazymsg('check.encoding.different'), 'parser')
214-
_pack1_cmp = pack1 # type: bytes
215-
_pack2_cmp = pack2 # type: bytes
214+
_pack1_cmp: bytes = pack1
215+
_pack2_cmp: bytes = pack2
216216
log.debug(lazymsg('[{hex}]', hex=od(_pack1_cmp)), 'parser')
217217
log.debug(lazymsg('[{hex}]', hex=od(_pack2_cmp)), 'parser')
218218
return False
@@ -309,7 +309,7 @@ def _make_nlri(neighbor: Neighbor, routes: str) -> list[NLRI]:
309309
nlris: list[NLRI] = []
310310
try:
311311
while announced:
312-
_announced = announced # type: Buffer
312+
_announced: Buffer = announced
313313
log.debug(lazymsg('parsing NLRI {announced}', announced=_announced), 'parser')
314314
nlri_parsed, announced = NLRI.unpack_nlri(afi, safi, announced, Action.ANNOUNCE, addpath, negotiated_in)
315315
if nlri_parsed is not NLRI.INVALID:
@@ -318,7 +318,7 @@ def _make_nlri(neighbor: Neighbor, routes: str) -> list[NLRI]:
318318
log.error(lazymsg('nlri.parse.failed afi={a} safi={s}', a=afi, s=safi), 'parser')
319319
from exabgp.debug import string_exception
320320

321-
_exc_nlri = exc # type: BaseException
321+
_exc_nlri: BaseException = exc
322322
log.error(lazymsg('nlri.parse.error error={msg}', msg=string_exception(_exc_nlri)), 'parser')
323323
if getenv().debug.pdb:
324324
raise
@@ -400,7 +400,7 @@ def _make_update(neighbor: Neighbor, raw: bytes) -> UpdateCollection | None:
400400
if kind == BGP_MSG_UPDATE:
401401
log.debug(lazymsg('message.type type=update'), 'parser')
402402
else:
403-
_kind = kind # type: int
403+
_kind: int = kind
404404
log.debug(lazymsg('message.type.abort type={kind} expected=update', kind=_kind), 'parser')
405405
return None
406406
else:
@@ -577,6 +577,6 @@ def _get_dummy_negotiated() -> Negotiated:
577577

578578
def check_notification(raw: bytes) -> bool:
579579
notification = Notification.unpack_message(raw[18:], _get_dummy_negotiated())
580-
_notification = notification # type: Notification
580+
_notification: Notification = notification
581581
log.info(lazymsg('notification.decoded notification={notification}', notification=_notification), 'parser')
582582
return True

0 commit comments

Comments
 (0)