From 39a041721141c14088ab8f04bae65fec37f96535 Mon Sep 17 00:00:00 2001 From: reuben olinsky Date: Sun, 19 May 2024 22:09:05 -0700 Subject: [PATCH] Adjust clippy settings (#27) --- Cargo.toml | 14 +++++++++++++- cli/Cargo.toml | 4 ++++ cli/src/main.rs | 8 ++++++-- cli/tests/integration_tests.rs | 2 +- interactive-shell/Cargo.toml | 15 +++++++++------ parser/Cargo.toml | 6 +++++- shell/Cargo.toml | 6 +++++- shell/src/builtins/complete.rs | 15 ++++++++------- shell/src/builtins/declare.rs | 1 + shell/src/builtins/mod.rs | 1 - shell/src/builtins/read.rs | 9 ++++----- shell/src/builtins/trap.rs | 13 ++++--------- shell/src/builtins/unset.rs | 17 +++++++---------- shell/src/expansion.rs | 2 ++ shell/src/jobs.rs | 1 - shell/src/patterns.rs | 1 + shell/src/shell.rs | 6 ++++-- 17 files changed, 74 insertions(+), 47 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 724d3e70..4c501e14 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,13 +4,25 @@ members = ["cli", "parser", "shell", "interactive-shell"] default-members = ["cli", "parser", "shell", "interactive-shell"] [workspace.package] +categories = ["command-line-utilities", "development-tools"] edition = "2021" +keywords = ["cli", "shell", "sh", "bash", "script"] license = "MIT" +readme = "README.md" +repository = "https://github.com/reubeno/brush" version = "0.1.0" [workspace.lints.clippy] all = { level = "deny", priority = -1 } pedantic = { level = "deny", priority = -1 } +cargo = { level = "deny", priority = -1 } +perf = { level = "deny", priority = -1 } +expect_used = "deny" +format_push_string = "deny" +panic = "deny" +panic_in_result_fn = "deny" +todo = "deny" +unwrap_in_result = "deny" bool_to_int_with_if = "allow" collapsible_else_if = "allow" collapsible_if = "allow" @@ -22,9 +34,9 @@ missing_panics_doc = "allow" must_use_candidate = "allow" redundant_closure_for_method_calls = "allow" redundant_else = "allow" +result_large_err = "allow" similar_names = "allow" struct_excessive_bools = "allow" -result_large_err = "allow" [profile.release] opt-level = "z" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index ffd4c11a..6fc14589 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -1,8 +1,12 @@ [package] name = "brush-cli" description = "brush shell" +categories.workspace = true edition.workspace = true +keywords.workspace = true license.workspace = true +readme.workspace = true +repository.workspace = true version.workspace = true [[bin]] diff --git a/cli/src/main.rs b/cli/src/main.rs index 4c1803c1..35240deb 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -146,10 +146,14 @@ fn main() { .without_time() .with_filter(filter); - tracing_subscriber::registry() + if tracing_subscriber::registry() .with(stderr_log_layer) .try_init() - .expect("Failed to initialize tracing."); + .is_err() + { + // Something went wrong; proceed on anyway but complain audibly. + eprintln!("warning: failed to initialize tracing."); + } // // Run. diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 7592acf4..b862d03b 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -855,7 +855,7 @@ impl TestCase { } WhichShell::NamedShell(name) => (std::process::Command::new(name), None), }, - ShellInvocation::ExecScript(_) => todo!("UNIMPLEMENTED: exec script test"), + ShellInvocation::ExecScript(_) => unimplemented!("exec script test"), }; for arg in &shell_config.default_args { diff --git a/interactive-shell/Cargo.toml b/interactive-shell/Cargo.toml index e5f90426..5eb5d54d 100644 --- a/interactive-shell/Cargo.toml +++ b/interactive-shell/Cargo.toml @@ -1,8 +1,12 @@ [package] name = "brush-interactive-shell" -description = "Shell" +description = "Interactive shell for brush" +categories.workspace = true edition.workspace = true +keywords.workspace = true license.workspace = true +readme.workspace = true +repository.workspace = true version.workspace = true [lib] @@ -15,10 +19,9 @@ workspace = true [dependencies] brush-parser = { path = "../parser" } brush-shell = { path = "../shell" } -rustyline = { git = "https://github.com/reubeno/rustyline", rev = "023a08093e9b8ff81f9d8e096e00a23e6e83167a", features = ["derive"] } -thiserror = "1.0.61" -tokio = { version = "1.37.0", features = [ - "macros", - "signal", +rustyline = { git = "https://github.com/reubeno/rustyline", rev = "023a08093e9b8ff81f9d8e096e00a23e6e83167a", features = [ + "derive", ] } +thiserror = "1.0.61" +tokio = { version = "1.37.0", features = ["macros", "signal"] } tracing = "0.1.40" diff --git a/parser/Cargo.toml b/parser/Cargo.toml index c0496fa2..44a680af 100644 --- a/parser/Cargo.toml +++ b/parser/Cargo.toml @@ -1,8 +1,12 @@ [package] name = "brush-parser" -description = "Parser" +description = "Shell parser for brush" +categories.workspace = true edition.workspace = true +keywords.workspace = true license.workspace = true +readme.workspace = true +repository.workspace = true version.workspace = true [lib] diff --git a/shell/Cargo.toml b/shell/Cargo.toml index a8f02bb7..143f53e9 100644 --- a/shell/Cargo.toml +++ b/shell/Cargo.toml @@ -1,8 +1,12 @@ [package] name = "brush-shell" -description = "brush Shell" +description = "Core for brush shell" +categories.workspace = true edition.workspace = true +keywords.workspace = true license.workspace = true +readme.workspace = true +repository.workspace = true version.workspace = true [lib] diff --git a/shell/src/builtins/complete.rs b/shell/src/builtins/complete.rs index 032b3ebd..d90ad8b5 100644 --- a/shell/src/builtins/complete.rs +++ b/shell/src/builtins/complete.rs @@ -1,5 +1,6 @@ use clap::{arg, Parser}; use std::collections::HashMap; +use std::fmt::Write as _; use std::io::Write; use crate::builtin::{BuiltinCommand, BuiltinExitCode}; @@ -350,25 +351,25 @@ impl CompleteCommand { } if let Some(glob_pattern) = &spec.glob_pattern { - s.push_str(&std::format!(" -G {glob_pattern}")); + write!(s, " -G {glob_pattern}")?; } if let Some(word_list) = &spec.word_list { - s.push_str(&std::format!(" -W {word_list}")); + write!(s, " -W {word_list}")?; } if let Some(function_name) = &spec.function_name { - s.push_str(&std::format!(" -F {function_name}")); + write!(s, " -F {function_name}")?; } if let Some(command) = &spec.command { - s.push_str(&std::format!(" -C {command}")); + write!(s, " -C {command}")?; } if let Some(filter_pattern) = &spec.filter_pattern { - s.push_str(&std::format!(" -X {filter_pattern}")); + write!(s, " -X {filter_pattern}")?; } if let Some(prefix) = &spec.prefix { - s.push_str(&std::format!(" -P {prefix}")); + write!(s, " -P {prefix}")?; } if let Some(suffix) = &spec.suffix { - s.push_str(&std::format!(" -S {suffix}")); + write!(s, " -S {suffix}")?; } if let Some(command_name) = command_name { diff --git a/shell/src/builtins/declare.rs b/shell/src/builtins/declare.rs index b1a2df1f..308f01a6 100644 --- a/shell/src/builtins/declare.rs +++ b/shell/src/builtins/declare.rs @@ -284,6 +284,7 @@ impl DeclareCommand { Ok(true) } + #[allow(clippy::unwrap_in_result)] fn declaration_to_name_and_value( declaration: &CommandArg, ) -> Result<(String, Option, Option, bool), error::Error> { diff --git a/shell/src/builtins/mod.rs b/shell/src/builtins/mod.rs index 335dc9d3..7919b26a 100644 --- a/shell/src/builtins/mod.rs +++ b/shell/src/builtins/mod.rs @@ -84,7 +84,6 @@ fn decl_builtin() -> BuiltinRegistration { } } -#[allow(dead_code)] fn special_decl_builtin() -> BuiltinRegistration { BuiltinRegistration { execute_func: exec_declaration_builtin::, diff --git a/shell/src/builtins/read.rs b/shell/src/builtins/read.rs index 44d24257..3f023a2a 100644 --- a/shell/src/builtins/read.rs +++ b/shell/src/builtins/read.rs @@ -82,7 +82,7 @@ impl BuiltinCommand for ReadCommand { return error::unimp("read -u"); } - let input_line = read_line(context.stdin())?; + let input_line = read_line(context.stdin()); if let Some(input_line) = input_line { let mut variable_names: VecDeque = self.variable_names.clone().into(); for field in input_line.split_ascii_whitespace() { @@ -105,8 +105,7 @@ impl BuiltinCommand for ReadCommand { } } -#[allow(clippy::unnecessary_wraps)] -fn read_line(mut file: openfiles::OpenFile) -> Result, crate::error::Error> { +fn read_line(mut file: openfiles::OpenFile) -> Option { let mut line = String::new(); let mut buffer = [0; 1]; // 1-byte buffer @@ -124,8 +123,8 @@ fn read_line(mut file: openfiles::OpenFile) -> Result, crate::err } if line.is_empty() { - Ok(None) + None } else { - Ok(Some(line)) + Some(line) } } diff --git a/shell/src/builtins/trap.rs b/shell/src/builtins/trap.rs index fb97d435..08dadcdd 100644 --- a/shell/src/builtins/trap.rs +++ b/shell/src/builtins/trap.rs @@ -40,7 +40,7 @@ impl BuiltinCommand for TrapCommand { } else if self.args.len() == 1 { let signal = self.args[0].as_str(); let signal_type = parse_signal(signal)?; - Self::remove_all_handlers(&mut context, signal_type)?; + Self::remove_all_handlers(&mut context, signal_type); Ok(BuiltinExitCode::Success) } else { let handler = &self.args[0]; @@ -50,7 +50,7 @@ impl BuiltinCommand for TrapCommand { signal_types.push(parse_signal(signal)?); } - Self::register_handler(&mut context, signal_types, handler.as_str())?; + Self::register_handler(&mut context, signal_types, handler.as_str()); Ok(BuiltinExitCode::Success) } } @@ -86,29 +86,24 @@ impl TrapCommand { Ok(()) } - #[allow(clippy::unnecessary_wraps)] fn remove_all_handlers( context: &mut crate::context::CommandExecutionContext<'_>, signal: traps::TrapSignal, - ) -> Result<(), error::Error> { + ) { context.shell.traps.remove_handlers(signal); - Ok(()) } - #[allow(clippy::unnecessary_wraps)] fn register_handler( context: &mut crate::context::CommandExecutionContext<'_>, signals: Vec, handler: &str, - ) -> Result<(), error::Error> { + ) { for signal in signals { context .shell .traps .register_handler(signal, handler.to_owned()); } - - Ok(()) } } diff --git a/shell/src/builtins/unset.rs b/shell/src/builtins/unset.rs index 5cb59971..d128c2fa 100644 --- a/shell/src/builtins/unset.rs +++ b/shell/src/builtins/unset.rs @@ -13,35 +13,32 @@ pub(crate) struct UnsetCommand { #[derive(Parser)] #[clap(group = clap::ArgGroup::new("name-interpretation").multiple(false).required(false))] -#[allow(clippy::struct_field_names)] pub(crate) struct UnsetNameInterpretation { #[arg( short = 'f', group = "name-interpretation", help = "treat each name as a shell function" )] - names_are_shell_functions: bool, + shell_functions: bool, #[arg( short = 'v', group = "name-interpretation", help = "treat each name as a shell variable" )] - names_are_shell_variables: bool, + shell_variables: bool, #[arg( short = 'n', group = "name-interpretation", help = "treat each name as a name reference" )] - names_are_name_references: bool, + name_references: bool, } impl UnsetNameInterpretation { pub fn unspecified(&self) -> bool { - !self.names_are_shell_functions - && !self.names_are_shell_variables - && !self.names_are_name_references + !self.shell_functions && !self.shell_variables && !self.name_references } } @@ -54,14 +51,14 @@ impl BuiltinCommand for UnsetCommand { // // TODO: implement nameref // - if self.name_interpretation.names_are_name_references { + if self.name_interpretation.name_references { return crate::error::unimp("unset: name references are not yet implemented"); } let unspecified = self.name_interpretation.unspecified(); for name in &self.names { - if unspecified || self.name_interpretation.names_are_shell_variables { + if unspecified || self.name_interpretation.shell_variables { let parameter = parser::word::parse_parameter(name, &context.shell.parser_options())?; @@ -90,7 +87,7 @@ impl BuiltinCommand for UnsetCommand { } // TODO: Check if functions can be readonly. - if unspecified || self.name_interpretation.names_are_shell_functions { + if unspecified || self.name_interpretation.shell_functions { if context.shell.funcs.remove(name).is_some() { continue; } diff --git a/shell/src/expansion.rs b/shell/src/expansion.rs index 448756c0..2a0bc299 100644 --- a/shell/src/expansion.rs +++ b/shell/src/expansion.rs @@ -1223,6 +1223,7 @@ impl<'a> WordExpander<'a> { Ok(value.to_string()) } + #[allow(clippy::unwrap_in_result)] fn uppercase_first_char( &mut self, s: String, @@ -1252,6 +1253,7 @@ impl<'a> WordExpander<'a> { } } + #[allow(clippy::unwrap_in_result)] fn lowercase_first_char( &mut self, s: String, diff --git a/shell/src/jobs.rs b/shell/src/jobs.rs index a1e13490..8b71919d 100644 --- a/shell/src/jobs.rs +++ b/shell/src/jobs.rs @@ -120,7 +120,6 @@ impl Display for JobAnnotation { pub struct Job { join_handles: VecDeque, - #[allow(dead_code)] pids: Vec, annotation: JobAnnotation, diff --git a/shell/src/patterns.rs b/shell/src/patterns.rs index 15ea4195..47b49935 100644 --- a/shell/src/patterns.rs +++ b/shell/src/patterns.rs @@ -380,6 +380,7 @@ pub(crate) fn remove_smallest_matching_suffix<'a>( } #[cfg(test)] +#[allow(clippy::panic_in_result_fn)] mod tests { use super::*; use anyhow::Result; diff --git a/shell/src/shell.rs b/shell/src/shell.rs index 29124d93..b8e534cc 100644 --- a/shell/src/shell.rs +++ b/shell/src/shell.rs @@ -1,5 +1,6 @@ use std::borrow::Cow; use std::collections::{HashMap, VecDeque}; +use std::fmt::Write as _; use std::io::Write; use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; @@ -529,10 +530,11 @@ impl Shell { error_message.push_str(inner.to_string().as_str()); if let Some(position) = position { - error_message.push_str(&format!( + write!( + error_message, " (detected near line {} column {})", position.line, position.column - )); + )?; } tracing::error!("{}", error_message);