From 7c8c5cf66e61ee1963080b81f2e75367940bca51 Mon Sep 17 00:00:00 2001 From: gesh Date: Wed, 13 May 2026 23:23:31 +0300 Subject: [PATCH 1/6] Use String::new instead of casting "" More uniform with other empty values --- Cargo.toml | 1 - src/server/state.rs | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6bdb8e2..73ae997 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -87,7 +87,6 @@ no_effect = { level = "allow", priority = 1 } pedantic = "warn" ## TODO, disabled since they currently fail ### Straightforward fix, for next PR -manual_string_new = { level = "allow", priority = 1 } items_after_statements = { level = "allow", priority = 1 } ignored_unit_patterns = { level = "allow", priority = 1 } implicit_clone = { level = "allow", priority = 1 } diff --git a/src/server/state.rs b/src/server/state.rs index d039037..67b600c 100644 --- a/src/server/state.rs +++ b/src/server/state.rs @@ -613,7 +613,7 @@ mod test { conf, Some(0), Some(0), - Some("".to_string()), + Some(String::new()), eventer, notifier, Refresher::new(), @@ -747,7 +747,7 @@ mod test { conf, Some(0), Some(0), - Some("".to_string()), + Some(String::new()), eventer, notifier, Refresher::new(), @@ -829,7 +829,7 @@ mod test { conf, Some(0), Some(0), - Some("".to_string()), + Some(String::new()), eventer, notifier, Refresher::new(), @@ -877,7 +877,7 @@ mod test { conf, Some(0), Some(0), - Some("".to_string()), + Some(String::new()), eventer, notifier, Refresher::new(), From 64b909265c2114d37ef63d315753e3599d3afe1e Mon Sep 17 00:00:00 2001 From: gesh Date: Thu, 14 May 2026 00:00:35 +0300 Subject: [PATCH 2/6] Remove unnecessary noops --- src/config.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/config.rs b/src/config.rs index 633ca61..b080ebe 100644 --- a/src/config.rs +++ b/src/config.rs @@ -934,10 +934,7 @@ mod test { Err(e) if e.contains( "Account x has an 'http' redirect but the HTTP server is set to 'none'", - ) => - { - (); - } + ) => {} Err(e) => panic!("{e:?}"), _ => panic!(), } @@ -955,10 +952,7 @@ mod test { Err(e) if e.contains( "Account x has an 'https' redirect but the HTTPS server is set to 'none'", - ) => - { - (); - } + ) => {} Err(e) => panic!("{e:?}"), _ => panic!(), } From ee0dc682fa6db1c496387d60a183ed5ae07bce44 Mon Sep 17 00:00:00 2001 From: gesh Date: Wed, 13 May 2026 21:40:17 +0300 Subject: [PATCH 3/6] Be more explicit about matching against () Protect against type changes in the scrutinee --- Cargo.toml | 1 - src/config.rs | 4 ++-- src/server/mod.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 73ae997..7ea69f0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -88,7 +88,6 @@ pedantic = "warn" ## TODO, disabled since they currently fail ### Straightforward fix, for next PR items_after_statements = { level = "allow", priority = 1 } -ignored_unit_patterns = { level = "allow", priority = 1 } implicit_clone = { level = "allow", priority = 1 } unused_self = { level = "allow", priority = 1 } ### Lint guidance seems wrong, needs decision diff --git a/src/config.rs b/src/config.rs index b080ebe..b3fa8e8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -572,10 +572,10 @@ impl Account { let mut url = Url::parse(&self.redirect_uri)?; if https_port.is_some() && self.redirect_uri.to_lowercase().starts_with("https") { url.set_port(https_port) - .map_err(|_| "Cannot set https port")?; + .map_err(|()| "Cannot set https port")?; } else { url.set_port(http_port) - .map_err(|_| "Cannot set http port")?; + .map_err(|()| "Cannot set http port")?; } Ok(url) } diff --git a/src/server/mod.rs b/src/server/mod.rs index 7272f8a..1a6d6b9 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -170,7 +170,7 @@ fn request(pstate: Arc, mut stream: UnixStream) -> Result<() } "restore" => { match pstate.restore(rest.to_vec()) { - Ok(_) => stream.write_all(b"ok:")?, + Ok(()) => stream.write_all(b"ok:")?, Err(e) => stream.write_all(format!("error:{e:}").as_bytes())?, } return Ok(()); From fc1104cf23d0e7ecade170a9a4041542c93480d2 Mon Sep 17 00:00:00 2001 From: gesh Date: Wed, 13 May 2026 21:46:17 +0300 Subject: [PATCH 4/6] Be more explicit about using clone() Also remove unnecessary clones (Instant impls Copy) Cannot enable lints until [grmtools#639] is merged since the generated code for the lexer will cause these to unblockably complain. When it is, re-enable implicit_clone and redundant_clone [grmtools#639]: https://github.com/softdevteam/grmtools/pull/639 --- Cargo.toml | 7 +++++-- src/main.rs | 2 +- src/server/http_server.rs | 2 +- src/server/notifier.rs | 2 +- src/server/refresher.rs | 4 ++-- src/server/state.rs | 4 ++-- 6 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7ea69f0..990dd01 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -88,7 +88,6 @@ pedantic = "warn" ## TODO, disabled since they currently fail ### Straightforward fix, for next PR items_after_statements = { level = "allow", priority = 1 } -implicit_clone = { level = "allow", priority = 1 } unused_self = { level = "allow", priority = 1 } ### Lint guidance seems wrong, needs decision similar_names = { level = "allow", priority = 1 } @@ -107,10 +106,14 @@ needless_pass_by_value = { level = "allow", priority = 1 } nursery = "warn" ## TODO, disabled since they currently fail ### Straightforward fix, for next PR -redundant_clone = { level = "allow", priority = 1 } needless_collect = { level = "allow", priority = 1 } ### Would change style, need buy-in + style guide option_if_let_else = { level = "allow", priority = 1 } ### Needs analysis + oversight for correctness missing_const_for_fn = { level = "allow", priority = 1 } significant_drop_tightening = { level = "allow", priority = 1 } + +## Blocked +### By: https://github.com/softdevteam/grmtools/pull/639 +implicit_clone = { level = "allow", priority = 1 } +redundant_clone = { level = "allow", priority = 1 } diff --git a/src/main.rs b/src/main.rs index 33735e0..f182b9d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -310,7 +310,7 @@ fn main() { // Files in this directory MAY be subjected to periodic clean-up. To ensure that your files // are not removed, they should have their access time timestamp modified at least once every // 6 hours of monotonic time - let sock_path_cl = sock_path.clone(); + let sock_path_cl = sock_path; thread::spawn(move || loop { thread::sleep(Duration::from_hours(6)); let _ = utimensat( diff --git a/src/server/http_server.rs b/src/server/http_server.rs index f9a9c8b..9f2dd74 100644 --- a/src/server/http_server.rs +++ b/src/server/http_server.rs @@ -209,7 +209,7 @@ fn request( (Some("Bearer"), Some(expires_in), Some(access_token), refresh_token) => { let now = Instant::now(); let expiry = expiry_instant(&ct_lk, act_id, now, expires_in)?; - let act_name = ct_lk.account(act_id).name.to_owned(); + let act_name = ct_lk.account(act_id).name.clone(); ct_lk.tokenstate_replace( act_id, TokenState::Active { diff --git a/src/server/notifier.rs b/src/server/notifier.rs index da49537..627160e 100644 --- a/src/server/notifier.rs +++ b/src/server/notifier.rs @@ -84,7 +84,7 @@ impl Notifier { let url = url.clone(); let act = ct_lk.account(act_id); if let Some(ref cmd) = ct_lk.config().auth_notify_cmd { - auth_cmds.push((act.name.to_owned(), cmd.clone(), url)); + auth_cmds.push((act.name.clone(), cmd.clone(), url)); } ct_lk.tokenstate_replace(act_id, ts); } diff --git a/src/server/refresher.rs b/src/server/refresher.rs index d4dab5d..f690839 100644 --- a/src/server/refresher.rs +++ b/src/server/refresher.rs @@ -357,7 +357,7 @@ impl Refresher { // If the second case occurs, we assume that the user knows that the token // really needs refreshing, and we treat the token as if it had expired. if let Some(t) = lra.checked_add(act.refresh_retry(ct_lk.config())) { - return Some(t.to_owned()); + return Some(t); } } @@ -372,7 +372,7 @@ impl Refresher { { expiry = cmp::min(expiry, t); } - Some(expiry.to_owned()) + Some(expiry) } _ => None, } diff --git a/src/server/state.rs b/src/server/state.rs index 67b600c..408adcb 100644 --- a/src/server/state.rs +++ b/src/server/state.rs @@ -784,7 +784,7 @@ mod test { } { - pstate.restore(dump.clone()).unwrap(); + pstate.restore(dump).unwrap(); let ct_lk = pstate.ct_lock(); let x_id = ct_lk.validate_act_name("x").unwrap(); @@ -843,7 +843,7 @@ mod test { } { - pstate.restore(dump.clone()).unwrap(); + pstate.restore(dump).unwrap(); let ct_lk = pstate.ct_lock(); let x_id = ct_lk.validate_act_name("x").unwrap(); From 0514cb3ca8dd6e203b6919df6926d6410c41371d Mon Sep 17 00:00:00 2001 From: gesh Date: Sun, 24 May 2026 23:24:50 +0300 Subject: [PATCH 5/6] Document why we globally disable needless_collect Specifically in our case, its integration with borrow checking is buggy, so causes pointless false positives. See eg https://github.com/rust-lang/rust-clippy/issues/6066 --- Cargo.toml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 990dd01..1519922 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -104,9 +104,11 @@ redundant_else = { level = "allow", priority = 1 } needless_pass_by_value = { level = "allow", priority = 1 } nursery = "warn" -## TODO, disabled since they currently fail -### Straightforward fix, for next PR +# Overzealous, false positives suggest broken code, see +# https://github.com/rust-lang/rust-clippy/issues/6066 and refs there needless_collect = { level = "allow", priority = 1 } + +## TODO, disabled since they currently fail ### Would change style, need buy-in + style guide option_if_let_else = { level = "allow", priority = 1 } ### Needs analysis + oversight for correctness From 51c3cb1f1ee7be52f729a242859efa953ff68269 Mon Sep 17 00:00:00 2001 From: gesh Date: Sun, 24 May 2026 23:17:53 +0300 Subject: [PATCH 6/6] Document why we disable elided_lifetimes_in_paths Original motivation for this lint was cases like fn(&T) -> UnclearLifetime where the lifetime of the output type wasn't obvious without diving into the implementation (and it could thus hide a hidden dependency on the &T). The lint as written is a very blunt instrument, and also covers the innocuous fn(ContainsLifetime) -> NoLifetime case we have throughout the codebase. Work is being done to narrow the scope of the lint, and in particular to make our usecase accepted, so disable it globally for now. See also: https://github.com/rust-lang/rust/issues/91639#issuecomment-2659700523 https://github.com/rust-lang/rust/pull/120808 https://rust-lang.zulipchat.com/#narrow/channel/213817/near/528092054 --- Cargo.toml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1519922..8f13c0a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,10 +72,17 @@ rust_2018_idioms = "deny" rust_2021_compatibility = "deny" rust_2024_compatibility = "deny" let_underscore = "warn" +# Especially heavyweight to resolve, for not much added readability +# This lint is also being revised, and current direction of travel +# (as of 2026-05-24) is to allow the fn(ContainsLifetime) -> NoLifetime cases +# present in the codebase, and to focus on the fn(&T) -> UnclearLifetime case +# instead. See also +# https://github.com/rust-lang/rust/issues/91639#issuecomment-2659700523 +# https://github.com/rust-lang/rust/pull/120808 +# https://rust-lang.zulipchat.com/#narrow/channel/213817/near/528092054 +elided_lifetimes_in_paths = { level = "allow", priority = 1 } ## TODO, disabled since they currently fail -### Would change style -elided_lifetimes_in_paths = { level = "allow", priority = 1 } ### Needs analysis + oversight for correctness tail_expr_drop_order = { level = "allow", priority = 1 }