Auto merge of #11483 - y21:path_ends_with, r=Alexendoo

new lint: `path_ends_with_ext`

Closes #11479

Not sure if it needs more test cases. I couldn't come up with any other ones, but it is a pretty simple lint logic wise with not too many checks

changelog: new lint: [`path_ends_with_ext`]

Author: bors

CHANGELOG.md

@@ -5245,6 +5245,7 @@ Released 2018-09-13
 [`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
 [`partialeq_to_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_to_none
 [`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite
+[`path_ends_with_ext`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_ends_with_ext
 [`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch
 [`permissions_set_readonly_false`]: https://rust-lang.github.io/rust-clippy/master/index.html#permissions_set_readonly_false
 [`positional_named_format_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#positional_named_format_parameters
@@ -5575,5 +5576,6 @@ Released 2018-09-13
 [`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
 [`absolute-paths-max-segments`]: https://doc.rust-lang.org/clippy/lint_configuration.html#absolute-paths-max-segments
 [`absolute-paths-allowed-crates`]: https://doc.rust-lang.org/clippy/lint_configuration.html#absolute-paths-allowed-crates
+[`allowed-dotfiles`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-dotfiles
 [`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow
 <!-- end autogenerated links to configuration documentation -->

book/src/lint_configuration.md

@@ -751,6 +751,16 @@ Which crates to allow absolute paths from
 * [`absolute_paths`](https://rust-lang.github.io/rust-clippy/master/index.html#absolute_paths)
 
 
+## `allowed-dotfiles`
+Additional dotfiles (files or directories starting with a dot) to allow
+
+**Default Value:** `{}` (`rustc_data_structures::fx::FxHashSet<String>`)
+
+---
+**Affected lints:**
+* [`path_ends_with_ext`](https://rust-lang.github.io/rust-clippy/master/index.html#path_ends_with_ext)
+
+
 ## `enforce-iter-loop-reborrow`
 #### Example
 ```

clippy_lints/src/declared_lints.rs

@@ -399,6 +399,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::OR_FUN_CALL_INFO,
     crate::methods::OR_THEN_UNWRAP_INFO,
     crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
+    crate::methods::PATH_ENDS_WITH_EXT_INFO,
     crate::methods::RANGE_ZIP_WITH_LEN_INFO,
     crate::methods::READONLY_WRITE_LOCK_INFO,
     crate::methods::READ_LINE_WITHOUT_TRIM_INFO,

clippy_lints/src/lib.rs

@@ -662,12 +662,19 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     let allow_unwrap_in_tests = conf.allow_unwrap_in_tests;
     let suppress_restriction_lint_in_const = conf.suppress_restriction_lint_in_const;
     store.register_late_pass(move |_| Box::new(approx_const::ApproxConstant::new(msrv())));
+    let allowed_dotfiles = conf
+        .allowed_dotfiles
+        .iter()
+        .cloned()
+        .chain(methods::DEFAULT_ALLOWED_DOTFILES.iter().copied().map(ToOwned::to_owned))
+        .collect::<FxHashSet<_>>();
     store.register_late_pass(move |_| {
         Box::new(methods::Methods::new(
             avoid_breaking_exported_api,
             msrv(),
             allow_expect_in_tests,
             allow_unwrap_in_tests,
+            allowed_dotfiles.clone(),
         ))
     });
     store.register_late_pass(move |_| Box::new(matches::Matches::new(msrv())));

clippy_lints/src/methods/mod.rs

@@ -74,6 +74,7 @@ mod option_map_unwrap_or;
 mod or_fun_call;
 mod or_then_unwrap;
 mod path_buf_push_overwrite;
+mod path_ends_with_ext;
 mod range_zip_with_len;
 mod read_line_without_trim;
 mod readonly_write_lock;
@@ -120,6 +121,8 @@ use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
 use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, peel_blocks, return_ty};
 use if_chain::if_chain;
+pub use path_ends_with_ext::DEFAULT_ALLOWED_DOTFILES;
+use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
 use rustc_hir::{Expr, ExprKind, Node, Stmt, StmtKind, TraitItem, TraitItemKind};
 use rustc_hir_analysis::hir_ty_to_ty;
@@ -3563,11 +3566,51 @@ declare_clippy_lint! {
     "calls to `.take()` or `.skip()` that are out of bounds"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Looks for calls to `Path::ends_with` calls where the argument looks like a file extension.
+    ///
+    /// By default, Clippy has a short list of known filenames that start with a dot
+    /// but aren't necessarily file extensions (e.g. the `.git` folder), which are allowed by default.
+    /// The `allowed-dotfiles` configuration can be used to allow additional
+    /// file extensions that Clippy should not lint.
+    ///
+    /// ### Why is this bad?
+    /// This doesn't actually compare file extensions. Rather, `ends_with` compares the given argument
+    /// to the last **component** of the path and checks if it matches exactly.
+    ///
+    /// ### Known issues
+    /// File extensions are often at most three characters long, so this only lints in those cases
+    /// in an attempt to avoid false positives.
+    /// Any extension names longer than that are assumed to likely be real path components and are
+    /// therefore ignored.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # use std::path::Path;
+    /// fn is_markdown(path: &Path) -> bool {
+    ///     path.ends_with(".md")
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # use std::path::Path;
+    /// fn is_markdown(path: &Path) -> bool {
+    ///     path.extension().is_some_and(|ext| ext == "md")
+    /// }
+    /// ```
+    #[clippy::version = "1.74.0"]
+    pub PATH_ENDS_WITH_EXT,
+    suspicious,
+    "attempting to compare file extensions using `Path::ends_with`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
     allow_expect_in_tests: bool,
     allow_unwrap_in_tests: bool,
+    allowed_dotfiles: FxHashSet<String>,
 }
 
 impl Methods {
@@ -3577,12 +3620,14 @@ impl Methods {
         msrv: Msrv,
         allow_expect_in_tests: bool,
         allow_unwrap_in_tests: bool,
+        allowed_dotfiles: FxHashSet<String>,
     ) -> Self {
         Self {
             avoid_breaking_exported_api,
             msrv,
             allow_expect_in_tests,
             allow_unwrap_in_tests,
+            allowed_dotfiles,
         }
     }
 }
@@ -3703,6 +3748,7 @@ impl_lint_pass!(Methods => [
     FILTER_MAP_BOOL_THEN,
     READONLY_WRITE_LOCK,
     ITER_OUT_OF_BOUNDS,
+    PATH_ENDS_WITH_EXT,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -3978,6 +4024,7 @@ impl Methods {
                     if let ExprKind::MethodCall(.., span) = expr.kind {
                         case_sensitive_file_extension_comparisons::check(cx, expr, span, recv, arg);
                     }
+                    path_ends_with_ext::check(cx, recv, arg, expr, &self.msrv, &self.allowed_dotfiles);
                 },
                 ("expect", [_]) => {
                     match method_call(recv) {

clippy_lints/src/methods/path_ends_with_ext.rs

@@ -0,0 +1,53 @@
+use super::PATH_ENDS_WITH_EXT;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::msrvs;
+use clippy_utils::msrvs::Msrv;
+use clippy_utils::source::snippet;
+use clippy_utils::ty::is_type_diagnostic_item;
+use rustc_ast::{LitKind, StrStyle};
+use rustc_data_structures::fx::FxHashSet;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::sym;
+use std::fmt::Write;
+
+pub const DEFAULT_ALLOWED_DOTFILES: &[&str] = &[
+    "git", "svn", "gem", "npm", "vim", "env", "rnd", "ssh", "vnc", "smb", "nvm", "bin",
+];
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    recv: &Expr<'_>,
+    path: &Expr<'_>,
+    expr: &Expr<'_>,
+    msrv: &Msrv,
+    allowed_dotfiles: &FxHashSet<String>,
+) {
+    if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv).peel_refs(), sym::Path)
+        && !path.span.from_expansion()
+        && let ExprKind::Lit(lit) = path.kind
+        && let LitKind::Str(path, StrStyle::Cooked) = lit.node
+        && let Some(path) = path.as_str().strip_prefix('.')
+        && (1..=3).contains(&path.len())
+        && !allowed_dotfiles.contains(path)
+        && path.chars().all(char::is_alphanumeric)
+    {
+        let mut sugg = snippet(cx, recv.span, "..").into_owned();
+        if msrv.meets(msrvs::OPTION_IS_SOME_AND) {
+            let _ = write!(sugg, r#".extension().is_some_and(|ext| ext == "{path}")"#);
+        } else {
+            let _ = write!(sugg, r#".extension().map_or(false, |ext| ext == "{path}")"#);
+        };
+
+        span_lint_and_sugg(
+            cx,
+            PATH_ENDS_WITH_EXT,
+            expr.span,
+            "this looks like a failed attempt at checking for the file extension",
+            "try",
+            sugg,
+            Applicability::MaybeIncorrect
+        );
+    }
+}

clippy_lints/src/utils/conf.rs

@@ -561,6 +561,11 @@ define_Conf! {
     /// Which crates to allow absolute paths from
     (absolute_paths_allowed_crates: rustc_data_structures::fx::FxHashSet<String> =
         rustc_data_structures::fx::FxHashSet::default()),
+    /// Lint: PATH_ENDS_WITH_EXT.
+    ///
+    /// Additional dotfiles (files or directories starting with a dot) to allow
+    (allowed_dotfiles: rustc_data_structures::fx::FxHashSet<String> =
+        rustc_data_structures::fx::FxHashSet::default()),
     /// Lint: EXPLICIT_ITER_LOOP
     ///
     /// Whether to recommend using implicit into iter for reborrowed values.

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -10,6 +10,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            allow-print-in-tests
            allow-private-module-inception
            allow-unwrap-in-tests
+           allowed-dotfiles
            allowed-idents-below-min-chars
            allowed-scripts
            arithmetic-side-effects-allowed
@@ -82,6 +83,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            allow-print-in-tests
            allow-private-module-inception
            allow-unwrap-in-tests
+           allowed-dotfiles
            allowed-idents-below-min-chars
            allowed-scripts
            arithmetic-side-effects-allowed

tests/ui/path_ends_with_ext.fixed

@@ -0,0 +1,36 @@
+#![warn(clippy::path_ends_with_ext)]
+use std::path::Path;
+
+macro_rules! arg {
+    () => {
+        ".md"
+    };
+}
+
+fn test(path: &Path) {
+    path.extension().is_some_and(|ext| ext == "md");
+    //~^ ERROR: this looks like a failed attempt at checking for the file extension
+
+    // some "extensions" are allowed by default
+    path.ends_with(".git");
+
+    // most legitimate "dotfiles" are longer than 3 chars, so we allow them as well
+    path.ends_with(".bashrc");
+
+    // argument from expn shouldn't trigger
+    path.ends_with(arg!());
+
+    path.ends_with("..");
+    path.ends_with("./a");
+    path.ends_with(".");
+    path.ends_with("");
+}
+
+// is_some_and was stabilized in 1.70, so suggest map_or(false, ..) if under that
+#[clippy::msrv = "1.69"]
+fn under_msv(path: &Path) -> bool {
+    path.extension().map_or(false, |ext| ext == "md")
+    //~^ ERROR: this looks like a failed attempt at checking for the file extension
+}
+
+fn main() {}

tests/ui/path_ends_with_ext.rs

@@ -0,0 +1,36 @@
+#![warn(clippy::path_ends_with_ext)]
+use std::path::Path;
+
+macro_rules! arg {
+    () => {
+        ".md"
+    };
+}
+
+fn test(path: &Path) {
+    path.ends_with(".md");
+    //~^ ERROR: this looks like a failed attempt at checking for the file extension
+
+    // some "extensions" are allowed by default
+    path.ends_with(".git");
+
+    // most legitimate "dotfiles" are longer than 3 chars, so we allow them as well
+    path.ends_with(".bashrc");
+
+    // argument from expn shouldn't trigger
+    path.ends_with(arg!());
+
+    path.ends_with("..");
+    path.ends_with("./a");
+    path.ends_with(".");
+    path.ends_with("");
+}
+
+// is_some_and was stabilized in 1.70, so suggest map_or(false, ..) if under that
+#[clippy::msrv = "1.69"]
+fn under_msv(path: &Path) -> bool {
+    path.ends_with(".md")
+    //~^ ERROR: this looks like a failed attempt at checking for the file extension
+}
+
+fn main() {}

tests/ui/path_ends_with_ext.stderr

@@ -0,0 +1,17 @@
+error: this looks like a failed attempt at checking for the file extension
+  --> $DIR/path_ends_with_ext.rs:11:5
+   |
+LL |     path.ends_with(".md");
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: try: `path.extension().is_some_and(|ext| ext == "md")`
+   |
+   = note: `-D clippy::path-ends-with-ext` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::path_ends_with_ext)]`
+
+error: this looks like a failed attempt at checking for the file extension
+  --> $DIR/path_ends_with_ext.rs:32:5
+   |
+LL |     path.ends_with(".md")
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: try: `path.extension().map_or(false, |ext| ext == "md")`
+
+error: aborting due to 2 previous errors
+