Fix ignored clippy lints (#12160)

# Description
Fixes some ignored clippy lints.

# User-Facing Changes
Changes some signatures and return types to `&dyn Command` instead of
`&Box<dyn Command`, but I believe this is only an internal change.

crates/nu-command/src/debug/inspect_table.rs

@@ -48,7 +48,6 @@ pub fn build_table(value: Value, description: String, termsize: usize) -> String
 
     add_padding_to_widths(&mut widths);
 
-    #[allow(clippy::manual_clamp)]
     let width = val_table_width.max(desc_table_width).min(termsize);
 
     let mut desc_table = Table::from_iter([[String::from("description"), desc]]);

crates/nu-command/src/filesystem/util.rs

@@ -39,7 +39,6 @@ pub fn try_interaction(
     (interaction, confirmed)
 }
 
-#[allow(dead_code)]
 fn get_interactive_confirmation(prompt: String) -> Result<bool, Box<dyn Error>> {
     let input = Input::new()
         .with_prompt(prompt)

crates/nu-command/src/filters/reverse.rs

@@ -66,7 +66,6 @@ impl Command for Reverse {
     ) -> Result<PipelineData, ShellError> {
         let metadata = input.metadata();
 
-        #[allow(clippy::needless_collect)]
         let v: Vec<_> = input.into_iter_strict(call.head)?.collect();
         let iter = v.into_iter().rev();
         Ok(iter.into_pipeline_data_with_metadata(metadata, engine_state.ctrlc.clone()))

crates/nu-command/src/filters/utils.rs

@@ -41,9 +41,7 @@ pub fn boolean_fold(
 
     let ctrlc = engine_state.ctrlc.clone();
 
-    // TODO: This Clippy lint is incorrectly triggered in our CI for come reason
+    let eval_block = get_eval_block(engine_state);
-    #[allow(clippy::needless_borrow)]
-    let eval_block = get_eval_block(&engine_state);
 
     for value in input.into_interruptible_iter(ctrlc) {
         // with_env() is used here to ensure that each iteration uses

crates/nu-command/src/help/help_commands.rs

@@ -7,7 +7,6 @@ use nu_protocol::{
     record, span, Category, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData,
     ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value,
 };
-use std::borrow::Borrow;
 
 #[derive(Clone)]
 pub struct HelpCommands;
@@ -127,7 +126,7 @@ fn build_help_commands(engine_state: &EngineState, span: Span) -> Vec<Value> {
 
     for (_, decl_id) in commands {
         let decl = engine_state.get_decl(decl_id);
-        let sig = decl.signature().update_from_command(decl.borrow());
+        let sig = decl.signature().update_from_command(decl);
 
         let key = sig.name;
         let usage = sig.usage;

crates/nu-command/src/network/http/client.rs

@@ -421,7 +421,6 @@ pub struct RequestFlags {
     pub full: bool,
 }
 
-#[allow(clippy::needless_return)]
 fn transform_response_using_content_type(
     engine_state: &EngineState,
     stack: &mut Stack,
@@ -464,9 +463,9 @@ fn transform_response_using_content_type(
 
     let output = response_to_buffer(resp, engine_state, span);
     if flags.raw {
-        return Ok(output);
+        Ok(output)
     } else if let Some(ext) = ext {
-        return match engine_state.find_decl(format!("from {ext}").as_bytes(), &[]) {
+        match engine_state.find_decl(format!("from {ext}").as_bytes(), &[]) {
             Some(converter_id) => engine_state.get_decl(converter_id).run(
                 engine_state,
                 stack,
@@ -474,10 +473,10 @@ fn transform_response_using_content_type(
                 output,
             ),
             None => Ok(output),
-        };
+        }
     } else {
-        return Ok(output);
+        Ok(output)
-    };
+    }
 }
 
 pub fn check_response_redirection(

crates/nu-command/src/strings/detect_columns.rs

@@ -109,7 +109,6 @@ fn detect_columns(
     let config = engine_state.get_config();
     let input = input.collect_string("", config)?;
 
-    #[allow(clippy::needless_collect)]
     let input: Vec<_> = input
         .lines()
         .skip(num_rows_to_skip.unwrap_or_default())

crates/nu-command/src/viewers/table.rs

@@ -989,7 +989,6 @@ enum TableView {
     },
 }
 
-#[allow(clippy::manual_filter)]
 fn maybe_strip_color(output: String, config: &Config) -> String {
     // the terminal is for when people do ls from vim, there should be no coloring there
     if !config.use_ansi_coloring || !std::io::stdout().is_terminal() {

crates/nu-command/tests/commands/table.rs

@@ -2346,7 +2346,6 @@ fn join_lines(lines: impl IntoIterator<Item = impl AsRef<str>>) -> String {
 }
 
 // util function to easier copy && paste
-#[allow(dead_code)]
 fn _print_lines(s: &str, w: usize) {
     eprintln!("{:#?}", _split_str_by_width(s, w));
 }

crates/nu-command/tests/commands/where_.rs

@@ -1,5 +1,5 @@
 use nu_test_support::nu;
-#[allow(unused_imports)]
+#[cfg(feature = "sqlite")]
 use nu_test_support::pipeline;
 
 #[test]

crates/nu-command/tests/format_conversions/csv.rs

@@ -183,7 +183,6 @@ fn from_csv_text_with_tab_separator_to_table() {
 }
 
 #[test]
-#[allow(clippy::needless_raw_string_hashes)]
 fn from_csv_text_with_comments_to_table() {
     Playground::setup("filter_from_csv_test_5", |dirs, sandbox| {
         sandbox.with_files(vec![FileWithContentToBeTrimmed(
@@ -377,7 +376,7 @@ fn from_csv_text_with_wrong_type_separator() {
 fn table_with_record_error() {
     let actual = nu!(pipeline(
         r#"
-            [[a b]; [1 2] [3 {a: 1 b: 2}]] 
+            [[a b]; [1 2] [3 {a: 1 b: 2}]]
             | to csv
         "#
     ));

crates/nu-command/tests/format_conversions/tsv.rs

@@ -106,7 +106,6 @@ fn from_tsv_text_to_table() {
 }
 
 #[test]
-#[allow(clippy::needless_raw_string_hashes)]
 fn from_tsv_text_with_comments_to_table() {
     Playground::setup("filter_from_tsv_test_2", |dirs, sandbox| {
         sandbox.with_files(vec![FileWithContentToBeTrimmed(

crates/nu-engine/src/env.rs

@@ -19,11 +19,9 @@ const ENV_PATH_NAME: &str = "PATH";
 
 const ENV_CONVERSIONS: &str = "ENV_CONVERSIONS";
 
-#[allow(dead_code)]
 enum ConversionResult {
     Ok(Value),
     ConversionError(ShellError), // Failure during the conversion itself
-    GeneralError(ShellError),    // Other error not directly connected to running the conversion
     CellPathError, // Error looking up the ENV_VAR.to_/from_string fields in $env.ENV_CONVERSIONS
 }
 
@@ -46,7 +44,6 @@ pub fn convert_env_values(engine_state: &mut EngineState, stack: &Stack) -> Opti
                 let _ = new_scope.insert(name.to_string(), v);
             }
             ConversionResult::ConversionError(e) => error = error.or(Some(e)),
-            ConversionResult::GeneralError(_) => continue,
             ConversionResult::CellPathError => {
                 let _ = new_scope.insert(name.to_string(), val.clone());
             }
@@ -101,7 +98,6 @@ pub fn env_to_string(
     match get_converted_value(engine_state, stack, env_name, value, "to_string") {
         ConversionResult::Ok(v) => Ok(v.coerce_into_string()?),
         ConversionResult::ConversionError(e) => Err(e),
-        ConversionResult::GeneralError(e) => Err(e),
         ConversionResult::CellPathError => match value.coerce_string() {
             Ok(s) => Ok(s),
             Err(_) => {

crates/nu-explore/src/pager/mod.rs

@@ -148,7 +148,6 @@ impl<'a> Pager<'a> {
 }
 
 #[derive(Debug, Clone)]
-#[allow(dead_code)]
 pub enum Transition {
     Ok,
     Exit,

crates/nu-glob/src/lib.rs

@@ -73,6 +73,7 @@ extern crate doc_comment;
 doctest!("../README.md");
 
 use std::cmp;
+use std::cmp::Ordering;
 use std::error::Error;
 use std::fmt;
 use std::fs;
@@ -346,7 +347,6 @@ impl Error for GlobError {
         self.error.description()
     }
 
-    #[allow(unknown_lints, bare_trait_objects)]
     fn cause(&self) -> Option<&dyn Error> {
         Some(&self.error)
     }
@@ -505,7 +505,6 @@ impl Iterator for Paths {
 
 /// A pattern parsing error.
 #[derive(Debug)]
-#[allow(missing_copy_implementations)]
 pub struct PatternError {
     /// The approximate character index of where the error occurred.
     pub pos: usize,
@@ -630,53 +629,58 @@ impl Pattern {
 
                     let count = i - old;
 
-                    #[allow(clippy::comparison_chain)]
+                    match count.cmp(&2) {
-                    if count > 2 {
+                        Ordering::Greater => {
-                        return Err(PatternError {
+                            return Err(PatternError {
-                            pos: old + 2,
+                                pos: old + 2,
-                            msg: ERROR_WILDCARDS,
+                                msg: ERROR_WILDCARDS,
-                        });
+                            });
-                    } else if count == 2 {
+                        }
-                        // ** can only be an entire path component
+                        Ordering::Equal => {
-                        // i.e. a/**/b is valid, but a**/b or a/**b is not
+                            // ** can only be an entire path component
-                        // invalid matches are treated literally
+                            // i.e. a/**/b is valid, but a**/b or a/**b is not
-                        let is_valid = if i == 2 || path::is_separator(chars[i - count - 1]) {
+                            // invalid matches are treated literally
-                            // it ends in a '/'
+                            let is_valid = if i == 2 || path::is_separator(chars[i - count - 1]) {
-                            if i < chars.len() && path::is_separator(chars[i]) {
+                                // it ends in a '/'
-                                i += 1;
+                                if i < chars.len() && path::is_separator(chars[i]) {
-                                true
+                                    i += 1;
-                            // or the pattern ends here
+                                    true
-                            // this enables the existing globbing mechanism
+                                // or the pattern ends here
-                            } else if i == chars.len() {
+                                // this enables the existing globbing mechanism
-                                true
+                                } else if i == chars.len() {
-                            // `**` ends in non-separator
+                                    true
+                                // `**` ends in non-separator
+                                } else {
+                                    return Err(PatternError {
+                                        pos: i,
+                                        msg: ERROR_RECURSIVE_WILDCARDS,
+                                    });
+                                }
+                            // `**` begins with non-separator
                             } else {
                                 return Err(PatternError {
-                                    pos: i,
+                                    pos: old - 1,
                                     msg: ERROR_RECURSIVE_WILDCARDS,
                                 });
-                            }
+                            };
-                        // `**` begins with non-separator
-                        } else {
-                            return Err(PatternError {
-                                pos: old - 1,
-                                msg: ERROR_RECURSIVE_WILDCARDS,
-                            });
-                        };
 
-                        if is_valid {
+                            if is_valid {
-                            // collapse consecutive AnyRecursiveSequence to a
+                                // collapse consecutive AnyRecursiveSequence to a
-                            // single one
+                                // single one
 
-                            let tokens_len = tokens.len();
+                                let tokens_len = tokens.len();
 
-                            if !(tokens_len > 1 && tokens[tokens_len - 1] == AnyRecursiveSequence) {
+                                if !(tokens_len > 1
-                                is_recursive = true;
+                                    && tokens[tokens_len - 1] == AnyRecursiveSequence)
-                                tokens.push(AnyRecursiveSequence);
+                                {
+                                    is_recursive = true;
+                                    tokens.push(AnyRecursiveSequence);
+                                }
                             }
                         }
-                    } else {
+                        Ordering::Less => {
-                        tokens.push(AnySequence);
+                            tokens.push(AnySequence);
+                        }
                     }
                 }
                 '[' => {
@@ -1051,7 +1055,6 @@ fn chars_eq(a: char, b: char, case_sensitive: bool) -> bool {
 }
 
 /// Configuration options to modify the behaviour of `Pattern::matches_with(..)`.
-#[allow(missing_copy_implementations)]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct MatchOptions {
     /// Whether or not patterns should be matched in a case-sensitive manner.

crates/nu-parser/src/parser.rs

@@ -577,9 +577,8 @@ fn first_kw_idx(
             ..
         }) = signature.get_positional(idx)
         {
-            #[allow(clippy::needless_range_loop)]
+            for (span_idx, &span) in spans.iter().enumerate().skip(spans_idx) {
-            for span_idx in spans_idx..spans.len() {
+                let contents = working_set.get_span_contents(span);
-                let contents = working_set.get_span_contents(spans[span_idx]);
 
                 if contents == kw {
                     return (Some(idx), span_idx);
@@ -3201,12 +3200,11 @@ pub fn parse_signature(working_set: &mut StateWorkingSet, span: Span) -> Express
 }
 
 pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> Box<Signature> {
-    #[allow(clippy::enum_variant_names)]
     enum ParseMode {
-        ArgMode,
+        Arg,
-        AfterCommaArgMode,
+        AfterCommaArg,
-        TypeMode,
+        Type,
-        DefaultValueMode,
+        DefaultValue,
     }
 
     #[derive(Debug)]
@@ -3237,7 +3235,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
     }
 
     let mut args: Vec<Arg> = vec![];
-    let mut parse_mode = ParseMode::ArgMode;
+    let mut parse_mode = ParseMode::Arg;
 
     for token in &output {
         match token {
@@ -3251,13 +3249,13 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                 // The : symbol separates types
                 if contents == b":" {
                     match parse_mode {
-                        ParseMode::ArgMode => {
+                        ParseMode::Arg => {
-                            parse_mode = ParseMode::TypeMode;
+                            parse_mode = ParseMode::Type;
                         }
-                        ParseMode::AfterCommaArgMode => {
+                        ParseMode::AfterCommaArg => {
                             working_set.error(ParseError::Expected("parameter or flag", span));
                         }
-                        ParseMode::TypeMode | ParseMode::DefaultValueMode => {
+                        ParseMode::Type | ParseMode::DefaultValue => {
                             // We're seeing two types for the same thing for some reason, error
                             working_set.error(ParseError::Expected("type", span));
                         }
@@ -3266,13 +3264,13 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                 // The = symbol separates a variable from its default value
                 else if contents == b"=" {
                     match parse_mode {
-                        ParseMode::TypeMode | ParseMode::ArgMode => {
+                        ParseMode::Type | ParseMode::Arg => {
-                            parse_mode = ParseMode::DefaultValueMode;
+                            parse_mode = ParseMode::DefaultValue;
                         }
-                        ParseMode::AfterCommaArgMode => {
+                        ParseMode::AfterCommaArg => {
                             working_set.error(ParseError::Expected("parameter or flag", span));
                         }
-                        ParseMode::DefaultValueMode => {
+                        ParseMode::DefaultValue => {
                             // We're seeing two default values for some reason, error
                             working_set.error(ParseError::Expected("default value", span));
                         }
@@ -3281,20 +3279,20 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                 // The , symbol separates params only
                 else if contents == b"," {
                     match parse_mode {
-                        ParseMode::ArgMode => parse_mode = ParseMode::AfterCommaArgMode,
+                        ParseMode::Arg => parse_mode = ParseMode::AfterCommaArg,
-                        ParseMode::AfterCommaArgMode => {
+                        ParseMode::AfterCommaArg => {
                             working_set.error(ParseError::Expected("parameter or flag", span));
                         }
-                        ParseMode::TypeMode => {
+                        ParseMode::Type => {
                             working_set.error(ParseError::Expected("type", span));
                         }
-                        ParseMode::DefaultValueMode => {
+                        ParseMode::DefaultValue => {
                             working_set.error(ParseError::Expected("default value", span));
                         }
                     }
                 } else {
                     match parse_mode {
-                        ParseMode::ArgMode | ParseMode::AfterCommaArgMode => {
+                        ParseMode::Arg | ParseMode::AfterCommaArg => {
                             // Long flag with optional short form following with no whitespace, e.g. --output, --age(-a)
                             if contents.starts_with(b"--") && contents.len() > 2 {
                                 // Split the long flag from the short flag with the ( character as delimiter.
@@ -3400,7 +3398,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                                         working_set.error(ParseError::Expected("short flag", span));
                                     }
                                 }
-                                parse_mode = ParseMode::ArgMode;
+                                parse_mode = ParseMode::Arg;
                             }
                             // Mandatory short flag, e.g. -e (must be one character)
                             else if contents.starts_with(b"-") && contents.len() > 1 {
@@ -3438,12 +3436,12 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                                     },
                                     type_annotated: false,
                                 });
-                                parse_mode = ParseMode::ArgMode;
+                                parse_mode = ParseMode::Arg;
                             }
                             // Short flag alias for long flag, e.g. --b (-a)
                             // This is the same as the short flag in --b(-a)
                             else if contents.starts_with(b"(-") {
-                                if matches!(parse_mode, ParseMode::AfterCommaArgMode) {
+                                if matches!(parse_mode, ParseMode::AfterCommaArg) {
                                     working_set
                                         .error(ParseError::Expected("parameter or flag", span));
                                 }
@@ -3506,7 +3504,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                                     required: false,
                                     type_annotated: false,
                                 });
-                                parse_mode = ParseMode::ArgMode;
+                                parse_mode = ParseMode::Arg;
                             }
                             // Rest param
                             else if let Some(contents) = contents.strip_prefix(b"...") {
@@ -3530,7 +3528,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                                     var_id: Some(var_id),
                                     default_value: None,
                                 }));
-                                parse_mode = ParseMode::ArgMode;
+                                parse_mode = ParseMode::Arg;
                             }
                             // Normal param
                             else {
@@ -3559,10 +3557,10 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                                     required: true,
                                     type_annotated: false,
                                 });
-                                parse_mode = ParseMode::ArgMode;
+                                parse_mode = ParseMode::Arg;
                             }
                         }
-                        ParseMode::TypeMode => {
+                        ParseMode::Type => {
                             if let Some(last) = args.last_mut() {
                                 let syntax_shape = parse_shape_name(
                                     working_set,
@@ -3604,9 +3602,9 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                                     }
                                 }
                             }
-                            parse_mode = ParseMode::ArgMode;
+                            parse_mode = ParseMode::Arg;
                         }
-                        ParseMode::DefaultValueMode => {
+                        ParseMode::DefaultValue => {
                             if let Some(last) = args.last_mut() {
                                 let expression = parse_value(working_set, span, &SyntaxShape::Any);
 
@@ -3727,7 +3725,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
                                     }
                                 }
                             }
-                            parse_mode = ParseMode::ArgMode;
+                            parse_mode = ParseMode::Arg;
                         }
                     }
                 }

crates/nu-parser/tests/test_parser.rs

@@ -86,13 +86,12 @@ fn test_int(
             },
         ) = &expressions.elements[0]
         {
-            compare_rhs_binaryOp(test_tag, &expected_val, observed_val);
+            compare_rhs_binary_op(test_tag, &expected_val, observed_val);
         }
     }
 }
 
-#[allow(non_snake_case)]
+fn compare_rhs_binary_op(
-fn compare_rhs_binaryOp(
     test_tag: &str,
     expected: &Expr, // the rhs expr we hope to see (::Int, ::Float, not ::B)
     observed: &Expr, // the Expr actually provided: can be ::Int, ::Float, ::String,

crates/nu-protocol/src/engine/engine_state.rs

@@ -12,7 +12,6 @@ use crate::{
     ShellError, Signature, Span, Type, VarId, VirtualPathId,
 };
 use crate::{Category, Value};
-use std::borrow::Borrow;
 use std::collections::HashMap;
 use std::num::NonZeroUsize;
 use std::path::Path;
@@ -788,11 +787,11 @@ impl EngineState {
         self.vars[var_id].const_val = Some(val);
     }
 
-    #[allow(clippy::borrowed_box)]
+    pub fn get_decl(&self, decl_id: DeclId) -> &dyn Command {
-    pub fn get_decl(&self, decl_id: DeclId) -> &Box<dyn Command> {
         self.decls
             .get(decl_id)
             .expect("internal error: missing declaration")
+            .as_ref()
     }
 
     /// Get all commands within scope, sorted by the commands' names
@@ -823,8 +822,7 @@ impl EngineState {
         decls.into_iter()
     }
 
-    #[allow(clippy::borrowed_box)]
+    pub fn get_signature(&self, decl: &dyn Command) -> Signature {
-    pub fn get_signature(&self, decl: &Box<dyn Command>) -> Signature {
         if let Some(block_id) = decl.get_block_id() {
             *self.blocks[block_id].signature.clone()
         } else {
@@ -838,7 +836,7 @@ impl EngineState {
             .map(|(_, id)| {
                 let decl = self.get_decl(id);
 
-                self.get_signature(decl).update_from_command(decl.borrow())
+                self.get_signature(decl).update_from_command(decl)
             })
             .collect()
     }
@@ -856,7 +854,7 @@ impl EngineState {
             .map(|(_, id)| {
                 let decl = self.get_decl(id);
 
-                let signature = self.get_signature(decl).update_from_command(decl.borrow());
+                let signature = self.get_signature(decl).update_from_command(decl);
 
                 (
                     signature,

crates/nu-protocol/src/engine/state_working_set.rs

@@ -685,8 +685,7 @@ impl<'a> StateWorkingSet<'a> {
         }
     }
 
-    #[allow(clippy::borrowed_box)]
+    pub fn get_decl(&self, decl_id: DeclId) -> &dyn Command {
-    pub fn get_decl(&self, decl_id: DeclId) -> &Box<dyn Command> {
         let num_permanent_decls = self.permanent_state.num_decls();
         if decl_id < num_permanent_decls {
             self.permanent_state.get_decl(decl_id)
@@ -695,6 +694,7 @@ impl<'a> StateWorkingSet<'a> {
                 .decls
                 .get(decl_id - num_permanent_decls)
                 .expect("internal error: missing declaration")
+                .as_ref()
         }
     }
 

crates/nu-protocol/src/example.rs

@@ -1,5 +1,5 @@
 use crate::Value;
-#[allow(unused_imports)]
+#[cfg(feature = "plugin")]
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug)]