From 77e266193d2da6451aadb6b421288beac7ae7c14 Mon Sep 17 00:00:00 2001 From: Abdelrahman Ashraf Date: Thu, 30 May 2024 14:48:13 +0700 Subject: [PATCH] =?UTF-8?q?chore:=20=F0=9F=A4=96=20setup=20Clippy=20lint?= =?UTF-8?q?=20(#121)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: 🤖 setup Clippy lint * chore: 🤖 fix linting issues * chore: 🤖 add clippy to makefile * chore: 🤖 uncomment back the build --- .github/workflows/check-pr.yaml | 5 ++++ Makefile | 9 +++++++ dotlottie-fms/src/dolottie_manager.rs | 2 ++ dotlottie-rs/src/dotlottie_player.rs | 32 ++++++++--------------- dotlottie-rs/src/layout.rs | 16 +++++++----- dotlottie-rs/src/lottie_renderer/mod.rs | 18 ++++++++----- dotlottie-rs/src/thorvg.rs | 32 +++++++++++++++-------- dotlottie-rs/tests/frame_interpolation.rs | 8 ++---- dotlottie-rs/tests/markers.rs | 6 ++--- dotlottie-rs/tests/play.rs | 2 +- dotlottie-rs/tests/theming.rs | 6 +---- 11 files changed, 75 insertions(+), 61 deletions(-) diff --git a/.github/workflows/check-pr.yaml b/.github/workflows/check-pr.yaml index 818d5f84..59397026 100644 --- a/.github/workflows/check-pr.yaml +++ b/.github/workflows/check-pr.yaml @@ -29,3 +29,8 @@ jobs: run: make demo-player - name: Run Tests run: make test + - name: Run Clippy + run: make clippy + # Make sure CI fails on all warnings, including Clippy lints + env: + RUSTFLAGS: "-Dwarnings" diff --git a/Makefile b/Makefile index 1bb9f441..9d1f7de4 100644 --- a/Makefile +++ b/Makefile @@ -953,6 +953,14 @@ bench: cargo bench --manifest-path $(FMS)/Cargo.toml cargo bench --manifest-path $(RUNTIME_FFI)/Cargo.toml +.PHONY: clippy +clippy: + $(info $(YELLOW)Running clippy for workspace$(NC)) + cargo clippy --manifest-path $(CORE)/Cargo.toml --all-targets --all-features + # fms has a lot of clippy warnings and errors, so we're ignoring them for now + # cargo clippy --manifest-path $(FMS)/Cargo.toml --all-targets --all-features + cargo clippy --manifest-path $(RUNTIME_FFI)/Cargo.toml --all-targets --all-features + .PHONY: help help: @echo "Welcome to the $(GREEN)dotlottie-player$(NC) build system!" @@ -994,5 +1002,6 @@ help: @echo " - $(YELLOW)distclean$(NC) - clean up everything" @echo " - $(YELLOW)test$(NC) - run all tests" @echo " - $(YELLOW)bench$(NC) - run all benchmarks" + @echo " - $(YELLOW)clippy$(NC) - run clippy on all projects" @echo @echo diff --git a/dotlottie-fms/src/dolottie_manager.rs b/dotlottie-fms/src/dolottie_manager.rs index b629ff7f..1bfdf7ab 100644 --- a/dotlottie-fms/src/dolottie_manager.rs +++ b/dotlottie-fms/src/dolottie_manager.rs @@ -79,6 +79,7 @@ impl DotLottieManager { } /// Advances to the next animation and returns it's animation data as a string. + #[allow(dead_code)] fn next_animation(&mut self) -> Result { let mut i = 0; let new_active_animation_id: String; @@ -104,6 +105,7 @@ impl DotLottieManager { } /// Reverses to the previous animation and returns it's animation data as a string. + #[allow(dead_code)] fn previous_animation(&mut self) -> Result { let new_active_animation_id: String; let mut i = 0; diff --git a/dotlottie-rs/src/dotlottie_player.rs b/dotlottie-rs/src/dotlottie_player.rs index 6d53b2aa..4bef9ee4 100644 --- a/dotlottie-rs/src/dotlottie_player.rs +++ b/dotlottie-rs/src/dotlottie_player.rs @@ -165,24 +165,15 @@ impl DotLottieRuntime { } pub fn is_playing(&self) -> bool { - match self.playback_state { - PlaybackState::Playing => true, - _ => false, - } + matches!(self.playback_state, PlaybackState::Playing) } pub fn is_paused(&self) -> bool { - match self.playback_state { - PlaybackState::Paused => true, - _ => false, - } + matches!(self.playback_state, PlaybackState::Paused) } pub fn is_stopped(&self) -> bool { - match self.playback_state { - PlaybackState::Stopped => true, - _ => false, - } + matches!(self.playback_state, PlaybackState::Stopped) } pub fn play(&mut self) -> bool { @@ -555,11 +546,11 @@ impl DotLottieRuntime { } fn flip_direction_if_needed(&mut self, new_mode: Mode) { - let should_flip = match (new_mode, self.direction) { + let should_flip = matches!( + (new_mode, self.direction), (Mode::Forward | Mode::Bounce, Direction::Reverse) - | (Mode::Reverse | Mode::ReverseBounce, Direction::Forward) => true, - _ => false, - }; + | (Mode::Reverse | Mode::ReverseBounce, Direction::Forward) + ); if should_flip { self.direction = self.direction.flip(); @@ -568,14 +559,13 @@ impl DotLottieRuntime { } fn update_background_color(&mut self, new_config: &Config) { - if self.config.background_color != new_config.background_color { - if self + if self.config.background_color != new_config.background_color + && self .renderer .set_background_color(new_config.background_color) .is_ok() - { - self.config.background_color = new_config.background_color; - } + { + self.config.background_color = new_config.background_color; } } diff --git a/dotlottie-rs/src/layout.rs b/dotlottie-rs/src/layout.rs index c14cf9ea..f2ebec60 100644 --- a/dotlottie-rs/src/layout.rs +++ b/dotlottie-rs/src/layout.rs @@ -14,18 +14,20 @@ pub struct Layout { pub align: Vec, } -impl Layout { - pub fn new(fit: Fit, align: Vec) -> Self { +impl Default for Layout { + fn default() -> Self { Self { - fit, - align: validate_normalize_align(align), + fit: Fit::Contain, + align: vec![0.5, 0.5], } } +} - pub fn default() -> Self { +impl Layout { + pub fn new(fit: Fit, align: Vec) -> Self { Self { - fit: Fit::Contain, - align: vec![0.5, 0.5], + fit, + align: validate_normalize_align(align), } } diff --git a/dotlottie-rs/src/lottie_renderer/mod.rs b/dotlottie-rs/src/lottie_renderer/mod.rs index 8f9c0230..4cca5921 100644 --- a/dotlottie-rs/src/lottie_renderer/mod.rs +++ b/dotlottie-rs/src/lottie_renderer/mod.rs @@ -28,6 +28,12 @@ pub struct LottieRenderer { layout: Layout, } +impl Default for LottieRenderer { + fn default() -> Self { + Self::new() + } +} + impl LottieRenderer { pub fn new() -> Self { let thorvg_canvas = Canvas::new(TvgEngine::TvgEngineSw, 0); @@ -115,13 +121,13 @@ impl LottieRenderer { pub fn total_frames(&self) -> Result { self.thorvg_animation .get_total_frame() - .map_err(|e| LottieRendererError::ThorvgError(e)) + .map_err(LottieRendererError::ThorvgError) } pub fn duration(&self) -> Result { self.thorvg_animation .get_duration() - .map_err(|e| LottieRendererError::ThorvgError(e)) + .map_err(LottieRendererError::ThorvgError) } pub fn current_frame(&self) -> f32 { @@ -156,7 +162,7 @@ impl LottieRenderer { let total_frames = self .thorvg_animation .get_total_frame() - .map_err(|e| LottieRendererError::ThorvgError(e))?; + .map_err(LottieRendererError::ThorvgError)?; if no < 0.0 || no >= total_frames { return Err(LottieRendererError::InvalidArgument(format!( @@ -179,7 +185,7 @@ impl LottieRenderer { return Ok(()); } - if width <= 0 || height <= 0 { + if width.max(0) == 0 || height.max(0) == 0 { return Err(LottieRendererError::InvalidArgument( "Width and height must be greater than 0".to_string(), )); @@ -240,13 +246,13 @@ impl LottieRenderer { self.thorvg_background_shape .fill((red, green, blue, alpha)) - .map_err(|e| LottieRendererError::ThorvgError(e)) + .map_err(LottieRendererError::ThorvgError) } pub fn load_theme_data(&mut self, slots: &str) -> Result<(), LottieRendererError> { self.thorvg_animation .set_slots(slots) - .map_err(|e| LottieRendererError::ThorvgError(e)) + .map_err(LottieRendererError::ThorvgError) } pub fn set_layout(&mut self, layout: &Layout) -> Result<(), LottieRendererError> { diff --git a/dotlottie-rs/src/thorvg.rs b/dotlottie-rs/src/thorvg.rs index 08505e6f..5bc712d9 100644 --- a/dotlottie-rs/src/thorvg.rs +++ b/dotlottie-rs/src/thorvg.rs @@ -1,5 +1,6 @@ #![allow(non_upper_case_globals)] #![allow(non_snake_case)] +#![allow(non_camel_case_types)] use std::{ffi::CString, ptr}; use thiserror::Error; @@ -59,7 +60,10 @@ fn convert_tvg_result(result: Tvg_Result, function_name: &str) -> Result<(), Tvg Tvg_Result_TVG_RESULT_NOT_SUPPORTED => Err(TvgError::NotSupported { function_name: func_name, }), - Tvg_Result_TVG_RESULT_UNKNOWN | _ => Err(TvgError::Unknown { + Tvg_Result_TVG_RESULT_UNKNOWN => Err(TvgError::Unknown { + function_name: func_name, + }), + _ => Err(TvgError::Unknown { function_name: func_name, }), } @@ -93,13 +97,7 @@ impl Canvas { } } - pub fn set_viewport( - &mut self, - x: i32, - y: i32, - w: i32, - h: i32, - ) -> Result<(), TvgError> { + pub fn set_viewport(&mut self, x: i32, y: i32, w: i32, h: i32) -> Result<(), TvgError> { let result = unsafe { tvg_canvas_set_viewport(self.raw_canvas, x, y, w, h) }; convert_tvg_result(result, "tvg_canvas_set_viewport") @@ -179,6 +177,12 @@ pub struct Animation { raw_paint: *mut Tvg_Paint, } +impl Default for Animation { + fn default() -> Self { + Self::new() + } +} + impl Animation { pub fn new() -> Self { let raw_animation = unsafe { tvg_animation_new() }; @@ -253,7 +257,7 @@ impl Animation { convert_tvg_result(result, "tvg_animation_get_total_frame")?; - return Ok(total_frame); + Ok(total_frame) } pub fn get_duration(&self) -> Result { @@ -264,7 +268,7 @@ impl Animation { convert_tvg_result(result, "tvg_animation_get_duration")?; - return Ok(duration); + Ok(duration) } pub fn set_frame(&mut self, frame_no: f32) -> Result<(), TvgError> { @@ -280,7 +284,7 @@ impl Animation { convert_tvg_result(result, "tvg_animation_get_frame")?; - return Ok(curr_frame); + Ok(curr_frame) } pub fn set_slots(&mut self, slots: &str) -> Result<(), TvgError> { @@ -313,6 +317,12 @@ pub struct Shape { raw_shape: *mut Tvg_Paint, } +impl Default for Shape { + fn default() -> Self { + Self::new() + } +} + impl Shape { pub fn new() -> Self { Shape { diff --git a/dotlottie-rs/tests/frame_interpolation.rs b/dotlottie-rs/tests/frame_interpolation.rs index 815ddbb7..d1f8e4ae 100644 --- a/dotlottie-rs/tests/frame_interpolation.rs +++ b/dotlottie-rs/tests/frame_interpolation.rs @@ -53,12 +53,8 @@ mod tests { assert_eq!(rendered_frames[rendered_frames.len() - 1], total_frames); - for i in 0..rendered_frames.len() { - assert!( - rendered_frames[i] == rendered_frames[i].floor(), - "Frame {} is interpolated.", - i - ); + for (i, frame) in rendered_frames.iter().enumerate() { + assert!(*frame == frame.floor(), "Frame {} is interpolated.", i); } } } diff --git a/dotlottie-rs/tests/markers.rs b/dotlottie-rs/tests/markers.rs index 6ccb7e68..ebed04c3 100644 --- a/dotlottie-rs/tests/markers.rs +++ b/dotlottie-rs/tests/markers.rs @@ -39,8 +39,7 @@ mod tests { assert_eq!(actual_markers.len(), 4); - let expected_markers = vec![ - Marker { + let expected_markers = [Marker { name: "Marker_1".to_string(), time: 0.0, duration: 10.0, @@ -59,8 +58,7 @@ mod tests { name: "Marker_4".to_string(), time: 30.0, duration: 12.0, - }, - ]; + }]; for marker in actual_markers { let expected = expected_markers diff --git a/dotlottie-rs/tests/play.rs b/dotlottie-rs/tests/play.rs index dd2209b1..7091b167 100644 --- a/dotlottie-rs/tests/play.rs +++ b/dotlottie-rs/tests/play.rs @@ -156,6 +156,6 @@ mod tests { } } - assert_eq!((rendered_frames[0] - mid_frame).abs() <= 1.0, true); + assert!((rendered_frames[0] - mid_frame).abs() <= 1.0); } } diff --git a/dotlottie-rs/tests/theming.rs b/dotlottie-rs/tests/theming.rs index a2542002..fe3ca9d3 100644 --- a/dotlottie-rs/tests/theming.rs +++ b/dotlottie-rs/tests/theming.rs @@ -5,11 +5,7 @@ use crate::test_utils::{HEIGHT, WIDTH}; #[cfg(test)] mod tests { - use std::{ - fs::{self, File}, - io::Read, - path::Path, - }; + use super::*;