Merge pull request #1390

Author: Cobrand

changelog.md

@@ -8,6 +8,8 @@ when upgrading from a version of rust-sdl2 to another.
 
 [PR #1378](https://github.com/Rust-SDL2/rust-sdl2/pull/1378) **BREAKING CHANGE** Change `Keycode` to be a struct rather than an enum. Fix `Keycode::from_scancode` for non-QWERTY keyboard layouts.
 
+[PR #1390](https://github.com/Rust-SDL2/rust-sdl2/pull/1390) Apply clippy fixes, fix deprecations and other code quality improvements.
+
 [PR #1368](https://github.com/Rust-SDL2/rust-sdl2/pull/1368) Remove unnecessary unsafe in Window interface. Make Window `Clone`.
 
 [PR #1366](https://github.com/Rust-SDL2/rust-sdl2/pull/1366) Add Primary Selection bindings.
@@ -22,7 +24,7 @@ when upgrading from a version of rust-sdl2 to another.
 
 [PR #1250](https://github.com/Rust-SDL2/rust-sdl2/pull/1250) Add `lib64` to native library search path when using bundled feature
 
-[PR #1240](https://github.com/Rust-SDL2/rust-sdl2/pull/1240) **BREAKING CHANGE** Take `PixelMasks` by refrence
+[PR #1240](https://github.com/Rust-SDL2/rust-sdl2/pull/1240) **BREAKING CHANGE** Take `PixelMasks` by reference
 
 [PR #1254](https://github.com/Rust-SDL2/rust-sdl2/pull/1254) **BREAKING CHANGE** Make `SdlDrop` and `SubsystemDrop` safer; forbid external code from constructing `SdlDrop`
 
@@ -34,8 +36,6 @@ when upgrading from a version of rust-sdl2 to another.
 
 [PR #1337](https://github.com/Rust-SDL2/rust-sdl2/pull/1337) Fix "Cannot initialize Sdl from more than one thread" for tests / CI
 
-[PR #1337](https://github.com/Rust-SDL2/rust-sdl2/pull/1337) Fix "Cannot initialize Sdl from more than one thread" for tests / CI
-
 [PR #1346](https://github.com/Rust-SDL2/rust-sdl2/pull/1346) Add basic Shaped Window support
 
 [PR #1314](https://github.com/Rust-SDL2/rust-sdl2/pull/1314) Add "ALWAYS ON TOP" support for X11

sdl2-sys/build.rs

@@ -36,8 +36,8 @@ macro_rules! add_msvc_includes_to_bindings {
 fn init_submodule(sdl_path: &Path) {
     if !sdl_path.join("CMakeLists.txt").exists() {
         Command::new("git")
-            .args(&["submodule", "update", "--init"])
-            .current_dir(sdl_path.clone())
+            .args(["submodule", "update", "--init"])
+            .current_dir(sdl_path)
             .status()
             .expect("Git is needed to retrieve the SDL source files");
     }
@@ -436,15 +436,17 @@ fn copy_library_file(src_path: &Path, target_path: &Path) {
     for path in &[target_path, &deps_path] {
         let dst_path = path.join(src_path.file_name().expect("Path missing filename"));
 
-        fs::copy(&src_path, &dst_path).expect(&format!(
-            "Failed to copy SDL2 dynamic library from {} to {}",
-            src_path.to_string_lossy(),
-            dst_path.to_string_lossy()
-        ));
+        fs::copy(src_path, &dst_path).unwrap_or_else(|_| {
+            panic!(
+                "Failed to copy SDL2 dynamic library from {} to {}",
+                src_path.to_string_lossy(),
+                dst_path.to_string_lossy()
+            )
+        });
     }
 }
 
-fn copy_dynamic_libraries(sdl2_compiled_path: &PathBuf, target_os: &str) {
+fn copy_dynamic_libraries(sdl2_compiled_path: &Path, target_os: &str) {
     let target_path = find_cargo_target_dir();
 
     // Windows binaries do not embed library search paths, so successfully
@@ -651,35 +653,35 @@ fn generate_bindings(target: &str, host: &str, headers_paths: &[String]) {
     // Set correct target triple for bindgen when cross-compiling
     if target != host {
         bindings = bindings.clang_arg("-target");
-        bindings = bindings.clang_arg(target.clone());
+        bindings = bindings.clang_arg(target);
 
         if cfg!(feature = "image") {
             image_bindings = image_bindings.clang_arg("-target");
-            image_bindings = image_bindings.clang_arg(target.clone());
+            image_bindings = image_bindings.clang_arg(target);
         }
 
         if cfg!(feature = "ttf") {
             ttf_bindings = ttf_bindings.clang_arg("-target");
-            ttf_bindings = ttf_bindings.clang_arg(target.clone());
+            ttf_bindings = ttf_bindings.clang_arg(target);
         }
 
         if cfg!(feature = "mixer") {
             mixer_bindings = mixer_bindings.clang_arg("-target");
-            mixer_bindings = mixer_bindings.clang_arg(target.clone());
+            mixer_bindings = mixer_bindings.clang_arg(target);
         }
 
         if cfg!(feature = "gfx") {
             gfx_framerate_bindings = gfx_framerate_bindings.clang_arg("-target");
-            gfx_framerate_bindings = gfx_framerate_bindings.clang_arg(target.clone());
+            gfx_framerate_bindings = gfx_framerate_bindings.clang_arg(target);
 
             gfx_primitives_bindings = gfx_primitives_bindings.clang_arg("-target");
-            gfx_primitives_bindings = gfx_primitives_bindings.clang_arg(target.clone());
+            gfx_primitives_bindings = gfx_primitives_bindings.clang_arg(target);
 
             gfx_imagefilter_bindings = gfx_imagefilter_bindings.clang_arg("-target");
-            gfx_imagefilter_bindings = gfx_imagefilter_bindings.clang_arg(target.clone());
+            gfx_imagefilter_bindings = gfx_imagefilter_bindings.clang_arg(target);
 
             gfx_rotozoom_bindings = gfx_rotozoom_bindings.clang_arg("-target");
-            gfx_rotozoom_bindings = gfx_rotozoom_bindings.clang_arg(target.clone());
+            gfx_rotozoom_bindings = gfx_rotozoom_bindings.clang_arg(target);
         }
     }
 
@@ -943,5 +945,5 @@ fn generate_bindings(target: &str, host: &str, headers_paths: &[String]) {
 }
 
 fn get_os_from_triple(triple: &str) -> Option<&str> {
-    triple.splitn(3, "-").nth(2)
+    triple.splitn(3, '-').nth(2)
 }

src/sdl2/audio.rs

@@ -774,10 +774,7 @@ impl<'a, Channel: AudioFormatNum> AudioQueue<Channel> {
 
         let mut obtained = MaybeUninit::uninit();
         unsafe {
-            let device = match device.into() {
-                Some(device) => Some(CString::new(device).unwrap()),
-                None => None,
-            };
+            let device = device.into().map(|device| CString::new(device).unwrap());
             // Warning: map_or consumes its argument; `device.map_or()` would therefore consume the
             // CString and drop it, making device_ptr a dangling pointer! To avoid that we downgrade
             // device to an Option<&_> first.
@@ -801,7 +798,7 @@ impl<'a, Channel: AudioFormatNum> AudioQueue<Channel> {
                     Ok(AudioQueue {
                         subsystem: a.clone(),
                         device_id,
-                        phantom: PhantomData::default(),
+                        phantom: PhantomData,
                         spec,
                     })
                 }
@@ -850,7 +847,7 @@ impl<'a, Channel: AudioFormatNum> AudioQueue<Channel> {
             sys::SDL_QueueAudio(
                 self.device_id.id(),
                 data.as_ptr() as *const c_void,
-                (data.len() * mem::size_of::<Channel>()) as u32,
+                mem::size_of_val(data) as u32,
             )
         };
         result == 0
@@ -863,7 +860,7 @@ impl<'a, Channel: AudioFormatNum> AudioQueue<Channel> {
             sys::SDL_QueueAudio(
                 self.device_id.id(),
                 data.as_ptr() as *const c_void,
-                (data.len() * mem::size_of::<Channel>()) as u32,
+                mem::size_of_val(data) as u32,
             )
         };
         if result == 0 {
@@ -918,10 +915,7 @@ impl<CB: AudioCallback> AudioDevice<CB> {
 
         let mut obtained = MaybeUninit::uninit();
         unsafe {
-            let device = match device.into() {
-                Some(device) => Some(CString::new(device).unwrap()),
-                None => None,
-            };
+            let device = device.into().map(|device| CString::new(device).unwrap());
             // Warning: map_or consumes its argument; `device.map_or()` would therefore consume the
             // CString and drop it, making device_ptr a dangling pointer! To avoid that we downgrade
             // device to an Option<&_> first.

src/sdl2/common.rs

@@ -36,13 +36,4 @@ impl fmt::Display for IntegerOrSdlError {
     }
 }
 
-impl Error for IntegerOrSdlError {
-    fn description(&self) -> &str {
-        use self::IntegerOrSdlError::*;
-
-        match *self {
-            IntegerOverflows(_, _) => "integer overflow",
-            SdlError(ref e) => e,
-        }
-    }
-}
+impl Error for IntegerOrSdlError {}

src/sdl2/controller.rs

@@ -41,14 +41,10 @@ impl fmt::Display for AddMappingError {
 }
 
 impl error::Error for AddMappingError {
-    fn description(&self) -> &str {
-        use self::AddMappingError::*;
-
-        match *self {
-            InvalidMapping(_) => "invalid mapping",
-            InvalidFilePath(_) => "invalid file path",
-            ReadError(_) => "read error",
-            SdlError(ref e) => e,
+    fn source(&self) -> Option<&(dyn error::Error + 'static)> {
+        match self {
+            Self::InvalidMapping(err) => Some(err),
+            Self::InvalidFilePath(_) | Self::ReadError(_) | Self::SdlError(_) => None,
         }
     }
 }
@@ -126,9 +122,7 @@ impl GameControllerSubsystem {
     /// Return `true` if controller events are processed.
     #[doc(alias = "SDL_GameControllerEventState")]
     pub fn event_state(&self) -> bool {
-        unsafe {
-            sys::SDL_GameControllerEventState(sys::SDL_QUERY as i32) == sys::SDL_ENABLE as i32
-        }
+        unsafe { sys::SDL_GameControllerEventState(sys::SDL_QUERY) == sys::SDL_ENABLE as i32 }
     }
 
     /// Add a new controller input mapping from a mapping string.
@@ -172,7 +166,7 @@ impl GameControllerSubsystem {
 
     /// Load controller input mappings from an SDL [`RWops`] object.
     #[doc(alias = "SDL_GameControllerAddMappingsFromRW")]
-    pub fn load_mappings_from_rw<'a>(&self, rw: RWops<'a>) -> Result<i32, AddMappingError> {
+    pub fn load_mappings_from_rw(&self, rw: RWops<'_>) -> Result<i32, AddMappingError> {
         use self::AddMappingError::*;
 
         let result = unsafe { sys::SDL_GameControllerAddMappingsFromRW(rw.raw(), 0) };