diff --git a/ci/shell.nix b/ci/shell.nix index 10d8a29c9..ed0f31ccc 100644 --- a/ci/shell.nix +++ b/ci/shell.nix @@ -29,8 +29,8 @@ let "thumbv7m-none-eabi" # T1 ]; # we use rustfmt from nixpkgs because it's built with the nighly flag needed for wrap_comments - # to use official binary, remove rustfmt from buildInputs below and uncomment next line: - # extensions = [ "rustfmt" ]; + # to use official binary, remove rustfmt from buildInputs and add it to extensions: + extensions = [ "clippy" ]; }; in with nixpkgs; diff --git a/ci/test.yml b/ci/test.yml index 32a42cbfc..77c5d97ae 100644 --- a/ci/test.yml +++ b/ci/test.yml @@ -9,6 +9,7 @@ core unit test: script: - nix-shell --run "poetry run make -C core test | ts -s" - nix-shell --run "poetry run make -C core test_rust | ts -s" + - nix-shell --run "poetry run make -C core clippy | ts -s" core device ui test: stage: test diff --git a/core/Makefile b/core/Makefile index 265a36663..72655529d 100644 --- a/core/Makefile +++ b/core/Makefile @@ -118,6 +118,9 @@ mypy: src/apps/webauthn \ src/trezor/ui +clippy: + cd embed/rust ; cargo clippy + ## code generation: templates: ## render Mako templates (for lists of coins, tokens, etc.) diff --git a/core/embed/rust/build.rs b/core/embed/rust/build.rs index 56230f12e..8f21e222c 100644 --- a/core/embed/rust/build.rs +++ b/core/embed/rust/build.rs @@ -150,7 +150,7 @@ fn generate_micropython_bindings() { .lines() .skip_while(|s| !s.contains("search starts here:")) .take_while(|s| !s.contains("End of search list.")) - .filter(|s| s.starts_with(" ")) + .filter(|s| s.starts_with(' ')) .map(|s| format!("-I{}", s.trim())); bindings = bindings.clang_args(include_args); diff --git a/core/embed/rust/src/error.rs b/core/embed/rust/src/error.rs index 7ad5a8aca..1497ad8e1 100644 --- a/core/embed/rust/src/error.rs +++ b/core/embed/rust/src/error.rs @@ -4,6 +4,7 @@ use cstr_core::CStr; use crate::micropython::{ffi, obj::Obj, qstr::Qstr}; #[derive(Debug)] +#[allow(clippy::enum_variant_names)] pub enum Error { TypeError, OutOfRange, @@ -22,7 +23,7 @@ impl Error { /// exception, because the object is not guaranteed to remain intact. /// Micropython might reuse the same space for creating a different /// exception. - pub unsafe fn to_obj(self) -> Obj { + pub unsafe fn into_obj(self) -> Obj { unsafe { // SAFETY: // - first argument is a reference to a valid exception type @@ -34,7 +35,7 @@ impl Error { Error::AllocationFailed => ffi::mp_obj_new_exception(&ffi::mp_type_MemoryError), Error::CaughtException(obj) => obj, Error::KeyError(key) => { - ffi::mp_obj_new_exception_args(&ffi::mp_type_KeyError, 1, &key.into()) + ffi::mp_obj_new_exception_args(&ffi::mp_type_KeyError, 1, &key) } Error::ValueError(msg) => { if let Ok(msg) = msg.try_into() { @@ -45,7 +46,7 @@ impl Error { } Error::ValueErrorParam(msg, param) => { if let Ok(msg) = msg.try_into() { - let args: [Obj; 2] = [msg, param.into()]; + let args: [Obj; 2] = [msg, param]; ffi::mp_obj_new_exception_args(&ffi::mp_type_ValueError, 2, args.as_ptr()) } else { ffi::mp_obj_new_exception(&ffi::mp_type_ValueError) diff --git a/core/embed/rust/src/lib.rs b/core/embed/rust/src/lib.rs index 8f4eaf7c3..19ff60e81 100644 --- a/core/embed/rust/src/lib.rs +++ b/core/embed/rust/src/lib.rs @@ -30,5 +30,5 @@ fn panic(_info: &PanicInfo) -> ! { // TODO: Ideally we would take the file and line info out of // `PanicInfo::location()`. - trezorhal::common::fatal_error(&empty, &msg, &empty, 0, &empty); + trezorhal::common::fatal_error(empty, msg, empty, 0, empty); } diff --git a/core/embed/rust/src/micropython/func.rs b/core/embed/rust/src/micropython/func.rs index 038157555..b1d573c29 100644 --- a/core/embed/rust/src/micropython/func.rs +++ b/core/embed/rust/src/micropython/func.rs @@ -4,7 +4,7 @@ pub type Func = ffi::mp_obj_fun_builtin_fixed_t; impl Func { /// Convert a "static const" function to a MicroPython object. - pub const fn to_obj(&'static self) -> Obj { + pub const fn as_obj(&'static self) -> Obj { // SAFETY: // - We are an object struct with a base and a type. // - 'static lifetime holds us in place. diff --git a/core/embed/rust/src/micropython/map.rs b/core/embed/rust/src/micropython/map.rs index 895d35d4e..81ae02d71 100644 --- a/core/embed/rust/src/micropython/map.rs +++ b/core/embed/rust/src/micropython/map.rs @@ -40,7 +40,7 @@ impl Map { } impl Map { - pub fn from_fixed<'a>(table: &'a [MapElem]) -> MapRef<'a> { + pub fn from_fixed(table: &[MapElem]) -> MapRef { let mut map = MaybeUninit::uninit(); // SAFETY: `mp_map_init_fixed_table` completely initializes all fields of `map`. unsafe { diff --git a/core/embed/rust/src/micropython/obj.rs b/core/embed/rust/src/micropython/obj.rs index 8305f8a75..28b865470 100644 --- a/core/embed/rust/src/micropython/obj.rs +++ b/core/embed/rust/src/micropython/obj.rs @@ -129,7 +129,8 @@ impl TryFrom for bool { // SAFETY: // - `obj` can be anything uPy understands. // EXCEPTION: Can call Python code (on custom instances) and therefore raise. - if catch_exception(|| unsafe { ffi::mp_obj_is_true(obj) })? { + let result = catch_exception(|| unsafe { ffi::mp_obj_is_true(obj) })?; + if result { Ok(true) } else { Ok(false) diff --git a/core/embed/rust/src/micropython/runtime.rs b/core/embed/rust/src/micropython/runtime.rs index 2bcb22bc7..c33bb40b5 100644 --- a/core/embed/rust/src/micropython/runtime.rs +++ b/core/embed/rust/src/micropython/runtime.rs @@ -14,8 +14,8 @@ pub unsafe fn raise_exception(err: Error) -> ! { unsafe { // SAFETY: // - argument must be an exception instance - // (err.to_obj() should return the right thing) - ffi::nlr_jump(err.to_obj().as_ptr()); + // (err.into_obj() should return the right thing) + ffi::nlr_jump(err.into_obj().as_ptr()); } panic!(); } diff --git a/core/embed/rust/src/micropython/typ.rs b/core/embed/rust/src/micropython/typ.rs index 4666ed562..79c7dc455 100644 --- a/core/embed/rust/src/micropython/typ.rs +++ b/core/embed/rust/src/micropython/typ.rs @@ -19,7 +19,7 @@ impl Type { } } - pub fn to_base(&'static self) -> ObjBase { + pub fn as_base(&'static self) -> ObjBase { ObjBase { type_: self } } } diff --git a/core/embed/rust/src/protobuf/defs.rs b/core/embed/rust/src/protobuf/defs.rs index eb6188535..ea98f4006 100644 --- a/core/embed/rust/src/protobuf/defs.rs +++ b/core/embed/rust/src/protobuf/defs.rs @@ -126,8 +126,7 @@ pub fn find_name_by_msg_offset(msg_offset: u16) -> Result { name_defs .iter() - .filter(|def| def.msg_offset == msg_offset) - .next() + .find(|def| def.msg_offset == msg_offset) .map(|def| def.msg_name) .ok_or_else(|| Error::KeyError(msg_offset.into())) } diff --git a/core/embed/rust/src/protobuf/encode.rs b/core/embed/rust/src/protobuf/encode.rs index b83bee8a9..28f162bd5 100644 --- a/core/embed/rust/src/protobuf/encode.rs +++ b/core/embed/rust/src/protobuf/encode.rs @@ -29,7 +29,7 @@ pub extern "C" fn protobuf_len(obj: Obj) -> Obj { Encoder.encode_message(stream, &obj.def(), &obj)?; - Ok(stream.len.try_into()?) + stream.len.try_into() }; unsafe { util::try_or_raise(block) } } @@ -46,7 +46,7 @@ pub extern "C" fn protobuf_encode(buf: Obj, obj: Obj) -> Obj { Encoder.encode_message(stream, &obj.def(), &obj)?; - Ok(stream.len().try_into()?) + stream.len().try_into() }; unsafe { util::try_or_raise(block) } } diff --git a/core/embed/rust/src/protobuf/obj.rs b/core/embed/rust/src/protobuf/obj.rs index d699826fc..82c5ef019 100644 --- a/core/embed/rust/src/protobuf/obj.rs +++ b/core/embed/rust/src/protobuf/obj.rs @@ -28,7 +28,7 @@ pub struct MsgObj { impl MsgObj { pub fn alloc_with_capacity(capacity: usize, msg: &MsgDef) -> Result, Error> { Gc::new(Self { - base: Self::obj_type().to_base(), + base: Self::obj_type().as_base(), map: Map::with_capacity(capacity)?, msg_wire_id: msg.wire_id, msg_offset: msg.offset, @@ -100,13 +100,13 @@ impl MsgObj { } } -impl Into for Gc { - fn into(self) -> Obj { +impl From> for Obj { + fn from(value: Gc) -> Self { // SAFETY: - // - We are GC-allocated. - // - We are `repr(C)`. - // - We have a `base` as the first field with the correct type. - unsafe { Obj::from_ptr(Self::into_raw(self).cast()) } + // - `value` is GC-allocated. + // - `value` is `repr(C)`. + // - `value` has a `base` as the first field with the correct type. + unsafe { Self::from_ptr(Gc::into_raw(value).cast()) } } } @@ -155,7 +155,7 @@ pub struct MsgDefObj { impl MsgDefObj { pub fn alloc(def: MsgDef) -> Result, Error> { let this = Gc::new(Self { - base: Self::obj_type().to_base(), + base: Self::obj_type().as_base(), def, })?; Ok(this) @@ -175,13 +175,13 @@ impl MsgDefObj { } } -impl Into for Gc { - fn into(self) -> Obj { +impl From> for Obj { + fn from(value: Gc) -> Self { // SAFETY: - // - We are GC-allocated. - // - We are `repr(C)`. - // - We have a `base` as the first field with the correct type. - unsafe { Obj::from_ptr(Self::into_raw(self).cast()) } + // - `value` is GC-allocated. + // - `value` is `repr(C)`. + // - `value` has a `base` as the first field with the correct type. + unsafe { Self::from_ptr(Gc::into_raw(value).cast()) } } } @@ -234,7 +234,7 @@ unsafe extern "C" fn msg_def_obj_attr(self_in: Obj, attr: ffi::qstr, dest: *mut // dest[0] = function_obj // dest[1] = self unsafe { - dest.write(MSG_DEF_OBJ_IS_TYPE_OF_OBJ.to_obj()); + dest.write(MSG_DEF_OBJ_IS_TYPE_OF_OBJ.as_obj()); dest.offset(1).write(self_in); } }