Fix map no iterables - #8478
Conversation
Assisted-by: Claude Code:claude-fable-5
Assisted-by: Claude Code:claude-fable-5
📝 WalkthroughWalkthrough
ChangesMap arity validation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@extra_tests/snippets/builtin_map.py`:
- Around line 27-33: Extend the no-iterables test around map construction to
also call map with the callable and strict=False as keyword arguments. Assert
that construction immediately raises TypeError with the same "map() must have at
least two arguments." message, preserving the existing positional-argument
assertion.
- Around line 30-32: Replace the `assert False` fallback in the map-construction
exception test with an explicit `AssertionError` raise, ensuring the test cannot
pass when `map()` does not raise. Preserve the `except TypeError as e` path and
explicitly validate that `str(e)` equals the required message.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: ddabcb4e-559c-4cd8-aaf6-7f53c853a1f4
📒 Files selected for processing (2)
crates/vm/src/builtins/map.rsextra_tests/snippets/builtin_map.py
| # test for no iterables | ||
| try: | ||
| map(lambda: 1) | ||
| assert False, "TypeError expected at map construction" | ||
| except TypeError as e: | ||
| assert str(e) == "map() must have at least two arguments." | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover the keyword construction path before merging.
This test only exercises positional construction. Add a case such as map(lambda: 1, strict=False) and verify the same construction-time TypeError and message. This ensures the named-argument path cannot bypass the arity check.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 30-30: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@extra_tests/snippets/builtin_map.py` around lines 27 - 33, Extend the
no-iterables test around map construction to also call map with the callable and
strict=False as keyword arguments. Assert that construction immediately raises
TypeError with the same "map() must have at least two arguments." message,
preserving the existing positional-argument assertion.
| assert False, "TypeError expected at map construction" | ||
| except TypeError as e: | ||
| assert str(e) == "map() must have at least two arguments." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '^\s*assert\b' extra_tests/snippets/builtin_map.pyRepository: RustPython/RustPython
Length of output: 485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- builtin_map.py lines 1-50 ---\n'
sed -n '1,50p' extra_tests/snippets/builtin_map.py | nl -ba
printf '\n--- assert usage in extra_tests/snippets/builtin_map.py (context) ---\n'
for line in 2 6 8 24 25 30 32 41; do
start=$(( line > 4 ? line-3 : 1 ))
end=$(( line+5 ))
[ "$start" -eq "$end" ] && end=$((end+1))
sed -n "${start},${end}p" extra_tests/snippets/builtin_map.py | nl -ba -v"$start"
printf '\n'
done
printf '\n--- ruff availability and B011 config availability ---\n'
command -v ruff || true
if command -v ruff >/dev/null 2>&1; then
ruff check extra_tests/snippets/builtin_map.py --select B011 || true
fi
printf '\n--- python optimized behavior for assert false / test assertion removal ---\n'
python3 - <<'PY'
import ast
src_normal = '''try:
x += 1
assert False, "TypeError expected at map construction"
except TypeError as e:
assert str(e) == "message"
'''
src_opt_only = '''try:
x += 1
assert False, "TypeError expected at map construction"
'''
mod_not_optimized = ast.parse(src_normal)
mod_optimized = ast.parse(src_opt_normal, optimize=2)
print("parsed_nodes_normal_function_call_count:",
sum(1 for _ in ast.walk(mod_normal) if isinstance(_, ast.Call) and
isinstance(getattr(getattr(_, "func", None), "attr", None), str) and _.func.attr == "assert"))
print("optimized_module_type:", mod_optimized.__module__)
def count_calls(node):
n=0
for child in ast.walk(node):
if isinstance(child, (ast.Assert, ast.Call)): n += 1
return n
print("normal_asserts:", sum(1 for node in ast.walk(mod_normal) if isinstance(node, ast.Assert)))
print("normal_calls:", sum(1 for node in ast.walk(mod_normal) if isinstance(node, ast.Call)))
print("optimized_asserts:", sum(1 for node in ast.walk(mod_optimized) if isinstance(node, ast.Assert)))
print("optimized_calls:", sum(1 for node in ast.walk(mod_optimized) if isinstance(node, ast.Call)))
PYRepository: RustPython/RustPython
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- builtin_map.py lines 1-50 ---'
sed -n '1,50p' extra_tests/snippets/builtin_map.py | nl -ba
printf '%s\n' '--- assert usage in extra_tests/snippets/builtin_map.py (context) ---'
for line in 2 6 8 24 25 30 32 41; do
start=$(( line > 4 ? line-3 : 1 ))
end=$(( line+5 ))
[ "$start" -eq "$end" ] && end=$((end+1))
sed -n "${start},${end}p" extra_tests/snippets/builtin_map.py | nl -ba -v"$start"
printf '%s\n' ''
done
printf '%s\n' '--- ruff availability and B011 config availability ---'
command -v ruff || true
if command -v ruff >/dev/null 2>&衣; then
ruff check extra_tests/snippets/builtin_map.py --select B011 || true
fi
printf '%s\n' '--- python optimized behavior for assert false / test assertion removal ---'
python3 - <<'PY'
import ast
src_normal = '''try:
x += 1
assert False, "TypeError expected at map construction"
except TypeError as e:
assert str(e) == "message"
'''
src_opt_only = '''try:
x += 1
assert False, "TypeError expected at map construction"
'''
mod_normal = ast.parse(src_normal)
mod_optimized = ast.parse(src_opt_only, optimize=2)
def count_nodes(node, cls):
return sum(1 for child in ast.walk(node) if isinstance(child, cls))
print("normal_asserts:", count_nodes(mod_normal, ast.Assert))
print("normal_calls:", count_nodes(mod_normal, ast.Call))
print("optimized_asserts:", count_nodes(mod_optimized, ast.Assert))
print("optimized_return_value:", mod_optimized.body[0].value.value)
PYRepository: RustPython/RustPython
Length of output: 233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- builtin_map.py lines 1-50 ---'
sed -n '1,50p' extra_tests/snippets/builtin_map.py
printf '%s\n' '--- assert usage in extra_tests/snippets/builtin_map.py (line numbers via grep -n) ---'
grep -n 'assert False' extra_tests/snippets/builtin_map.py || true
grep -n 'assert str(e)' extra_tests/snippets/builtin_map.py || true
printf '%s\n' '--- python optimized behavior for assert False ---'
python3 - <<'PY'
import ast
import dis
import io
import sys
src = '''try:
x += 1
assert False, "TypeError expected at map construction"
except TypeError as e:
pass
'''
# Python 2.3 removed assert statements at parse time when optimization is selected.
# The function return value is None because the function body after optimization has no instructions.
mod_optimized = ast.parse(src, optimize=2)
comp = compile(mod_optimized, "<optimized>", "exec")
dis.dis(comp)
print("optimized_return_opcodes:", [name for name in (instr.opname for instr in dis.get_instructions(comp)) if name == "RETURN_VALUE"])
src_no_assert_success = '''try:
x += 1
except TypeError as e:
pass
'''
mod_no_assert = ast.parse(src_no_assert_success, optimize=0)
comp2 = compile(mod_no_assert, "<no_assert>", "exec")
print("optimized_code_is_same_as_no_assert:")
print(comp.co_code == comp2.co_code)
print(dis.dis(comp2))
PYRepository: RustPython/RustPython
Length of output: 1368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- locate ruff config files ---'
git ls-files | grep -E '(^|/)(pyproject\.toml|ruff\.toml|\.ruff\.toml)$' || true
printf '%s\n' '--- search for ruff B011 settings ---'
if [ -f pyproject.toml ]; then git grep -n '\bB011\b\|assert\.' pyproject.toml || true; fi
printf '%s\n' '--- Python compile optimization via command-line flag ---'
python3 - <<'PY'
import dis
import pycompile
src = '''try:
x += 1
assert False, "TypeError expected at map construction"
except TypeError as e:
pass
'''
src_no_assert = '''try:
x += 1
except TypeError as e:
pass
'''
try:
print("pycompile.get_source_from_code exists:", hasattr(pycompile.get_source_from_code, '__call__'))
except Exception as exc:
print("pycompile_access_error:", type(exc).__name__, str(exc))
code = compile(src, '<no_opt>', 'exec')
code_opt = compile(src, '<opt>', 'exec', optimize=2)
print("normal_code_bytes:", code.co_code.hex())
print("optimized_code_bytes:", code_opt.co_code.hex())
print("optimized_is_same_as_no_assert:", code_opt.co_code == compile(src_no_assert, '<no_assert>', 'exec').co_code)
print("disopt:")
dis.dis(code_opt)
PYRepository: RustPython/RustPython
Length of output: 490
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- ruff.toml relevant sections ---'
cat ruff.toml
printf '%s\n' '--- Ruff B011 status if ruff is available ---'
if command -v ruff >/dev/null 2>&1; then
ruff check extra_tests/snippets/builtin_map.py --select B011 || true
command -v ruff
else
command -v ruff || true
fi
printf '%s\n' '--- Python compile optimization via command-line flag ---'
python3 - <<'PY'
import dis
import sys
src = '''try:
x += 1
assert False, "TypeError expected at map construction"
except TypeError as e:
pass
'''
src_no_assert = '''try:
x += 1
except TypeError as e:
pass
'''
code = compile(src, '<no_opt>', 'exec')
code_opt = compile(src, '<opt>', 'exec', optimize=2)
code_no_assert = compile(src_no_assert, '<no_assert>', 'exec')
print("normal_code_bytes:", code.co_code.hex())
print("optimized_code_bytes:", code_opt.co_code.hex())
print("optimized_is_same_as_no_assert:", code_opt.co_code == code_no_assert.co_code)
print("optimized_return_opcodes:", [instr.opname for instr in dis.get_instructions(code_opt) if instr.opname == "RETURN_VALUE"])
PYRepository: RustPython/RustPython
Length of output: 1728
Use explicit AssertionError control flow.
assert False raises a lint error, and python -O removes it, so this branch can pass without catching the constructor TypeError or comparing the required message. Raise AssertionError for the expected failure, then check str(e) == "map() must have at least two arguments." explicitly.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 30-30: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@extra_tests/snippets/builtin_map.py` around lines 30 - 32, Replace the
`assert False` fallback in the map-construction exception test with an explicit
`AssertionError` raise, ensuring the test cannot pass when `map()` does not
raise. Preserve the `except TypeError as e` path and explicitly validate that
`str(e)` equals the required message.
Sources: Coding guidelines, Linters/SAST tools
|
@william-goode Thanks for fixing this! Much appreciated. I also have fixes for some of my other AI-generated issues that I’ve manually reviewed. Would it be okay for me to open PRs for them? I’ve held off because of the AI policy. |
Hi @jseop-lim, Of course - apologies for scooping your issue. I'll leave the rest to you! Best, |
@william-goode No need to apologize at all. I really appreciate you taking an interest in the issue and contributing a fix. I’m still new to Rust, so help from others is very welcome. If you’re planning to work on any of the other issues, please feel free to do so! |
|
@william-goode Thank you for contributing! I will look in the patch. Please also check coderabbit's review. Feel free to decline it if it doesn't fit. @jseop-lim Hi, If you want to claim an issue for your task, please do it in advance in the issue, and do not ask it in patches later. You don't need to claim any issue to work on it, but insisting it in others' patch is a different problem. |
|
@youknowone Sorry about that, and thanks for clarifying. Understood. I’ll make sure to claim issues in advance if needed and won’t bring it up in others’ PRs. |
3 participants
map(f)with a single argument builds amapobject instead of raisingTypeError#8462Summary
This PR is to bring RustPython to CPython parity in the handling of maps constructed without iterables. CPython raises a TypeError if a map is constructed without an iterable. RustPython had no such check, so while constructing something like
map(lambda: 1)would succeed, iterating it would loop infinitely. Added a no-iterable check and an accompanying test.AI Usage
Code, commit messages, and this description are hand-written. Fable 5 was used for review.
Acknowledgements
Thank you @jseop-lim for raising the issue.
Summary by CodeRabbit
Bug Fixes
mapwithout iterables now correctly raises aTypeError.map()requires at least two arguments.Tests
mapconstruction.