Skip to content

Commit

Permalink
chore: 🤖 setup Clippy lint (#121)
Browse files Browse the repository at this point in the history
* chore: 🤖 setup Clippy lint

* chore: 🤖 fix linting issues

* chore: 🤖 add clippy to makefile

* chore: 🤖 uncomment back the build
  • Loading branch information
theashraf authored May 30, 2024
1 parent d066ab5 commit 77e2661
Show file tree
Hide file tree
Showing 11 changed files with 75 additions and 61 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/check-pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
9 changes: 9 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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!"
Expand Down Expand Up @@ -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
2 changes: 2 additions & 0 deletions dotlottie-fms/src/dolottie_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, DotLottieError> {
let mut i = 0;
let new_active_animation_id: String;
Expand All @@ -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<String, DotLottieError> {
let new_active_animation_id: String;
let mut i = 0;
Expand Down
32 changes: 11 additions & 21 deletions dotlottie-rs/src/dotlottie_player.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
}

Expand Down
16 changes: 9 additions & 7 deletions dotlottie-rs/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,20 @@ pub struct Layout {
pub align: Vec<f32>,
}

impl Layout {
pub fn new(fit: Fit, align: Vec<f32>) -> 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<f32>) -> Self {
Self {
fit: Fit::Contain,
align: vec![0.5, 0.5],
fit,
align: validate_normalize_align(align),
}
}

Expand Down
18 changes: 12 additions & 6 deletions dotlottie-rs/src/lottie_renderer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -115,13 +121,13 @@ impl LottieRenderer {
pub fn total_frames(&self) -> Result<f32, LottieRendererError> {
self.thorvg_animation
.get_total_frame()
.map_err(|e| LottieRendererError::ThorvgError(e))
.map_err(LottieRendererError::ThorvgError)
}

pub fn duration(&self) -> Result<f32, LottieRendererError> {
self.thorvg_animation
.get_duration()
.map_err(|e| LottieRendererError::ThorvgError(e))
.map_err(LottieRendererError::ThorvgError)
}

pub fn current_frame(&self) -> f32 {
Expand Down Expand Up @@ -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!(
Expand All @@ -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(),
));
Expand Down Expand Up @@ -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> {
Expand Down
32 changes: 21 additions & 11 deletions dotlottie-rs/src/thorvg.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
}),
}
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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() };
Expand Down Expand Up @@ -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<f32, TvgError> {
Expand All @@ -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> {
Expand All @@ -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> {
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 2 additions & 6 deletions dotlottie-rs/tests/frame_interpolation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
6 changes: 2 additions & 4 deletions dotlottie-rs/tests/markers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion dotlottie-rs/tests/play.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
6 changes: 1 addition & 5 deletions dotlottie-rs/tests/theming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;

Expand Down

0 comments on commit 77e2661

Please sign in to comment.