chore: Script to find undocumented attributes #5987
3 issues
code-review: Found 3 issues (1 medium, 2 low)
Medium
Potential IndexError when accessing node.args without bounds check - `scripts/find_undocumented_set_data_keys.py:122-125`
The get_key_node() function accesses node.args[0] and node.args[1] without verifying that the args list has enough elements. If the script encounters any function call named set_data or set_data_normalized that has fewer positional arguments than expected (e.g., keyword-only arguments or a different function with the same name), this will raise an IndexError. While this may be unlikely in the current codebase, it makes the script fragile for future changes.
Low
New script lacks test coverage - `scripts/find_undocumented_set_data_keys.py:1`
This PR adds a new utility script with significant logic (AST parsing, namespace building, pattern matching) but no corresponding tests. The PR guidelines explicitly state to add tests to validate changes. Test coverage would help catch edge cases in the AST traversal and key resolution logic.
O(n) membership check on dict_values instead of O(1) set lookup - `scripts/find_undocumented_set_data_keys.py:236`
vars(ATTRIBUTE_NAMES).values() returns a dict_values object, not a set. The membership check resolved.value not in convention_keys on line 259 performs O(n) iteration for each resolved key, resulting in O(n*m) time complexity. Converting to a set would make lookups O(1).
Duration: 1m 47s · Tokens: 311.8k in / 4.4k out · Cost: $0.50 (+merge: $0.00, +fix_gate: $0.01)
Annotations
Check warning on line 125 in scripts/find_undocumented_set_data_keys.py
sentry-warden / warden: code-review
Potential IndexError when accessing node.args without bounds check
The `get_key_node()` function accesses `node.args[0]` and `node.args[1]` without verifying that the args list has enough elements. If the script encounters any function call named `set_data` or `set_data_normalized` that has fewer positional arguments than expected (e.g., keyword-only arguments or a different function with the same name), this will raise an `IndexError`. While this may be unlikely in the current codebase, it makes the script fragile for future changes.