about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-02-12 21:57:49 +0000
committerbors <bors@rust-lang.org>2023-02-12 21:57:49 +0000
commit298f1397982a4e7cf718931bff399fcc6bffd9e1 (patch)
tree7b0d5212a0d700529e2e4e6dd56869b4904ea9e0
parent6f353fdf0a4ea505dd70c315478e29dce5e873e0 (diff)
parent1f77866991568557e69e2135369ab2a512052a6b (diff)
downloadrust-298f1397982a4e7cf718931bff399fcc6bffd9e1.tar.gz
rust-298f1397982a4e7cf718931bff399fcc6bffd9e1.zip
Auto merge of #10317 - m-ou-se:suspicious-command-arg-space, r=Manishearth
Add `suspicious_command_arg_space` lint

Fixes #10316

---

changelog: New lint: [`suspicious_command_arg_space`]
[#10317](https://github.com/rust-lang/rust-clippy/pull/10317)
<!-- changelog_checked -->
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/methods/mod.rs31
-rw-r--r--clippy_lints/src/methods/suspicious_command_arg_space.rs39
-rw-r--r--clippy_utils/src/paths.rs1
-rw-r--r--tests/ui/suspicious_command_arg_space.rs10
-rw-r--r--tests/ui/suspicious_command_arg_space.stderr25
7 files changed, 108 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 659e8aebcd5..30727beb8e7 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 457a25826e7..c1feabf05aa 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 f1e8be7f2b8..d8c018f7591 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 00000000000..73632c5a357
--- /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 95eebab7567..4aae0f7284e 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 00000000000..bdc6113a250
--- /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 00000000000..9bc0ca93aec
--- /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
+