From 42531e017c62650486d8457aed83604bb3bc2b70 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Wed, 31 Jul 2024 20:37:40 +0200 Subject: [PATCH] Clippy fixes from stable and nightly (#13455) - **Doccomment style fixes** - **Forgotten stuff in `nu-pretty-hex`** - **Don't `for` around an `Option`** - and more I think the suggestions here are a net positive, some of the suggestions moved into #13498 feel somewhat arbitrary, I also raised https://github.com/rust-lang/rust-clippy/issues/13188 as the nightly `byte_char_slices` would require either a global allow or otherwise a ton of granular allows or possibly confusing bytestring literals. --- .../nu-cli/src/commands/history/history_.rs | 60 ++-- crates/nu-command/src/conversions/fill.rs | 8 +- crates/nu-command/src/network/http/client.rs | 6 +- .../nu-command/tests/commands/random/mod.rs | 1 - .../nu-command/tests/commands/random/uuid.rs | 2 +- crates/nu-parser/src/parse_keywords.rs | 18 +- crates/nu-parser/src/parser.rs | 2 +- crates/nu-path/src/path.rs | 4 +- crates/nu-path/src/tilde.rs | 2 +- crates/nu-pretty-hex/src/lib.rs | 5 - crates/nu-pretty-hex/tests/tests.rs | 295 +++++++++--------- .../nu-protocol/src/pipeline/pipeline_data.rs | 12 +- crates/nu-protocol/src/signature.rs | 5 +- crates/nu-system/src/foreground.rs | 3 +- crates/nuon/src/from.rs | 2 +- 15 files changed, 185 insertions(+), 240 deletions(-) diff --git a/crates/nu-cli/src/commands/history/history_.rs b/crates/nu-cli/src/commands/history/history_.rs index cdf85eea72..1f3d1082fa 100644 --- a/crates/nu-cli/src/commands/history/history_.rs +++ b/crates/nu-cli/src/commands/history/history_.rs @@ -156,58 +156,34 @@ fn create_history_record(idx: usize, entry: HistoryItem, long: bool, head: Span) //2. Create a record of either short or long columns and values let item_id_value = Value::int( - match entry.id { - Some(id) => { - let ids = id.to_string(); - match ids.parse::() { - Ok(i) => i, - _ => 0i64, - } - } - None => 0i64, - }, + entry + .id + .and_then(|id| id.to_string().parse::().ok()) + .unwrap_or_default(), head, ); let start_timestamp_value = Value::string( - match entry.start_timestamp { - Some(time) => time.to_string(), - None => "".into(), - }, + entry + .start_timestamp + .map(|time| time.to_string()) + .unwrap_or_default(), head, ); let command_value = Value::string(entry.command_line, head); let session_id_value = Value::int( - match entry.session_id { - Some(sid) => { - let sids = sid.to_string(); - match sids.parse::() { - Ok(i) => i, - _ => 0i64, - } - } - None => 0i64, - }, - head, - ); - let hostname_value = Value::string( - match entry.hostname { - Some(host) => host, - None => "".into(), - }, - head, - ); - let cwd_value = Value::string( - match entry.cwd { - Some(cwd) => cwd, - None => "".into(), - }, + entry + .session_id + .and_then(|id| id.to_string().parse::().ok()) + .unwrap_or_default(), head, ); + let hostname_value = Value::string(entry.hostname.unwrap_or_default(), head); + let cwd_value = Value::string(entry.cwd.unwrap_or_default(), head); let duration_value = Value::duration( - match entry.duration { - Some(d) => d.as_nanos().try_into().unwrap_or(0), - None => 0, - }, + entry + .duration + .and_then(|d| d.as_nanos().try_into().ok()) + .unwrap_or(0), head, ); let exit_status_value = Value::int(entry.exit_status.unwrap_or(0), head); diff --git a/crates/nu-command/src/conversions/fill.rs b/crates/nu-command/src/conversions/fill.rs index eaf8c05da1..3bd205814a 100644 --- a/crates/nu-command/src/conversions/fill.rs +++ b/crates/nu-command/src/conversions/fill.rs @@ -150,13 +150,9 @@ fn fill( FillAlignment::Left }; - let width = if let Some(arg) = width_arg { arg } else { 1 }; + let width = width_arg.unwrap_or(1); - let character = if let Some(arg) = character_arg { - arg - } else { - " ".to_string() - }; + let character = character_arg.unwrap_or_else(|| " ".to_string()); let arg = Arguments { width, diff --git a/crates/nu-command/src/network/http/client.rs b/crates/nu-command/src/network/http/client.rs index deeb768eea..e348bedc25 100644 --- a/crates/nu-command/src/network/http/client.rs +++ b/crates/nu-command/src/network/http/client.rs @@ -141,17 +141,17 @@ pub fn request_add_authorization_header( let login = match (user, password) { (Some(user), Some(password)) => { let mut enc_str = String::new(); - base64_engine.encode_string(&format!("{user}:{password}"), &mut enc_str); + base64_engine.encode_string(format!("{user}:{password}"), &mut enc_str); Some(enc_str) } (Some(user), _) => { let mut enc_str = String::new(); - base64_engine.encode_string(&format!("{user}:"), &mut enc_str); + base64_engine.encode_string(format!("{user}:"), &mut enc_str); Some(enc_str) } (_, Some(password)) => { let mut enc_str = String::new(); - base64_engine.encode_string(&format!(":{password}"), &mut enc_str); + base64_engine.encode_string(format!(":{password}"), &mut enc_str); Some(enc_str) } _ => None, diff --git a/crates/nu-command/tests/commands/random/mod.rs b/crates/nu-command/tests/commands/random/mod.rs index 5a6b4c4b4a..a879b524b8 100644 --- a/crates/nu-command/tests/commands/random/mod.rs +++ b/crates/nu-command/tests/commands/random/mod.rs @@ -3,5 +3,4 @@ mod chars; mod dice; mod float; mod int; -#[cfg(feature = "uuid_crate")] mod uuid; diff --git a/crates/nu-command/tests/commands/random/uuid.rs b/crates/nu-command/tests/commands/random/uuid.rs index c38db6da27..4182824cab 100644 --- a/crates/nu-command/tests/commands/random/uuid.rs +++ b/crates/nu-command/tests/commands/random/uuid.rs @@ -1,5 +1,5 @@ use nu_test_support::nu; -use uuid_crate::Uuid; +use uuid::Uuid; #[test] fn generates_valid_uuid4() { diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 3e2b1827b2..6b7245f1ce 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -169,11 +169,7 @@ pub fn parse_def_predecl(working_set: &mut StateWorkingSet, spans: &[Span]) { // Now, pos should point at the next span after the def-like call. // Skip all potential flags, like --env, --wrapped or --help: - while pos < spans.len() - && working_set - .get_span_contents(spans[pos]) - .starts_with(&[b'-']) - { + while pos < spans.len() && working_set.get_span_contents(spans[pos]).starts_with(b"-") { pos += 1; } @@ -202,12 +198,8 @@ pub fn parse_def_predecl(working_set: &mut StateWorkingSet, spans: &[Span]) { let mut signature_pos = None; while pos < spans.len() { - if working_set - .get_span_contents(spans[pos]) - .starts_with(&[b'[']) - || working_set - .get_span_contents(spans[pos]) - .starts_with(&[b'(']) + if working_set.get_span_contents(spans[pos]).starts_with(b"[") + || working_set.get_span_contents(spans[pos]).starts_with(b"(") { signature_pos = Some(pos); break; @@ -424,7 +416,7 @@ pub fn parse_def( let mut decl_name_span = None; for span in rest_spans { - if !working_set.get_span_contents(*span).starts_with(&[b'-']) { + if !working_set.get_span_contents(*span).starts_with(b"-") { decl_name_span = Some(*span); break; } @@ -554,7 +546,7 @@ pub fn parse_def( for arg_name in &signature.optional_positional { verify_not_reserved_variable_name(working_set, &arg_name.name, sig.span); } - for arg_name in &signature.rest_positional { + if let Some(arg_name) = &signature.rest_positional { verify_not_reserved_variable_name(working_set, &arg_name.name, sig.span); } for flag_name in &signature.get_names() { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index a5559f5c52..8332d2f383 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -5926,7 +5926,7 @@ pub fn discover_captures_in_closure( seen.push(var_id); } } - for positional in &block.signature.rest_positional { + if let Some(positional) = &block.signature.rest_positional { if let Some(var_id) = positional.var_id { seen.push(var_id); } diff --git a/crates/nu-path/src/path.rs b/crates/nu-path/src/path.rs index 916fffdf72..9d7b64af27 100644 --- a/crates/nu-path/src/path.rs +++ b/crates/nu-path/src/path.rs @@ -660,10 +660,10 @@ impl Path { /// the current directory. /// /// * On Unix, a path is absolute if it starts with the root, - /// so [`is_absolute`](Path::is_absolute) and [`has_root`](Path::has_root) are equivalent. + /// so [`is_absolute`](Path::is_absolute) and [`has_root`](Path::has_root) are equivalent. /// /// * On Windows, a path is absolute if it has a prefix and starts with the root: - /// `c:\windows` is absolute, while `c:temp` and `\temp` are not. + /// `c:\windows` is absolute, while `c:temp` and `\temp` are not. /// /// # Examples /// diff --git a/crates/nu-path/src/tilde.rs b/crates/nu-path/src/tilde.rs index 95c91addf8..ac48bdfc3f 100644 --- a/crates/nu-path/src/tilde.rs +++ b/crates/nu-path/src/tilde.rs @@ -121,7 +121,7 @@ fn expand_tilde_with_another_user_home(path: &Path) -> PathBuf { return match path.to_str() { Some(file_path) => { let mut file = file_path.to_string(); - match file_path.find(|c| c == '/' || c == '\\') { + match file_path.find(['/', '\\']) { None => { file.remove(0); user_home_dir(&file) diff --git a/crates/nu-pretty-hex/src/lib.rs b/crates/nu-pretty-hex/src/lib.rs index 02a8d553f3..201f586f75 100644 --- a/crates/nu-pretty-hex/src/lib.rs +++ b/crates/nu-pretty-hex/src/lib.rs @@ -1,5 +1,3 @@ -// #![no_std] - //! A Rust library providing pretty hex dump. //! //! A `simple_hex()` way renders one-line hex dump, and a `pretty_hex()` way renders @@ -59,8 +57,5 @@ //! 0018: db b1 bc 35 bf ee ...5.. //! ``` -#[cfg(feature = "alloc")] -extern crate alloc; - mod pretty_hex; pub use pretty_hex::*; diff --git a/crates/nu-pretty-hex/tests/tests.rs b/crates/nu-pretty-hex/tests/tests.rs index b59362a077..8c481f6fb0 100644 --- a/crates/nu-pretty-hex/tests/tests.rs +++ b/crates/nu-pretty-hex/tests/tests.rs @@ -1,158 +1,147 @@ -// #![no_std] - -#[cfg(feature = "alloc")] -extern crate alloc; -extern crate nu_pretty_hex; - -#[cfg(feature = "alloc")] -use alloc::{format, string::String, vec, vec::Vec}; use nu_pretty_hex::*; -#[cfg(feature = "alloc")] -#[test] -fn test_simple() { - let bytes: Vec = (0..16).collect(); - let expected = "00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f"; - assert_eq!(expected, simple_hex(&bytes)); - assert_eq!(expected, bytes.hex_dump().to_string()); - assert_eq!(simple_hex(&bytes), config_hex(&bytes, HexConfig::simple())); - - let mut have = String::new(); - simple_hex_write(&mut have, &bytes).unwrap(); - assert_eq!(expected, have); - - let str = "string"; - let string: String = String::from("string"); - let slice: &[u8] = &[0x73, 0x74, 0x72, 0x69, 0x6e, 0x67]; - assert_eq!(simple_hex(&str), "73 74 72 69 6e 67"); - assert_eq!(simple_hex(&str), simple_hex(&string)); - assert_eq!(simple_hex(&str), simple_hex(&slice)); - - assert!(simple_hex(&vec![]).is_empty()); -} - -#[cfg(feature = "alloc")] -#[test] -fn test_pretty() { - let bytes: Vec = (0..256).map(|x| x as u8).collect(); - let want = include_str!("256.txt"); - - let mut hex = String::new(); - pretty_hex_write(&mut hex, &bytes).unwrap(); - assert_eq!(want, hex); - assert_eq!(want, format!("{:?}", bytes.hex_dump())); - assert_eq!(want, pretty_hex(&bytes)); - assert_eq!(want, config_hex(&bytes, HexConfig::default())); - - assert_eq!("Length: 0 (0x0) bytes\n", pretty_hex(&vec![])); -} - -#[cfg(feature = "alloc")] -#[test] -fn test_config() { - let cfg = HexConfig { - title: false, - ascii: false, - width: 0, - group: 0, - chunk: 0, - }; - assert!(config_hex(&vec![], cfg).is_empty()); - assert_eq!("2425262728", config_hex(&"$%&'(", cfg)); - - let v = include_bytes!("data"); - let cfg = HexConfig { - title: false, - group: 8, - ..HexConfig::default() - }; - let hex = "0000: 6b 4e 1a c3 af 03 d2 1e 7e 73 ba c8 bd 84 0f 83 kN......~s......\n\ - 0010: 89 d5 cf 90 23 67 4b 48 db b1 bc 35 bf ee ....#gKH...5.."; - assert_eq!(hex, config_hex(&v, cfg)); - assert_eq!(hex, format!("{:?}", v.hex_conf(cfg))); - let mut str = String::new(); - hex_write(&mut str, v, cfg).unwrap(); - assert_eq!(hex, str); - - assert_eq!( - config_hex( - &v, - HexConfig { - ascii: false, - ..cfg - } - ), - "0000: 6b 4e 1a c3 af 03 d2 1e 7e 73 ba c8 bd 84 0f 83\n\ - 0010: 89 d5 cf 90 23 67 4b 48 db b1 bc 35 bf ee" - ); - - assert_eq!( - config_hex( - &v, - HexConfig { - ascii: false, - group: 4, - chunk: 2, - ..cfg - } - ), - "0000: 6b4e 1ac3 af03 d21e 7e73 bac8 bd84 0f83\n\ - 0010: 89d5 cf90 2367 4b48 dbb1 bc35 bfee" - ); - - let v: Vec = (0..21).collect(); - let want = r##"Length: 21 (0x15) bytes -0000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ -0010: 10 11 12 13 14 ....."##; - assert_eq!(want, pretty_hex(&v)); - - let v: Vec = (0..13).collect(); - assert_eq!( - config_hex( - &v, - HexConfig { - title: false, - ascii: true, - width: 11, - group: 2, - chunk: 3 - } - ), - "0000: 000102 030405 060708 090a ...........\n\ - 000b: 0b0c .." - ); - - let v: Vec = (0..19).collect(); - assert_eq!( - config_hex( - &v, - HexConfig { - title: false, - ascii: true, - width: 16, - group: 3, - chunk: 3 - } - ), - "0000: 000102 030405 060708 090a0b 0c0d0e 0f ................\n\ - 0010: 101112 ..." - ); - - let cfg = HexConfig { - title: false, - group: 0, - ..HexConfig::default() - }; - assert_eq!( - format!("{:?}", v.hex_conf(cfg)), - "0000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................\n\ - 0010: 10 11 12 ..." - ); - assert_eq!( - v.hex_conf(cfg).to_string(), - "00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12" - ); -} +// #[test] +// fn test_simple() { +// let bytes: Vec = (0..16).collect(); +// let expected = "00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f"; +// assert_eq!(expected, simple_hex(&bytes)); +// assert_eq!(expected, bytes.hex_dump().to_string()); +// assert_eq!(simple_hex(&bytes), config_hex(&bytes, HexConfig::simple())); +// +// let mut have = String::new(); +// simple_hex_write(&mut have, &bytes).unwrap(); +// assert_eq!(expected, have); +// +// let str = "string"; +// let string: String = String::from("string"); +// let slice: &[u8] = &[0x73, 0x74, 0x72, 0x69, 0x6e, 0x67]; +// assert_eq!(simple_hex(&str), "73 74 72 69 6e 67"); +// assert_eq!(simple_hex(&str), simple_hex(&string)); +// assert_eq!(simple_hex(&str), simple_hex(&slice)); +// +// assert!(simple_hex(&vec![]).is_empty()); +// } +// +// #[test] +// fn test_pretty() { +// let bytes: Vec = (0..256).map(|x| x as u8).collect(); +// let want = include_str!("256.txt"); +// +// let mut hex = String::new(); +// pretty_hex_write(&mut hex, &bytes).unwrap(); +// assert_eq!(want, hex); +// assert_eq!(want, format!("{:?}", bytes.hex_dump())); +// assert_eq!(want, pretty_hex(&bytes)); +// assert_eq!(want, config_hex(&bytes, HexConfig::default())); +// +// assert_eq!("Length: 0 (0x0) bytes\n", pretty_hex(&vec![])); +// } +// +// #[test] +// fn test_config() { +// let cfg = HexConfig { +// title: false, +// ascii: false, +// width: 0, +// group: 0, +// chunk: 0, +// }; +// assert!(config_hex(&vec![], cfg).is_empty()); +// assert_eq!("2425262728", config_hex(&"$%&'(", cfg)); +// +// let v = include_bytes!("data"); +// let cfg = HexConfig { +// title: false, +// group: 8, +// ..HexConfig::default() +// }; +// let hex = "0000: 6b 4e 1a c3 af 03 d2 1e 7e 73 ba c8 bd 84 0f 83 kN......~s......\n\ +// 0010: 89 d5 cf 90 23 67 4b 48 db b1 bc 35 bf ee ....#gKH...5.."; +// assert_eq!(hex, config_hex(&v, cfg)); +// assert_eq!(hex, format!("{:?}", v.hex_conf(cfg))); +// let mut str = String::new(); +// hex_write(&mut str, v, cfg).unwrap(); +// assert_eq!(hex, str); +// +// assert_eq!( +// config_hex( +// &v, +// HexConfig { +// ascii: false, +// ..cfg +// } +// ), +// "0000: 6b 4e 1a c3 af 03 d2 1e 7e 73 ba c8 bd 84 0f 83\n\ +// 0010: 89 d5 cf 90 23 67 4b 48 db b1 bc 35 bf ee" +// ); +// +// assert_eq!( +// config_hex( +// &v, +// HexConfig { +// ascii: false, +// group: 4, +// chunk: 2, +// ..cfg +// } +// ), +// "0000: 6b4e 1ac3 af03 d21e 7e73 bac8 bd84 0f83\n\ +// 0010: 89d5 cf90 2367 4b48 dbb1 bc35 bfee" +// ); +// +// let v: Vec = (0..21).collect(); +// let want = r##"Length: 21 (0x15) bytes +// 0000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................ +// 0010: 10 11 12 13 14 ....."##; +// assert_eq!(want, pretty_hex(&v)); +// +// let v: Vec = (0..13).collect(); +// assert_eq!( +// config_hex( +// &v, +// HexConfig { +// title: false, +// ascii: true, +// width: 11, +// group: 2, +// chunk: 3 +// } +// ), +// "0000: 000102 030405 060708 090a ...........\n\ +// 000b: 0b0c .." +// ); +// +// let v: Vec = (0..19).collect(); +// assert_eq!( +// config_hex( +// &v, +// HexConfig { +// title: false, +// ascii: true, +// width: 16, +// group: 3, +// chunk: 3 +// } +// ), +// "0000: 000102 030405 060708 090a0b 0c0d0e 0f ................\n\ +// 0010: 101112 ..." +// ); +// +// let cfg = HexConfig { +// title: false, +// group: 0, +// ..HexConfig::default() +// }; +// assert_eq!( +// format!("{:?}", v.hex_conf(cfg)), +// "0000: 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ................\n\ +// 0010: 10 11 12 ..." +// ); +// assert_eq!( +// v.hex_conf(cfg).to_string(), +// "00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12" +// ); +// } // This test case checks that hex_write works even without the alloc crate. // Decorators to this function like simple_hex_write or PrettyHex::hex_dump() diff --git a/crates/nu-protocol/src/pipeline/pipeline_data.rs b/crates/nu-protocol/src/pipeline/pipeline_data.rs index 2ae8027f06..91fd0e1315 100644 --- a/crates/nu-protocol/src/pipeline/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline/pipeline_data.rs @@ -19,12 +19,12 @@ const LINE_ENDING_PATTERN: &[char] = &['\r', '\n']; /// We've tried a few variations of this structure. Listing these below so we have a record. /// /// * We tried always assuming a stream in Nushell. This was a great 80% solution, but it had some rough edges. -/// Namely, how do you know the difference between a single string and a list of one string. How do you know -/// when to flatten the data given to you from a data source into the stream or to keep it as an unflattened -/// list? +/// Namely, how do you know the difference between a single string and a list of one string. How do you know +/// when to flatten the data given to you from a data source into the stream or to keep it as an unflattened +/// list? /// /// * We tried putting the stream into Value. This had some interesting properties as now commands "just worked -/// on values", but lead to a few unfortunate issues. +/// on values", but lead to a few unfortunate issues. /// /// The first is that you can't easily clone Values in a way that felt largely immutable. For example, if /// you cloned a Value which contained a stream, and in one variable drained some part of it, then the second @@ -37,8 +37,8 @@ const LINE_ENDING_PATTERN: &[char] = &['\r', '\n']; /// concrete list values rather than streams, and be able to view them without non-local effects. /// /// * A balance of the two approaches is what we've landed on: Values are thread-safe to pass, and we can stream -/// them into any sources. Streams are still available to model the infinite streams approach of original -/// Nushell. +/// them into any sources. Streams are still available to model the infinite streams approach of original +/// Nushell. #[derive(Debug)] pub enum PipelineData { Empty, diff --git a/crates/nu-protocol/src/signature.rs b/crates/nu-protocol/src/signature.rs index f8b83063d7..c67e8a604e 100644 --- a/crates/nu-protocol/src/signature.rs +++ b/crates/nu-protocol/src/signature.rs @@ -467,13 +467,12 @@ impl Signature { /// Checks if short or long are already present /// Panics if one of them is found fn check_names(&self, name: impl Into, short: Option) -> (String, Option) { - let s = short.map(|c| { + let s = short.inspect(|c| { assert!( - !self.get_shorts().contains(&c), + !self.get_shorts().contains(c), "There may be duplicate short flags for '-{}'", c ); - c }); let name = { diff --git a/crates/nu-system/src/foreground.rs b/crates/nu-system/src/foreground.rs index bd78f616aa..93f7dd35c0 100644 --- a/crates/nu-system/src/foreground.rs +++ b/crates/nu-system/src/foreground.rs @@ -61,9 +61,8 @@ impl ForegroundChild { pipeline_state: Some(pipeline_state.clone()), } }) - .map_err(|e| { + .inspect_err(|_e| { foreground_pgroup::reset(); - e }) } else { command.spawn().map(|child| Self { diff --git a/crates/nuon/src/from.rs b/crates/nuon/src/from.rs index 2138e50ac5..ded1512a6a 100644 --- a/crates/nuon/src/from.rs +++ b/crates/nuon/src/from.rs @@ -12,7 +12,7 @@ use std::sync::Arc; /// > **Note** /// > [`Span`] can be passed to [`from_nuon`] if there is context available to the caller, e.g. when /// > using this function in a command implementation such as -/// [`from nuon`](https://www.nushell.sh/commands/docs/from_nuon.html). +/// > [`from nuon`](https://www.nushell.sh/commands/docs/from_nuon.html). /// /// also see [`super::to_nuon`] for the inverse operation pub fn from_nuon(input: &str, span: Option) -> Result {