diff --git a/CHANGELOG.md b/CHANGELOG.md index 659e8aebc..30727beb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4764,6 +4764,7 @@ Released 2018-09-13 [`suboptimal_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#suboptimal_flops [`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl [`suspicious_assignment_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_assignment_formatting +[`suspicious_command_arg_space`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_command_arg_space [`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting [`suspicious_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_map [`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 457a25826..c1feabf05 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -378,6 +378,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::SKIP_WHILE_NEXT_INFO, crate::methods::STABLE_SORT_PRIMITIVE_INFO, crate::methods::STRING_EXTEND_CHARS_INFO, + crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO, crate::methods::SUSPICIOUS_MAP_INFO, crate::methods::SUSPICIOUS_SPLITN_INFO, crate::methods::SUSPICIOUS_TO_OWNED_INFO, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index f1e8be7f2..d8c018f75 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -80,6 +80,7 @@ mod skip_while_next; mod stable_sort_primitive; mod str_splitn; mod string_extend_chars; +mod suspicious_command_arg_space; mod suspicious_map; mod suspicious_splitn; mod suspicious_to_owned; @@ -3162,6 +3163,32 @@ declare_clippy_lint! { "collecting an iterator when collect is not needed" } +declare_clippy_lint! { + /// ### What it does + /// + /// Checks for `Command::arg()` invocations that look like they + /// should be multiple arguments instead, such as `arg("-t ext2")`. + /// + /// ### Why is this bad? + /// + /// `Command::arg()` does not split arguments by space. An argument like `arg("-t ext2")` + /// will be passed as a single argument to the command, + /// which is likely not what was intended. + /// + /// ### Example + /// ```rust + /// std::process::Command::new("echo").arg("-n hello").spawn().unwrap(); + /// ``` + /// Use instead: + /// ```rust + /// std::process::Command::new("echo").args(["-n", "hello"]).spawn().unwrap(); + /// ``` + #[clippy::version = "1.67.0"] + pub SUSPICIOUS_COMMAND_ARG_SPACE, + suspicious, + "single command line argument that looks like it should be multiple arguments" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -3289,6 +3316,7 @@ impl_lint_pass!(Methods => [ SEEK_FROM_CURRENT, SEEK_TO_START_INSTEAD_OF_REWIND, NEEDLESS_COLLECT, + SUSPICIOUS_COMMAND_ARG_SPACE, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -3496,6 +3524,9 @@ impl Methods { unnecessary_lazy_eval::check(cx, expr, recv, arg, "and"); } }, + ("arg", [arg]) => { + suspicious_command_arg_space::check(cx, recv, arg, span); + } ("as_deref" | "as_deref_mut", []) => { needless_option_as_deref::check(cx, expr, recv, name); }, diff --git a/clippy_lints/src/methods/suspicious_command_arg_space.rs b/clippy_lints/src/methods/suspicious_command_arg_space.rs new file mode 100644 index 000000000..73632c5a3 --- /dev/null +++ b/clippy_lints/src/methods/suspicious_command_arg_space.rs @@ -0,0 +1,39 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::paths; +use clippy_utils::ty::match_type; +use rustc_ast as ast; +use rustc_errors::{Applicability, Diagnostic}; +use rustc_hir as hir; +use rustc_lint::LateContext; +use rustc_span::Span; + +use super::SUSPICIOUS_COMMAND_ARG_SPACE; + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, recv: &'tcx hir::Expr<'_>, arg: &'tcx hir::Expr<'_>, span: Span) { + let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + + if match_type(cx, ty, &paths::STD_PROCESS_COMMAND) + && let hir::ExprKind::Lit(lit) = &arg.kind + && let ast::LitKind::Str(s, _) = &lit.node + && let Some((arg1, arg2)) = s.as_str().split_once(' ') + && arg1.starts_with('-') + && arg1.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + { + span_lint_and_then( + cx, + SUSPICIOUS_COMMAND_ARG_SPACE, + arg.span, + "single argument that looks like it should be multiple arguments", + |diag: &mut Diagnostic| { + diag.multipart_suggestion_verbose( + "consider splitting the argument", + vec![ + (span, "args".to_string()), + (arg.span, format!("[{arg1:?}, {arg2:?}]")), + ], + Applicability::MaybeIncorrect, + ); + } + ); + } +} diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 95eebab75..4aae0f728 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -115,6 +115,7 @@ pub const STD_FS_CREATE_DIR: [&str; 3] = ["std", "fs", "create_dir"]; pub const STD_IO_SEEK: [&str; 3] = ["std", "io", "Seek"]; pub const STD_IO_SEEK_FROM_CURRENT: [&str; 4] = ["std", "io", "SeekFrom", "Current"]; pub const STD_IO_SEEKFROM_START: [&str; 4] = ["std", "io", "SeekFrom", "Start"]; +pub const STD_PROCESS_COMMAND: [&str; 3] = ["std", "process", "Command"]; pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"]; pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"]; pub const STRING_NEW: [&str; 4] = ["alloc", "string", "String", "new"]; diff --git a/tests/ui/suspicious_command_arg_space.rs b/tests/ui/suspicious_command_arg_space.rs new file mode 100644 index 000000000..bdc6113a2 --- /dev/null +++ b/tests/ui/suspicious_command_arg_space.rs @@ -0,0 +1,10 @@ +fn main() { + // Things it should warn about: + std::process::Command::new("echo").arg("-n hello").spawn().unwrap(); + std::process::Command::new("cat").arg("--number file").spawn().unwrap(); + + // Things it should not warn about: + std::process::Command::new("echo").arg("hello world").spawn().unwrap(); + std::process::Command::new("a").arg("--fmt=%a %b %c").spawn().unwrap(); + std::process::Command::new("b").arg("-ldflags=-s -w").spawn().unwrap(); +} diff --git a/tests/ui/suspicious_command_arg_space.stderr b/tests/ui/suspicious_command_arg_space.stderr new file mode 100644 index 000000000..9bc0ca93a --- /dev/null +++ b/tests/ui/suspicious_command_arg_space.stderr @@ -0,0 +1,25 @@ +error: single argument that looks like it should be multiple arguments + --> $DIR/suspicious_command_arg_space.rs:3:44 + | +LL | std::process::Command::new("echo").arg("-n hello").spawn().unwrap(); + | ^^^^^^^^^^ + | + = note: `-D clippy::suspicious-command-arg-space` implied by `-D warnings` +help: consider splitting the argument + | +LL | std::process::Command::new("echo").args(["-n", "hello"]).spawn().unwrap(); + | ~~~~ ~~~~~~~~~~~~~~~ + +error: single argument that looks like it should be multiple arguments + --> $DIR/suspicious_command_arg_space.rs:4:43 + | +LL | std::process::Command::new("cat").arg("--number file").spawn().unwrap(); + | ^^^^^^^^^^^^^^^ + | +help: consider splitting the argument + | +LL | std::process::Command::new("cat").args(["--number", "file"]).spawn().unwrap(); + | ~~~~ ~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 2 previous errors +