GeezerVison

crates/vm/src/vm/interpreter.rs (1)

466-521: Consider extracting a small helper to reduce repetition in alias injection.

The find-then-push pattern is repeated three times with minor variations. A small closure or helper could reduce the boilerplate while keeping intent clear.

♻️ Suggested refactor

     // Collect and add frozen module aliases for test modules
     let mut entries: Vec<_> = iter.collect();
+
+    let find_code = |entries: &[(&'static str, FrozenModule)], name: &str| {
+        entries.iter().find(|(n, _)| *n == name).map(|(_, m)| m.code)
+    };
+
-    if let Some(hello_code) = entries
-        .iter()
-        .find(|(n, _)| *n == "__hello__")
-        .map(|(_, m)| m.code)
-    {
+    if let Some(hello_code) = find_code(&entries, "__hello__") {
         entries.push(("__hello_alias__", FrozenModule { code: hello_code, package: false }));
         entries.push(("__phello_alias__", FrozenModule { code: hello_code, package: true }));
         entries.push(("__phello_alias__.spam", FrozenModule { code: hello_code, package: false }));
     }
-    if let Some(code) = entries
-        .iter()
-        .find(|(n, _)| *n == "__phello__")
-        .map(|(_, m)| m.code)
-    {
+    if let Some(code) = find_code(&entries, "__phello__") {
         entries.push(("__phello__.__init__", FrozenModule { code, package: false }));
     }
-    if let Some(code) = entries
-        .iter()
-        .find(|(n, _)| *n == "__phello__.ham")
-        .map(|(_, m)| m.code)
-    {
+    if let Some(code) = find_code(&entries, "__phello__.ham") {
         entries.push(("__phello__.ham.__init__", FrozenModule { code, package: false }));
     }

crates/vm/src/import.rs (1)

242-265: Prefer Option<&PyObjectRef> over &Option<PyObjectRef>.

Clippy flags &Option<T> in function signatures (clippy::ref_option). Using Option<&PyObjectRef> is more idiomatic—it avoids an unnecessary indirection and callers can simply pass spec.as_ref().

♻️ Proposed fix

-pub(crate) fn get_spec_file_origin(
-    spec: &Option<PyObjectRef>,
-    vm: &VirtualMachine,
-) -> Option<String> {
-    let spec = spec.as_ref()?;
+pub(crate) fn get_spec_file_origin(
+    spec: Option<&PyObjectRef>,
+    vm: &VirtualMachine,
+) -> Option<String> {
+    let spec = spec?;

As per coding guidelines, "Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints that are introduced by your changes."

crates/vm/src/frame.rs (1)

3298-3309: Unnecessary per-call string allocation — prefer &str or interned identifiers.

vm.ctx.new_str("__spec__") and vm.ctx.new_str("_initializing") allocate new PyStr objects every time is_module_initializing is called. Elsewhere in this file (e.g., line 278), raw &str is passed directly to get_attr. The same pattern works here.

♻️ Proposed fix

 fn is_module_initializing(module: &PyObject, vm: &VirtualMachine) -> bool {
-    let Ok(spec) = module.get_attr(&vm.ctx.new_str("__spec__"), vm) else {
+    let Ok(spec) = module.get_attr("__spec__", vm) else {
         return false;
     };
     if vm.is_none(&spec) {
         return false;
     }
-    let Ok(initializing_attr) = spec.get_attr(&vm.ctx.new_str("_initializing"), vm) else {
+    let Ok(initializing_attr) = spec.get_attr("_initializing", vm) else {
         return false;
     };
     initializing_attr.try_to_bool(vm).unwrap_or(false)
 }

crates/vm/src/stdlib/imp.rs (1)

254-254: Return type readability: consider a type alias for this complex tuple.

Option<(Option<PyRef<PyMemoryView>>, bool, Option<PyStrRef>)> is hard to parse at a glance. A named struct (even file-local) would improve readability, though this is non-blocking.

crates/vm/src/import.rs (1)

504-542: Silently swallowing rich_compare_bool error on line 519 is acceptable for a warning-only path, but worth a brief comment.

The unwrap_or(false) on the rich_compare_bool result means a broken __ne__ on a package string silently suppresses the deprecation warning rather than propagating the error. This is intentional (we're inside a best-effort warning block), but a one-line // Ignore comparison errors; this is only advisory. would clarify intent for future readers.