Fix all clippy warnings, and deny them in CI

* Fix all clippy lints

Also deny clippy warnings in CI

* Changelog

* Fix lints left out

* Changelog: specify that clippy denies warnings in CI

* Update PR template with clippy recommendations

.github/pull_request_template.md

@@ -4,7 +4,7 @@
 # Checklist
   - [ ] `CHANGELOG.md` for the BSP or HAL updated
   - [ ] All new or modified code is well documented, especially public items
-  - [ ] No new warnings or clippy suggestions have been introduced (see CI or check locally)
+  - [ ] No new warnings or clippy suggestions have been introduced - CI will **deny** clippy warnings by default! You may `#[allow]` certain lints where reasonable, but ideally justify those with a short comment. 
 
 ## If Adding a new Board
   - [ ] Board CI added to `crates.json`

.github/workflows/build-hal.yml

@@ -38,12 +38,4 @@ jobs:
         set -ex
         features=$(cat ./crates.json | jq -Mr --arg pac "${{matrix.pac}}" -c '.hal_build_variants["${{matrix.pac}}"].features | join(",")')
         target=$(cat ./crates.json | jq -Mr --arg pac "${{matrix.pac}}" -c '.hal_build_variants["${{matrix.pac}}"].target')
-        cargo build --features=${features} --target=${target} --manifest-path=./hal/Cargo.toml
-
-    - name: Clippy HAL for ${{ matrix.pac }}
-      if: ${{ matrix.toolchain == 'nightly' }}
-      run: |
-        set -ex
-        features=$(cat ./crates.json | jq -Mr --arg pac "${{matrix.pac}}" -c '.hal_build_variants["${{matrix.pac}}"].features | join(",")')
-        target=$(cat ./crates.json | jq -Mr --arg pac "${{matrix.pac}}" -c '.hal_build_variants["${{matrix.pac}}"].target')
-        cargo clippy --features=${features} --target=${target} --manifest-path=./hal/Cargo.toml
+        cargo clippy --features=${features} --target=${target} --manifest-path=./hal/Cargo.toml -- -D warnings

hal/CHANGELOG.md

@@ -1,5 +1,7 @@
 # Unreleased Changes
 
+- CI/CD pipeline now uses `cargo clippy` instead of `cargo build` and denies clippy warnings by default
+- Fix HAL clippy lints
 - Add compile error for combined `library` and `dma` features
 - Add `dma` feature to docs metadata
 - Update the PACs to svd2rust 0.30.2.

hal/src/gpio/dynpin.rs

@@ -57,6 +57,8 @@
 //! operation, the trait functions will return
 //! [`InvalidPinType`](Error::InvalidPinType).
 
+#![allow(clippy::bool_comparison)]
+
 use core::convert::TryFrom;
 
 use paste::paste;

hal/src/gpio/pin.rs

@@ -94,6 +94,7 @@
 //! [`AnyKind`]: crate::typelevel#anykind-trait-pattern
 
 #![allow(clippy::zero_prefixed_literal)]
+#![allow(clippy::bool_comparison)]
 
 use core::convert::Infallible;
 use core::marker::PhantomData;

hal/src/lib.rs

@@ -104,4 +104,3 @@ pub use crate::thumbv7em::*;
 
 #[macro_use]
 mod bsp_peripherals_macro;
-pub use bsp_peripherals_macro::*;

hal/src/rtc.rs

@@ -455,7 +455,7 @@ impl TimerParams {
                                               * (rust-lang/rust#51999) */
         };
 
-        let cycles: u32 = ticks / divider_value as u32;
+        let cycles: u32 = ticks / divider_value;
 
         TimerParams { divider, cycles }
     }

hal/src/sercom/spi.rs

@@ -721,6 +721,8 @@ where
 
     /// Obtain a reference to the PAC `SERCOM` struct
     ///
+    /// # Safety
+    ///
     /// Directly accessing the `SERCOM` could break the invariants of the
     /// type-level tracking in this module, so it is unsafe.
     #[inline]
@@ -1241,6 +1243,8 @@ where
 
     /// Read from the DATA register
     ///
+    /// # Safety
+    ///
     /// Reading from the data register directly is `unsafe`, because it will
     /// clear the RXC flag, which could break assumptions made elsewhere in
     /// this module.
@@ -1251,6 +1255,8 @@ where
 
     /// Write to the DATA register
     ///
+    /// # Safety
+    ///
     /// Writing to the data register directly is `unsafe`, because it will clear
     /// the DRE flag, which could break assumptions made elsewhere in this
     /// module.

hal/src/sercom/spi_future.rs

@@ -489,6 +489,8 @@ where
         if self.rcvd < self.sent {
             let buf = unsafe { buf.get_unchecked_mut(self.rcvd..) };
             let mut data = buf.iter_mut();
+            // Allow this lint as it will put out a warning on thumbv7em but not thumbv6m
+            #[allow(clippy::unnecessary_cast)]
             let word = unsafe { self.spi.as_mut().read_data() as u32 };
             let bytes = word.to_le_bytes();
             let mut iter = bytes.iter();
@@ -544,6 +546,8 @@ where
     /// Consume the [`SpiFuture`] and free its components without checking for
     /// completion
     ///
+    /// # Safety
+    ///
     /// Ending the transaction prematurely could leave the [`Spi`] in an
     /// inconsistent state. It is not safe to call this function unless the
     /// transaction is complete.

hal/src/sercom/uart/config.rs

@@ -88,7 +88,7 @@ impl<P: ValidPads> Config<P> {
 
         // Enable internal clock mode
         registers.configure_mode();
-        registers.configure_pads(P::RXPO as u8, P::TXPO as u8);
+        registers.configure_pads(P::RXPO, P::TXPO);
         registers.set_char_size(EightBit::SIZE);
 
         Self {

hal/src/thumbv6m/usb/bus.rs

@@ -24,8 +24,9 @@ use usb_device::{Result as UsbResult, UsbDirection, UsbError};
 
 /// EndpointTypeBits represents valid values for the EPTYPE fields in
 /// the EPCFGn registers.
-#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+#[derive(Debug, Default, PartialEq, Eq, Clone, Copy)]
 pub enum EndpointTypeBits {
+    #[default]
     Disabled = 0,
     Control = 1,
     Isochronous = 2,
@@ -35,12 +36,6 @@ pub enum EndpointTypeBits {
     DualBank = 5,
 }
 
-impl Default for EndpointTypeBits {
-    fn default() -> Self {
-        EndpointTypeBits::Disabled
-    }
-}
-
 impl From<EndpointType> for EndpointTypeBits {
     fn from(ep_type: EndpointType) -> EndpointTypeBits {
         match ep_type {

hal/src/thumbv7em/aes.rs

@@ -304,7 +304,7 @@ impl Aes {
     /// Enable-protected register
     #[inline]
     fn ctrla(&self) -> &CTRLA {
-        &(*self.aes()).ctrla
+        &self.aes().ctrla
     }
 
     /// Control B

hal/src/thumbv7em/can.rs

@@ -73,6 +73,7 @@ impl<ID: PclkId + AhbId, PS: PclkSourceId, RX, TX, CAN> Dependencies<ID, PS, RX,
     /// Destroy an instance of `Dependencies` struct.
     ///
     /// Releases all enclosed objects back to the user.
+    #[allow(clippy::type_complexity)]
     pub fn free<S>(self, gclk0: S) -> (Pclk<ID, PS>, HertzU32, AhbClk<ID>, RX, TX, CAN, S::Dec)
     where
         S: Source<Id = Gclk0Id> + Decrement,

hal/src/thumbv7em/clock/v2.rs

@@ -853,6 +853,8 @@
 //!
 //! [interior mutability]: https://doc.rust-lang.org/reference/interior-mutability.html
 
+#![allow(clippy::manual_range_contains)]
+
 use typenum::U0;
 
 use crate::time::Hertz;

hal/src/thumbv7em/clock/v2/gclk.rs

@@ -1166,6 +1166,7 @@ where
     /// factors to only the valid ones for the given [`Gclk`]. See the
     /// [`GclkDivider`] trait for more details.
     #[inline]
+    #[allow(clippy::should_implement_trait)]
     pub fn div(mut self, div: G::Divider) -> Self {
         self.settings.div = div;
         self
@@ -1320,6 +1321,7 @@ impl<I: GclkSourceId> EnabledGclk0<I, U1> {
     ///
     /// `Gclk0` will remain fully enabled during the swap.
     #[inline]
+    #[allow(clippy::type_complexity)]
     pub fn swap_pin_for_source<S>(
         self,
         source: S,

hal/src/thumbv7em/pukcc.rs

@@ -297,7 +297,7 @@ impl Pukcc {
             let service_params = &mut pukcl_params.params.ZpEcDsaGenerateFast;
             service_params.nu1ModBase = modulo_p.pukcc_base();
             service_params.nu1CnsBase = cns.pukcc_base();
-            service_params.u2ModLength = C::MOD_LENGTH as u16;
+            service_params.u2ModLength = C::MOD_LENGTH;
             service_params.nu1ScalarNumber = k_cr.pukcc_base();
             service_params.nu1OrderPointBase = order_point.pukcc_base();
             service_params.nu1PrivateKey = private_key_cr.pukcc_base();
@@ -754,6 +754,7 @@ impl Pukcc {
 
 /// An error type representing failure modes a [`Pukcc::self_test`] service
 #[derive(Debug)]
+#[allow(dead_code)]
 pub struct SelfTestFailure(c_abi::SelfTest);
 
 /// An error type representing failure modes for a

hal/src/thumbv7em/qspi.rs

@@ -244,6 +244,7 @@ impl Qspi<OneShot> {
     /// Return the consumed pins and the QSPI peripheral
     ///
     /// Order: `(qspi, sck, cs, io0, io1, io2, io3)`
+    #[allow(clippy::type_complexity)]
     pub fn free(
         self,
     ) -> (

hal/src/thumbv7em/usb/bus.rs

@@ -24,8 +24,9 @@ use usb_device::{Result as UsbResult, UsbDirection, UsbError};
 
 /// EndpointTypeBits represents valid values for the EPTYPE fields in
 /// the EPCFGn registers.
-#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+#[derive(Debug, Default, PartialEq, Eq, Clone, Copy)]
 pub enum EndpointTypeBits {
+    #[default]
     Disabled = 0,
     Control = 1,
     Isochronous = 2,
@@ -35,12 +36,6 @@ pub enum EndpointTypeBits {
     DualBank = 5,
 }
 
-impl Default for EndpointTypeBits {
-    fn default() -> Self {
-        EndpointTypeBits::Disabled
-    }
-}
-
 impl From<EndpointType> for EndpointTypeBits {
     fn from(ep_type: EndpointType) -> EndpointTypeBits {
         match ep_type {

hal/src/timer_params.rs

@@ -36,7 +36,7 @@ impl TimerParams {
             _ => 1024,
         };
 
-        let cycles: u32 = ticks / divider as u32;
+        let cycles: u32 = ticks / divider;
 
         if cycles > u16::max_value() as u32 {
             panic!("cycles {} is out of range for a 16 bit counter", cycles);