8000 disallow __new__, __init__ by youknowone · Pull Request #6446 · RustPython/RustPython · GitHub
[go: up one dir, main page]
Skip to content

disallow __new__, __init__ #6446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 3 commits into from
Dec 22, 2025
Merged

disallow __new__, __init__ #6446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
364756e
disallow __new__, __init__
youknowone Dec 16, 2025
0d791f9
migrate to Initializer
youknowone Dec 16, 2025
78c23c5
apply review
youknowone Dec 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 74 additions & 51 deletions crates/derive-impl/src/pyclass.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ impl FromStr for AttrName {

#[derive(Default)]
struct ImplContext {
is_trait: bool,
attribute_items: ItemNursery,
method_items: MethodNursery,
getset_items: GetSetNursery,
Expand Down Expand Up @@ -232,7 +233,10 @@ pub(crate) fn impl_pyclass_impl(attr: PunctuatedNestedMeta, item: Item) -> Resul
}
}
Item::Trait(mut trai) => {
let mut context = ImplContext::default();
let mut context = ImplContext {
is_trait: true,
..Default::default()
};
let mut has_extend_slots = false;
for item in &trai.items {
let has = match item {
Expand Down Expand Up @@ -710,21 +714,16 @@ pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> R
};

// Check if with(Constructor) is specified. If Constructor trait is used, don't generate slot_new
let mut has_slot_new = false;

let mut extra_attrs = Vec::new();
let mut with_items = vec![];
for nested in &attr {
if let NestedMeta::Meta(Meta::List(MetaList { path, nested, .. })) = nested {
// If we already found the constructor trait, no need to keep looking for it
if !has_slot_new && path.is_ident("with") {
// Check if Constructor is in the list
if path.is_ident("with") {
for meta in nested {
if let NestedMeta::Meta(Meta::Path(p)) = meta
&& p.is_ident("Constructor")
{
has_slot_new = true;
}
with_items.push(meta.get_ident().expect("with() has non-ident item").clone());
}
continue;
}
extra_attrs.push(NestedMeta::Meta(Meta::List(MetaList {
path: path.clone(),
Expand All @@ -734,43 +733,40 @@ pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> R
}
}

let mut has_slot_init = false;
let with_contains = |with_items: &[Ident], s: &str| {
// Check if Constructor is in the list
with_items.iter().any(|ident| ident == s)
};

let syn::ItemImpl {
generics,
self_ty,
items,
..
} = &imp;
for item in items {
// FIXME: better detection or correct wrapper implementation
let Some(ident) = item.get_ident() else {
continue;
};
let item_name = ident.to_string();
match item_name.as_str() {
"slot_new" => {
has_slot_new = true;
}
"slot_init" => {
has_slot_init = true;
}
_ => continue,
}
}

// TODO: slot_new, slot_init must be Constructor or Initializer later

let slot_new = if has_slot_new {
let slot_new = if with_contains(&with_items, "Constructor") {
quote!()
} else {
with_items.push(Ident::new("Constructor", Span::call_site()));
quote! {
#[pyslot]
pub fn slot_new(
cls: ::rustpython_vm::builtins::PyTypeRef,
args: ::rustpython_vm::function::FuncArgs,
vm: &::rustpython_vm::VirtualMachine,
) -> ::rustpython_vm::PyResult {
<Self as ::rustpython_vm::class::PyClassDef>::Base::slot_new(cls, args, vm)
impl ::rustpython_vm::types::Constructor for #self_ty {
type Args = ::rustpython_vm::function::FuncArgs;

fn slot_new(
cls: ::rustpython_vm::builtins::PyTypeRef,
args: ::rustpython_vm::function::FuncArgs,
vm: &::rustpython_vm::VirtualMachine,
) -> ::rustpython_vm::PyResult {
<Self as ::rustpython_vm::class::PyClassDef>::Base::slot_new(cls, args, vm)
}
fn py_new(
_cls: &::rustpython_vm::Py<::rustpython_vm::builtins::PyType>,
_args: Self::Args,
_vm: &::rustpython_vm::VirtualMachine
) -> ::rustpython_vm::PyResult<Self> {
unreachable!("slot_new is defined")
}
}
}
};
Expand All @@ -779,19 +775,29 @@ pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> R
// from `BaseException` in `SimpleExtendsException` macro.
// See: `(initproc)BaseException_init`
// spell-checker:ignore initproc
let slot_init = if has_slot_init {
let slot_init = if with_contains(&with_items, "Initializer") {
quote!()
} else {
// FIXME: this is a generic logic for types not only for exceptions
with_items.push(Ident::new("Initializer", Span::call_site()));
quote! {
#[pyslot]
#[pymethod(name="__init__")]
pub fn slot_init(
zelf: ::rustpython_vm::PyObjectRef,
args: ::rustpython_vm::function::FuncArgs,
vm: &::rustpython_vm::VirtualMachine,
) -> ::rustpython_vm::PyResult<()> {
<Self as ::rustpython_vm::class::PyClassDef>::Base::slot_init(zelf, args, vm)
impl ::rustpython_vm::types::Initializer for #self_ty {
type Args = ::rustpython_vm::function::FuncArgs;

fn slot_init(
zelf: ::rustpython_vm::PyObjectRef,
args: ::rustpython_vm::function::FuncArgs,
vm: &::rustpython_vm::VirtualMachine,
) -> ::rustpython_vm::PyResult<()> {
<Self as ::rustpython_vm::class::PyClassDef>::Base::slot_init(zelf, args, vm)
}

fn init(
_zelf: ::rustpython_vm::PyRef<Self>,
_args: Self::Args,
_vm: &::rustpython_vm::VirtualMachine
) -> ::rustpython_vm::PyResult<()> {
unreachable!("slot_init is defined")
}
}
}
};
Expand All @@ -803,13 +809,13 @@ pub(crate) fn impl_pyexception_impl(attr: PunctuatedNestedMeta, item: Item) -> R
};

Ok(quote! {
#[pyclass(flags(BASETYPE, HAS_DICT) #extra_attrs_tokens)]
#[pyclass(flags(BASETYPE, HAS_DICT), with(#(#with_items),*) #extra_attrs_tokens)]
impl #generics #self_ty {
#(#items)*

#slot_new
#slot_init
}

#slot_new
#slot_init
})
}

Expand Down Expand Up @@ -892,6 +898,23 @@ where
let item_meta = MethodItemMeta::from_attr(ident.clone(), &item_attr)?;

let py_name = item_meta.method_name()?;

// Disallow __new__ and __init__ as pymethod in impl blocks (not in traits)
if !args.context.is_trait {
if py_name == "__new__" {
return Err(syn::Error::new(
ident.span(),
"#[pymethod] cannot define '__new__'. Use #[pyclass(with(Constructor))] instead.",
));
}
if py_name == "__init__" {
return Err(syn::Error::new(
ident.span(),
"#[pymethod] cannot define '__init__'. Use #[pyclass(with(Initializer))] instead.",
));
}
}

let raw = item_meta.raw()?;
let sig_doc = text_signature(func.sig(), &py_name);

Expand Down
41 changes: 22 additions & 19 deletions crates/stdlib/src/sqlite.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1540,7 +1540,7 @@ mod _sqlite {
size: Option<c_int>,
}

#[pyclass(with(Constructor, IterNext, Iterable), flags(BASETYPE))]
#[pyclass(with(Constructor, Initializer, IterNext, Iterable), flags(BASETYPE))]
impl Cursor {
fn new(
connection: PyRef<Connection>,
Expand Down Expand Up @@ -1571,24 +1571,6 @@ mod _sqlite {
}
}

#[pymethod]
fn __init__(&self, _connection: PyRef<Connection>, _vm: &VirtualMachine) -> PyResult<()> {
let mut guard = self.inner.lock();
if guard.is_some() {
// Already initialized (e.g., from a call to super().__init__)
return Ok(());
}
*guard = Some(CursorInner {
description: None,
row_cast_map: vec![],
lastrowid: -1,
rowcount: -1,
statement: None,
closed: false,
});
Ok(())
}

fn check_cursor_state(inner: Option<&CursorInner>, vm: &VirtualMachine) -> PyResult<()> {
match inner {
Some(inner) if inner.closed => Err(new_programming_error(
Expand Down Expand Up @@ -1949,6 +1931,27 @@ mod _sqlite {
}
}

impl Initializer for Cursor {
type Args = PyRef<Connection>;

Comment on lines +1934 to +1936
Copy link
Contributor
@coderabbitai coderabbitai bot Dec 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for the Cursor implementation in sqlite.rs and examine Constructor/Initializer patterns
cd crates/stdlib/src || exit 1

# Look at lines around 1923 and 1934-1936 for Constructor and Initializer
echo "=== Constructor for Cursor (around line 1923) ==="
sed -n '1920,1930p' sqlite.rs

echo ""
echo "=== Initializer for Cursor (around line 1934-1936) ==="
sed -n '1933,1950p' sqlite.rs

Repository: RustPython/RustPython

Length of output: 1095

🏁 Script executed:

#!/bin/bash
# Check Connection class pattern
cd crates/stdlib/src || exit 1

echo "=== Connection class Constructor (around line 853-937) ==="
grep -n "impl Constructor for Connection" sqlite.rs -A 10

echo ""
echo "=== Connection class Initializer ==="
grep -n "impl Initializer for Connection" sqlite.rs -A 10

echo ""
echo "=== Search for other Constructor/Initializer patterns in stdlib ==="
find crates/stdlib/src -name "*.rs" -exec grep -l "impl Constructor\|impl Initializer" {} \; | head -5

Repository: RustPython/RustPython

Length of output: 1345

🏁 Script executed:

#!/bin/bash
# Get correct path and search for ConnectArgs definition and other patterns
find . -name "sqlite.rs" | head -1

echo ""
echo "=== Search for ConnectArgs definition ==="
rg "struct ConnectArgs|type ConnectArgs" crates/stdlib/src/sqlite.rs

echo ""
echo "=== Search for other Constructor/Initializer in sqlite.rs ==="
rg "impl Constructor|impl Initializer" crates/stdlib/src/sqlite.rs -B 1 -A 5

echo ""
echo "=== Search for Constructor/Initializer patterns across stdlib ==="
rg "impl Constructor|impl Initializer" . -t rs --max-count=20

Repository: RustPython/RustPython

Length of output: 1448

🏁 Script executed:

#!/bin/bash
# Search for Constructor/Initializer patterns across the codebase
echo "=== Constructor/Initializer patterns in codebase ==="
rg "impl Constructor|impl Initializer" --max-count=30 -A 3

echo ""
echo "=== Row class implementation ==="
rg "impl Constructor for Row" crates/stdlib/src/sqlite.rs -A 10

echo ""
echo "=== Check if Row has Initializer ==="
rg "impl Initializer for Row" crates/stdlib/src/sqlite.rs -A 5

Repository: RustPython/RustPython

Length of output: 39811

🏁 Script executed:

#!/bin/bash
# Check how Constructor and Initializer are invoked in the framework
echo "=== Look for how Constructor and Initializer are called ==="
rg "impl.*Constructor\|impl.*Initializer" crates/vm/src/object/ -A 20 | head -100

echo ""
echo "=== Check if there's framework code handling argument conversion ==="
rg "fn py_new|fn init" crates/vm/src/object/ -B 2 -A 5 | head -80

Repository: RustPython/RustPython

Length of output: 688

🏁 Script executed:

#!/bin/bash
# Search for Constructor and Initializer trait definitions
echo "=== Constructor trait definition ==="
rg "trait Constructor" crates/vm/src/ -A 10

echo ""
echo "=== Initializer trait definition ==="
rg "trait Initializer" crates/vm/src/ -A 10

echo ""
echo "=== Look for code that calls Constructor::py_new and Initializer::init ==="
rg "Constructor::py_new|Initializer::init" crates/vm/src/ -B 2 -A 2 | head -60

Repository: RustPython/RustPython

Length of output: 2361

🏁 Script executed:

#!/bin/bash
# Look at the full slot_init implementation
echo "=== Full Initializer::slot_init implementation ==="
rg "fn slot_init" crates/vm/src/types/slot.rs -A 25

echo ""
echo "=== Check FromArgs trait ==="
rg "trait FromArgs" crates/vm/src/ -A 5

Repository: RustPython/RustPython

Length of output: 1640

🏁 Script executed:

#!/bin/bash
# Verify if Row has Initializer by doing a more thorough search
echo "=== Search for Row impl blocks in sqlite.rs ==="
rg "impl.*for Row" crates/stdlib/src/sqlite.rs -A 3

echo ""
echo "=== Check the line numbers for Row to understand its structure ==="
rg -n "^struct Row" crates/stdlib/src/sqlite.rs

echo ""
echo "=== Look for all impl blocks after Row struct ==="
rg -n "impl.*Row" crates/stdlib/src/sqlite.rs

Repository: RustPython/RustPython

Length of output: 1480

🏁 Script executed:

#!/bin/bash
# Check if there's automatic FromArgs handling for tuples vs non-tuples
echo "=== Search for FromArgs implementation for tuples ==="
rg "impl.*FromArgs.*for.*\(" crates/vm/src/function/ -A 5 | head -80

echo ""
echo "=== Check if there are FromArgs implementations handling conversion ==="
rg "impl.*FromArgs" crates/vm/src/function/argument.rs -A 8 | head -120

Repository: RustPython/RustPython

Length of output: 3127

🏁 Script executed:

#!/bin/bash
# Look at the actual tuple FromArgs implementation more carefully
echo "=== Full tuple_from_py_func_args macro ==="
rg "tuple_from_py_func_args" crates/vm/src/function/argument.rs -A 50 | head -150

echo ""
echo "=== Test if single-element tuple behaves same as non-tuple ==="
echo "Looking for how args.bind works..."
rg "fn bind\|pub fn bind" crates/vm/src/ -B 2 -A 8 | head -80

Repository: RustPython/RustPython

Length of output: 1422

🏁 Script executed:

#!/bin/bash
# Verify the actual framework behavior - let's check if there are any tests for Cursor
echo "=== Check for Cursor tests to understand expected behavior ==="
find . -name "*.rs" -path "*/tests/*" -o -name "*test*" | xargs grep -l "Cursor\|sqlite" 2>/dev/null | head -5

echo ""
echo "=== Check if there's documentation about how Constructor and Initializer Args should relate ==="
rg "Constructor.*Args|Initializer.*Args" crates/vm/src/types/slot.rs -B 3 -A 3

echo ""
echo "=== Final confirmation: check the exact parameter handling in init function ==="
sed -n '1936,1945p' crates/stdlib/src/sqlite.rs

Repository: RustPython/RustPython

Length of output: 723

Argument type inconsistency between Constructor and Initializer.

The Initializer::Args is PyRef<Connection> (unwrapped), but Constructor::Args is (PyRef<Connection>,) (single-element tuple). This differs from the Connection class pattern (lines 853-937), where both Constructor and Initializer use the same ConnectArgs type.

While the RustPython framework's FromArgs trait can handle both forms, this inconsistency reduces maintainability and contradicts the established pattern throughout the codebase. Align argument types for consistency.

🤖 Prompt for AI Agents 
In crates/stdlib/src/sqlite.rs around lines 1934-1936, the Initializer::Args
type is declared as PyRef<Connection> while the corresponding Constructor::Args
is (PyRef<Connection>,); change Initializer::Args to the single-element tuple
type (PyRef<Connection>,) so both Constructor and Initializer use the identical
argument shape, and update any pattern matching or parameter destructuring in
the initializer implementation to unpack the tuple accordingly.

fn init(zelf: PyRef<Self>, _connection: Self::Args, _vm: &VirtualMachine) -> PyResult<()> {
let mut guard = zelf.inner.lock();
if guard.is_some() {
// Already initialized (e.g., from a call to super().__init__)
return Ok(());
}
*guard = Some(CursorInner {
description: None,
row_cast_map: vec![],
lastrowid: -1,
rowcount: -1,
statement: None,
closed: false,
});
Ok(())
}
}

impl SelfIter for Cursor {}
impl IterNext for Cursor {
fn next(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyIterReturn> {
Expand Down
2 changes: 1 addition & 1 deletion crates/stdlib/src/ssl/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub(crate) mod ssl_error {
use crate::vm::{
PyPayload, PyRef, PyResult, VirtualMachine,
builtins::{PyBaseExceptionRef, PyOSError, PyStrRef},
types::Constructor,
types::{Constructor, Initializer},
};

// Error type constants - exposed as pyattr and available for internal use
Expand Down
32 changes: 21 additions & 11 deletions crates/vm/src/builtins/object.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ use super::{PyDictRef, PyList, PyStr, PyStrRef, PyType, PyTypeRef};
use crate::common::hash::PyHash;
use crate::types::PyTypeFlags;
use crate::{
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyResult, VirtualMachine,
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
class::PyClassImpl,
convert::ToPyResult,
function::{Either, FuncArgs, PyArithmeticValue, PyComparisonValue, PySetterValue},
types::{Constructor, PyComparisonOp},
types::{Constructor, Initializer, PyComparisonOp},
};
use itertools::Itertools;

Expand Down Expand Up @@ -115,6 +115,18 @@ impl Constructor for PyBaseObject {
}
}

impl Initializer for PyBaseObject {
type Args = FuncArgs;

fn slot_init(_zelf: PyObjectRef, _args: FuncArgs, _vm: &VirtualMachine) -> PyResult<()> {
Ok(())
}

fn init(_zelf: PyRef<Self>, _args: Self::Args, _vm: &VirtualMachine) -> PyResult<()> {
unreachable!("slot_init is defined")
}
}

// TODO: implement _PyType_GetSlotNames properly
fn type_slot_names(typ: &Py<PyType>, vm: &VirtualMachine) -> PyResult<Option<super::PyListRef>> {
// let attributes = typ.attributes.read();
Expand Down Expand Up @@ -235,7 +247,7 @@ fn object_getstate_default(obj: &PyObject, required: bool, vm: &VirtualMachine)
// getstate.call((), vm)
// }

#[pyclass(with(Constructor), flags(BASETYPE))]
#[pyclass(with(Constructor, Initializer), flags(BASETYPE))]
impl PyBaseObject {
#[pymethod(raw)]
fn __getstate__(vm: &VirtualMachine, args: FuncArgs) -> PyResult {
Expand Down Expand Up @@ -444,19 +456,17 @@ impl PyBaseObject {
obj.str(vm)
}

#[pyslot]
#[pymethod]
fn __init__(_zelf: PyObjectRef, _args: FuncArgs, _vm: &VirtualMachine) -> PyResult<()> {
Ok(())
}

#[pygetset]
fn __class__(obj: PyObjectRef) -> PyTypeRef {
obj.class().to_owned()
}

#[pygetset(name = "__class__", setter)]
fn set_class(instance: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
#[pygetset(setter)]
fn set___class__(
instance: PyObjectRef,
value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<()> {
match value.downcast::<PyType>() {
Ok(cls) => {
let both_module = instance.class().fast_issubclass(vm.ctx.types.module_type)
Expand Down
Loading
Loading
0