Skip to content

Commit

Permalink
chore: lots of clippy lints
Browse files Browse the repository at this point in the history
Transition from rust-version 1.78 -> 1.83 introduced new clippy rules.

- Use `expect` macro instead of `allow` for crate specific lints
- Add a `reason` attribute instead of keeping inline comments with
  explanations
- Use default `as _` for traits which are brought into scope but unused
- Small improvements related to borrowing lints
  • Loading branch information
n-dusan committed Dec 16, 2024
1 parent de914bf commit 7200830
Show file tree
Hide file tree
Showing 27 changed files with 192 additions and 111 deletions.
2 changes: 1 addition & 1 deletion src/db/init.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::db::{DatabaseConnection, DatabaseKind, Db};
use crate::db::{DatabaseConnection, DatabaseKind, Db as _};
use std::env;
use std::path::{Path, PathBuf};
/// Connects to a database and applies migrations.
Expand Down
5 changes: 2 additions & 3 deletions src/db/mod.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
//! Database related module.
#![allow(clippy::unreachable)]
use async_trait::async_trait;
use sqlx::Transaction;
use std::str::FromStr;
use std::str::FromStr as _;

use sqlx::any::{self, AnyPoolOptions};
use sqlx::AnyPool;
use sqlx::ConnectOptions;
use sqlx::ConnectOptions as _;
use tracing::instrument;

/// Database initialization.
Expand Down
2 changes: 1 addition & 1 deletion src/db/models/publication/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use async_trait::async_trait;
use chrono::NaiveDate;
use serde::{Deserialize, Serialize};
use sqlx::{any::AnyRow, FromRow, Row};
use sqlx::{any::AnyRow, FromRow, Row as _};

pub mod manager;

Expand Down
2 changes: 1 addition & 1 deletion src/db/models/publication_version/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl super::TxManager for DatabaseTransaction {
}

/// Recursively find all publication versions starting from a given publication ID.
///
/// This is necessary because publication versions can be the same across publications.
/// To make versions query simpler, we walk the publication hierarchy starting from
/// `publication_name` looking for related publications.
Expand Down
2 changes: 1 addition & 1 deletion src/db/models/publication_version/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use async_trait::async_trait;
use serde::{Deserialize, Serialize};
use sqlx::{any::AnyRow, FromRow, Row};
use sqlx::{any::AnyRow, FromRow, Row as _};

pub mod manager;

Expand Down
57 changes: 36 additions & 21 deletions src/history/changes.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
//! Module for inserting changes into the database
#![allow(
clippy::exit,
#![expect(
clippy::shadow_reuse,
reason = "Bindings that shadow other bindings in the same scope are used to make the code a bit more legible in this module"
)]
#![expect(
clippy::future_not_send,
clippy::string_add
reason = "We don't worry about git2-rs not implementing `Send` trait"
)]
use super::rdf::graph::Bag;
use crate::db::models::changed_library_document::{self, ChangedLibraryDocument};
Expand All @@ -20,7 +22,7 @@ use crate::db::models::publication_version;
use crate::db::models::status::Status;
use crate::db::models::{document, document_element};
use crate::db::models::{stele, version};
use crate::db::{DatabaseTransaction, Tx};
use crate::db::{DatabaseTransaction, Tx as _};
use crate::history::rdf::graph::StelaeGraph;
use crate::history::rdf::namespaces::{dcterms, oll};
use crate::server::errors::CliError;
Expand All @@ -33,7 +35,7 @@ use crate::{
db::{self, DatabaseConnection},
stelae::archive::Archive,
};
use anyhow::Context;
use anyhow::Context as _;
use chrono::DateTime;
use git2::{TreeWalkMode, TreeWalkResult};
use sophia::api::ns::rdfs;
Expand Down Expand Up @@ -184,7 +186,6 @@ async fn load_delta_for_stele(
///
/// # Errors
/// Errors if the delta cannot be loaded from the publications
#[allow(clippy::unwrap_used)]
async fn load_delta_from_publications(
tx: &mut DatabaseTransaction,
rdf_repo: &Repo,
Expand Down Expand Up @@ -266,7 +267,7 @@ async fn load_delta_from_publications(
})?;
let (last_valid_pub_name, last_valid_codified_date) =
referenced_publication_information(&pub_graph);
let publication_hash = md5::compute(pub_name.clone() + stele);
let publication_hash = md5::compute(format!("{}{}", pub_name.clone(), stele));
let last_inserted_pub_id = if let Some(valid_pub_name) = last_valid_pub_name {
let last_inserted_pub =
publication::TxManager::find_by_name_and_stele(tx, &valid_pub_name, stele).await?;
Expand Down Expand Up @@ -310,7 +311,7 @@ async fn load_delta_for_publication(

insert_document_changes(
tx,
&last_inserted_date,
last_inserted_date.as_ref(),
pub_document_versions,
pub_graph,
&publication,
Expand All @@ -319,7 +320,7 @@ async fn load_delta_for_publication(

insert_library_changes(
tx,
&last_inserted_date,
last_inserted_date.as_ref(),
pub_collection_versions,
pub_graph,
&publication,
Expand All @@ -336,7 +337,7 @@ async fn load_delta_for_publication(
/// Insert document changes into the database
async fn insert_document_changes(
tx: &mut DatabaseTransaction,
last_inserted_date: &Option<NaiveDate>,
last_inserted_date: Option<&NaiveDate>,
pub_document_versions: Vec<&SimpleTerm<'_>>,
pub_graph: &StelaeGraph,
publication: &Publication,
Expand All @@ -354,8 +355,12 @@ async fn insert_document_changes(
}
}
version::TxManager::create(tx, &codified_date).await?;
let pub_version_hash =
md5::compute(publication.name.clone() + &codified_date + &publication.stele);
let pub_version_hash = md5::compute(format!(
"{}{}{}",
publication.name.clone(),
&codified_date,
&publication.stele
));
publication_version::TxManager::create(
tx,
&pub_version_hash,
Expand Down Expand Up @@ -396,9 +401,12 @@ async fn insert_document_changes(
)?;
for el_status in statuses {
let status = Status::from_string(&el_status)?;
let document_change_hash = md5::compute(
pub_version_hash.clone() + &doc_mpath.clone() + &status.to_int().to_string(),
);
let document_change_hash = md5::compute(format!(
"{}{}{}",
pub_version_hash.clone(),
&doc_mpath.clone(),
&status.to_int().to_string()
));
document_changes_bulk.push(DocumentChange::new(
document_change_hash,
status.to_int(),
Expand All @@ -417,7 +425,7 @@ async fn insert_document_changes(
/// Insert library changes into the database
async fn insert_library_changes(
tx: &mut DatabaseTransaction,
last_inserted_date: &Option<NaiveDate>,
last_inserted_date: Option<&NaiveDate>,
pub_collection_versions: Vec<&SimpleTerm<'_>>,
pub_graph: &StelaeGraph,
publication: &Publication,
Expand Down Expand Up @@ -449,8 +457,12 @@ async fn insert_library_changes(
url.clone(),
publication.stele.clone(),
));
let pub_version_hash =
md5::compute(publication.name.clone() + &codified_date + &publication.stele);
let pub_version_hash = md5::compute(format!(
"{}{}{}",
publication.name.clone(),
&codified_date,
&publication.stele
));
library_changes_bulk.push(LibraryChange::new(
pub_version_hash.clone(),
library_status.to_int(),
Expand All @@ -473,9 +485,12 @@ async fn insert_library_changes(
continue;
};
let status = Status::from_string(&found_status)?;
let document_change_hash = md5::compute(
pub_version_hash.clone() + &doc_mpath.clone() + &status.to_int().to_string(),
);
let document_change_hash = md5::compute(format!(
"{}{}{}",
pub_version_hash.clone(),
&doc_mpath.clone(),
&status.to_int().to_string()
));
changed_library_document_bulk.push(ChangedLibraryDocument::new(
document_change_hash,
library_mpath.clone(),
Expand Down
24 changes: 15 additions & 9 deletions src/history/rdf/graph.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
#![allow(
#![expect(
clippy::module_name_repetitions,
reason = "Call our graph `StelaeGraph`, which repeats the name graph, but is used to differentiate between our wrapper and the underlying sophia graph."
)]
#![expect(
clippy::min_ident_chars,
clippy::pattern_type_mismatch
reason = "Nomenclature for RDF is `s`, `p, `o` instead of `subject`, `predicate`, `object`"
)]
#![expect(
clippy::pattern_type_mismatch,
reason = "Bypass sophia internal & ref match on SimpleTerm"
)]
/// The helper methods for working with RDF in Stelae.
use anyhow::Context;
use sophia::api::graph::{GTripleSource, Graph};
use anyhow::Context as _;
use sophia::api::graph::{GTripleSource, Graph as _};
use sophia::api::ns::NsTerm;
use sophia::api::MownStr;
use sophia::api::{prelude::*, term::SimpleTerm};
Expand Down Expand Up @@ -94,7 +101,7 @@ impl StelaeGraph {
subject: Option<&'graph SimpleTerm>,
predicate: Option<NsTerm<'graph>>,
object: Option<NsTerm<'graph>>,
) -> anyhow::Result<SimpleTerm> {
) -> anyhow::Result<SimpleTerm<'graph>> {
let triple = self.get_next_triples_matching(subject, predicate, object)?;
let SimpleTerm::Iri(iri) = &triple.o() else {
anyhow::bail!("Expected literal language, got - {:?}", triple.o());
Expand All @@ -111,7 +118,7 @@ impl StelaeGraph {
subject: Option<&'graph SimpleTerm>,
predicate: Option<NsTerm<'graph>>,
object: Option<NsTerm<'graph>>,
) -> anyhow::Result<[&'graph SimpleTerm<'_>; 3]> {
) -> anyhow::Result<[&'graph SimpleTerm<'graph>; 3]> {
let triple = self
.triples_matching_inner(subject, predicate, object)
.next()
Expand Down Expand Up @@ -151,7 +158,7 @@ impl StelaeGraph {
subject: Option<&'graph SimpleTerm>,
predicate: Option<NsTerm<'graph>>,
object: Option<NsTerm<'graph>>,
) -> anyhow::Result<Vec<&SimpleTerm>> {
) -> anyhow::Result<Vec<&'graph SimpleTerm<'graph>>> {
let triples_iter = self.triples_matching_inner(subject, predicate, object);
let iris = triples_iter
.into_iter()
Expand Down Expand Up @@ -184,10 +191,9 @@ impl Bag<'_> {
///
/// # Errors
/// Errors if the items are not found.
#[allow(clippy::separated_literal_suffix)]
pub fn items(&self) -> anyhow::Result<Vec<SimpleTerm>> {
let container = &self.uri;
let mut i = 1_u32;
let mut i: u32 = 1;
let mut items = vec![];
loop {
let el_uri = format!("http://www.w3.org/1999/02/22-rdf-syntax-ns#_{i}");
Expand Down
71 changes: 52 additions & 19 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,44 +50,77 @@
// > If you enable a restriction lint for your crate it is recommended to also fix code that
// > this lint triggers on. However, those lints are really strict by design and you might want
// > to #[allow] them in some special cases, with a comment justifying that.
#![allow(clippy::blanket_clippy_restriction_lints)]
#![allow(
clippy::blanket_clippy_restriction_lints,
reason = "See above explanation."
)]
#![warn(clippy::restriction)]
//
//
// =========================================================================
// Individually blanket-allow single lints relevant to this whole crate
// =========================================================================
#![allow(clippy::implicit_return, reason = "This is idiomatic Rust")]
#![allow(
// This is idiomatic Rust
clippy::implicit_return,
// Multiple deps are currently pinning `hermit-abi` — December 2022
clippy::multiple_crate_versions,
// We're not interested in becoming no-std compatible
reason = "Multiple deps are currently pinning `hermit-abi` — December 2022"
)]
#![allow(
clippy::std_instead_of_alloc,
reason = "We're not interested in becoming no-std compatible"
)]
#![allow(
clippy::std_instead_of_core,
// TODO: But I think the mod.rs is more conventional — @tombh
reason = "Import items from std instead of core"
)]
#![allow(
clippy::mod_module_files,
// Although performance is of course important for this application, it is not currently
// such that it would benefit from explicit inline suggestions. Besides, not specifying
// `#[inline]` doesn't mean that a function won't be inlined. And if performance does start
// to become a problem, there are other avenues to explore before deciding on which functions
// would benefit from explicit inlining.
reason = "TODO: But I think the mod.rs is more conventional — @tombh"
)]
#![allow(
clippy::missing_inline_in_public_items,
// I think marking `#[non_exhaustive]` is more for structs/enums that are imported into other crates
reason = "
Although performance is of course important for this application, it is not currently
such that it would benefit from explicit inline suggestions. Besides, not specifying
`#[inline]` doesn't mean that a function won't be inlined. And if performance does start
to become a problem, there are other avenues to explore before deciding on which functions
would benefit from explicit inlining
"
)]
#![allow(
clippy::exhaustive_structs,
reason = "I think marking `#[non_exhaustive]` is more for structs/enums that are imported into other crates"
)]
#![allow(
clippy::exhaustive_enums,
reason = "I think marking `#[non_exhaustive]` is more for structs/enums that are imported into other crates"
)]
#![allow(
clippy::question_mark_used,
reason = "We rely on propagating errors with question mark extensively"
)]
#![allow(
clippy::semicolon_outside_block,
// We tend to break up long functions into smaller ones, so this lint is not useful
reason = "Opt in to have semicolon in the outside block across codebase"
)]
#![allow(
clippy::single_call_fn,
reason = "We tend to break up long functions into smaller ones, so this lint is not useful"
)]
#![allow(
clippy::arithmetic_side_effects,
// We'll allow unimplemented! in code, but disallow todo!
reason = "Our arithmetic is very simple for now, so no side effects are expected at the time of writing this"
)]
#![allow(
clippy::unimplemented,
reason = "We'll allow unimplemented! in code, but disallow todo!"
)]
#![allow(
clippy::renamed_function_params,
reason = "
Sometimes collides with `min_ident_chars`, in cases where trait params consist of a single char.
So we disallow single chars, and allow renamed_function_params.
"
)]

pub mod db;
Expand Down
10 changes: 8 additions & 2 deletions src/server/api/routes.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
//! A central place to register App routes.
#![allow(clippy::exit)]
#![expect(
clippy::exit,
reason = "We exit with 1 error code on any application errors"
)]
use std::{process, sync::OnceLock};

use crate::server::api::state;
Expand Down Expand Up @@ -215,7 +218,10 @@ fn initialize_dynamic_routes<
/// * `state` - The application state
/// # Errors
/// Will error if unable to register routes (e.g. if git repository cannot be opened)
#[allow(clippy::iter_over_hash_type)]
#[expect(
clippy::iter_over_hash_type,
reason = "List of repositories that are registered as routes are always sorted, even with iterating over hash type"
)]
fn register_routes<T: Global>(cfg: &mut web::ServiceConfig, state: &T) -> anyhow::Result<()> {
for stele in state.archive().stelae.values() {
if let Some(repositories) = stele.repositories.as_ref() {
Expand Down
Loading

0 comments on commit 7200830

Please sign in to comment.