Implement a lint for replacable Command::new

CHANGELOG.md

@@ -6075,6 +6075,7 @@ Released 2018-09-13
 [`unnecessary_safety_comment`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_comment
 [`unnecessary_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_doc
 [`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports
+[`unnecessary_shell_command`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_shell_command
 [`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by
 [`unnecessary_struct_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_struct_initialization
 [`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned
@@ -6221,6 +6222,7 @@ Released 2018-09-13
 [`trivial-copy-size-limit`]: https://doc.rust-lang.org/clippy/lint_configuration.html#trivial-copy-size-limit
 [`type-complexity-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#type-complexity-threshold
 [`unnecessary-box-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#unnecessary-box-size
+[`unnecessary-commands`]: https://doc.rust-lang.org/clippy/lint_configuration.html#unnecessary-commands
 [`unreadable-literal-lint-fractions`]: https://doc.rust-lang.org/clippy/lint_configuration.html#unreadable-literal-lint-fractions
 [`upper-case-acronyms-aggressive`]: https://doc.rust-lang.org/clippy/lint_configuration.html#upper-case-acronyms-aggressive
 [`vec-box-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#vec-box-size-threshold

README.md

@@ -5,7 +5,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
 You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.

book/src/README.md

@@ -6,7 +6,7 @@
 A collection of lints to catch common mistakes and improve your
 [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint
 level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how

book/src/lint_configuration.md

@@ -893,6 +893,16 @@ The byte size a `T` in `Box<T>` can have, below which it triggers the `clippy::u
 * [`unnecessary_box_returns`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns)
 
 
+## `unnecessary-commands`
+The list of shell commands to recommend replacing.
+
+**Default Value:** `{ curl = "use `std::net`, `ureq`, or `reqwest` instead", jq = "use `serde_json`, or another native json parser instead", ls = "use `std::fs::read_dir` instead", sed = "use `regex`, or the methods on `str` instead", wget = "use `std::net`, `ureq`, or `reqwest` instead" }`
+
+---
+**Affected lints:**
+* [`unnecessary_shell_commands`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_shell_commands)
+
+
 ## `unreadable-literal-lint-fractions`
 Should the fraction of a decimal be linted to include separators.
 

clippy_config/src/conf.rs

@@ -7,6 +7,8 @@ use rustc_span::edit_distance::edit_distance;
 use rustc_span::{BytePos, Pos, SourceFile, Span, SyntaxContext};
 use serde::de::{IgnoredAny, IntoDeserializer, MapAccess, Visitor};
 use serde::{Deserialize, Deserializer, Serialize};
+use std::borrow::Cow;
+use std::collections::BTreeMap;
 use std::fmt::{Debug, Display, Formatter};
 use std::ops::Range;
 use std::path::PathBuf;
@@ -648,6 +650,15 @@ define_Conf! {
     /// The byte size a `T` in `Box<T>` can have, below which it triggers the `clippy::unnecessary_box` lint
     #[lints(unnecessary_box_returns)]
     unnecessary_box_size: u64 = 128,
+    /// The list of shell commands to recommend replacing.
+    #[lints(unnecessary_shell_commands)]
+    unnecessary_commands: BTreeMap<Box<str>, Option<Cow<'static, str>>> = [
+        ["ls",   "use `std::fs::read_dir` instead"],
+        ["curl", "use `std::net`, `ureq`, or `reqwest` instead"],
+        ["wget", "use `std::net`, `ureq`, or `reqwest` instead"],
+        ["sed",  "use `regex`, or the methods on `str` instead"],
+        ["jq",   "use `serde_json`, or another native json parser instead"],
+    ].into_iter().map(|[k, v]| (Box::<str>::from(k), Some(Cow::Borrowed(v)))).collect(),
     /// Should the fraction of a decimal be linted to include separators.
     #[lints(unreadable_literal)]
     unreadable_literal_lint_fractions: bool = true,

clippy_lints/src/declared_lints.rs

@@ -744,6 +744,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_INFO,
     crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
     crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
+    crate::unnecessary_shell_command::UNNECESSARY_SHELL_COMMAND_INFO,
     crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO,
     crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO,
     crate::unnested_or_patterns::UNNESTED_OR_PATTERNS_INFO,

clippy_lints/src/lib.rs

@@ -367,6 +367,7 @@ mod unnecessary_literal_bound;
 mod unnecessary_map_on_constructor;
 mod unnecessary_owned_empty_strings;
 mod unnecessary_self_imports;
+mod unnecessary_shell_command;
 mod unnecessary_struct_initialization;
 mod unnecessary_wraps;
 mod unnested_or_patterns;
@@ -949,5 +950,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
     store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
     store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound));
+    store.register_late_pass(move |_| Box::new(unnecessary_shell_command::UnnecessaryShellCommand::new(conf)));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/unnecessary_shell_command.rs

@@ -0,0 +1,107 @@
+use std::borrow::Cow;
+use std::collections::BTreeMap;
+
+use clippy_config::Conf;
+use clippy_utils::consts::{ConstEvalCtxt, Constant};
+use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
+use clippy_utils::path_def_id;
+use clippy_utils::ty::get_inherent_method;
+use rustc_hir::def_id::DefId;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::impl_lint_pass;
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Detects calls to `std::process::Command::new` that can easily be replaced with Rust code.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// "Shelling out" is slow, non-portable, and generally unnecessary (especially to these programs).
+    ///
+    /// ### Example
+    /// ```no_run
+    /// use std::io;
+    /// use std::process::Command;
+    ///
+    /// fn list_files() -> io::Result<Vec<String>> {
+    ///     let output = Command::new("ls").output()?;
+    ///     if !output.status.success() {
+    ///         return Err(io::Error::new(
+    ///             io::ErrorKind::Other,
+    ///             String::from_utf8_lossy(&output.stderr)
+    ///         ));
+    ///     }
+    ///
+    ///     let stdout = std::str::from_utf8(&output.stdout).expect("should be UTF-8 output");
+    ///     Ok(stdout.split_whitespace().map(String::from).collect())
+    /// }
+    /// // example code where clippy issues a warning
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// fn list_files() -> std::io::Result<Vec<String>> {
+    ///     let mut buf = Vec::new();
+    ///     for entry_res in std::fs::read_dir(".")? {
+    ///         let path = entry_res?.path();
+    ///         let os_name = path.into_os_string();
+    ///         buf.push(os_name.into_string().expect("should be UTF-8 paths"))
+    ///     }
+    ///
+    ///     Ok(buf)
+    /// }
+    /// ```
+    #[clippy::version = "1.84.0"]
+    pub UNNECESSARY_SHELL_COMMAND,
+    pedantic,
+    "using the simple shell utilities instead of Rust code"
+}
+
+pub struct UnnecessaryShellCommand {
+    std_process_command_new: Option<DefId>,
+    unnecessary_commands: &'static BTreeMap<Box<str>, Option<Cow<'static, str>>>,
+}
+
+impl UnnecessaryShellCommand {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self {
+            std_process_command_new: None,
+            unnecessary_commands: &conf.unnecessary_commands,
+        }
+    }
+}
+
+impl_lint_pass!(UnnecessaryShellCommand => [UNNECESSARY_SHELL_COMMAND]);
+
+impl LateLintPass<'_> for UnnecessaryShellCommand {
+    fn check_crate(&mut self, cx: &LateContext<'_>) {
+        if let Some(command_did) = cx.tcx.get_diagnostic_item(sym::Command)
+            && let Some(fn_item) = get_inherent_method(cx, command_did, sym::new)
+        {
+            self.std_process_command_new = Some(fn_item.def_id);
+        }
+    }
+
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        let Some(std_process_command_new) = self.std_process_command_new else {
+            return;
+        };
+
+        if let ExprKind::Call(func, [command]) = expr.kind
+            && path_def_id(cx, func) == Some(std_process_command_new)
+            && let Some(Constant::Str(command_lit)) = ConstEvalCtxt::new(cx).eval(command)
+            && let command_lit = command_lit.strip_suffix(".exe").unwrap_or(&command_lit)
+            && let Some(help) = self.unnecessary_commands.get(command_lit)
+        {
+            let lint = UNNECESSARY_SHELL_COMMAND;
+            let msg = "unnecessarily shelling out for trivial operation";
+            if let Some(help) = help.as_deref() {
+                span_lint_and_help(cx, lint, command.span, msg, None, help);
+            } else {
+                span_lint(cx, lint, command.span, msg);
+            }
+        }
+    }
+}

clippy_utils/src/ty.rs

@@ -1323,22 +1323,29 @@ pub fn deref_chain<'cx, 'tcx>(cx: &'cx LateContext<'tcx>, ty: Ty<'tcx>) -> impl
 
 /// Checks if a Ty<'_> has some inherent method Symbol.
 ///
-/// This does not look for impls in the type's `Deref::Target` type.
-/// If you need this, you should wrap this call in `clippy_utils::ty::deref_chain().any(...)`.
+/// This is a helper for [`get_inherent_method`].
 pub fn get_adt_inherent_method<'a>(cx: &'a LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> Option<&'a AssocItem> {
     if let Some(ty_did) = ty.ty_adt_def().map(AdtDef::did) {
-        cx.tcx.inherent_impls(ty_did).iter().find_map(|&did| {
-            cx.tcx
-                .associated_items(did)
-                .filter_by_name_unhygienic(method_name)
-                .next()
-                .filter(|item| item.kind == AssocKind::Fn)
-        })
+        get_inherent_method(cx, ty_did, method_name)
     } else {
         None
     }
 }
 
+/// Checks if the [`DefId`] of a Ty has some inherent method Symbol.
+///
+/// This does not look for impls in the type's `Deref::Target` type.
+/// If you need this, you should wrap this call in `clippy_utils::ty::deref_chain().any(...)`.
+pub fn get_inherent_method<'a>(cx: &'a LateContext<'_>, ty_did: DefId, method_name: Symbol) -> Option<&'a AssocItem> {
+    cx.tcx.inherent_impls(ty_did).iter().find_map(|&did| {
+        cx.tcx
+            .associated_items(did)
+            .filter_by_name_unhygienic(method_name)
+            .next()
+            .filter(|item| item.kind == AssocKind::Fn)
+    })
+}
+
 /// Get's the type of a field by name.
 pub fn get_field_by_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, name: Symbol) -> Option<Ty<'tcx>> {
     match *ty.kind() {

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -71,6 +71,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            trivial-copy-size-limit
            type-complexity-threshold
            unnecessary-box-size
+           unnecessary-commands
            unreadable-literal-lint-fractions
            upper-case-acronyms-aggressive
            vec-box-size-threshold
@@ -155,6 +156,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            trivial-copy-size-limit
            type-complexity-threshold
            unnecessary-box-size
+           unnecessary-commands
            unreadable-literal-lint-fractions
            upper-case-acronyms-aggressive
            vec-box-size-threshold
@@ -239,6 +241,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
            trivial-copy-size-limit
            type-complexity-threshold
            unnecessary-box-size
+           unnecessary-commands
            unreadable-literal-lint-fractions
            upper-case-acronyms-aggressive
            vec-box-size-threshold

tests/ui/unnecessary_shell_command.rs

@@ -0,0 +1,18 @@
+#![warn(clippy::unnecessary_shell_command)]
+
+use std::process::Command;
+
+fn main() {
+    let _ = Command::new("ls");
+    //~^ error: unnecessarily shelling out for trivial operation
+    let _ = Command::new("curl");
+    //~^ error: unnecessarily shelling out for trivial operation
+    let _ = Command::new("wget");
+    //~^ error: unnecessarily shelling out for trivial operation
+    let _ = Command::new("sed");
+    //~^ error: unnecessarily shelling out for trivial operation
+    let _ = Command::new("jq");
+    //~^ error: unnecessarily shelling out for trivial operation
+
+    let _ = Command::new("ffmpeg");
+}

tests/ui/unnecessary_shell_command.stderr

@@ -0,0 +1,44 @@
+error: unnecessarily shelling out for trivial operation
+  --> tests/ui/unnecessary_shell_command.rs:6:26
+   |
+LL |     let _ = Command::new("ls");
+   |                          ^^^^
+   |
+   = help: use `std::fs::read_dir` instead
+   = note: `-D clippy::unnecessary-shell-command` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::unnecessary_shell_command)]`
+
+error: unnecessarily shelling out for trivial operation
+  --> tests/ui/unnecessary_shell_command.rs:8:26
+   |
+LL |     let _ = Command::new("curl");
+   |                          ^^^^^^
+   |
+   = help: use `std::net`, `ureq`, or `reqwest` instead
+
+error: unnecessarily shelling out for trivial operation
+  --> tests/ui/unnecessary_shell_command.rs:10:26
+   |
+LL |     let _ = Command::new("wget");
+   |                          ^^^^^^
+   |
+   = help: use `std::net`, `ureq`, or `reqwest` instead
+
+error: unnecessarily shelling out for trivial operation
+  --> tests/ui/unnecessary_shell_command.rs:12:26
+   |
+LL |     let _ = Command::new("sed");
+   |                          ^^^^^
+   |
+   = help: use `regex`, or the methods on `str` instead
+
+error: unnecessarily shelling out for trivial operation
+  --> tests/ui/unnecessary_shell_command.rs:14:26
+   |
+LL |     let _ = Command::new("jq");
+   |                          ^^^^
+   |
+   = help: use `serde_json`, or another native json parser instead
+
+error: aborting due to 5 previous errors
+