Clear the clippy warnings and gate on them in CI

.github/workflows/ci.yml

@@ -31,6 +31,17 @@ jobs:
           components: rustfmt
       - run: cargo fmt --check
 
+  clippy:
+    name: Clippy
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+        with:
+          components: clippy
+      - uses: Swatinem/rust-cache@v2
+      - run: cargo clippy --locked --all-targets -- -D warnings
+
   msrv:
     name: Minimum supported Rust version
     runs-on: ubuntu-latest

src/agg/mod.rs

@@ -93,7 +93,7 @@ mod tests {
     fn test_singleton_is_singleton() {
         for aggregator in aggregators(&fixtures::by_id()) {
             for taxon in fixtures::taxon_list() {
-                assert_matches!(aggregator.counting_aggregate(&vec![taxon.id]), Ok(tid) if tid == taxon.id);
+                assert_matches!(aggregator.counting_aggregate(&[taxon.id]), Ok(tid) if tid == taxon.id);
             }
         }
     }
@@ -102,13 +102,11 @@ mod tests {
     fn test_invalid_taxa() {
         for aggregator in aggregators(&fixtures::by_id()) {
             assert_matches!(
-                aggregator.counting_aggregate(&vec![5]).unwrap_err(),
+                aggregator.counting_aggregate(&[5]).unwrap_err(),
                 Error::Taxon(taxon::Error::UnknownTaxon(5))
             );
             assert_matches!(
-                aggregator
-                    .counting_aggregate(&vec![1, 2, 5, 1])
-                    .unwrap_err(),
+                aggregator.counting_aggregate(&[1, 2, 5, 1]).unwrap_err(),
                 Error::Taxon(taxon::Error::UnknownTaxon(5))
             );
         }

src/agg/rank.rs

@@ -32,7 +32,7 @@ impl<T: Iterator<Item = (String, TaxonId)>> RankAggregator<T> {
 
     fn raise_to_rank(&self, taxon: TaxonId, target: Rank) -> Option<TaxonId> {
         let mut ancestor = Some(taxon);
-        while ancestor.map_or(false, |a| self.ranks[a].map_or(true, |r| target < r)) {
+        while ancestor.is_some_and(|a| self.ranks[a].is_none_or(|r| target < r)) {
             ancestor = ancestor.and_then(|a| self.ancestors[a]);
         }
         ancestor
@@ -58,7 +58,7 @@ impl<T: Iterator<Item = (String, TaxonId)>> Iterator for RankAggregator<T> {
             let (mut aggregate, mut aggregate_rank) =
                 self.with_rank(initial_tid).expect("reeeeeeee");
 
-            while self.records.peek().map_or(false, |(s, _)| *s == sequence) {
+            while self.records.peek().is_some_and(|(s, _)| *s == sequence) {
                 let (_, next) = self.records.next().expect("we just peeked at it");
                 let (next_taxon, next_rank) = self.with_rank(next).expect("reeeeeee");
 

src/commands/prot2kmer2lca.rs

@@ -165,7 +165,7 @@ where
             let mut chunk_output = String::new();
             for read in chunk {
                 // Ignore empty reads and reads with a length smaller than k
-                if let Some(prot) = read.sequence.get(0).filter(|p| p.len() >= k) {
+                if let Some(prot) = read.sequence.first().filter(|p| p.len() >= k) {
                     chunk_output.push_str(&format!(">{}\n", read.header));
                     let mut lcas = (0..(prot.len() - k + 1))
                         .map(|i| &prot[i..i + k])

src/commands/prot2tryp2lca.rs

@@ -121,7 +121,7 @@ pub fn prot2tryp2lca(args: ProtToTrypToLca) -> errors::Result<()> {
                             }
                         })
                     {
-                        if let Some(lca) = fst.get(&peptide).map(Some).unwrap_or(default) {
+                        if let Some(lca) = fst.get(peptide).map(Some).unwrap_or(default) {
                             chunk_output.push_str(&format!("{}\n", lca));
                         }
                     }

src/commands/snaptaxon.rs

@@ -78,7 +78,9 @@ pub struct SnapTaxon {
 pub fn snaptaxon(args: SnapTaxon) -> errors::Result<()> {
     let taxons = taxon::read_taxa_file(&args.taxon_file)?;
     if args.rank.map(|r| r == rank::Rank::NoRank).unwrap_or(false) {
-        return Err(errors::Error::InvalidInvocation("Snap to an actual rank.".into()).into());
+        return Err(errors::Error::InvalidInvocation(
+            "Snap to an actual rank.".into(),
+        ));
     }
 
     // Parsing the taxons

src/commands/taxa2agg.rs

@@ -138,8 +138,7 @@ pub fn taxa2agg(args: TaxaToAgg) -> errors::Result<()> {
         (m, s) => Err(errors::Error::InvalidInvocation(format!(
             "{:?} and {:?} cannot be combined",
             m, s
-        ))
-        .into()),
+        ))),
     };
     let aggregator = aggregator?;
 
@@ -208,7 +207,7 @@ impl FromStr for Method {
         match s {
             "tree" => Ok(Method::Tree),
             "rmq" => Ok(Method::RangeMinimumQuery),
-            _ => Err(Error::ParseMethodError(s.to_string()).into()),
+            _ => Err(Error::ParseMethodError(s.to_string())),
         }
     }
 }
@@ -237,7 +236,7 @@ impl FromStr for Strategy {
             "lca*" => Ok(Strategy::LowestCommonAncestor),
             "hybrid" => Ok(Strategy::Hybrid),
             "mrtl" => Ok(Strategy::MaximumRootToLeafPath),
-            _ => Err(Error::ParseStrategyError(s.to_string()).into()),
+            _ => Err(Error::ParseStrategyError(s.to_string())),
         }
     }
 }

src/commands/taxa2freq.rs

@@ -90,7 +90,9 @@ pub struct TaxaToFreq {
 pub fn taxa2freq(args: TaxaToFreq) -> errors::Result<()> {
     let taxons = taxon::read_taxa_file(&args.taxon_file)?;
     if args.rank == rank::Rank::NoRank {
-        return Err(errors::Error::InvalidInvocation("Snap to an actual rank.".into()).into());
+        return Err(errors::Error::InvalidInvocation(
+            "Snap to an actual rank.".into(),
+        ));
     }
     let numfiles = args.input_files.len();
 

src/commands/translate.rs

@@ -169,7 +169,7 @@ impl FromStr for Frame {
             "1R" => Ok(Frame::Reverse1),
             "2R" => Ok(Frame::Reverse2),
             "3R" => Ok(Frame::Reverse3),
-            _ => Err(Error::ParseFrameError(s.to_string()).into()),
+            _ => Err(Error::ParseFrameError(s.to_string())),
         }
     }
 }

src/commands/uniq.rs

@@ -61,12 +61,9 @@ pub fn uniq(args: Uniq) -> errors::Result<()> {
     for record in fasta::Reader::new(io::stdin(), false).records() {
         let mut record = record?;
         if let Some(ref delimiter) = args.delimiter {
-            record.header.truncate(
-                record
-                    .header
-                    .find(delimiter)
-                    .unwrap_or_else(|| record.header.len()),
-            );
+            record
+                .header
+                .truncate(record.header.find(delimiter).unwrap_or(record.header.len()));
         }
         if let Some(ref mut rec) = last {
             if rec.header == record.header {

src/dna/mod.rs

@@ -55,7 +55,7 @@ impl From<Nucleotide> for u8 {
     }
 }
 
-impl<'a> From<&'a u8> for Nucleotide {
+impl From<&u8> for Nucleotide {
     fn from(ch: &u8) -> Self {
         Nucleotide::from(*ch)
     }
@@ -69,13 +69,13 @@ pub struct Frame<'a>(&'a [Nucleotide]);
 #[derive(Debug, PartialEq)]
 pub struct Strand(Vec<Nucleotide>);
 
-impl<'a> From<&'a [u8]> for Strand {
+impl From<&[u8]> for Strand {
     fn from(read: &[u8]) -> Self {
         Strand(read.iter().map(Nucleotide::from).collect())
     }
 }
 
-impl<'a> From<&'a Vec<String>> for Strand {
+impl From<&Vec<String>> for Strand {
     fn from(lines: &Vec<String>) -> Self {
         Strand(
             lines

src/dna/translation.rs

@@ -11,7 +11,7 @@ use crate::dna::{Frame, Nucleotide};
 #[derive(Debug, PartialEq, Eq, Hash)]
 pub struct Codon(Nucleotide, Nucleotide, Nucleotide);
 
-impl<'a> From<&'a [Nucleotide]> for Codon {
+impl From<&[Nucleotide]> for Codon {
     fn from(b: &[Nucleotide]) -> Self {
         Codon(b[0], b[1], b[2])
     }

src/io/fasta.rs

@@ -42,8 +42,7 @@ impl<R: Read> Reader<R> {
         };
 
         if !header.starts_with('>') {
-            return Err(errors::Error::Io(io::Error::new(
-                io::ErrorKind::Other,
+            return Err(errors::Error::Io(io::Error::other(
                 "Expected > at beginning of fasta header.",
             )));
         }
@@ -165,16 +164,16 @@ impl<'a, W: Write> Writer<'a, W> {
         write!(self.buffer, ">{}", record.header)?;
         let sequence = record.sequence.join(self.separator);
         if !self.wrap {
-            self.buffer.write_all(&[b'\n'])?;
+            self.buffer.write_all(b"\n")?;
             self.buffer.write_all(sequence.as_bytes())?;
         } else {
             for subseq in sequence.as_bytes().chunks(FASTA_WIDTH) {
-                self.buffer.write_all(&[b'\n'])?;
+                self.buffer.write_all(b"\n")?;
                 self.buffer.write_all(subseq)?;
             }
         }
         if !sequence.is_empty() {
-            self.buffer.write_all(&[b'\n'])?;
+            self.buffer.write_all(b"\n")?;
         }
         Ok(())
     }

src/io/fastq.rs

@@ -30,8 +30,7 @@ impl<R: Read> Reader<R> {
             Some(header) => header?,
         };
         if !header.starts_with('@') {
-            return Err(errors::Error::Io(io::Error::new(
-                io::ErrorKind::Other,
+            return Err(errors::Error::Io(io::Error::other(
                 "Expected @ at beginning of fastq header.",
             )));
         }
@@ -59,8 +58,7 @@ impl<R: Read> Reader<R> {
             .map(|line| !line.starts_with('+'))
             .unwrap_or(false)
         {
-            return Err(errors::Error::Io(io::Error::new(
-                io::ErrorKind::Other,
+            return Err(errors::Error::Io(io::Error::other(
                 "Expected a + as separator.",
             )));
         }
@@ -71,8 +69,7 @@ impl<R: Read> Reader<R> {
             if let Some(line) = self.lines.next() {
                 quality.push_str(&line?)
             } else {
-                return Err(errors::Error::Io(io::Error::new(
-                    io::ErrorKind::Other,
+                return Err(errors::Error::Io(io::Error::other(
                     "Expected as many quality lines as \
                                                             sequence lines.",
                 )));

src/rank.rs

@@ -108,6 +108,14 @@ impl Rank {
     }
 }
 
+// Deliberately not `Some(self.cmp(other))`: NoRank is treated as incomparable, so that
+// `score()` falls through every branch for it and `raise_to_rank` stops climbing. Making
+// this canonical would give NoRank a score of 12 and change aggregation.
+//
+// Note this does contradict the Ord impl below, which reports NoRank as Less rather than
+// incomparable, so `min()` and `<` disagree about it. Left as is because both behaviours
+// are relied on; worth revisiting deliberately rather than as a lint fix.
+#[allow(clippy::non_canonical_partial_ord_impl)]
 impl PartialOrd for Rank {
     fn partial_cmp(&self, other: &Rank) -> Option<Ordering> {
         if self == &Rank::NoRank || other == &Rank::NoRank {

src/rmq/lca.rs

@@ -49,8 +49,8 @@ impl LCACalculator {
     fn first_occurence(&self, taxon_id: TaxonId) -> taxon::Result<usize> {
         self.first_occurences
             .get(&taxon_id)
-            .ok_or_else(|| taxon::Error::UnknownTaxon(taxon_id).into())
-            .map(|t| *t)
+            .ok_or(taxon::Error::UnknownTaxon(taxon_id))
+            .copied()
     }
 }
 
@@ -102,30 +102,30 @@ mod tests {
     #[test]
     fn test_two_on_same_path() {
         let aggregator = LCACalculator::new(fixtures::tree());
-        assert_matches!(aggregator.counting_aggregate(&vec![12884, 185752]), Ok(185752));
-        assert_matches!(aggregator.counting_aggregate(&vec![185752, 12884]), Ok(185752));
-        assert_matches!(aggregator.counting_aggregate(&vec![1, 2]), Ok(2));
-        assert_matches!(aggregator.counting_aggregate(&vec![2, 1]), Ok(2));
+        assert_matches!(aggregator.counting_aggregate(&[12884, 185752]), Ok(185752));
+        assert_matches!(aggregator.counting_aggregate(&[185752, 12884]), Ok(185752));
+        assert_matches!(aggregator.counting_aggregate(&[1, 2]), Ok(2));
+        assert_matches!(aggregator.counting_aggregate(&[2, 1]), Ok(2));
     }
 
     #[test]
     fn test_two_on_fork() {
         let aggregator = LCACalculator::new(fixtures::tree());
-        assert_matches!(aggregator.counting_aggregate(&vec![2, 10239]), Ok(1));
-        assert_matches!(aggregator.counting_aggregate(&vec![10239, 2]), Ok(1));
-        assert_matches!(aggregator.counting_aggregate(&vec![185751, 185752]), Ok(12884));
-        assert_matches!(aggregator.counting_aggregate(&vec![185752, 185751]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[2, 10239]), Ok(1));
+        assert_matches!(aggregator.counting_aggregate(&[10239, 2]), Ok(1));
+        assert_matches!(aggregator.counting_aggregate(&[185751, 185752]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[185752, 185751]), Ok(12884));
     }
 
     #[test]
     fn test_three_on_triangle() {
         let aggregator = LCACalculator::new(fixtures::tree());
-        assert_matches!(aggregator.counting_aggregate(&vec![12884, 185751, 185752]), Ok(12884));
-        assert_matches!(aggregator.counting_aggregate(&vec![12884, 185752, 185751]), Ok(12884));
-        assert_matches!(aggregator.counting_aggregate(&vec![185751, 12884, 185752]), Ok(12884));
-        assert_matches!(aggregator.counting_aggregate(&vec![185752, 12884, 185751]), Ok(12884));
-        assert_matches!(aggregator.counting_aggregate(&vec![185751, 185752, 12884]), Ok(12884));
-        assert_matches!(aggregator.counting_aggregate(&vec![185752, 185751, 12884]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[12884, 185751, 185752]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[12884, 185752, 185751]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[185751, 12884, 185752]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[185752, 12884, 185751]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[185751, 185752, 12884]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[185752, 185751, 12884]), Ok(12884));
     }
 
     fn taxon(id: TaxonId, parent: TaxonId) -> Taxon {
@@ -155,10 +155,10 @@ mod tests {
     #[test]
     fn test_with_deeper_interns() {
         let large_aggregator = LCACalculator::new(TaxonTree::new(&large_taxon_list()));
-        assert_matches!(large_aggregator.counting_aggregate(&vec![9, 7]),  Ok(3));
-        assert_matches!(large_aggregator.counting_aggregate(&vec![9, 10]), Ok(3));
-        assert_matches!(large_aggregator.counting_aggregate(&vec![7, 9]),  Ok(3));
-        assert_matches!(large_aggregator.counting_aggregate(&vec![14, 8]), Ok(3));
-        assert_matches!(large_aggregator.counting_aggregate(&vec![14, 8]), Ok(3));
+        assert_matches!(large_aggregator.counting_aggregate(&[9, 7]),  Ok(3));
+        assert_matches!(large_aggregator.counting_aggregate(&[9, 10]), Ok(3));
+        assert_matches!(large_aggregator.counting_aggregate(&[7, 9]),  Ok(3));
+        assert_matches!(large_aggregator.counting_aggregate(&[14, 8]), Ok(3));
+        assert_matches!(large_aggregator.counting_aggregate(&[14, 8]), Ok(3));
     }
 }

src/rmq/mix.rs

@@ -72,7 +72,7 @@ impl agg::Aggregator for MixCalculator {
             for (&right, &count) in taxons.iter() {
                 let lca = self.lca_aggregator.lca(left, right)?;
                 if lca == left || lca == right {
-                    let mut weight = weights.entry(left).or_insert_with(Weights::new);
+                    let weight = weights.entry(left).or_insert_with(Weights::new);
                     if lca == left {
                         weight.lca += count;
                     }
@@ -89,7 +89,7 @@ impl agg::Aggregator for MixCalculator {
             .iter()
             .max_by_key(|&(_, w)| NotNan::new(factorize(*w, self.factor)).unwrap())
             .map(|tup| *tup.0)
-            .ok_or_else(|| agg::Error::EmptyInput)
+            .ok_or(agg::Error::EmptyInput)
     }
 }
 
@@ -103,25 +103,25 @@ mod tests {
     #[test]
     fn test_full_rtl() {
         let aggregator = MixCalculator::new(fixtures::tree(), 0.0);
-        assert_matches!(aggregator.counting_aggregate(&vec![12884, 185751]), Ok(185751));
-        assert_matches!(aggregator.counting_aggregate(&vec![12884, 185751, 185752, 185752]), Ok(185752));
-        assert_matches!(aggregator.counting_aggregate(&vec![1, 1, 10239, 10239, 10239, 12884, 185751, 185752]), Ok(10239));
+        assert_matches!(aggregator.counting_aggregate(&[12884, 185751]), Ok(185751));
+        assert_matches!(aggregator.counting_aggregate(&[12884, 185751, 185752, 185752]), Ok(185752));
+        assert_matches!(aggregator.counting_aggregate(&[1, 1, 10239, 10239, 10239, 12884, 185751, 185752]), Ok(10239));
     }
 
     #[test]
     fn test_full_lca() {
         let aggregator = MixCalculator::new(fixtures::tree(), 1.0);
-        assert_matches!(aggregator.counting_aggregate(&vec![12884, 185751]), Ok(12884));
-        assert_matches!(aggregator.counting_aggregate(&vec![12884, 185751, 185752, 185752]), Ok(12884));
-        assert_matches!(aggregator.counting_aggregate(&vec![1, 1, 10239, 10239, 10239, 12884, 185751, 185752]), Ok(1));
+        assert_matches!(aggregator.counting_aggregate(&[12884, 185751]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[12884, 185751, 185752, 185752]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[1, 1, 10239, 10239, 10239, 12884, 185751, 185752]), Ok(1));
     }
 
     /* third example might fail because 12884 and 185751 have the same score. */
     #[test]
     fn test_one_half() {
         let aggregator = MixCalculator::new(fixtures::tree(), 0.5);
-        assert_matches!(aggregator.counting_aggregate(&vec![12884, 12884, 185751]), Ok(12884));
-        assert_matches!(aggregator.counting_aggregate(&vec![12884, 185751, 185751]), Ok(185751));
-        assert_matches!(aggregator.counting_aggregate(&vec![1, 12884, 12884, 185751, 185752]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[12884, 12884, 185751]), Ok(12884));
+        assert_matches!(aggregator.counting_aggregate(&[12884, 185751, 185751]), Ok(185751));
+        assert_matches!(aggregator.counting_aggregate(&[1, 12884, 12884, 185751, 185752]), Ok(12884));
     }
 }