fix clippy (#10659)

This pr fix clippy warnings in latest clippy version(1.72.0):

Unfortunally it's not easy to handle for [try
fold](https://rust-lang.github.io/rust-clippy/master/index.html#/manual_try_fold)
warning in `start command`

Refer to known issue:
> This lint doesn’t take into account whether a function does something
on the failure case, i.e., whether short-circuiting will affect
behavior. Refactoring to try_fold is not desirable in those cases.

That's the case for our code, which does something on the failure case.

So this pr is making a little refactor on `try_commands`.
This commit is contained in:
WindSoilder 2023-10-09 22:31:15 +08:00 committed by GitHub
parent 2ef34a3b4b
commit 0c67d742f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 56 additions and 76 deletions

View File

@ -98,7 +98,7 @@ fn timezone_offset_internal(
}; };
match s.len() { match s.len() {
len if len >= 2 => &s[2..], len if len >= 2 => &s[2..],
len if len == 0 => s, 0 => s,
_ => return Err(ParseErrorKind::TooShort), _ => return Err(ParseErrorKind::TooShort),
}; };

View File

@ -90,7 +90,6 @@ pub fn get_pipeline_elements(
let value_span = value.span(); let value_span = value.span();
let value_span_start = value_span.start as i64; let value_span_start = value_span.start as i64;
let value_span_end = value_span.end as i64; let value_span_end = value_span.end as i64;
let command_name = command_name;
let record = record! { let record = record! {
"cmd_index" => Value::string(index, span), "cmd_index" => Value::string(index, span),

View File

@ -158,8 +158,10 @@ impl Command for Ls {
let glob_options = if all { let glob_options = if all {
None None
} else { } else {
let mut glob_options = MatchOptions::new(); let glob_options = MatchOptions {
glob_options.recursive_match_hidden_dir = false; recursive_match_hidden_dir: false,
..Default::default()
};
Some(glob_options) Some(glob_options)
}; };
let (prefix, paths) = nu_engine::glob_from(&glob_path, &cwd, call_span, glob_options)?; let (prefix, paths) = nu_engine::glob_from(&glob_path, &cwd, call_span, glob_options)?;

View File

@ -151,40 +151,38 @@ fn try_commands(
span: Span, span: Span,
) -> Result<(), ShellError> { ) -> Result<(), ShellError> {
let env_vars_str = env_to_strings(engine_state, stack)?; let env_vars_str = env_to_strings(engine_state, stack)?;
commands let cmd_run_result = commands.into_iter().map(|mut cmd| {
.into_iter() let status = cmd
.map(|mut cmd| { .envs(&env_vars_str)
let status = cmd .stdin(Stdio::null())
.envs(&env_vars_str) .stdout(Stdio::null())
.stdin(Stdio::null()) .stderr(Stdio::null())
.stdout(Stdio::null()) .status();
.stderr(Stdio::null()) match status {
.status(); Ok(status) if status.success() => Ok(()),
match status { Ok(status) => Err(format!(
Ok(status) if status.success() => Ok(()), "\nCommand `{}` failed with {}",
Ok(status) => Err(format!( format_command(&cmd),
"\nCommand `{}` failed with {}", status
format_command(&cmd), )),
status Err(err) => Err(format!(
)), "\nCommand `{}` failed with {}",
Err(err) => Err(format!( format_command(&cmd),
"\nCommand `{}` failed with {}", err
format_command(&cmd), )),
err }
)), });
}
}) for one_result in cmd_run_result {
.take_while_inclusive(|result| result.is_err()) if let Err(err_msg) = one_result {
.fold(Err("".to_string()), |combined_result, next_result| { return Err(ShellError::ExternalCommand {
combined_result.or_else(|combined_message| { label: "No command found to start with this path".to_string(),
next_result.map_err(|next_message| combined_message + &next_message) help: "Try different path or install appropriate command\n".to_string() + &err_msg,
}) span,
}) });
.map_err(|message| ShellError::ExternalCommand { }
label: "No command found to start with this path".to_string(), }
help: "Try different path or install appropriate command\n".to_string() + &message, Ok(())
span,
})
} }
fn format_command(command: &std::process::Command) -> String { fn format_command(command: &std::process::Command) -> String {

View File

@ -232,7 +232,7 @@ fn detect_columns(
.iter() .iter()
.take(end_index) .take(end_index)
.skip(start_index) .skip(start_index)
.map(|v| v.as_string().unwrap_or(String::default())) .map(|v| v.as_string().unwrap_or_default())
.join(" "); .join(" ");
let binding = Value::string(combined, Span::unknown()); let binding = Value::string(combined, Span::unknown());
let last_seg = vals.split_off(end_index); let last_seg = vals.split_off(end_index);

View File

@ -107,8 +107,7 @@ fn action(
Value::Error { .. } => input.clone(), Value::Error { .. } => input.clone(),
Value::Binary { val, .. } => match base64_config.action_type { Value::Binary { val, .. } => match base64_config.action_type {
ActionType::Encode => { ActionType::Encode => {
let mut enc_vec = Vec::new(); let mut enc_vec = vec![0; val.len() * 4 / 3 + 4];
enc_vec.resize(val.len() * 4 / 3 + 4, 0);
let bytes_written = match base64_engine.encode_slice(val, &mut enc_vec) { let bytes_written = match base64_engine.encode_slice(val, &mut enc_vec) {
Ok(bytes_written) => bytes_written, Ok(bytes_written) => bytes_written,
Err(err) => { Err(err) => {

View File

@ -133,7 +133,7 @@ pub fn create_file_at(full_path: impl AsRef<Path>) -> Result<(), std::io::Error>
#[rstest] #[rstest]
#[case(".", r#"'*z'"#, &["ablez", "baker", "charliez"], &["ablez", "charliez"], "simple glob")] #[case(".", r#"'*z'"#, &["ablez", "baker", "charliez"], &["ablez", "charliez"], "simple glob")]
#[case(".", r#"'qqq'"#, &["ablez", "baker", "charliez"], &[], "glob matches none")] #[case(".", r#"'qqq'"#, &["ablez", "baker", "charliez"], &[], "glob matches none")]
#[case("foo/bar", r#"'*[\]}]*'"#, &[r#"foo/bar/ab}le"#, "foo/bar/baker", r#"foo/bar/cha]rlie"#], &[r#"foo/bar/ab}le"#, r#"foo/bar/cha]rlie"#], "glob has quoted metachars")] #[case("foo/bar", r"'*[\]}]*'", &[r#"foo/bar/ab}le"#, "foo/bar/baker", r#"foo/bar/cha]rlie"#], &[r#"foo/bar/ab}le"#, r#"foo/bar/cha]rlie"#], "glob has quoted metachars")]
#[case("foo/bar", r#"'../*'"#, &["foo/able", "foo/bar/baker", "foo/charlie"], &["foo/able", "foo/bar", "foo/charlie"], "glob matches files in parent")] #[case("foo/bar", r#"'../*'"#, &["foo/able", "foo/bar/baker", "foo/charlie"], &["foo/able", "foo/bar", "foo/charlie"], "glob matches files in parent")]
#[case("foo", r#"'./{a,b}*'"#, &["foo/able", "foo/bar/baker", "foo/charlie"], &["foo/able", "foo/bar"], "glob with leading ./ matches peer files")] #[case("foo", r#"'./{a,b}*'"#, &["foo/able", "foo/bar/baker", "foo/charlie"], &["foo/able", "foo/bar"], "glob with leading ./ matches peer files")]
fn glob_files_in_parent( fn glob_files_in_parent(

View File

@ -82,7 +82,7 @@ pub fn glob_from(
}; };
let pattern = pattern.to_string_lossy().to_string(); let pattern = pattern.to_string_lossy().to_string();
let glob_options = options.unwrap_or_else(MatchOptions::new); let glob_options = options.unwrap_or_default();
let glob = nu_glob::glob_with(&pattern, glob_options).map_err(|err| { let glob = nu_glob::glob_with(&pattern, glob_options).map_err(|err| {
nu_protocol::ShellError::GenericError( nu_protocol::ShellError::GenericError(

View File

@ -161,7 +161,7 @@ pub struct Paths {
/// ``` /// ```
/// Paths are yielded in alphabetical order. /// Paths are yielded in alphabetical order.
pub fn glob(pattern: &str) -> Result<Paths, PatternError> { pub fn glob(pattern: &str) -> Result<Paths, PatternError> {
glob_with(pattern, MatchOptions::new()) glob_with(pattern, MatchOptions::default())
} }
/// Return an iterator that produces all the `Path`s that match the given /// Return an iterator that produces all the `Path`s that match the given
@ -716,7 +716,7 @@ impl Pattern {
/// assert!(Pattern::new("d*g").unwrap().matches("doog")); /// assert!(Pattern::new("d*g").unwrap().matches("doog"));
/// ``` /// ```
pub fn matches(&self, str: &str) -> bool { pub fn matches(&self, str: &str) -> bool {
self.matches_with(str, MatchOptions::new()) self.matches_with(str, MatchOptions::default())
} }
/// Return if the given `Path`, when converted to a `str`, matches this /// Return if the given `Path`, when converted to a `str`, matches this
@ -1020,7 +1020,7 @@ fn chars_eq(a: char, b: char, case_sensitive: bool) -> bool {
/// Configuration options to modify the behaviour of `Pattern::matches_with(..)`. /// Configuration options to modify the behaviour of `Pattern::matches_with(..)`.
#[allow(missing_copy_implementations)] #[allow(missing_copy_implementations)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct MatchOptions { pub struct MatchOptions {
/// Whether or not patterns should be matched in a case-sensitive manner. /// Whether or not patterns should be matched in a case-sensitive manner.
/// This currently only considers upper/lower case relationships between /// This currently only considers upper/lower case relationships between
@ -1045,27 +1045,9 @@ pub struct MatchOptions {
pub recursive_match_hidden_dir: bool, pub recursive_match_hidden_dir: bool,
} }
impl MatchOptions { // Overwrite default behavior, because we want to make `recursive_match_hidden_dir` to true.
/// Constructs a new `MatchOptions` with default field values. This is used impl Default for MatchOptions {
/// when calling functions that do not take an explicit `MatchOptions` fn default() -> Self {
/// parameter.
///
/// This function always returns this value:
///
/// ```rust,ignore
/// MatchOptions {
/// case_sensitive: true,
/// require_literal_separator: false,
/// require_literal_leading_dot: false
/// recursive_match_hidden_dir: true,
/// }
/// ```
///
/// # Note
/// The behavior of this method doesn't match `default()`'s. This returns
/// `case_sensitive` as `true` while `default()` does it as `false`.
// FIXME: Consider unity the behavior with `default()` in a next major release.
pub fn new() -> Self {
Self { Self {
case_sensitive: true, case_sensitive: true,
require_literal_separator: false, require_literal_separator: false,
@ -1274,7 +1256,7 @@ mod test {
for c in "ABCDEFGHIJKLMNOPQRSTUVWXYZ".chars() { for c in "ABCDEFGHIJKLMNOPQRSTUVWXYZ".chars() {
let options = MatchOptions { let options = MatchOptions {
case_sensitive: false, case_sensitive: false,
..MatchOptions::new() ..MatchOptions::default()
}; };
assert!(pat.matches_with(&c.to_string(), options)); assert!(pat.matches_with(&c.to_string(), options));
} }

View File

@ -9,7 +9,6 @@ use libproc::libproc::thread_info::ThreadInfo;
use libproc::processes::{pids_by_type, ProcFilter}; use libproc::processes::{pids_by_type, ProcFilter};
use mach2::mach_time; use mach2::mach_time;
use std::cmp; use std::cmp;
use std::ffi::OsStr;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::thread; use std::thread;
use std::time::{Duration, Instant}; use std::time::{Duration, Instant};
@ -186,7 +185,7 @@ fn get_path_info(pid: i32, mut size: size_t) -> Option<PathInfo> {
let exe = Path::new(get_unchecked_str(cp, start).as_str()).to_path_buf(); let exe = Path::new(get_unchecked_str(cp, start).as_str()).to_path_buf();
let name = exe let name = exe
.file_name() .file_name()
.unwrap_or_else(|| OsStr::new("")) .unwrap_or_default()
.to_str() .to_str()
.unwrap_or("") .unwrap_or("")
.to_owned(); .to_owned();
@ -406,7 +405,7 @@ impl ProcessInfo {
self.curr_path self.curr_path
.as_ref() .as_ref()
.map(|cur_path| cur_path.cwd.display().to_string()) .map(|cur_path| cur_path.cwd.display().to_string())
.unwrap_or_else(|| "".to_string()) .unwrap_or_default()
} }
} }

View File

@ -233,7 +233,7 @@ pub fn root() -> PathBuf {
} }
pub fn binaries() -> PathBuf { pub fn binaries() -> PathBuf {
let build_target = std::env::var("CARGO_BUILD_TARGET").unwrap_or(String::new()); let build_target = std::env::var("CARGO_BUILD_TARGET").unwrap_or_default();
let profile = if let Ok(env_profile) = std::env::var("NUSHELL_CARGO_PROFILE") { let profile = if let Ok(env_profile) = std::env::var("NUSHELL_CARGO_PROFILE") {
env_profile env_profile

View File

@ -212,6 +212,7 @@ macro_rules! nu_with_plugins {
} }
use crate::{Outcome, NATIVE_PATH_ENV_VAR}; use crate::{Outcome, NATIVE_PATH_ENV_VAR};
use std::fmt::Write;
use std::{ use std::{
path::Path, path::Path,
process::{Command, Stdio}, process::{Command, Stdio},
@ -299,14 +300,14 @@ pub fn nu_with_plugin_run_test(cwd: impl AsRef<Path>, plugins: &[&str], command:
let registrations: String = plugins let registrations: String = plugins
.iter() .iter()
.map(|plugin_name| { .fold(String::new(), |mut output, plugin_name| {
let plugin = with_exe(plugin_name); let plugin = with_exe(plugin_name);
let plugin_path = nu_path::canonicalize_with(&plugin, &test_bins) let plugin_path = nu_path::canonicalize_with(&plugin, &test_bins)
.unwrap_or_else(|_| panic!("failed to canonicalize plugin {} path", &plugin)); .unwrap_or_else(|_| panic!("failed to canonicalize plugin {} path", &plugin));
let plugin_path = plugin_path.to_string_lossy(); let plugin_path = plugin_path.to_string_lossy();
format!("register {plugin_path};") let _ = write!(output, "register {plugin_path};");
}) output
.collect(); });
let commands = format!("{registrations}{command}"); let commands = format!("{registrations}{command}");
let target_cwd = crate::fs::in_directory(&cwd); let target_cwd = crate::fs::in_directory(&cwd);