Clippy clean, and rustfmt and clippy part of the checks

cargo clippy --workspace --all-targets had 24 warnings; its own fixes took
most, the rest by hand: two type aliases for long types, a struct built
whole, a checked division, a doc comment's stray blank line. Its fix for
the stale-feed fold was wrong: `any` stops at the first feed it removes,
where the fold removed every one, so that is a loop instead.

A pre-commit hook in .githooks refuses unformatted Rust; clippy, being a
build, is in CLAUDE.md's checks with -D warnings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-17 21:59:37 +00:00
parent 692a510217
commit c90a66a24f
10 changed files with 89 additions and 57 deletions

3
.git-blame-ignore-revs Normal file
View File

@@ -0,0 +1,3 @@
# Formatting only: `git config blame.ignoreRevsFile .git-blame-ignore-revs` makes blame skip these.
# rustfmt adopted
692a5102179b958b5d15cecc4f5e6f16b2955b76

7
.githooks/pre-commit Executable file
View File

@@ -0,0 +1,7 @@
#!/bin/sh
# Refuses a commit whose Rust is not as rustfmt writes it. Clippy is left to the checks before a
# push (CLAUDE.md, Tests): it is a build, and Tower's builds go one at a time.
git diff --cached --name-only --diff-filter=ACM | grep -q '\.rs$' || exit 0
cargo fmt --all --check >/dev/null 2>&1 && exit 0
echo "pre-commit: the Rust is not formatted; run cargo fmt --all" >&2
exit 1

View File

@@ -171,6 +171,8 @@ Patching that file by guessing an anchor string has failed repeatedly. Read the
## Tests ## Tests
```sh ```sh
cargo fmt --all --check # rustfmt, as rustfmt.toml says; the pre-commit hook refuses a commit without it
cargo clippy --workspace --all-targets -- -D warnings # no warnings, ever
cargo test # ~80 tests: parsing, filters, retention, schedules, SQL, per-user state cargo test # ~80 tests: parsing, filters, retention, schedules, SQL, per-user state
npx tsc -p . # type-checks web/src npx tsc -p . # type-checks web/src
node tests/page-smoke.js node tests/page-smoke.js
@@ -194,6 +196,13 @@ session was stopped. So:
* Builds and test binaries live under `target/`, a link to `/mnt/fast`; linking through * Builds and test binaries live under `target/`, a link to `/mnt/fast`; linking through
`/mnt/user` (FUSE) once took the load to 248 on its own. `/mnt/user` (FUSE) once took the load to 248 on its own.
The Rust is formatted by rustfmt (`rustfmt.toml`) and kept free of clippy's warnings. A clone
turns on the hook once: `git config core.hooksPath .githooks`; and `git config blame.ignoreRevsFile
.git-blame-ignore-revs` keeps blame past the commit that first formatted everything. A clippy
suggestion is not always the same code: on adopting it, `fold(false, |any, id| remove(id) || any)`
was offered as `any(...)`, which stops at the first and would have removed one feed of many. A lint
that is wrong for a line gets `#[allow(clippy::...)]` there with a comment saying why.
Things about the browser suite that have cost time: Things about the browser suite that have cost time:
* It starts **its own daemon and database** under `/tmp/ipx-ui-test`, wiped once per run. Playwright * It starts **its own daemon and database** under `/tmp/ipx-ui-test`, wiped once per run. Playwright

View File

@@ -424,8 +424,7 @@ impl Store {
} }
pub async fn due_one(&self, id: &str) -> Result<Option<Due>> { pub async fn due_one(&self, id: &str) -> Result<Option<Due>> {
Ok(self self.rows("SELECT id, url, etag, last_modified FROM feeds WHERE id = $1", vec![id.into()])
.rows("SELECT id, url, etag, last_modified FROM feeds WHERE id = $1", vec![id.into()])
.await? .await?
.first() .first()
.map(|r| -> Result<Due> { .map(|r| -> Result<Due> {
@@ -436,7 +435,7 @@ impl Store {
last_modified: r.try_get("", "last_modified")?, last_modified: r.try_get("", "last_modified")?,
}) })
}) })
.transpose()?) .transpose()
} }
pub async fn save(&self, id: &str, got: Refreshed) -> Result<()> { pub async fn save(&self, id: &str, got: Refreshed) -> Result<()> {
@@ -507,8 +506,8 @@ impl Store {
/// A feed's last error as its page says it: explained once it has been failing a day, or else /// A feed's last error as its page says it: explained once it has been failing a day, or else
/// only that the check failed. Never the error itself, which is for the log. /// only that the check failed. Never the error itself, which is for the log.
fn failure_words(err: &str, failing_for: i64) -> String { fn failure_words(err: &str, failing_for: i64) -> String {
Some(err) (failing_for >= FLAG_AFTER_SECS)
.filter(|_| failing_for >= FLAG_AFTER_SECS) .then_some(err)
.and_then(ipx_feed::explain_failure) .and_then(ipx_feed::explain_failure)
.map_or_else(|| "Its last check failed.".to_owned(), |f| f.reason.to_owned()) .map_or_else(|| "Its last check failed.".to_owned(), |f| f.reason.to_owned())
} }

View File

@@ -386,8 +386,11 @@ fn patreon_campaign(head: &[u8]) -> Option<String> {
}) })
} }
/// A campaign's name, and its shows as (id, title).
type PatreonShows = (Option<String>, Vec<(String, String)>);
/// A campaign's name and its shows as (id, title), from Patreon's JSON:API answer. /// A campaign's name and its shows as (id, title), from Patreon's JSON:API answer.
fn parse_patreon_shows(json: &[u8]) -> Result<(Option<String>, Vec<(String, String)>)> { fn parse_patreon_shows(json: &[u8]) -> Result<PatreonShows> {
let v: serde_json::Value = serde_json::from_slice(json).context("Patreon's answer is not JSON")?; let v: serde_json::Value = serde_json::from_slice(json).context("Patreon's answer is not JSON")?;
// Missing is not the same as none. Read as no shows, the creator feed would be scanned as // Missing is not the same as none. Read as no shows, the creator feed would be scanned as
// a plain feed, claim every show's files, and leave the shows empty once the list returned. // a plain feed, claim every show's files, and leave the shows empty once the list returned.

View File

@@ -771,7 +771,6 @@ impl Db {
/// Returns true when this enclosure URL is new. False means we have downloaded it /// Returns true when this enclosure URL is new. False means we have downloaded it
/// before, or deliberately reaped it -- either way it is not fetched again. /// before, or deliberately reaped it -- either way it is not fetched again.
pub async fn mark_downloaded(&self, url: &str, path: &std::path::Path, bytes: u64) -> Result<()> { pub async fn mark_downloaded(&self, url: &str, path: &std::path::Path, bytes: u64) -> Result<()> {
self.exec( self.exec(
"UPDATE enclosures SET state = 'done', path = $2, bytes_done = $3, "UPDATE enclosures SET state = 'done', path = $2, bytes_done = $3,
@@ -1789,7 +1788,7 @@ impl Db {
/// False when they do not subscribe to it, as `set_pinned`. /// False when they do not subscribe to it, as `set_pinned`.
pub async fn set_kept(&self, user_id: i64, feed_id: &str, on: bool) -> Result<bool> { pub async fn set_kept(&self, user_id: i64, feed_id: &str, on: bool) -> Result<bool> {
let r = subscriptions::Entity::update_many() let r = subscriptions::Entity::update_many()
.col_expr(subscriptions::Column::Kept, Expr::val(on).into()) .col_expr(subscriptions::Column::Kept, Expr::val(on))
.filter(subscriptions::Column::UserId.eq(user_id)) .filter(subscriptions::Column::UserId.eq(user_id))
.filter(subscriptions::Column::FeedId.eq(feed_id)) .filter(subscriptions::Column::FeedId.eq(feed_id))
.exec(&self.orm) .exec(&self.orm)
@@ -1819,7 +1818,7 @@ impl Db {
pub async fn set_pinned(&self, user_id: i64, feed_id: &str, on: bool) -> Result<bool> { pub async fn set_pinned(&self, user_id: i64, feed_id: &str, on: bool) -> Result<bool> {
let r = subscriptions::Entity::update_many() let r = subscriptions::Entity::update_many()
.col_expr(subscriptions::Column::Pinned, Expr::val(on).into()) .col_expr(subscriptions::Column::Pinned, Expr::val(on))
.filter(subscriptions::Column::UserId.eq(user_id)) .filter(subscriptions::Column::UserId.eq(user_id))
.filter(subscriptions::Column::FeedId.eq(feed_id)) .filter(subscriptions::Column::FeedId.eq(feed_id))
.exec(&self.orm) .exec(&self.orm)
@@ -1927,7 +1926,7 @@ impl Db {
use sea_orm::sea_query::ExprTrait; use sea_orm::sea_query::ExprTrait;
let now = now(); let now = now();
users::Entity::update_many() users::Entity::update_many()
.col_expr(users::Column::LastLogin, Expr::val(now).into()) .col_expr(users::Column::LastLogin, Expr::val(now))
.filter(users::Column::Id.eq(id)) .filter(users::Column::Id.eq(id))
.filter(Expr::expr(Func::coalesce([Expr::col(users::Column::LastLogin), Expr::val(0)])).lte(now - 3600)) .filter(Expr::expr(Func::coalesce([Expr::col(users::Column::LastLogin), Expr::val(0)])).lte(now - 3600))
.exec(&self.orm) .exec(&self.orm)
@@ -1978,7 +1977,7 @@ impl Db {
.and_then(|(_, u)| u); .and_then(|(_, u)| u);
if found.is_some() { if found.is_some() {
sessions::Entity::update_many() sessions::Entity::update_many()
.col_expr(sessions::Column::Seen, Expr::val(now()).into()) .col_expr(sessions::Column::Seen, Expr::val(now()))
.filter(sessions::Column::Token.eq(token)) .filter(sessions::Column::Token.eq(token))
.exec(&self.orm) .exec(&self.orm)
.await?; .await?;
@@ -2035,7 +2034,7 @@ impl Db {
.await?; .await?;
let Some((token, Some(user))) = found else { return Ok(None) }; let Some((token, Some(user))) = found else { return Ok(None) };
api_tokens::Entity::update_many() api_tokens::Entity::update_many()
.col_expr(api_tokens::Column::LastUsed, Expr::val(now()).into()) .col_expr(api_tokens::Column::LastUsed, Expr::val(now()))
.filter(api_tokens::Column::Id.eq(token.id)) .filter(api_tokens::Column::Id.eq(token.id))
.exec(&self.orm) .exec(&self.orm)
.await?; .await?;
@@ -2711,7 +2710,7 @@ impl Db {
} }
if name.is_some() || words.is_some() || description.is_some() { if name.is_some() || words.is_some() || description.is_some() {
signal_verdicts::Entity::update_many() signal_verdicts::Entity::update_many()
.col_expr(signal_verdicts::Column::Recheck, Expr::val(true).into()) .col_expr(signal_verdicts::Column::Recheck, Expr::val(true))
.filter(signal_verdicts::Column::SignalId.eq(id)) .filter(signal_verdicts::Column::SignalId.eq(id))
.filter(signal_verdicts::Column::Hit.eq(true)) .filter(signal_verdicts::Column::Hit.eq(true))
.exec(&self.orm) .exec(&self.orm)

View File

@@ -85,7 +85,7 @@ pub fn filename_for(url: &str, content_disposition: Option<&str>) -> String {
} }
let from_url = url::Url::parse(url) let from_url = url::Url::parse(url)
.ok() .ok()
.and_then(|u| u.path_segments().and_then(|s| s.filter(|p| !p.is_empty()).next_back()).map(str::to_owned)) .and_then(|u| u.path_segments().and_then(|mut s| s.rfind(|p| !p.is_empty())).map(str::to_owned))
.map(|seg| percent_encoding::percent_decode_str(&seg).decode_utf8_lossy().into_owned()) .map(|seg| percent_encoding::percent_decode_str(&seg).decode_utf8_lossy().into_owned())
.unwrap_or_default(); .unwrap_or_default();
sanitize(&from_url) sanitize(&from_url)

View File

@@ -397,11 +397,13 @@ async fn main() -> Result<()> {
Command::Export { file } => export(&ctx, &file).await, Command::Export { file } => export(&ctx, &file).await,
_ => run(&ctx, wire_cmd.expect("only List and Daemon have no wire form")).await, _ => run(&ctx, wire_cmd.expect("only List and Daemon have no wire form")).await,
}; };
if result.is_ok() && changes_catalogue && ipc::daemon_is_live(&socket).await { if result.is_ok()
if let Err(e) = ipc::reload(&socket).await { && changes_catalogue
&& ipc::daemon_is_live(&socket).await
&& let Err(e) = ipc::reload(&socket).await
{
eprintln!("the running daemon did not take the change ({e:#}); restart it, or it may undo it"); eprintln!("the running daemon did not take the change ({e:#}); restart it, or it may undo it");
} }
}
// The batch exporter holds the last few seconds of spans; without this they are lost. // The batch exporter holds the last few seconds of spans; without this they are lost.
if let Some(p) = otel { if let Some(p) = otel {
let _ = p.shutdown(); let _ = p.shutdown();
@@ -573,8 +575,8 @@ pub(crate) async fn status(ctx: &Ctx) -> Event {
use std::sync::atomic::Ordering::Relaxed; use std::sync::atomic::Ordering::Relaxed;
let st = &metrics::STATS; let st = &metrics::STATS;
st.feeds.store(feeds as i64, Relaxed); st.feeds.store(feeds as i64, Relaxed);
st.pending.store(pending as i64, Relaxed); st.pending.store(pending, Relaxed);
st.downloaded.store(downloaded as i64, Relaxed); st.downloaded.store(downloaded, Relaxed);
Event::Status { feeds, pending, downloaded } Event::Status { feeds, pending, downloaded }
} }
Err(e) => Event::Error { msg: format!("{e:#}") }, Err(e) => Event::Error { msg: format!("{e:#}") },
@@ -1086,8 +1088,10 @@ async fn assemble_config(db: &db::Db, mut cfg: config::Config, path: &std::path:
} }
async fn export(ctx: &Ctx, file: &std::path::Path) -> Result<()> { async fn export(ctx: &Ctx, file: &std::path::Path) -> Result<()> {
let mut doc = opml::OPML::default(); let mut doc = opml::OPML {
doc.head = Some(opml::Head { title: Some("ipx subscriptions".into()), ..Default::default() }); head: Some(opml::Head { title: Some("ipx subscriptions".into()), ..Default::default() }),
..Default::default()
};
for (id, feed) in &ctx.cfg().feeds { for (id, feed) in &ctx.cfg().feeds {
let title = ctx.db.feed_summary(id).await.ok().and_then(|s| s.title).unwrap_or_else(|| id.clone()); let title = ctx.db.feed_summary(id).await.ok().and_then(|s| s.title).unwrap_or_else(|| id.clone());
doc.add_feed(&title, &feed.url); doc.add_feed(&title, &feed.url);
@@ -1139,7 +1143,12 @@ async fn clean_directory(ctx: &Ctx) -> Result<()> {
return Ok(()); return Ok(());
} }
let mut cfg = ctx.edit_cfg().await; let mut cfg = ctx.edit_cfg().await;
if stale.iter().fold(false, |any, id| cfg.feeds.remove(id).is_some() || any) { // Every one removed, so not `any`, which would stop at the first.
let mut removed = false;
for id in &stale {
removed |= cfg.feeds.remove(id).is_some();
}
if removed {
ctx.store_cfg(cfg).await?; ctx.store_cfg(cfg).await?;
} }
for id in &stale { for id in &stale {
@@ -1208,15 +1217,16 @@ async fn fetch(ctx: &Arc<Ctx>, only: Option<&str>, force: bool, scope: &[String]
state.last_modified = None; state.last_modified = None;
} }
if !force && let Some(at) = ctx.due(&cfg, &sub.cfg, id, &state, subscribed.contains_key(id)) { if !force
if at > db::now() { && let Some(at) = ctx.due(&cfg, &sub.cfg, id, &state, subscribed.contains_key(id))
&& at > db::now()
{
ctx.out.emit(Event::FeedSkip { ctx.out.emit(Event::FeedSkip {
feed: id.clone(), feed: id.clone(),
reason: format!("not due for {}", duration((at - db::now()) as u64)), reason: format!("not due for {}", duration((at - db::now()) as u64)),
}); });
continue; continue;
} }
}
due.push((sub, state)); due.push((sub, state));
} }
@@ -1672,7 +1682,7 @@ async fn scan_one(
force: bool, force: bool,
// The body the scan already fetched for it, if it did (#104), and where it moved, if it did; // The body the scan already fetched for it, if it did (#104), and where it moved, if it did;
// with the site's icon when a forced scan looked it up meanwhile, "" for none (#228). // with the site's icon when a forced scan looked it up meanwhile, "" for none (#228).
prefetched: Option<(Result<(feed::Fetched, Option<String>)>, Option<String>)>, prefetched: Option<Prefetched>,
) -> Result<Outcome> { ) -> Result<Outcome> {
let (prefetched, icon) = match prefetched { let (prefetched, icon) = match prefetched {
Some((got, icon)) => (Some(got), icon), Some((got, icon)) => (Some(got), icon),
@@ -2004,6 +2014,9 @@ async fn scan_one(
Ok(Outcome::Feed(scan)) Ok(Outcome::Feed(scan))
} }
/// A feed's body fetched by the scan ahead of its turn, and the site's icon if one was looked up.
type Prefetched = (Result<(feed::Fetched, Option<String>)>, Option<String>);
async fn sync_opml(ctx: &Arc<Ctx>, parent_id: &str, parent: &config::Feed, bytes: &[u8]) -> Result<Outcome> { async fn sync_opml(ctx: &Arc<Ctx>, parent_id: &str, parent: &config::Feed, bytes: &[u8]) -> Result<Outcome> {
let listed = feed::parse_opml(bytes)?; let listed = feed::parse_opml(bytes)?;
if let Some(title) = feed::opml_title(bytes) { if let Some(title) = feed::opml_title(bytes) {
@@ -2381,9 +2394,9 @@ async fn torrent_one(
ctx.torrents() ctx.torrents()
.await? .await?
.fetch(&cfg, url, dest_dir, |done, total| { .fetch(&cfg, url, dest_dir, |done, total| {
if total > 0 { if let Some(pct) = (done * 100).checked_div(total).map(|p| p as i64)
let pct = (done * 100 / total) as i64; && pct > last_pct
if pct > last_pct { {
last_pct = pct; last_pct = pct;
ctx.out.emit(Event::Progress { ctx.out.emit(Event::Progress {
feed: feed_id.to_string(), feed: feed_id.to_string(),
@@ -2394,7 +2407,6 @@ async fn torrent_one(
total: Some(total), total: Some(total),
}); });
} }
}
}) })
.await .await
} }
@@ -2522,8 +2534,8 @@ mod tests {
fn feed() -> config::Feed { fn feed() -> config::Feed {
// Whatever `ipx add` would write, which is the shape every code path sees. // Whatever `ipx add` would write, which is the shape every code path sees.
let mut cfg = config::Config::default(); let mut cfg = config::Config::default();
let f = add_one_cfg(&mut cfg, "http://x/f.xml", None, vec![]);
f add_one_cfg(&mut cfg, "http://x/f.xml", None, vec![])
} }
/// The feed entry `add` builds, without the network round trip it does for a title. /// The feed entry `add` builds, without the network round trip it does for a title.

View File

@@ -231,7 +231,7 @@ fn full(items: &[Unjudged]) -> bool {
} }
pub fn refuses_key(e: &anyhow::Error) -> bool { pub fn refuses_key(e: &anyhow::Error) -> bool {
matches!(status(e), Some(401 | 402 | 403)) matches!(status(e), Some(401..=403))
} }
fn rate_limited(e: &anyhow::Error) -> bool { fn rate_limited(e: &anyhow::Error) -> bool {

View File

@@ -174,11 +174,11 @@ async fn auth(State(state): State<WebState>, mut req: Request, next: Next) -> Re
} }
// 2. A session cookie from signing in here. // 2. A session cookie from signing in here.
if user.is_none() { if user.is_none()
if let Some(sid) = cookie(&req, SESSION_COOKIE) { && let Some(sid) = cookie(&req, SESSION_COOKIE)
{
user = state.ctx.db.session_user(&sid, cfg.web.session_days.max(1) * 86_400).await.unwrap_or(None); user = state.ctx.db.session_user(&sid, cfg.web.session_days.max(1) * 86_400).await.unwrap_or(None);
} }
}
// 3. The shared token, which is the admin: the healthcheck and any scripts predate // 3. The shared token, which is the admin: the healthcheck and any scripts predate
// accounts and must keep working. // accounts and must keep working.
@@ -213,11 +213,11 @@ async fn auth(State(state): State<WebState>, mut req: Request, next: Next) -> Re
req.extensions_mut().insert(user); req.extensions_mut().insert(user);
req.extensions_mut().insert(Proxied(by_proxy)); req.extensions_mut().insert(Proxied(by_proxy));
let mut resp = next.run(req).await; let mut resp = next.run(req).await;
if let Some(c) = set_cookie { if let Some(c) = set_cookie
if let Ok(v) = header::HeaderValue::from_str(&c) { && let Ok(v) = header::HeaderValue::from_str(&c)
{
resp.headers_mut().insert(header::SET_COOKIE, v); resp.headers_mut().insert(header::SET_COOKIE, v);
} }
}
resp resp
} }
@@ -938,7 +938,7 @@ async fn feed_rows(state: &WebState, user: &crate::db::User, only: Option<&str>)
failing: s failing: s
.error_since .error_since
.filter(|since| crate::db::now() - since >= FLAG_AFTER_SECS) .filter(|since| crate::db::now() - since >= FLAG_AFTER_SECS)
.and_then(|_| s.last_error.as_deref()) .and(s.last_error.as_deref())
.and_then(crate::feed::explain_failure) .and_then(crate::feed::explain_failure)
.map(|f| FailingRow { reason: f.reason, new_url: f.new_url }), .map(|f| FailingRow { reason: f.reason, new_url: f.new_url }),
last_error: s.last_error, last_error: s.last_error,