Skip to content

nitpicks #5886

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 12 commits into from
Jul 2, 2025
Merged

nitpicks #5886

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
319065e
Use `Self` instead of struct name
ShaharNaveh Jun 30, 2025
8c8ccdc
General fixes
ShaharNaveh Jun 30, 2025
df03be4
General
ShaharNaveh Jun 29, 2025
bcfe02a
General fixes
ShaharNaveh Jul 2, 2025
91b5666
More nitpicks
ShaharNaveh Jul 2, 2025
9aed7aa
Added newlines
ShaharNaveh Jul 2, 2025
d9aeed7
More nitpicks
ShaharNaveh Jul 2, 2025
3992208
Use Self where possible
ShaharNaveh Jul 2, 2025
7a6e815
Use Self where possible
ShaharNaveh Jul 2, 2025
48b332c
Use Self
ShaharNaveh Jul 2, 2025
75ac4c4
Use Self more
ShaharNaveh Jul 2, 2025
da95cca
Use Self
ShaharNaveh Jul 2, 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
Prev Previous commit
Next Next commit
Use Self
  • Loading branch information
@ShaharNaveh
ShaharNaveh committed Jul 2, 2025
commit 48b332cf68215de39aa210608fef311c11468c66
15 changes: 11 additions & 4 deletions vm/src/stdlib/ast/module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ impl Node for Mod {
Self::FunctionType(cons) => cons.ast_to_object(vm, source_code),
}
}

fn ast_from_object(
vm: &VirtualMachine,
source_code: &SourceCodeOwned,
Expand All @@ -59,10 +60,11 @@ impl Node for Mod {
})
}
}

// constructor
impl Node for ruff::ModModule {
fn ast_to_object(self, vm: &VirtualMachine, source_code: &SourceCodeOwned) -> PyObjectRef {
let ruff::ModModule {
let Self {
body,
// type_ignores,
range,
Expand All @@ -85,12 +87,13 @@ impl Node for ruff::ModModule {
node_add_location(&dict, range, vm, source_code);
node.into()
}

fn ast_from_object(
vm: &VirtualMachine,
source_code: &SourceCodeOwned,
object: PyObjectRef,
) -> PyResult<Self> {
Ok(ruff::ModModule {
Ok(Self {
body: Node::ast_from_object(
vm,
source_code,
Expand Down Expand Up @@ -123,6 +126,7 @@ impl Node for ModInteractive {
node_add_location(&dict, range, vm, source_code);
node.into()
}

fn ast_from_object(
vm: &VirtualMachine,
source_code: &SourceCodeOwned,
Expand All @@ -138,6 +142,7 @@ impl Node for ModInteractive {
})
}
}

// constructor
impl Node for ruff::ModExpression {
fn ast_to_object(self, vm: &VirtualMachine, source_code: &SourceCodeOwned) -> PyObjectRef {
Expand All @@ -151,6 +156,7 @@ impl Node for ruff::ModExpression {
node_add_location(&dict, range, vm, source_code);
node.into()
}

fn ast_from_object(
vm: &VirtualMachine,
source_code: &SourceCodeOwned,
Expand All @@ -176,7 +182,7 @@ pub(super) struct ModFunctionType {
// constructor
impl Node for ModFunctionType {
fn ast_to_object(self, vm: &VirtualMachine, source_code: &SourceCodeOwned) -> PyObjectRef {
let ModFunctionType {
let Self {
argtypes,
returns,
range,
Expand All @@ -196,12 +202,13 @@ impl Node for ModFunctionType {
node_add_location(&dict, range, vm, source_code);
node.into()
}

fn ast_from_object(
vm: &VirtualMachine,
source_code: &SourceCodeOwned,
object: PyObjectRef,
) -> PyResult<Self> {
Ok(ModFunctionType {
Ok(Self {
argtypes: {
let argtypes: BoxedSlice<_> = Node::ast_from_object(
vm,
Expand Down
3 changes: 2 additions & 1 deletion vm/src/stdlib/ast/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub(crate) trait Node: Sized {
source_code: &SourceCodeOwned,
object: PyObjectRef,
) -> PyResult<Self>;

/// Used in `Option::ast_from_object`; if `true`, that impl will return None.
fn is_none(&self) -> bool {
false
Expand Down Expand Up @@ -44,7 +45,7 @@ impl<T: Node> Node for Box<T> {
source_code: &SourceCodeOwned,
object: PyObjectRef,
) -> PyResult<Self> {
T::ast_from_object(vm, source_code, object).map(Box::new)
T::ast_from_object(vm, source_code, object).map(Self::new)
Copy link
Copy Markdown
Member

@youknowone youknowone Jul 2, 2025

Choose a reason for hiding this comment

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

This is logically right but lookins so unfamiliar 😲

Copy link
Copy Markdown
Contributor Author

@ShaharNaveh ShaharNaveh Jul 2, 2025

Choose a reason for hiding this comment

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

It can be reverted, just say the word:)

}

fn is_none(&self) -> bool {
Expand Down
123 changes: 65 additions & 58 deletions vm/src/stdlib/ast/operator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,25 @@ use super::*;
impl Node for ruff::BoolOp {
fn ast_to_object(self, vm: &VirtualMachine, _source_code: &SourceCodeOwned) -> PyObjectRef {
let node_type = match self {
ruff::BoolOp::And => pyast::NodeBoolOpAnd::static_type(),
ruff::BoolOp::Or => pyast::NodeBoolOpOr::static_type(),
Self::And => pyast::NodeBoolOpAnd::static_type(),
Self::Or => pyast::NodeBoolOpOr::static_type(),
};
NodeAst
.into_ref_with_type(vm, node_type.to_owned())
.unwrap()
.into()
}

fn ast_from_object(
_vm: &VirtualMachine,
_source_code: &SourceCodeOwned,
_object: PyObjectRef,
) -> PyResult<Self> {
let _cls = _object.class();
Ok(if _cls.is(pyast::NodeBoolOpAnd::static_type()) {
ruff::BoolOp::And
Self::And
} else if _cls.is(pyast::NodeBoolOpOr::static_type()) {
ruff::BoolOp::Or
Self::Or
} else {
return Err(_vm.new_type_error(format!(
"expected some sort of boolop, but got {}",
Expand All @@ -30,61 +31,63 @@ impl Node for ruff::BoolOp {
})
}
}

// sum
impl Node for ruff::Operator {
fn ast_to_object(self, vm: &VirtualMachine, _source_code: &SourceCodeOwned) -> PyObjectRef {
let node_type = match self {
ruff::Operator::Add => pyast::NodeOperatorAdd::static_type(),
ruff::Operator::Sub => pyast::NodeOperatorSub::static_type(),
ruff::Operator::Mult => pyast::NodeOperatorMult::static_type(),
ruff::Operator::MatMult => pyast::NodeOperatorMatMult::static_type(),
ruff::Operator::Div => pyast::NodeOperatorDiv::static_type(),
ruff::Operator::Mod => pyast::NodeOperatorMod::static_type(),
ruff::Operator::Pow => pyast::NodeOperatorPow::static_type(),
ruff::Operator::LShift => pyast::NodeOperatorLShift::static_type(),
ruff::Operator::RShift => pyast::NodeOperatorRShift::static_type(),
ruff::Operator::BitOr => pyast::NodeOperatorBitOr::static_type(),
ruff::Operator::BitXor => pyast::NodeOperatorBitXor::static_type(),
ruff::Operator::BitAnd => pyast::NodeOperatorBitAnd::static_type(),
ruff::Operator::FloorDiv => pyast::NodeOperatorFloorDiv::static_type(),
Self::Add => pyast::NodeOperatorAdd::static_type(),
Self::Sub => pyast::NodeOperatorSub::static_type(),
Self::Mult => pyast::NodeOperatorMult::static_type(),
Self::MatMult => pyast::NodeOperatorMatMult::static_type(),
Self::Div => pyast::NodeOperatorDiv::static_type(),
Self::Mod => pyast::NodeOperatorMod::static_type(),
Self::Pow => pyast::NodeOperatorPow::static_type(),
Self::LShift => pyast::NodeOperatorLShift::static_type(),
Self::RShift => pyast::NodeOperatorRShift::static_type(),
Self::BitOr => pyast::NodeOperatorBitOr::static_type(),
Self::BitXor => pyast::NodeOperatorBitXor::static_type(),
Self::BitAnd => pyast::NodeOperatorBitAnd::static_type(),
Self::FloorDiv => pyast::NodeOperatorFloorDiv::static_type(),
};
NodeAst
.into_ref_with_type(vm, node_type.to_owned())
.unwrap()
.into()
}

fn ast_from_object(
_vm: &VirtualMachine,
_source_code: &SourceCodeOwned,
_object: PyObjectRef,
) -> PyResult<Self> {
let _cls = _object.class();
Ok(if _cls.is(pyast::NodeOperatorAdd::static_type()) {
ruff::Operator::Add
Self::Add
} else if _cls.is(pyast::NodeOperatorSub::static_type()) {
ruff::Operator::Sub
Self::Sub
} else if _cls.is(pyast::NodeOperatorMult::static_type()) {
ruff::Operator::Mult
Self::Mult
} else if _cls.is(pyast::NodeOperatorMatMult::static_type()) {
ruff::Operator::MatMult
Self::MatMult
} else if _cls.is(pyast::NodeOperatorDiv::static_type()) {
ruff::Operator::Div
Self::Div
} else if _cls.is(pyast::NodeOperatorMod::static_type()) {
ruff::Operator::Mod
Self::Mod
} else if _cls.is(pyast::NodeOperatorPow::static_type()) {
ruff::Operator::Pow
Self::Pow
} else if _cls.is(pyast::NodeOperatorLShift::static_type()) {
ruff::Operator::LShift
Self::LShift
} else if _cls.is(pyast::NodeOperatorRShift::static_type()) {
ruff::Operator::RShift
Self::RShift
} else if _cls.is(pyast::NodeOperatorBitOr::static_type()) {
ruff::Operator::BitOr
Self::BitOr
} else if _cls.is(pyast::NodeOperatorBitXor::static_type()) {
ruff::Operator::BitXor
Self::BitXor
} else if _cls.is(pyast::NodeOperatorBitAnd::static_type()) {
ruff::Operator::BitAnd
Self::BitAnd
} else if _cls.is(pyast::NodeOperatorFloorDiv::static_type()) {
ruff::Operator::FloorDiv
Self::FloorDiv
} else {
return Err(_vm.new_type_error(format!(
"expected some sort of operator, but got {}",
Expand All @@ -93,34 +96,36 @@ impl Node for ruff::Operator {
})
}
}

// sum
impl Node for ruff::UnaryOp {
fn ast_to_object(self, vm: &VirtualMachine, _source_code: &SourceCodeOwned) -> PyObjectRef {
let node_type = match self {
ruff::UnaryOp::Invert => pyast::NodeUnaryOpInvert::static_type(),
ruff::UnaryOp::Not => pyast::NodeUnaryOpNot::static_type(),
ruff::UnaryOp::UAdd => pyast::NodeUnaryOpUAdd::static_type(),
ruff::UnaryOp::USub => pyast::NodeUnaryOpUSub::static_type(),
Self::Invert => pyast::NodeUnaryOpInvert::static_type(),
Self::Not => pyast::NodeUnaryOpNot::static_type(),
Self::UAdd => pyast::NodeUnaryOpUAdd::static_type(),
Self::USub => pyast::NodeUnaryOpUSub::static_type(),
};
NodeAst
.into_ref_with_type(vm, node_type.to_owned())
.unwrap()
.into()
}

fn ast_from_object(
_vm: &VirtualMachine,
_source_code: &SourceCodeOwned,
_object: PyObjectRef,
) -> PyResult<Self> {
let _cls = _object.class();
Ok(if _cls.is(pyast::NodeUnaryOpInvert::static_type()) {
ruff::UnaryOp::Invert
Self::Invert
} else if _cls.is(pyast::NodeUnaryOpNot::static_type()) {
ruff::UnaryOp::Not
Self::Not
} else if _cls.is(pyast::NodeUnaryOpUAdd::static_type()) {
ruff::UnaryOp::UAdd
Self::UAdd
} else if _cls.is(pyast::NodeUnaryOpUSub::static_type()) {
ruff::UnaryOp::USub
Self::USub
} else {
return Err(_vm.new_type_error(format!(
"expected some sort of unaryop, but got {}",
Expand All @@ -129,52 +134,54 @@ impl Node for ruff::UnaryOp {
})
}
}

// sum
impl Node for ruff::CmpOp {
fn ast_to_object(self, vm: &VirtualMachine, _source_code: &SourceCodeOwned) -> PyObjectRef {
let node_type = match self {
ruff::CmpOp::Eq => pyast::NodeCmpOpEq::static_type(),
ruff::CmpOp::NotEq => pyast::NodeCmpOpNotEq::static_type(),
ruff::CmpOp::Lt => pyast::NodeCmpOpLt::static_type(),
ruff::CmpOp::LtE => pyast::NodeCmpOpLtE::static_type(),
ruff::CmpOp::Gt => pyast::NodeCmpOpGt::static_type(),
ruff::CmpOp::GtE => pyast::NodeCmpOpGtE::static_type(),
ruff::CmpOp::Is => pyast::NodeCmpOpIs::static_type(),
ruff::CmpOp::IsNot => pyast::NodeCmpOpIsNot::static_type(),
ruff::CmpOp::In => pyast::NodeCmpOpIn::static_type(),
ruff::CmpOp::NotIn => pyast::NodeCmpOpNotIn::static_type(),
Self::Eq => pyast::NodeCmpOpEq::static_type(),
Self::NotEq => pyast::NodeCmpOpNotEq::static_type(),
Self::Lt => pyast::NodeCmpOpLt::static_type(),
Self::LtE => pyast::NodeCmpOpLtE::static_type(),
Self::Gt => pyast::NodeCmpOpGt::static_type(),
Self::GtE => pyast::NodeCmpOpGtE::static_type(),
Self::Is => pyast::NodeCmpOpIs::static_type(),
Self::IsNot => pyast::NodeCmpOpIsNot::static_type(),
Self::In => pyast::NodeCmpOpIn::static_type(),
Self::NotIn => pyast::NodeCmpOpNotIn::static_type(),
};
NodeAst
.into_ref_with_type(vm, node_type.to_owned())
.unwrap()
.into()
}

fn ast_from_object(
_vm: &VirtualMachine,
_source_code: &SourceCodeOwned,
_object: PyObjectRef,
) -> PyResult<Self> {
let _cls = _object.class();
Ok(if _cls.is(pyast::NodeCmpOpEq::static_type()) {
ruff::CmpOp::Eq
Self::Eq
} else if _cls.is(pyast::NodeCmpOpNotEq::static_type()) {
ruff::CmpOp::NotEq
Self::NotEq
} else if _cls.is(pyast::NodeCmpOpLt::static_type()) {
ruff::CmpOp::Lt
Self::Lt
} else if _cls.is(pyast::NodeCmpOpLtE::static_type()) {
ruff::CmpOp::LtE
Self::LtE
} else if _cls.is(pyast::NodeCmpOpGt::static_type()) {
ruff::CmpOp::Gt
Self::Gt
} else if _cls.is(pyast::NodeCmpOpGtE::static_type()) {
ruff::CmpOp::GtE
Self::GtE
} else if _cls.is(pyast::NodeCmpOpIs::static_type()) {
ruff::CmpOp::Is
Self::Is
} else if _cls.is(pyast::NodeCmpOpIsNot::static_type()) {
ruff::CmpOp::IsNot
Self::IsNot
} else if _cls.is(pyast::NodeCmpOpIn::static_type()) {
ruff::CmpOp::In
Self::In
} else if _cls.is(pyast::NodeCmpOpNotIn::static_type()) {
ruff::CmpOp::NotIn
Self::NotIn
} else {
return Err(_vm.new_type_error(format!(
"expected some sort of cmpop, but got {}",
Expand Down