From ffd3c1eed962f52d1b8723702f2353a0e0f2e85a Mon Sep 17 00:00:00 2001 From: Theodore Bjernhed Date: Wed, 2 Jul 2025 12:04:47 +0200 Subject: [PATCH 1/6] style: remove redundant else block The `else` block adds unnecessary indentation and verbosity. To avoid this, we can simply move `return None` statement outside the `if` statement. This fixes the `redundant_else` clippy lint. --- Cargo.toml | 1 + axum-macros/src/debug_handler.rs | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 856f51f928..e7e3321f5d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,6 +46,7 @@ needless_pass_by_ref_mut = "warn" needless_pass_by_value = "warn" option_option = "warn" redundant_clone = "warn" +redundant_else = "warn" ref_option = "warn" rest_pat_in_fully_bound_structs = "warn" return_self_not_must_use = "warn" diff --git a/axum-macros/src/debug_handler.rs b/axum-macros/src/debug_handler.rs index c4838d56c7..2907af7ccf 100644 --- a/axum-macros/src/debug_handler.rs +++ b/axum-macros/src/debug_handler.rs @@ -536,9 +536,8 @@ fn check_input_order(item_fn: &ItemFn, kind: FunctionKind) -> Option compile_error!(#error); }); - } else { - return None; } + return None; } if types_that_consume_the_request.len() == 2 { From 77ef0da85275945168614f3614ddccd27157e034 Mon Sep 17 00:00:00 2001 From: Theodore Bjernhed Date: Wed, 2 Jul 2025 16:46:36 +0200 Subject: [PATCH 2/6] style: remove unnecessary `if` negation `if !..else` is a confusing syntax, as it does the inverse of what the rest of the lines says. By reordering the `if` statement we can make the `if` statement more clear. This fixes the `if_not_else` clippy lint. --- Cargo.toml | 1 + axum-macros/src/with_position.rs | 16 ++++++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e7e3321f5d..c137772e1f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ exit = "warn" filter_map_next = "warn" fn_params_excessive_bools = "warn" if_let_mutex = "warn" +if_not_else = "warn" implicit_clone = "warn" imprecise_flops = "warn" inefficient_to_string = "warn" diff --git a/axum-macros/src/with_position.rs b/axum-macros/src/with_position.rs index b5c1a60922..770efa5eaa 100644 --- a/axum-macros/src/with_position.rs +++ b/axum-macros/src/with_position.rs @@ -83,7 +83,14 @@ impl Iterator for WithPosition { fn next(&mut self) -> Option { match self.peekable.next() { Some(item) => { - if !self.handled_first { + if self.handled_first { + // Have seen the first item, and there's something left. + // Peek to see if this is the last item. + match self.peekable.peek() { + Some(_) => Some(Position::Middle(item)), + None => Some(Position::Last(item)), + } + } else { // Haven't seen the first item yet, and there is one to give. self.handled_first = true; // Peek to see if this is also the last item, @@ -92,13 +99,6 @@ impl Iterator for WithPosition { Some(_) => Some(Position::First(item)), None => Some(Position::Only(item)), } - } else { - // Have seen the first item, and there's something left. - // Peek to see if this is the last item. - match self.peekable.peek() { - Some(_) => Some(Position::Middle(item)), - None => Some(Position::Last(item)), - } } } // Iterator is finished. From 11eb562ff40fc110f3258f760ec7bac9d9e36c24 Mon Sep 17 00:00:00 2001 From: Theodore Bjernhed Date: Fri, 11 Jul 2025 21:58:04 +0200 Subject: [PATCH 3/6] style: remove explicit `iter` call for iterables Calling `.iter()` on items that are already iterable is redundant and makes the code more difficult to read. This commit also makes a couple of other related style changes to make the code easier to read. For example, removing the `iter` call on `self.routes` helped clippy find a violation of `for_kv_map`, which I also fixed. Clippy also recommended changing `["one", "two", "three", "four"].iter()` to `&["one", "two", "three", "four"]`, which does indeed match the previous behaviour. However, we can make the code easier to read by removing the unnecessary dereference of `*n` in the last `assert_eq` call by turning the slice into an array (`["one", "two", "three", "four"]`). This makes the code more readable, for free! This fixes the `explicit_iter_loop` clippy lint. --- Cargo.toml | 1 + axum/src/routing/path_router.rs | 2 +- axum/src/routing/tests/merge.rs | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c137772e1f..122acaf3ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ empty_enum = "warn" enum_glob_use = "warn" equatable_if_let = "warn" exit = "warn" +explicit_iter_loop = "warn" filter_map_next = "warn" fn_params_excessive_bools = "warn" if_let_mutex = "warn" diff --git a/axum/src/routing/path_router.rs b/axum/src/routing/path_router.rs index 329daa1b4b..d96a2717f5 100644 --- a/axum/src/routing/path_router.rs +++ b/axum/src/routing/path_router.rs @@ -102,7 +102,7 @@ where H: Handler, T: 'static, { - for (_, endpoint) in self.routes.iter_mut() { + for endpoint in self.routes.values_mut() { if let Endpoint::MethodRouter(rt) = endpoint { *rt = rt.clone().default_fallback(handler.clone()); } diff --git a/axum/src/routing/tests/merge.rs b/axum/src/routing/tests/merge.rs index b760184f54..a0272ef19d 100644 --- a/axum/src/routing/tests/merge.rs +++ b/axum/src/routing/tests/merge.rs @@ -62,11 +62,11 @@ async fn multiple_ors_balanced_differently() { async fn test(name: &str, app: Router) { let client = TestClient::new(app); - for n in ["one", "two", "three", "four"].iter() { + for n in ["one", "two", "three", "four"] { println!("running: {name} / {n}"); let res = client.get(&format!("/{n}")).await; assert_eq!(res.status(), StatusCode::OK); - assert_eq!(res.text().await, *n); + assert_eq!(res.text().await, n); } } } From 4b253319f6683637cd8606f95f54e4d5c51a1fb3 Mon Sep 17 00:00:00 2001 From: Theodore Bjernhed Date: Fri, 11 Jul 2025 23:30:08 +0200 Subject: [PATCH 4/6] chore: remove leftover `peek` call This is likely a leftover from a refactor. This fixes the `unused_peekable` clippy lint. --- Cargo.toml | 1 + axum/src/response/sse.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 122acaf3ba..353bfc07ff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -59,6 +59,7 @@ todo = "warn" trivially_copy_pass_by_ref = "warn" uninlined_format_args = "warn" unnested_or_patterns = "warn" +unused_peekable = "warn" unused_self = "warn" use_self = "warn" verbose_file_reads = "warn" diff --git a/axum/src/response/sse.rs b/axum/src/response/sse.rs index 43cd590a0d..ae486d5395 100644 --- a/axum/src/response/sse.rs +++ b/axum/src/response/sse.rs @@ -746,7 +746,7 @@ mod tests { fn parse_event(payload: &str) -> HashMap { let mut fields = HashMap::new(); - let mut lines = payload.lines().peekable(); + let mut lines = payload.lines(); while let Some(line) = lines.next() { if line.is_empty() { assert!(lines.next().is_none()); From 956dd254148c138805868d53b9c6e1b3cc4cb996 Mon Sep 17 00:00:00 2001 From: Theodore Bjernhed Date: Fri, 11 Jul 2025 23:46:47 +0200 Subject: [PATCH 5/6] style: remove semicolons at end of `if` statements Remove unnecessary semicolons at the end of control flow statements (`if` and `match`). These are not needed, and can safely be removed to avoid confusion, visual clutter, and improve readability of the code. This fixes the `unnecessary_semicolon` clippy lint. --- Cargo.toml | 1 + axum-macros/src/debug_handler.rs | 2 +- axum/src/extract/nested_path.rs | 2 +- axum/src/extract/path/de.rs | 2 +- axum/src/routing/mod.rs | 2 +- 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 353bfc07ff..070ce05de7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,6 +58,7 @@ suboptimal_flops = "warn" todo = "warn" trivially_copy_pass_by_ref = "warn" uninlined_format_args = "warn" +unnecessary_semicolon = "warn" unnested_or_patterns = "warn" unused_peekable = "warn" unused_self = "warn" diff --git a/axum-macros/src/debug_handler.rs b/axum-macros/src/debug_handler.rs index 2907af7ccf..4008e0b1fd 100644 --- a/axum-macros/src/debug_handler.rs +++ b/axum-macros/src/debug_handler.rs @@ -522,7 +522,7 @@ fn check_input_order(item_fn: &ItemFn, kind: FunctionKind) -> Option Deserializer<'de> for ValueDeserializer<'de> { )); } None => {} - }; + } self.value .take() diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 58e60bd607..2d357a86d5 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -264,7 +264,7 @@ where (false, false) => { panic!("Cannot merge two `Router`s that both have a fallback") } - }; + } panic_on_err!(this.path_router.merge(path_router)); From 1f9b929667879603c0cab10e4637bb172ac1ba4e Mon Sep 17 00:00:00 2001 From: Theodore Bjernhed Date: Wed, 2 Jul 2025 17:13:57 +0200 Subject: [PATCH 6/6] style: use `assert!` instead of `if..panic!` `assert!` is much more concise and readable than manually triggering a panic via an `if` statement. Once I did this, clippy also found a couple of conditions that could be simplified. I applied those simplifications. This fixes the `manual_assert` clippy lint. --- Cargo.toml | 1 + axum-extra/src/routing/mod.rs | 19 ++++++++-------- axum/benches/benches.rs | 4 +--- axum/src/response/sse.rs | 35 +++++++++++++++++------------- axum/src/routing/method_routing.rs | 9 ++++---- axum/src/routing/mod.rs | 14 +++++++----- axum/src/routing/path_router.rs | 9 ++++---- 7 files changed, 48 insertions(+), 43 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 070ce05de7..7077bac698 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ inefficient_to_string = "warn" linkedlist = "warn" lossy_float_literal = "warn" macro_use_imports = "warn" +manual_assert = "warn" manual_let_else = "warn" match_same_arms = "warn" match_wildcard_for_single_variants = "warn" diff --git a/axum-extra/src/routing/mod.rs b/axum-extra/src/routing/mod.rs index 45cb180e76..b6d74681c1 100644 --- a/axum-extra/src/routing/mod.rs +++ b/axum-extra/src/routing/mod.rs @@ -30,12 +30,12 @@ pub use self::typed::{SecondElementIs, TypedPath}; #[doc(hidden)] #[must_use] pub const fn __private_validate_static_path(path: &'static str) -> &'static str { - if path.is_empty() { - panic!("Paths must start with a `/`. Use \"/\" for root routes") - } - if path.as_bytes()[0] != b'/' { - panic!("Paths must start with /"); - } + assert!( + !path.is_empty(), + "Paths must start with a `/`. Use \"/\" for root routes" + ); + // `assert_eq!` is not allowed in constant functions so we have to do it manually. + assert!(path.as_bytes()[0] == b'/', "Paths must start with /"); path } @@ -356,9 +356,10 @@ where #[track_caller] fn validate_tsr_path(path: &str) { - if path == "/" { - panic!("Cannot add a trailing slash redirect route for `/`") - } + assert!( + path != "/", + "Cannot add a trailing slash redirect route for `/`" + ); } fn add_tsr_redirect_route(router: Router, path: &str) -> Router diff --git a/axum/benches/benches.rs b/axum/benches/benches.rs index c38ef91830..a6fd9da3de 100644 --- a/axum/benches/benches.rs +++ b/axum/benches/benches.rs @@ -230,9 +230,7 @@ fn install_rewrk() { let status = cmd .status() .unwrap_or_else(|_| panic!("failed to install rewrk")); - if !status.success() { - panic!("failed to install rewrk"); - } + assert!(status.success(), "failed to install rewrk"); } fn ensure_rewrk_is_installed() { diff --git a/axum/src/response/sse.rs b/axum/src/response/sse.rs index ae486d5395..23aec4c2bb 100644 --- a/axum/src/response/sse.rs +++ b/axum/src/response/sse.rs @@ -203,9 +203,10 @@ impl Event { where T: AsRef, { - if self.flags.contains(EventFlags::HAS_DATA) { - panic!("Called `Event::data` multiple times"); - } + assert!( + !self.flags.contains(EventFlags::HAS_DATA), + "Called `Event::data` multiple times" + ); for line in memchr_split(b'\n', data.as_ref().as_bytes()) { self.field("data", line); @@ -246,9 +247,10 @@ impl Event { self.0.flush() } } - if self.flags.contains(EventFlags::HAS_DATA) { - panic!("Called `Event::json_data` multiple times"); - } + assert!( + !self.flags.contains(EventFlags::HAS_DATA), + "Called `Event::json_data` multiple times" + ); let buffer = self.buffer.as_mut(); buffer.extend_from_slice(b"data: "); @@ -297,9 +299,10 @@ impl Event { where T: AsRef, { - if self.flags.contains(EventFlags::HAS_EVENT) { - panic!("Called `Event::event` multiple times"); - } + assert!( + !self.flags.contains(EventFlags::HAS_EVENT), + "Called `Event::event` multiple times" + ); self.flags.insert(EventFlags::HAS_EVENT); self.field("event", event.as_ref()); @@ -317,9 +320,10 @@ impl Event { /// /// Panics if this function has already been called on this event. pub fn retry(mut self, duration: Duration) -> Self { - if self.flags.contains(EventFlags::HAS_RETRY) { - panic!("Called `Event::retry` multiple times"); - } + assert!( + !self.flags.contains(EventFlags::HAS_RETRY), + "Called `Event::retry` multiple times" + ); self.flags.insert(EventFlags::HAS_RETRY); let buffer = self.buffer.as_mut(); @@ -364,9 +368,10 @@ impl Event { where T: AsRef, { - if self.flags.contains(EventFlags::HAS_ID) { - panic!("Called `Event::id` multiple times"); - } + assert!( + !self.flags.contains(EventFlags::HAS_ID), + "Called `Event::id` multiple times" + ); self.flags.insert(EventFlags::HAS_ID); let id = id.as_ref().as_bytes(); diff --git a/axum/src/routing/method_routing.rs b/axum/src/routing/method_routing.rs index c4ad4a07e4..4daca48c83 100644 --- a/axum/src/routing/method_routing.rs +++ b/axum/src/routing/method_routing.rs @@ -835,12 +835,11 @@ where S: Clone, { if endpoint_filter.contains(filter) { - if out.is_some() { - panic!( - "Overlapping method route. Cannot add two method routes that both handle \ + assert!( + !out.is_some(), + "Overlapping method route. Cannot add two method routes that both handle \ `{method_name}`", - ) - } + ); *out = endpoint.clone(); for method in methods { append_allow_header(allow_header, method); diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 2d357a86d5..4234cf2806 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -203,9 +203,10 @@ where #[doc(alias = "scope")] // Some web frameworks like actix-web use this term #[track_caller] pub fn nest(self, path: &str, router: Self) -> Self { - if path.is_empty() || path == "/" { - panic!("Nesting at the root is no longer supported. Use merge instead."); - } + assert!( + !(path.is_empty() || path == "/"), + "Nesting at the root is no longer supported. Use merge instead." + ); let RouterInner { path_router, @@ -229,9 +230,10 @@ where T::Response: IntoResponse, T::Future: Send + 'static, { - if path.is_empty() || path == "/" { - panic!("Nesting at the root is no longer supported. Use fallback_service instead."); - } + assert!( + !(path.is_empty() || path == "/"), + "Nesting at the root is no longer supported. Use fallback_service instead." + ); tap_inner!(self, mut this => { panic_on_err!(this.path_router.nest_service(path, service)); diff --git a/axum/src/routing/path_router.rs b/axum/src/routing/path_router.rs index d96a2717f5..3defffe29c 100644 --- a/axum/src/routing/path_router.rs +++ b/axum/src/routing/path_router.rs @@ -282,12 +282,11 @@ where >::Error: Into + 'static, >::Future: Send + 'static, { - if self.routes.is_empty() { - panic!( - "Adding a route_layer before any routes is a no-op. \ + assert!( + !self.routes.is_empty(), + "Adding a route_layer before any routes is a no-op. \ Add the routes you want the layer to apply to first." - ); - } + ); let routes = self .routes