Raise ValueError for None in required AST scalar fields

[changjoon-park]

Summary

When required scalar AST fields receive None (e.g. comprehension.target = None after manually constructing an AST), compile() raises mismatched errors compared to CPython:

Field	Current RustPython	CPython 3.14
alias.name	TypeError: Expected type 'str' but 'NoneType' found.	ValueError: field 'name' is required for alias
arg.arg	same TypeError	ValueError: field 'arg' is required for arg
comprehension.target / iter	ValueError: None disallowed in expression list	ValueError: field '<target/iter>' is required for comprehension
keyword.value	same generic ValueError	ValueError: field 'value' is required for keyword
match_case.pattern	TypeError: expected some sort of pattern, but got None	ValueError: field 'pattern' is required for match_case
withitem.context_expr	same generic ValueError	ValueError: field 'context_expr' is required for withitem
YieldFrom.value	same generic ValueError	ValueError: field 'value' is required for YieldFrom

These slip past the existing validate.rs semantic validator because Rust's non-Option field types (e.g. Alias.name: Identifier, Comprehension.target: Expr, MatchCase.pattern: Pattern) force Node::ast_from_object to fail at conversion time, before the validator pass can run. CPython's single-pass PyAST_Validate works because every AST attribute is a PyObject* and tolerates None until the validator decides; RustPython's strong typing forces this work to happen at conversion.

Fix

Add get_node_field_required next to the existing get_node_field / get_node_field_opt in crates/vm/src/stdlib/_ast.rs:

fn get_node_field_required(vm, obj, field, typ) -> PyResult {
    let value = get_node_field(vm, obj, field, typ)?;
    if vm.is_none(&value) {
        return Err(vm.new_value_error(format!("field '{field}' is required for {typ}")));
    }
    Ok(value)
}

Switch the eight call sites that read required scalar fields:

• alias.name, withitem.context_expr (other.rs)
• arg.arg, keyword.value (parameter.rs)
• comprehension.target, comprehension.iter, YieldFrom.value (expression.rs)
• match_case.pattern (pattern.rs)

Optional fields (read via get_node_field_opt: alias.asname, arg.annotation, Yield.value, Return.value, WithItem.optional_vars, ExceptHandler.type, etc.) and fields where None is legitimate (Constant.value representing the None literal) are unchanged.

Verification

Targeted probes (CPython 3.14.4 byte-identical)

$ ./target/release/rustpython /tmp/probe_none_checks.py
  alias.name: ValueError: field 'name' is required for alias
  arg.arg: ValueError: field 'arg' is required for arg
  comprehension.target: ValueError: field 'target' is required for comprehension
  comprehension.iter: ValueError: field 'iter' is required for comprehension
  keyword.value: ValueError: field 'value' is required for keyword
  match_case.pattern: ValueError: field 'pattern' is required for match_case
  withitem.context_expr: ValueError: field 'context_expr' is required for withitem

Identical to CPython 3.14.4 output for every case.

Test suites

$ ./target/release/rustpython -m unittest test.test_ast.test_ast
Ran 227 tests in 19s
OK (skipped=10, expected failures=36)

Was 38 expected failures; this PR unmasks test_none_checks and test_empty_yield_from.

$ ./target/release/rustpython -m unittest test.test_compile
Ran 185 tests in 4s
OK (skipped=50, expected failures=18)

No regressions.

Optional None paths still compile

Verified that legitimate None AST positions are unaffected:

• import x (alias.asname=None)
• def f(x): pass (arg.annotation=None, returns=None)
• def f(): yield (Yield.value=None)
• def f(): return (Return.value=None)
• with x: pass (WithItem.optional_vars=None)
• try: pass\nexcept: pass (ExceptHandler.type=None)
• Constant(value=None) (None literal)

All compile cleanly.

Pre-push

• cargo fmt --all --check clean
• cargo clippy -p rustpython-vm --all-targets -- -D warnings clean
• prek run --all-files — all 8 hooks pass

Scope

6 files, +35/-10 lines. Additive in spirit — no behaviour change on success paths.

Summary by CodeRabbit

• Bug Fixes
  • Enhanced AST field validation to enforce required fields during deserialization with improved CPython-compatible error messages.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 38352adc-c05d-44a7-b942-bb6887cf0986

📥 Commits

Reviewing files that changed from the base of the PR and between adafaf2 and d4903ba.

⛔ Files ignored due to path filters (1)

• Lib/test/test_ast/test_ast.py is excluded by !Lib/**

📒 Files selected for processing (5)

• crates/vm/src/stdlib/_ast.rs
• crates/vm/src/stdlib/_ast/expression.rs
• crates/vm/src/stdlib/_ast/other.rs
• crates/vm/src/stdlib/_ast/parameter.rs
• crates/vm/src/stdlib/_ast/pattern.rs

---

📝 Walkthrough

Walkthrough

The changes introduce a new validation helper get_node_field_required that enforces mandatory AST fields with stricter error handling, and apply it across multiple AST node deserialization implementations to require specific fields that were previously optional.

Changes

Cohort / File(s)	Summary
New required field validator crates/vm/src/stdlib/_ast.rs	Introduces get_node_field_required helper that validates mandatory AST fields, raising TypeError for missing attributes or ValueError for None values, complementing the optional get_node_field_opt.
Expression node validation crates/vm/src/stdlib/_ast/expression.rs	Updates ExprYieldFrom and Comprehension to require "value", "target", and "iter" fields via the new validator.
Other node validation crates/vm/src/stdlib/_ast/other.rs	Updates Alias and WithItem to require "name" and "context_expr" fields respectively.
Parameter and keyword validation crates/vm/src/stdlib/_ast/parameter.rs	Updates Parameter and Keyword to require "arg" and "value" fields respectively.
Pattern matching validation crates/vm/src/stdlib/_ast/pattern.rs	Updates MatchCase to require the "pattern" field during AST deserialization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

• ShaharNaveh
• youknowone

Poem

🐰 Through AST nodes we hop with care,
With required fields everywhere!
No missing values, none at all—
Each attribute heeds the call! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: introducing stricter validation for required AST scalar fields to raise ValueError when they are None, matching CPython behavior.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/ast.py
[x] lib: cpython/Lib/_ast_unparse.py
[x] test: cpython/Lib/test/test_unparse.py
[ ] test: cpython/Lib/test/test_type_comments.py (TODO: 15)

dependencies:

• ast

dependent tests: (136 tests)

• ast: test_ast test_builtin test_compile test_compiler_codegen test_dis test_fstring test_future_stmt test_site test_ssl test_type_comments test_ucn test_unparse
  • annotationlib: test_annotationlib test_functools test_inspect test_reprlib test_type_annotations test_type_params test_typing
    • dataclasses: test__colorize test_copy test_ctypes test_enum test_genericalias test_patma test_pprint test_pydoc test_regrtest test_zoneinfo
    • inspect: test_abc test_argparse test_asyncgen test_buffer test_code test_collections test_coroutines test_decimal test_generators test_grammar test_monitoring test_ntpath test_operator test_posixpath test_signal test_sqlite3 test_traceback test_types test_unittest test_yield_from test_zipimport
  • dbm.dumb: test_dbm_dumb
  • inspect:
    • bdb: test_bdb
    • cmd: test_cmd
    • importlib.metadata: test_importlib
    • pkgutil: test_pkgutil test_runpy
    • rlcompleter: test_rlcompleter
    • trace: test_trace
    • xmlrpc.server: test_docxmlrpc test_xmlrpc
  • pyclbr: test_pyclbr
  • traceback: test_asyncio test_code_module test_contextlib test_contextlib_async test_dictcomps test_exceptions test_http_cookiejar test_importlib test_iter test_listcomps test_pyexpat test_setcomps test_socket test_subprocess test_sys test_threadedtempfile test_threading test_unittest test_with
    • concurrent.futures.process: test_compileall test_concurrent_futures
    • http.cookiejar: test_urllib2
    • logging: test_asyncio test_hashlib test_logging test_support test_urllib2net
    • multiprocessing: test_asyncio test_concurrent_futures test_fcntl test_memoryview test_multiprocessing_main_handling test_re
    • py_compile: test_cmd_line_script test_importlib test_modulefinder test_py_compile
    • socketserver: test_imaplib test_socketserver test_wsgiref
    • threading: test_android test_asyncio test_bz2 test_concurrent_futures test_ctypes test_email test_fork1 test_frame test_ftplib test_gc test_httplib test_httpservers test_importlib test_io test_itertools test_largefile test_linecache test_opcache test_pathlib test_poll test_poplib test_queue test_robotparser test_sched test_smtplib test_super test_syslog test_termios test_threading_local test_time test_urllib2_localnet test_weakref test_winreg test_zstd
    • timeit: test_timeit

Legend:

• [+] path exists in CPython
• [x] up-to-date, [ ] outdated