Skip to content

new lint: path_ends_with_ext #11483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 -->
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())));
Expand Down
47 changes: 47 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
}
}
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down
53 changes: 53 additions & 0 deletions clippy_lints/src/methods/path_ends_with_ext.rs
Original file line number Diff line number Diff line change
@@ -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
);
}
}
5 changes: 5 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
36 changes: 36 additions & 0 deletions tests/ui/path_ends_with_ext.fixed
Original file line number Diff line number Diff line change
@@ -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() {}
36 changes: 36 additions & 0 deletions tests/ui/path_ends_with_ext.rs
Original file line number Diff line number Diff line change
@@ -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() {}
17 changes: 17 additions & 0 deletions tests/ui/path_ends_with_ext.stderr
Original file line number Diff line number Diff line change
@@ -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