Auto merge of rust-lang#10317 - m-ou-se:suspicious-command-arg-space,…

… r=Manishearth

Add `suspicious_command_arg_space` lint

Fixes rust-lang#10316

---

changelog: New lint: [`suspicious_command_arg_space`]
[rust-lang#10317](rust-lang/rust-clippy#10317)
<!-- changelog_checked -->

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

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,

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);
                 },

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,
+                );
+            }
+        );
+    }
+}

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"];

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();
+}

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
+