From ac4dcfe9c0145f42de56c47fa7430e06ccf5053b Mon Sep 17 00:00:00 2001 From: Shahar Naveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sat, 16 May 2026 19:56:47 +0300 Subject: [PATCH] Remove some `#[allow(clippy::*)]` (#7878) * Remove some `#[allow(clippy::*)]` * Fix after merge --- crates/codegen/src/compile.rs | 14 +-- crates/stdlib/src/json/machinery.rs | 4 - crates/vm/src/builtins/dict.rs | 6 - crates/vm/src/builtins/list.rs | 1 - crates/vm/src/builtins/module.rs | 9 +- crates/vm/src/lib.rs | 18 ++- crates/vm/src/sliceable.rs | 2 +- crates/vm/src/stdlib/_ast/pyast.rs | 2 - crates/vm/src/stdlib/_ast/statement.rs | 153 ++++++++++++------------- crates/vm/src/stdlib/_io.rs | 6 +- crates/vm/src/stdlib/_winapi.rs | 4 +- crates/vm/src/stdlib/os.rs | 6 +- 12 files changed, 103 insertions(+), 122 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index ee4012b5e..cef312459 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -5209,7 +5209,7 @@ impl Compiler { } // = compiler_function - #[allow(clippy::too_many_arguments)] + #[expect(clippy::too_many_arguments, reason = "ignore warning for now")] fn compile_function_def( &mut self, name: &str, @@ -7083,13 +7083,11 @@ impl Compiler { } // Check for overflow (INT_MAX < size - 1) - if size > (i32::MAX as usize + 1) { - return Err(self.error(CodegenErrorType::SyntaxError( + let size = u32::try_from(size).map_err(|_| { + self.error(CodegenErrorType::SyntaxError( "too many sub-patterns in mapping pattern".to_string(), - ))); - } - #[allow(clippy::cast_possible_truncation, reason = "checked right before")] - let size = size as u32; + )) + })?; // Step 2: If we have keys to match if size > 0 { @@ -9729,7 +9727,7 @@ impl Compiler { Ok(()) } - #[allow(clippy::too_many_arguments)] + #[expect(clippy::too_many_arguments, reason = "ignore warning for now")] fn compile_comprehension( &mut self, name: &str, diff --git a/crates/stdlib/src/json/machinery.rs b/crates/stdlib/src/json/machinery.rs index 30882588e..2acd6c8cb 100644 --- a/crates/stdlib/src/json/machinery.rs +++ b/crates/stdlib/src/json/machinery.rs @@ -44,10 +44,6 @@ static ESCAPE_CHARS: [&str; 0x20] = [ // And which one need to be escaped (1) // The characters that need escaping are 0x00 to 0x1F, 0x22 ("), 0x5C (\), 0x7F (DEL) // Non-ASCII unicode characters can be safely included in a JSON string -#[allow( - clippy::unusual_byte_groupings, - reason = "groups of 16 are intentional here" -)] static NEEDS_ESCAPING_BITSET: [u64; 4] = [ //fedcba9876543210_fedcba9876543210_fedcba9876543210_fedcba9876543210 0b0000000000000000_0000000000000100_1111111111111111_1111111111111111, // 3_2_1_0 diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index 1593c250f..d1b215597 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -251,7 +251,6 @@ impl PyDict { } // Python dict methods: -#[allow(clippy::len_without_is_empty)] #[pyclass( with( Py, @@ -929,7 +928,6 @@ macro_rules! dict_view { } fn item(vm: &VirtualMachine, key: PyObjectRef, value: PyObjectRef) -> PyObjectRef { - #[allow(clippy::redundant_closure_call)] $result_fn(vm, key, value) } @@ -1005,7 +1003,6 @@ macro_rules! dict_view { self.internal.lock().length_hint(|_| self.size.entries_size) } - #[allow(clippy::redundant_closure_call)] #[pymethod] fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef { let iter = builtins_iter(vm); @@ -1024,7 +1021,6 @@ macro_rules! dict_view { impl SelfIter for $iter_name {} impl IterNext for $iter_name { - #[allow(clippy::redundant_closure_call)] fn next(zelf: &Py, vm: &VirtualMachine) -> PyResult { let mut internal = zelf.internal.lock(); let next = if let IterStatus::Active(dict) = &internal.status { @@ -1076,7 +1072,6 @@ macro_rules! dict_view { } } - #[allow(clippy::redundant_closure_call)] #[pymethod] fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef { let iter = builtins_reversed(vm); @@ -1103,7 +1098,6 @@ macro_rules! dict_view { impl SelfIter for $reverse_iter_name {} impl IterNext for $reverse_iter_name { - #[allow(clippy::redundant_closure_call)] fn next(zelf: &Py, vm: &VirtualMachine) -> PyResult { let mut internal = zelf.internal.lock(); let next = if let IterStatus::Active(dict) = &internal.status { diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index a59d9367f..f4af27ea4 100644 --- a/crates/vm/src/builtins/list.rs +++ b/crates/vm/src/builtins/list.rs @@ -245,7 +245,6 @@ impl PyList { Self::from(self.borrow_vec().to_vec()).into_ref(&vm.ctx) } - #[allow(clippy::len_without_is_empty)] pub fn __len__(&self) -> usize { self.borrow_vec().len() } diff --git a/crates/vm/src/builtins/module.rs b/crates/vm/src/builtins/module.rs index f9d3df8df..51d90e9b3 100644 --- a/crates/vm/src/builtins/module.rs +++ b/crates/vm/src/builtins/module.rs @@ -82,10 +82,6 @@ impl PyModuleDef { } } -#[allow( - clippy::new_without_default, - reason = "avoid a misleading Default implementation" -)] #[pyclass(module = false, name = "module")] #[derive(Debug)] pub struct PyModule { @@ -112,7 +108,10 @@ pub struct ModuleInitArgs { } impl PyModule { - #[allow(clippy::new_without_default)] + #[expect( + clippy::new_without_default, + reason = "avoid a misleading Default implementation" + )] #[must_use] pub const fn new() -> Self { Self { diff --git a/crates/vm/src/lib.rs b/crates/vm/src/lib.rs index 5e2e88a07..cca5b4345 100644 --- a/crates/vm/src/lib.rs +++ b/crates/vm/src/lib.rs @@ -6,13 +6,19 @@ //! //! Some stdlib modules are implemented here, but most of them are in the `rustpython-stdlib` module. The -// to allow `mod foo {}` in foo.rs; clippy thinks this is a mistake/misunderstanding of -// how `mod` works, but we want this sometimes for pymodule declarations #![deny(clippy::disallowed_methods)] -#![allow(clippy::module_inception)] -// we want to mirror python naming conventions when defining python structs, so that does mean -// uppercase acronyms, e.g. TextIOWrapper instead of TextIoWrapper -#![allow(clippy::upper_case_acronyms)] +#![allow( + clippy::module_inception, + reason = " + to allow `mod foo {}` in foo.rs; clippy thinks this is a mistake/misunderstanding of + how `mod` works, but we want this sometimes for pymodule declarations" +)] +#![allow( + clippy::upper_case_acronyms, + reason = " +we want to mirror python naming conventions when defining python structs, so that does mean +uppercase acronyms, e.g. TextIOWrapper instead of TextIoWrapper" +)] #![doc(html_logo_url = "https://raw.githubusercontent.com/RustPython/RustPython/main/logo.png")] #![doc(html_root_url = "https://docs.rs/rustpython-vm/")] diff --git a/crates/vm/src/sliceable.rs b/crates/vm/src/sliceable.rs index 21fe6057e..dd4dd5097 100644 --- a/crates/vm/src/sliceable.rs +++ b/crates/vm/src/sliceable.rs @@ -160,7 +160,7 @@ impl SliceableSequenceMutOp for Vec { } } -#[allow(clippy::len_without_is_empty)] +#[expect(clippy::len_without_is_empty, reason = "Doesn't match CPython code")] pub trait SliceableSequenceOp { type Item; type Sliced; diff --git a/crates/vm/src/stdlib/_ast/pyast.rs b/crates/vm/src/stdlib/_ast/pyast.rs index c9017006f..7eae6f009 100644 --- a/crates/vm/src/stdlib/_ast/pyast.rs +++ b/crates/vm/src/stdlib/_ast/pyast.rs @@ -1,5 +1,3 @@ -#![allow(clippy::all)] - use super::*; use crate::builtins::{PyGenericAlias, PyTuple, PyTupleRef, PyTypeRef, make_union}; use crate::common::ascii; diff --git a/crates/vm/src/stdlib/_ast/statement.rs b/crates/vm/src/stdlib/_ast/statement.rs index 4c78ad4b7..43d1162a4 100644 --- a/crates/vm/src/stdlib/_ast/statement.rs +++ b/crates/vm/src/stdlib/_ast/statement.rs @@ -36,111 +36,99 @@ impl Node for ast::Stmt { } } - #[allow(clippy::if_same_then_else)] + #[expect(clippy::if_same_then_else, reason = "Looks better here")] fn ast_from_object( - _vm: &VirtualMachine, + vm: &VirtualMachine, source_file: &SourceFile, - _object: PyObjectRef, + object: PyObjectRef, ) -> PyResult { - let _cls = _object.class(); - Ok(if _cls.is(pyast::NodeStmtFunctionDef::static_type()) { + let cls = object.class(); + Ok(if cls.is(pyast::NodeStmtFunctionDef::static_type()) { Self::FunctionDef(ast::StmtFunctionDef::ast_from_object( - _vm, + vm, source_file, - _object, + object, )?) - } else if _cls.is(pyast::NodeStmtAsyncFunctionDef::static_type()) { + } else if cls.is(pyast::NodeStmtAsyncFunctionDef::static_type()) { Self::FunctionDef(ast::StmtFunctionDef::ast_from_object( - _vm, + vm, source_file, - _object, + object, )?) - } else if _cls.is(pyast::NodeStmtClassDef::static_type()) { - Self::ClassDef(ast::StmtClassDef::ast_from_object( - _vm, - source_file, - _object, - )?) - } else if _cls.is(pyast::NodeStmtReturn::static_type()) { - Self::Return(ast::StmtReturn::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtDelete::static_type()) { - Self::Delete(ast::StmtDelete::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtAssign::static_type()) { - Self::Assign(ast::StmtAssign::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtTypeAlias::static_type()) { + } else if cls.is(pyast::NodeStmtClassDef::static_type()) { + Self::ClassDef(ast::StmtClassDef::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtReturn::static_type()) { + Self::Return(ast::StmtReturn::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtDelete::static_type()) { + Self::Delete(ast::StmtDelete::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtAssign::static_type()) { + Self::Assign(ast::StmtAssign::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtTypeAlias::static_type()) { Self::TypeAlias(ast::StmtTypeAlias::ast_from_object( - _vm, + vm, source_file, - _object, + object, )?) - } else if _cls.is(pyast::NodeStmtAugAssign::static_type()) { + } else if cls.is(pyast::NodeStmtAugAssign::static_type()) { Self::AugAssign(ast::StmtAugAssign::ast_from_object( - _vm, + vm, source_file, - _object, + object, )?) - } else if _cls.is(pyast::NodeStmtAnnAssign::static_type()) { + } else if cls.is(pyast::NodeStmtAnnAssign::static_type()) { Self::AnnAssign(ast::StmtAnnAssign::ast_from_object( - _vm, + vm, source_file, - _object, + object, )?) - } else if _cls.is(pyast::NodeStmtFor::static_type()) { - Self::For(ast::StmtFor::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtAsyncFor::static_type()) { - Self::For(ast::StmtFor::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtWhile::static_type()) { - Self::While(ast::StmtWhile::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtIf::static_type()) { - Self::If(ast::StmtIf::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtWith::static_type()) { - Self::With(ast::StmtWith::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtAsyncWith::static_type()) { - Self::With(ast::StmtWith::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtMatch::static_type()) { - Self::Match(ast::StmtMatch::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtRaise::static_type()) { - Self::Raise(ast::StmtRaise::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtTry::static_type()) { - Self::Try(ast::StmtTry::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtTryStar::static_type()) { - Self::Try(ast::StmtTry::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtAssert::static_type()) { - Self::Assert(ast::StmtAssert::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtImport::static_type()) { - Self::Import(ast::StmtImport::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtImportFrom::static_type()) { + } else if cls.is(pyast::NodeStmtFor::static_type()) { + Self::For(ast::StmtFor::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtAsyncFor::static_type()) { + Self::For(ast::StmtFor::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtWhile::static_type()) { + Self::While(ast::StmtWhile::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtIf::static_type()) { + Self::If(ast::StmtIf::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtWith::static_type()) { + Self::With(ast::StmtWith::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtAsyncWith::static_type()) { + Self::With(ast::StmtWith::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtMatch::static_type()) { + Self::Match(ast::StmtMatch::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtRaise::static_type()) { + Self::Raise(ast::StmtRaise::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtTry::static_type()) { + Self::Try(ast::StmtTry::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtTryStar::static_type()) { + Self::Try(ast::StmtTry::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtAssert::static_type()) { + Self::Assert(ast::StmtAssert::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtImport::static_type()) { + Self::Import(ast::StmtImport::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtImportFrom::static_type()) { Self::ImportFrom(ast::StmtImportFrom::ast_from_object( - _vm, + vm, source_file, - _object, + object, )?) - } else if _cls.is(pyast::NodeStmtGlobal::static_type()) { - Self::Global(ast::StmtGlobal::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtNonlocal::static_type()) { - Self::Nonlocal(ast::StmtNonlocal::ast_from_object( - _vm, - source_file, - _object, - )?) - } else if _cls.is(pyast::NodeStmtExpr::static_type()) { - Self::Expr(ast::StmtExpr::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtPass::static_type()) { - Self::Pass(ast::StmtPass::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtBreak::static_type()) { - Self::Break(ast::StmtBreak::ast_from_object(_vm, source_file, _object)?) - } else if _cls.is(pyast::NodeStmtContinue::static_type()) { - Self::Continue(ast::StmtContinue::ast_from_object( - _vm, - source_file, - _object, - )?) - } else if _vm.is_none(&_object) { - return Err(_vm.new_value_error("None disallowed in statement list")); + } else if cls.is(pyast::NodeStmtGlobal::static_type()) { + Self::Global(ast::StmtGlobal::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtNonlocal::static_type()) { + Self::Nonlocal(ast::StmtNonlocal::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtExpr::static_type()) { + Self::Expr(ast::StmtExpr::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtPass::static_type()) { + Self::Pass(ast::StmtPass::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtBreak::static_type()) { + Self::Break(ast::StmtBreak::ast_from_object(vm, source_file, object)?) + } else if cls.is(pyast::NodeStmtContinue::static_type()) { + Self::Continue(ast::StmtContinue::ast_from_object(vm, source_file, object)?) + } else if vm.is_none(&object) { + return Err(vm.new_value_error("None disallowed in statement list")); } else { - return Err(_vm.new_type_error(format!( + return Err(vm.new_type_error(format!( "expected some sort of stmt, but got {}", - _object.repr(_vm)? + object.repr(vm)? ))); }) } @@ -201,6 +189,7 @@ impl Node for ast::StmtFunctionDef { node_add_location(&dict, range, vm, source_file); node.into() } + fn ast_from_object( _vm: &VirtualMachine, source_file: &SourceFile, diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index e25d6b609..65b7486df 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -147,7 +147,6 @@ mod _io { use num_traits::ToPrimitive; use std::io::{self, Cursor, SeekFrom, prelude::*}; - #[allow(clippy::let_and_return)] fn validate_whence(whence: i32) -> bool { host_io::validate_whence(whence) } @@ -204,7 +203,7 @@ mod _io { )) } - #[derive(FromArgs)] + #[derive(Clone, Copy, FromArgs)] pub(super) struct OptionalSize { // In a few functions, the default value is -1 rather than None. // Make sure the default value doesn't affect compatibility. @@ -213,7 +212,6 @@ mod _io { } impl OptionalSize { - #[allow(clippy::wrong_self_convention)] pub(super) fn to_usize(self) -> Option { self.size?.to_usize() } @@ -2907,7 +2905,7 @@ mod _io { Ok(()) } - #[allow(clippy::type_complexity)] + #[expect(clippy::type_complexity, reason = "ignore warning for now")] fn find_coder( buffer: &PyObject, encoding: &str, diff --git a/crates/vm/src/stdlib/_winapi.rs b/crates/vm/src/stdlib/_winapi.rs index 3ceed26d6..5d4865f1f 100644 --- a/crates/vm/src/stdlib/_winapi.rs +++ b/crates/vm/src/stdlib/_winapi.rs @@ -77,11 +77,11 @@ mod _winapi { } /// CreateFile - Create or open a file or I/O device. - #[pyfunction] - #[allow( + #[expect( clippy::too_many_arguments, reason = "matches Win32 CreateFile parameter structure" )] + #[pyfunction] fn CreateFile( file_name: PyStrRef, desired_access: u32, diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index bcca77528..f17add068 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -653,7 +653,10 @@ pub(super) mod _os { match self.stat(self.stat_dir_fd(), FollowSymlinks(follow_symlinks), vm) { Ok(stat_obj) => { let st_mode: i32 = stat_obj.get_attr("st_mode", vm)?.try_into_value(vm)?; - #[allow(clippy::unnecessary_cast)] + #[allow( + clippy::unnecessary_cast, + reason = "'st_mode' and 'S_IFMT' are not u32 on all platforms" + )] Ok((st_mode as u32 & libc::S_IFMT as u32) == mode_bits) } Err(e) => { @@ -1244,6 +1247,7 @@ pub(super) mod _os { #[cfg(not(windows))] #[allow(clippy::useless_conversion, reason = "needed for 32-bit platforms")] let st_blksize = i64::from(stat.st_blksize); + #[cfg(not(windows))] #[allow(clippy::useless_conversion, reason = "needed for 32-bit platforms")] let st_blocks = i64::from(stat.st_blocks);