Skip to content

Align more error messages with CPython 3.14.5#7928

Merged
youknowone merged 17 commits into
RustPython:mainfrom
ShaharNaveh:ruff-error-msg
May 19, 2026
Merged

Align more error messages with CPython 3.14.5#7928
youknowone merged 17 commits into
RustPython:mainfrom
ShaharNaveh:ruff-error-msg

Conversation

@ShaharNaveh
Copy link
Copy Markdown
Contributor

@ShaharNaveh ShaharNaveh commented May 19, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Improved syntax error messages: clearer wording, more accurate caret positioning and narrowing behavior, and broader coverage of parse/lex error cases (including tab/space inconsistencies).
    • Standardized wording for invalid nonlocal declarations at module/top level.
  • Refactor

    • Internal restructuring of syntax-error analysis and construction for simpler, more maintainable handling without changing user-facing behavior.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Refactors VM syntax-error construction: adds ruff_python_parser imports, updates macro-generated exception helper return types to fully-qualified builtins refs, introduces SyntaxErrorInfo to derive messages and caret behavior from parser errors, integrates it into new_syntax_error_maybe_incomplete; also updates a module-level nonlocal error message in the symbol table.

Changes

Syntax Error Handling Refactor

Layer / File(s) Summary
Exception helper return type and import setup
crates/vm/src/vm/vm_new.rs
Added ruff_python_parser error-type imports and removed duplicate imports. Updated define_exception_fn! macro-generated VirtualMachine::new_* helpers to return fully-qualified $crate::builtins::PyBaseExceptionRef.
SyntaxErrorInfo struct and error analysis logic
crates/vm/src/vm/vm_new.rs
Introduced internal SyntaxErrorInfo { msg, narrow_caret } and implemented analyze_compile_error mapping ParseErrorType/LexicalErrorType and string-matched OtherError cases to messages and caret-narrowing decisions.
new_syntax_error_maybe_incomplete refactoring
crates/vm/src/vm/vm_new.rs
Refactored to compute statement via source.map(...), derive (msg, narrow_caret) via SyntaxErrorInfo, apply a tab/space inconsistency message override when applicable, and construct the final syntax_error using the derived message.

SymbolTable Message Update

Layer / File(s) Summary
Nonlocal module-level error message
crates/codegen/src/symboltable.rs
Replaced the previous formatted module-level nonlocal error with a fixed message: "nonlocal declaration not allowed at module level".

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

I nibble tokens, parse the hay,
Map errors to words so they can say,
I narrow the caret where faults play,
Tidy messages hopped in a row,
The VM leaps forward — onward we go. 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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 objective of the pull request—aligning error messages with CPython 3.14.5, which is directly reflected in the changes to syntax error handling and the nonlocal declaration error message.
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

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

📦 Library Dependencies

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

[x] test: cpython/Lib/test/test_positional_only_arg.py

dependencies:

dependent tests: (no tests depend on positional_only_arg)

[ ] test: cpython/Lib/test/test_metaclass.py (TODO: 4)

dependencies:

dependent tests: (no tests depend on metaclass)

[ ] test: cpython/Lib/test/test_syntax.py (TODO: 305)

dependencies:

dependent tests: (no tests depend on syntax)

[x] test: cpython/Lib/test/test_named_expressions.py (TODO: 8)

dependencies:

dependent tests: (no tests depend on named_expressions)

Legend:

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/vm/src/vm/vm_new.rs (1)

661-661: 💤 Low value

Use and_then to avoid nested Option<Option<String>>.

source.map(...) with get_statement returning Option<String> creates Option<Option<String>>. While this works due to how ToPyObject is implemented for nested Options, using and_then is more idiomatic and flattens to Option<String>.

♻️ Suggested refactor
-        let statement = source.map(|src| get_statement(src, error.location()));
+        let statement = source.and_then(|src| get_statement(src, error.location()));
🤖 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 `@crates/vm/src/vm/vm_new.rs` at line 661, The current assignment let statement
= source.map(|src| get_statement(src, error.location())); produces an
Option<Option<String>> because get_statement returns Option<String>; change the
call to use and_then (i.e. source.and_then(|src| get_statement(src,
error.location()))) so the nested Option is flattened to Option<String>; update
the expression where statement is defined in vm_new.rs and keep the same
parameters (src and error.location()) when switching to and_then.
🤖 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.

Nitpick comments:
In `@crates/vm/src/vm/vm_new.rs`:
- Line 661: The current assignment let statement = source.map(|src|
get_statement(src, error.location())); produces an Option<Option<String>>
because get_statement returns Option<String>; change the call to use and_then
(i.e. source.and_then(|src| get_statement(src, error.location()))) so the nested
Option is flattened to Option<String>; update the expression where statement is
defined in vm_new.rs and keep the same parameters (src and error.location())
when switching to and_then.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 86bf8420-246f-487c-830a-d0e3819ae0c7

📥 Commits

Reviewing files that changed from the base of the PR and between 62b081b and 3ea23c9.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_syntax.py is excluded by !Lib/**
📒 Files selected for processing (1)
  • crates/vm/src/vm/vm_new.rs

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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 `@crates/vm/src/vm/vm_new.rs`:
- Around line 668-679: The cfg_select! block creates syntax_error_info immutably
in the default arm but the code later calls syntax_error_info.with_msg(&mut
self), causing a mutability mismatch; fix it by allocating a single mutable
binding: create let mut syntax_error_info = SyntaxErrorInfo::new(msg, false)
outside or before the cfg branch, then inside only run the parser-only behavior
under #[cfg(feature = "parser")] (call
syntax_error_info.analyze_compile_error(error) there) so the binding is always
mutable for later calls to syntax_error_info.with_msg; update the cfg_select! /
cfg attributes accordingly around analyze_compile_error to avoid duplicating the
allocation.
- Around line 100-105: Update the error strings for the lexical parse errors to
match CPython wording: in the ParseErrorType::Lexical arm handling
LexicalErrorType::LineContinuationError change the message to "unexpected
character after line continuation character" and in the arm for
LexicalErrorType::UnclosedStringError change the message to "unterminated string
literal" so the variants ParseErrorType::Lexical and
LexicalErrorType::UnclosedStringError produce exact CPython messages.
- Around line 115-132: Three parser error messages use the word "argument" but
CPython 3.14 uses "parameter"; update those message strings to use "parameter"
instead. Locate the ParseErrorType variants emitting these messages—specifically
ParseErrorType::DuplicateKeywordArgumentError (the "keyword argument repeated"
message) and ParseErrorType::VarParameterWithDefault (the "var-positional
argument cannot have default value" message)—and change their formatted/static
strings to use "parameter" (e.g., "keyword parameter repeated", "var-positional
parameter cannot have default value"); also update any other ParseErrorType
message in the same area that contains "argument" to use "parameter" for
consistency. Ensure only the wording changes, preserving formatting/argument
interpolation (e.g., {arg_name}) and tests.
🪄 Autofix (Beta)

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

Run ID: 0e9e28c2-1a3a-41d1-b684-53e2342a46da

📥 Commits

Reviewing files that changed from the base of the PR and between 3ea23c9 and bfec825.

📒 Files selected for processing (1)
  • crates/vm/src/vm/vm_new.rs

Comment thread crates/vm/src/vm/vm_new.rs Outdated
Comment on lines +115 to +132
ParseErrorType::ExpectedKeywordParam => "named arguments must follow bare *".into(),

ParseErrorType::EmptyImportNames => "Expected one or more names after 'import'".into(),

ParseErrorType::DuplicateKeywordArgumentError(arg_name) => {
format!("keyword argument repeated: {arg_name}")
}

ParseErrorType::UnparenthesizedGeneratorExpression => {
"Generator expression must be parenthesized".into()
}

ParseErrorType::NonDefaultParamAfterDefaultParam => {
"parameter without a default follows parameter with a default".into()
}

ParseErrorType::VarParameterWithDefault => {
"var-positional argument cannot have default value".into()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cd crates/vm/src/vm && wc -l vm_new.rs && head -150 vm_new.rs | tail -50

Repository: RustPython/RustPython

Length of output: 2004


🏁 Script executed:

cd crates/vm/src/vm && sed -n '115,132p' vm_new.rs

Repository: RustPython/RustPython

Length of output: 867


🏁 Script executed:

cd crates/vm/src/vm && sed -n '175,180p' vm_new.rs

Repository: RustPython/RustPython

Length of output: 335


🌐 Web query:

CPython 3.14 "named parameters must follow bare *" syntax error message

💡 Result:

In CPython, the SyntaxError message “named parameters must follow bare ” is raised when you put a bare asterisk () in a function parameter list in a way that does not allow any following keyword-only parameters. In other words, the grammar requires that “” be followed by keyword-only parameter definitions (e.g., “def f(, kwonly):...”); if “” is the last token (no keyword-only parameters after it), CPython reports this syntax error. Example that triggers it: >>> def f():... pass... SyntaxError: named arguments must follow bare * [1][2] Why this happens: a bare “” marks the transition to keyword-only parameters, so “” must be immediately followed by at least one keyword-only parameter (or otherwise the signature is incomplete/invalid). [1][2] Notes: - The error text you cited can appear in tooling with slightly different wording (“named arguments” vs “named parameters”), but the underlying CPython syntax rule is the same: after a bare “*”, you must have keyword-only parameters. [3][1] If you want, paste the exact CPython 3.14 source code snippet (and how you’re running it—REPL/file) and I can identify the specific invalid signature form that produces the error in that case.

Citations:


🌐 Web query:

CPython "var-positional parameter cannot have default value" error message

💡 Result:

In CPython/Python, you cannot give a default value to a var-positional parameter (*args). What you likely ran into is the generic “non-default/required parameter follows default” class of syntax checks: required parameters (those without defaults) must not appear after parameters that have defaults, and the *args position determines which parameters are “required”. In particular, a parameter list like def f(a=1, *args, b): ... is rejected because b is required but appears after a defaulted parameter, and the parser surfaces it as the “parameter without a default follows parameter with a default” SyntaxError [1]. A related CPython issue also discusses how this error applies to keyword-only parameters and how signatures involving *args can appear confusing in terms of whether a later parameter is treated as positional or keyword-only [2]. If your concrete code snippet is something like def func(*args=...): then the correct mental model is simply: the *args parameter is always the “collect remaining positional arguments” slot, so it’s not legal syntax to attach a default initializer to it (unlike normal named positional-or-keyword parameters).

Citations:


🌐 Web query:

CPython "at least one parameter must precede /" syntax error

💡 Result:

The SyntaxError text “at least one parameter must precede /” is raised when you use the positional-only separator “/” in a function parameter list without any parameters before it (i.e., an invalid signature like def f(/,...):...). Python’s syntax rule for positional-only parameters uses “/” to separate positional-only parameters (to the left) from the rest (to the right) [1][2]. PEP 570 defines that all parameters before “/” are positional-only, and that “/” is the delimiter between the left-hand parameter group and following parameters [1]. Therefore, the delimiter cannot appear as the very first element of the parameter list; at least one parameter must appear on the left side of “/”. Evidence in the official docs: - The tutorial section explains that positional-only parameters are placed before a “/”, and that “/” logically separates them from the rest of the parameters [2]. - PEP 570 similarly states that parameters left of “/” are treated as positional-only, implying “/” can only function as a separator after that left-hand parameter group [1]. If you share the exact snippet you’re running (including the full def statement), I can pinpoint precisely which part triggers the error.

Citations:


🌐 Web query:

CPython 3.14 test_syntax.py "named parameters" "var-positional parameter"

💡 Result:

In CPython’s syntax tests, a “var-positional” parameter is the *name part of a function signature. CPython rejects giving it a default value: SyntaxError: var-positional parameter cannot have default value is asserted in Lib/test/test_syntax.py (e.g. def foo(a,*b=3,c): ...). [1] This is consistent with how the grammar defines *args-style parameters: a var-positional parameter collects an arbitrary number of positional arguments, so it cannot simultaneously represent a single optional value via a default. The syntax test explicitly treats *b=3 (and also lambda a,*b=3,c: ...) as a syntax error. [1] Notes on “named parameters” (keyword arguments): what you can name in calls is separate from whether you may put a default on the *args parameter in the function definition. The test coverage for keyword argument behavior is different from the parser rule that disallows defaults on var-positional parameters. [1] Authoritative source inspected: CPython’s own test file Lib/test/test_syntax.py, which includes the exact SyntaxError cases for *b=3 / var-positional parameter cannot have default value. [1]

Citations:


🌐 Web query:

site:github.com cpython "named parameters must follow bare" 3.14

💡 Result:

In CPython 3.14, “named parameters must follow bare *” refers to a syntax rule for function parameter lists: once you include a bare * (meaning “start of keyword-only parameters”), any following parameters that are allowed after that * must be keyword-only parameters, and those keyword-only parameters must appear in the form “name[=default]” (i.e., named parameters). If you put something other than a keyword-only named parameter after the bare *, the parser raises a SyntaxError with that message. This message is part of CPython’s general validation of parameter lists around the * / keyword-only boundary (PEP 3102 describes the semantics of keyword-only parameters introduced via the bare * marker). [1] Relevant CPython issues discuss adjacent wording and related parser constraints around this exact error message: - CPython issue reports an error message specifically about “Named parameters must follow bare *” when defining a signature like def func(*): ... and notes the message should be corrected from “arguments” to “parameters” (mypy reproduces the CPython-derived message text). [2] - CPython issues discuss the accuracy and phrasing of parameter-related SyntaxError messages in this area (e.g., “non-default argument follows default argument” vs “parameter”). These indicate that CPython has been actively refining the precise text of such syntax errors, including those near the * / keyword-only syntax. [3][4] If you want, paste the exact function definition you’re compiling/running and I can tell you which specific token after * triggers the “must follow bare *” SyntaxError in CPython 3.14.

Citations:


🌐 Web query:

site:raw.githubusercontent.com/python/cpython "named parameters must follow bare" test_syntax

💡 Result:

I couldn’t find any authoritative technical source that actually contains the specific phrase “named parameters must follow bare test_syntax”. The web results I retrieved look unrelated (for example, Python docs about doctest and a sqlite3 note about named placeholders), and none describe a rule with that wording. If you meant a common Python syntax error rule, the relevant one is usually phrased as: positional/non-default arguments must come before default arguments (e.g., you can get “SyntaxError: non-default argument follows default argument”). However, that’s not the same as “named parameters must follow bare … test_syntax”, and I did not find a matching source for your exact query. Please paste the exact code snippet and the exact error message (and the language/tool name), and I’ll re-research specifically for that scenario.

Citations:


🌐 Web query:

site:raw.githubusercontent.com/python/cpython "at least one parameter must precede" test_syntax

💡 Result:

The phrase "at least one parameter must precede" is not a standard error message in Python's core libraries or common testing frameworks like pytest. Based on a search of Python's source code and documentation, this specific string does not appear as a defined error message. It is possible that the phrase is: 1. A misremembered or slightly altered version of a different error message (e.g., related to argument parsing or function signatures). 2. A specific error message from a third-party library or a custom application not indexed in general technical documentation. 3. A misunderstanding of a technical requirement, such as the rule in Python's typing module where a Union must contain at least one type [1], or requirements for positional arguments in functions [2]. If you are encountering this error in a specific context, please check the stack trace to identify the library or module raising the exception, as it is likely specific to that codebase.

Citations:


🌐 Web query:

CPython github.com/python/cpython "named parameters must follow bare *" OR "var-positional parameter cannot have default value" OR "at least one parameter must precede"

💡 Result:

In Python, function parameter syntax is strictly governed by rules designed to ensure unambiguous argument binding. The error messages you referenced correspond to specific violations of these rules: 1. Named parameters must follow bare *: This error occurs when you attempt to define a parameter without a default value after a bare * (which denotes the start of keyword-only parameters) without providing a default value, or more generally, when the structure of keyword-only arguments is violated [1][2]. A bare * indicates that all subsequent parameters must be passed as keyword arguments [3]. 2. Var-positional parameter cannot have default value: A var-positional parameter (defined as *args) collects any number of positional arguments into a tuple [4]. Because it consumes all remaining positional arguments, it cannot have a default value; assigning one would be syntactically meaningless as there is no scenario where the parameter would be "missing" to trigger the default [4]. 3. At least one parameter must precede: This error typically arises when using the positional-only parameter separator (/). The / must be preceded by at least one positional-only parameter [5]. Similarly, if you were to use other markers incorrectly, the parser requires them to be placed in a valid context relative to other parameters [6]. In summary, Python requires that: - Positional-only parameters (before /) must be defined first [3][7]. - Positional-or-keyword parameters follow [3][7]. - Var-positional parameters (*args) collect remaining positional arguments [4]. - Keyword-only parameters (after * or *args) must be passed by name [1][3]. - Var-keyword parameters (kwargs) collect remaining keyword arguments [8]. Violating the order of these categories or attempting to assign defaults to variadic parameters results in a SyntaxError during the parsing of the function definition [4][9][5].

Citations:


🌐 Web query:

CPython compile.c "named parameters must follow" OR "named arguments must follow"

💡 Result:

In CPython the message is "named arguments must follow bare *" (not “named parameters must follow …”), and it’s not emitted from compile.c.

Where it lives / how to find it:

  • Modern CPython (PEG parser / pegen): the error text is attached to an invalid-grammar production in Grammar/python.gram, e.g. invalid_star_etc / invalid_lambda_star_etc which raises RAISE_SYNTAX_ERROR("named arguments must follow bare *"). (git.joshthomas.dev)
  • Older CPython (pre-PEG era): the same string appears in Python/ast.c, where the AST builder raises ast_error(..., "named arguments must follow bare *") when * appears in a parameter list without any following keyword-only parameters. (chromium.googlesource.com)

Background/history: this exact SyntaxError wording dates back to the introduction of keyword-only parameters (PEP 3102 era); see the historical tracker discussion (migrated to GitHub) showing the same message. (bugs.python.org)

Citations:


🌐 Web query:

CPython grammar "var-positional parameter" "default value" error message

💡 Result:

In CPython grammar, the syntax for a var-positional parameter is “” followed by a parameter name, optionally with a type annotation, and there is no provision for a default value for that parameter. The grammar rule for star_etc only allows “'' param_no_default …” or “'' param_no_default_star_annotation …” (and other forms like ','), i.e., no “'=' expression” is allowed on that branch. [1] When you write code like def f(*args='Hello'): ..., CPython raises a SyntaxError with the message “var-positional argument cannot have default value”. This is documented in a CPython issue that includes the exact error text. [2] So the key points are: 1) Grammar: *args (var-positional) is defined without default (no “= expression” for that form). [1] 2) Error: trying to attach a default to *args yields “SyntaxError: var-positional argument cannot have default value”. [2] If you need the exact grammar section name in the official docs: the var-positional part appears under star_etc in the full grammar specification. [1]

Citations:


🌐 Web query:

CPython 3.14 syntax error messages "parameter" changes from "argument"

💡 Result:

In CPython 3.14, there is (or was) a parser error-message wording inconsistency around function definition “parameter lists”: some SyntaxError messages say “argument” when the formal construct is a “parameter”. A CPython issue documents the proposed/desired change to use “parameter” / “parameters” wording (e.g., “duplicate parameter …” and “named parameters …”) and notes that this kind of change is only made in the latest version to reduce breakage and may not be backported.[1] Specifically, the issue reports these current messages and proposed replacements: - It reports def func(param, param): ... raising SyntaxError: duplicate argument 'param' in function definition, and suggests it should be SyntaxError: duplicate parameter 'param' in function definition.[1] - It reports def func(param1, param2, *): ... raising SyntaxError: named arguments must follow bare *, and suggests it should be SyntaxError: named parameters must follow bare *.[1] So the answer to your query is: yes—CPython’s 3.14 SyntaxError strings for certain function-parameter-list problems have been observed/targeted for replacement of “argument” with “parameter”, but the CPython issue indicates the wording change is treated as a last-mile polish and may be limited to newer versions (not necessarily backported broadly).[1] (Your query mentions “today is 2026-05-19”; the issue is dated 2025 and discusses behavior around current development/later versions, not necessarily guaranteeing that every 3.14.x build already uses the new wording.)

Citations:


🌐 Web query:

CPython "at least one" "parameter" "precede" "/" positional-only

💡 Result:

In CPython/Python syntax, the forward-slash marker “/” for positional-only parameters must be preceded by at least one parameter (i.e., you cannot start a function signature with “/”). This is expressed by the rule that “All parameters left of the / are treated as positional-only”, and by the grammar/usage rule that “at least one argument must precede /” (and also that “/ may appear only once”). [1][2] Practical consequence: a signature like def f(/, x): is invalid, while def f(x, /): is valid. [1][2]

Citations:


Align error messages with CPython 3.14 by using parameter instead of argument.

CPython 3.14 has updated these error messages to use "parameter" wording for consistency. Update the three occurrences across lines 115–132 and 175–180.

Suggested changes
-            ParseErrorType::ExpectedKeywordParam => "named arguments must follow bare *".into(),
+            ParseErrorType::ExpectedKeywordParam => "named parameters must follow bare *".into(),
-                "var-positional argument cannot have default value".into()
+                "var-positional parameter cannot have default value".into()
-                "at least one argument must precede /".into()
+                "at least one parameter must precede /".into()
🤖 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 `@crates/vm/src/vm/vm_new.rs` around lines 115 - 132, Three parser error
messages use the word "argument" but CPython 3.14 uses "parameter"; update those
message strings to use "parameter" instead. Locate the ParseErrorType variants
emitting these messages—specifically
ParseErrorType::DuplicateKeywordArgumentError (the "keyword argument repeated"
message) and ParseErrorType::VarParameterWithDefault (the "var-positional
argument cannot have default value" message)—and change their formatted/static
strings to use "parameter" (e.g., "keyword parameter repeated", "var-positional
parameter cannot have default value"); also update any other ParseErrorType
message in the same area that contains "argument" to use "parameter" for
consistency. Ensure only the wording changes, preserving formatting/argument
interpolation (e.g., {arg_name}) and tests.

Comment on lines +668 to +679
cfg_select! {
feature = "parser" => {
let mut syntax_error_info = SyntaxErrorInfo::new(msg, false);
syntax_error_info.analyze_compile_error(error);
}
#[cfg(feature = "parser")]
crate::compiler::CompileError::Parse(rustpython_compiler::ParseError {
error: ruff_python_parser::ParseErrorType::OtherError(s),
..
}) => {
if s.starts_with("Expected an identifier, but found a keyword") {
msg = "invalid syntax".to_owned();
}
else if s.eq_ignore_ascii_case("positional patterns cannot follow keyword patterns") {
msg = "positional patterns follow keyword patterns".to_owned();
}
_ => {
let syntax_error_info = SyntaxErrorInfo::new(msg, false);
}
_ => {}
}
};

if syntax_error_type.is(self.ctx.exceptions.tab_error) {
msg = "inconsistent use of tabs and spaces in indentation".to_owned();
syntax_error_info.with_msg("inconsistent use of tabs and spaces in indentation");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix the mutability mismatch in syntax_error_info binding.

The _ cfg arm binds syntax_error_info immutably (line 675), but line 682 calls with_msg(&mut self) unconditionally. Extract the common SyntaxErrorInfo::new(msg, false) allocation and use #[cfg(feature = "parser")] for the conditional analyze_compile_error call:

Suggested fix
-        cfg_select! {
-            feature = "parser" => {
-                let mut syntax_error_info = SyntaxErrorInfo::new(msg, false);
-                syntax_error_info.analyze_compile_error(error);
-            }
-            _ => {
-                let syntax_error_info = SyntaxErrorInfo::new(msg, false);
-            }
-        };
+        let mut syntax_error_info = SyntaxErrorInfo::new(msg, false);
+        #[cfg(feature = "parser")]
+        syntax_error_info.analyze_compile_error(error);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cfg_select! {
feature = "parser" => {
let mut syntax_error_info = SyntaxErrorInfo::new(msg, false);
syntax_error_info.analyze_compile_error(error);
}
#[cfg(feature = "parser")]
crate::compiler::CompileError::Parse(rustpython_compiler::ParseError {
error: ruff_python_parser::ParseErrorType::OtherError(s),
..
}) => {
if s.starts_with("Expected an identifier, but found a keyword") {
msg = "invalid syntax".to_owned();
}
else if s.eq_ignore_ascii_case("positional patterns cannot follow keyword patterns") {
msg = "positional patterns follow keyword patterns".to_owned();
}
_ => {
let syntax_error_info = SyntaxErrorInfo::new(msg, false);
}
_ => {}
}
};
if syntax_error_type.is(self.ctx.exceptions.tab_error) {
msg = "inconsistent use of tabs and spaces in indentation".to_owned();
syntax_error_info.with_msg("inconsistent use of tabs and spaces in indentation");
let mut syntax_error_info = SyntaxErrorInfo::new(msg, false);
#[cfg(feature = "parser")]
syntax_error_info.analyze_compile_error(error);
if syntax_error_type.is(self.ctx.exceptions.tab_error) {
syntax_error_info.with_msg("inconsistent use of tabs and spaces in indentation");
🤖 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 `@crates/vm/src/vm/vm_new.rs` around lines 668 - 679, The cfg_select! block
creates syntax_error_info immutably in the default arm but the code later calls
syntax_error_info.with_msg(&mut self), causing a mutability mismatch; fix it by
allocating a single mutable binding: create let mut syntax_error_info =
SyntaxErrorInfo::new(msg, false) outside or before the cfg branch, then inside
only run the parser-only behavior under #[cfg(feature = "parser")] (call
syntax_error_info.analyze_compile_error(error) there) so the binding is always
mutable for later calls to syntax_error_info.with_msg; update the cfg_select! /
cfg attributes accordingly around analyze_compile_error to avoid duplicating the
allocation.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
crates/vm/src/vm/vm_new.rs (1)

686-700: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Mutability mismatch causes compile error when parser feature is disabled.

The cfg_select! macro creates syntax_error_info as mutable in the parser branch (line 688) but immutable in the _ branch (line 692). Line 697 unconditionally calls syntax_error_info.with_msg(...), which requires &mut self. This produces a compile error when the parser feature is disabled.

Fix
-        cfg_select! {
-            feature = "parser" => {
-                let mut syntax_error_info = SyntaxErrorInfo::new(msg, false);
-                syntax_error_info.analyze_compile_error(error);
-            }
-            _ => {
-                let syntax_error_info = SyntaxErrorInfo::new(msg, false);
-            }
-        };
+        let mut syntax_error_info = SyntaxErrorInfo::new(msg, false);
+        #[cfg(feature = "parser")]
+        syntax_error_info.analyze_compile_error(error);
🤖 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 `@crates/vm/src/vm/vm_new.rs` around lines 686 - 700, The variable
syntax_error_info is mutable only in the "parser" cfg_select! branch but used
mutably afterward by syntax_error_info.with_msg; make its mutability consistent
by declaring a mutable SyntaxErrorInfo before or within both branches. Locate
the cfg_select! block that constructs SyntaxErrorInfo (and the
SyntaxErrorInfo::new calls) and ensure the binding is created as let mut
syntax_error_info = ... in every branch (or move a single let mut
syntax_error_info = ... above the cfg_select!), so
syntax_error_type.is(self.ctx.exceptions.tab_error) and the later
syntax_error_info.with_msg(...) compile without error.
🤖 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 `@crates/vm/src/vm/vm_new.rs`:
- Line 115: Update the error message text in the ParseErrorType match arms to
use "parameter" instead of "argument" to match CPython 3.14 wording: change the
string for ParseErrorType::ExpectedKeywordParam from "named arguments must
follow bare *" to "named parameters must follow bare *", and likewise replace
"var-positional argument cannot have default value" and "at least one argument
must precede /" with "var-positional parameter cannot have default value" and
"at least one parameter must precede /" in their corresponding ParseErrorType
arms in vm_new.rs.
- Line 679: The variable statement is being set with source.map(|src|
get_statement(src, error.location())) which yields Option<Option<String>>
because get_statement returns Option<String>; change this to use and_then (or
otherwise flatten) so statement becomes Option<String> (e.g.,
source.and_then(|src| get_statement(src, error.location()))) before assigning it
to the text attribute used later; update the expression that constructs
statement (and any downstream use expecting Option<String>) to remove the nested
Option.

---

Duplicate comments:
In `@crates/vm/src/vm/vm_new.rs`:
- Around line 686-700: The variable syntax_error_info is mutable only in the
"parser" cfg_select! branch but used mutably afterward by
syntax_error_info.with_msg; make its mutability consistent by declaring a
mutable SyntaxErrorInfo before or within both branches. Locate the cfg_select!
block that constructs SyntaxErrorInfo (and the SyntaxErrorInfo::new calls) and
ensure the binding is created as let mut syntax_error_info = ... in every branch
(or move a single let mut syntax_error_info = ... above the cfg_select!), so
syntax_error_type.is(self.ctx.exceptions.tab_error) and the later
syntax_error_info.with_msg(...) compile without error.
🪄 Autofix (Beta)

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

Run ID: ee49b272-06ab-4817-b575-6a7172652d84

📥 Commits

Reviewing files that changed from the base of the PR and between 666443d and 42005e7.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_syntax.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • crates/codegen/src/symboltable.rs
  • crates/vm/src/vm/vm_new.rs
✅ Files skipped from review due to trivial changes (1)
  • crates/codegen/src/symboltable.rs

"cannot use starred expression here".into()
}

ParseErrorType::ExpectedKeywordParam => "named arguments must follow bare *".into(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align error messages with CPython 3.14 by using "parameter" instead of "argument".

The PR title indicates alignment with CPython 3.14.5, but these messages still use "argument" where CPython 3.14 uses "parameter":

  • Line 115: "named arguments must follow bare *" → should be "named parameters must follow bare *"
  • Line 132: "var-positional argument cannot have default value" → should be "var-positional parameter cannot have default value"
  • Line 192: "at least one argument must precede /" → should be "at least one parameter must precede /"
Suggested fix
-            ParseErrorType::ExpectedKeywordParam => "named arguments must follow bare *".into(),
+            ParseErrorType::ExpectedKeywordParam => "named parameters must follow bare *".into(),
             ParseErrorType::VarParameterWithDefault => {
-                "var-positional argument cannot have default value".into()
+                "var-positional parameter cannot have default value".into()
             }
             ParseErrorType::OtherError(s)
                 if s.eq_ignore_ascii_case(
                     "position-only parameter separator not allowed as first parameter",
                 ) =>
             {
-                "at least one argument must precede /".into()
+                "at least one parameter must precede /".into()
             }

Also applies to: 131-132, 187-193

🤖 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 `@crates/vm/src/vm/vm_new.rs` at line 115, Update the error message text in the
ParseErrorType match arms to use "parameter" instead of "argument" to match
CPython 3.14 wording: change the string for ParseErrorType::ExpectedKeywordParam
from "named arguments must follow bare *" to "named parameters must follow bare
*", and likewise replace "var-positional argument cannot have default value" and
"at least one argument must precede /" with "var-positional parameter cannot
have default value" and "at least one parameter must precede /" in their
corresponding ParseErrorType arms in vm_new.rs.

Comment thread crates/vm/src/vm/vm_new.rs Outdated
@youknowone youknowone merged commit 0063a6d into RustPython:main May 19, 2026
27 checks passed
@youknowone
Copy link
Copy Markdown
Member

great!

@ShaharNaveh ShaharNaveh deleted the ruff-error-msg branch May 20, 2026 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants