Merge pull request #388 from brendanzab/enable-clippy

Fix clippy lints

Author: brendanzab

.github/workflows/ci.yml

@@ -48,16 +48,16 @@ jobs:
   #   runs-on: ubuntu-latest
   #   strategy:
   #     matrix:
-  #       rust-toolchain: ["stable", "minimum"]
-  #   name: Clippy (${{ matrix.rust }})
+  #       rust-toolchain: ["stable"]
+  #   name: Clippy (${{ matrix.rust-toolchain }})
   #   steps:
   #     - name: Checkout repository
   #       uses: actions/checkout@v3
   #     - name: Install Nix
   #       uses: cachix/install-nix-action@v17
-  #
+
   #     - name: Run cargo clippy
-  #       run: nix develop .#${{ matrix.rust }} --command cargo clippy --deny warnings
+  #       run: nix develop .#${{ matrix.rust-toolchain }} --command cargo clippy -- --deny warnings
 
   nixpkgs-fmt:
     runs-on: ubuntu-latest

fathom/src/alloc.rs

@@ -69,8 +69,8 @@ impl<'a, Elem> Deref for SliceVec<'a, Elem> {
     }
 }
 
-impl<'a, Elem> Into<&'a [Elem]> for SliceVec<'a, Elem> {
-    fn into(self) -> &'a [Elem] {
+impl<'a, Elem> From<SliceVec<'a, Elem>> for &'a [Elem] {
+    fn from(slice: SliceVec<'a, Elem>) -> Self {
         // SAFETY: This is safe because we know that `self.elems[..self.next_index]`
         // only ever contains elements initialized with `MaybeUninit::new`.
         // We know this because:
@@ -79,12 +79,13 @@ impl<'a, Elem> Into<&'a [Elem]> for SliceVec<'a, Elem> {
         // - `self.next_index` is only incremented in `SliceBuilder::push`,
         //    and in that case we make sure `self.elems[self.next_index]`
         //    has been initialized before hand.
-        unsafe { slice_assume_init_ref(&self.elems[..self.next_index]) }
+        unsafe { slice_assume_init_ref(&slice.elems[..slice.next_index]) }
     }
 }
 
 // NOTE: This is the same implementation as `MaybeUninit::slice_assume_init_ref`,
 // which is currently unstable (see https://github.com/rust-lang/rust/issues/63569).
+#[allow(clippy::needless_lifetimes)] // These serve as important documentation
 pub unsafe fn slice_assume_init_ref<'a, T>(slice: &'a [MaybeUninit<T>]) -> &'a [T] {
     // SAFETY: casting slice to a `*const [T]` is safe since the caller guarantees that
     // `slice` is initialized, and`MaybeUninit` is guaranteed to have the same layout as `T`.

fathom/src/core.rs

@@ -552,7 +552,7 @@ def_prims! {
 }
 
 /// Formatting style for integers
-#[derive(Debug, Copy, Clone, PartialEq, PartialOrd)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd)]
 pub enum UIntStyle {
     Binary,
     Decimal,

fathom/src/core/binary.rs

@@ -180,7 +180,7 @@ impl<'data> BufferReader<'data> {
     /// Set the offset of the reader relative to the start position.
     pub fn set_offset(&mut self, offset: usize) -> Result<(), BufferError> {
         usize::checked_sub(offset, self.buffer.start_offset)
-            .ok_or_else(|| BufferError::SetOffsetBeforeStartOfBuffer { offset })
+            .ok_or(BufferError::SetOffsetBeforeStartOfBuffer { offset })
             .and_then(|relative_offset| self.set_relative_offset(relative_offset))
     }
 
@@ -329,7 +329,7 @@ impl<'arena, 'env, 'data> Context<'arena, 'env, 'data> {
                 ))
             }
             Value::FormatCond(_label, format, cond) => {
-                let value = self.read_format(reader, &format)?;
+                let value = self.read_format(reader, format)?;
                 let cond_res = self.elim_env().apply_closure(cond, value.clone());
 
                 match cond_res.as_ref() {
@@ -392,7 +392,7 @@ impl<'arena, 'env, 'data> Context<'arena, 'env, 'data> {
     ) -> Result<ArcValue<'arena>, ReadError<'arena>> {
         use crate::core::semantics::Elim::FunApp;
 
-        match (prim, &slice[..]) {
+        match (prim, slice) {
             (Prim::FormatU8, []) => read_const(reader, span, read_u8, |num| Const::U8(num, UIntStyle::Decimal)),
             (Prim::FormatU16Be, []) => read_const(reader, span, read_u16be, |num| Const::U16(num, UIntStyle::Decimal)),
             (Prim::FormatU16Le, []) => read_const(reader, span, read_u16le, |num| Const::U16(num, UIntStyle::Decimal)),
@@ -445,7 +445,7 @@ impl<'arena, 'env, 'data> Context<'arena, 'env, 'data> {
             Value::ConstLit(Const::U8(len, _)) => u64::from(*len),
             Value::ConstLit(Const::U16(len, _)) => u64::from(*len),
             Value::ConstLit(Const::U32(len, _)) => u64::from(*len),
-            Value::ConstLit(Const::U64(len, _)) => u64::from(*len),
+            Value::ConstLit(Const::U64(len, _)) => *len,
             _ => return Err(ReadError::InvalidValue(len.span())),
         };
 
@@ -552,27 +552,27 @@ impl<'arena, 'env, 'data> Context<'arena, 'env, 'data> {
         // that parsed reference alongside the position in `Const::Ref`.
 
         (self.cached_refs.get(&pos)?.iter())
-            .find(|r| self.conversion_env().is_equal(&r.format, &format))
+            .find(|r| self.conversion_env().is_equal(&r.format, format))
     }
 
     fn lookup_or_read_ref(
         &mut self,
         pos: usize,
         format: &ArcValue<'arena>,
     ) -> Result<ArcValue<'arena>, ReadError<'arena>> {
-        if let Some(parsed_ref) = self.lookup_ref(pos, &format) {
+        if let Some(parsed_ref) = self.lookup_ref(pos, format) {
             return Ok(parsed_ref.expr.clone());
         }
 
         // Read the data at the ref location
         let mut reader = self.initial_buffer.reader_with_offset(pos)?;
-        let expr = self.read_format(&mut reader, &format)?;
+        let expr = self.read_format(&mut reader, format)?;
 
         // We might have parsed the current reference during the above call to
         // `read_format`. It's unclear if this could ever happen in practice,
         // especially without succumbing to non-termination, but we'll panic
         // here just in case.
-        if let Some(_) = self.lookup_ref(pos, &format) {
+        if self.lookup_ref(pos, format).is_some() {
             panic!("recursion found when storing cached reference {}", pos);
         }
 
@@ -614,11 +614,11 @@ fn read_const<'arena, 'data, T>(
     ))
 }
 
-fn read_u8<'data>(reader: &mut BufferReader<'data>) -> Result<u8, BufferError> {
+fn read_u8(reader: &mut BufferReader<'_>) -> Result<u8, BufferError> {
     reader.read_byte()
 }
 
-fn read_s8<'data>(reader: &mut BufferReader<'data>) -> Result<i8, BufferError> {
+fn read_s8(reader: &mut BufferReader<'_>) -> Result<i8, BufferError> {
     reader.read_byte().map(|b| b as i8)
 }
 

fathom/src/core/semantics.rs

@@ -74,7 +74,7 @@ impl<'arena> Value<'arena> {
 
     pub fn match_prim_spine(&self) -> Option<(Prim, &[Elim<'arena>])> {
         match self {
-            Value::Stuck(Head::Prim(prim), spine) => Some((*prim, &spine)),
+            Value::Stuck(Head::Prim(prim), spine) => Some((*prim, spine)),
             _ => None,
         }
     }
@@ -1156,7 +1156,7 @@ impl<'arena, 'env> ConversionEnv<'arena, 'env> {
             (Value::RecordLit(labels0, exprs0), Value::RecordLit(labels1, exprs1)) => {
                 labels0 == labels1
                     && Iterator::zip(exprs0.iter(), exprs1.iter())
-                        .all(|(expr0, expr1)| self.is_equal(&expr0, &expr1))
+                        .all(|(expr0, expr1)| self.is_equal(expr0, expr1))
             }
             (Value::RecordLit(labels, exprs), _) => {
                 self.is_equal_record_lit(labels, exprs, &value1)
@@ -1167,7 +1167,7 @@ impl<'arena, 'env> ConversionEnv<'arena, 'env> {
 
             (Value::ArrayLit(elem_exprs0), Value::ArrayLit(elem_exprs1)) => {
                 Iterator::zip(elem_exprs0.iter(), elem_exprs1.iter())
-                    .all(|(elem_expr0, elem_expr1)| self.is_equal(&elem_expr0, &elem_expr1))
+                    .all(|(elem_expr0, elem_expr1)| self.is_equal(elem_expr0, elem_expr1))
             }
 
             (Value::FormatRecord(labels0, formats0), Value::FormatRecord(labels1, formats1))

fathom/src/driver.rs

@@ -97,7 +97,7 @@ impl<'surface, 'core> Driver<'surface, 'core> {
                 .with_notes(vec![
                     match location {
                         Some(location) => format!("panicked at: {}", location),
-                        None => format!("panicked at: unknown location"),
+                        None => "panicked at: unknown location".to_owned(),
                     },
                     format!("please file a bug report at: {}", BUG_REPORT_URL),
                     // TODO: print rust backtrace
@@ -140,7 +140,7 @@ impl<'surface, 'core> Driver<'surface, 'core> {
 
     /// Load a source string into the file database.
     pub fn load_source_string(&mut self, name: String, source: String) -> FileId {
-        self.files.add(name.to_owned(), source)
+        self.files.add(name, source)
     }
 
     /// Load a source file into the file database using a reader.
@@ -272,7 +272,7 @@ impl<'surface, 'core> Driver<'surface, 'core> {
         Status::Ok
     }
 
-    pub fn read_and_emit_format<'data>(
+    pub fn read_and_emit_format(
         &mut self,
         module_file_id: Option<FileId>,
         format_file_id: FileId,
@@ -380,7 +380,7 @@ impl<'surface, 'core> Driver<'surface, 'core> {
                 context.space(),
                 context.sequence(
                     context.text("["),
-                    exprs.iter().map(|expr| context.term(&expr)),
+                    exprs.iter().map(|expr| context.term(expr)),
                     context.text(","),
                     context.text("]"),
                 ),
@@ -455,7 +455,7 @@ impl<'surface, 'core> Driver<'surface, 'core> {
                     )
                     .with_notes(vec![
                         "The predicate on a conditional format did not succeed.".to_string(),
-                        format!("failed value: {}", doc.pretty(self.emit_width).to_string()),
+                        format!("failed value: {}", doc.pretty(self.emit_width)),
                     ])
             }
             ReadError::UnwrappedNone(_) => Diagnostic::error()

fathom/src/env.rs

@@ -250,7 +250,7 @@ impl<Entry> SliceEnv<Entry> {
     }
 
     /// Iterate over the elements in the environment.
-    pub fn iter<'this>(&'this self) -> impl 'this + DoubleEndedIterator<Item = &'this Entry> {
+    pub fn iter(&self) -> impl DoubleEndedIterator<Item = &Entry> {
         self.entries.iter()
     }
 }
@@ -326,7 +326,7 @@ impl<Entry> SharedEnv<Entry> {
     }
 
     /// Iterate over the elements in the environment.
-    pub fn iter<'this>(&'this self) -> impl 'this + DoubleEndedIterator<Item = &'this Entry> {
+    pub fn iter(&self) -> impl DoubleEndedIterator<Item = &Entry> {
         self.entries.iter()
     }
 }

fathom/src/lib.rs

@@ -1,4 +1,7 @@
 #![doc = include_str!("../../README.md")]
+#![allow(clippy::bool_assert_comparison)]
+#![allow(clippy::len_without_is_empty)]
+#![allow(clippy::new_without_default)]
 
 // Supporting modules
 mod alloc;

fathom/src/source.rs

@@ -124,8 +124,8 @@ impl ByteRange {
     }
 }
 
-impl Into<std::ops::Range<usize>> for ByteRange {
-    fn into(self) -> std::ops::Range<usize> {
-        self.start..self.end
+impl From<ByteRange> for std::ops::Range<usize> {
+    fn from(range: ByteRange) -> Self {
+        range.start..range.end
     }
 }

fathom/src/surface.rs

@@ -10,7 +10,11 @@ use scoped_arena::Scope;
 use crate::source::{ByteRange, FileId};
 use crate::{StringId, StringInterner};
 
-lalrpop_mod!(grammar, "/surface/grammar.rs");
+lalrpop_mod!(
+    #[allow(clippy::all)]
+    grammar,
+    "/surface/grammar.rs"
+);
 mod lexer;
 pub mod pretty;