Move CaseSensitiveFileExtensionComparisons into Methods lint pass

This commit is contained in:
Jason Newcomb 2022-06-05 13:21:04 -04:00
parent ba6a459528
commit e3b77974d0
6 changed files with 77 additions and 91 deletions

View file

@ -1,85 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_help;
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_hir::{Expr, ExprKind, PathSegment};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{source_map::Spanned, symbol::sym, Span};
declare_clippy_lint! {
/// ### What it does
/// Checks for calls to `ends_with` with possible file extensions
/// and suggests to use a case-insensitive approach instead.
///
/// ### Why is this bad?
/// `ends_with` is case-sensitive and may not detect files with a valid extension.
///
/// ### Example
/// ```rust
/// fn is_rust_file(filename: &str) -> bool {
/// filename.ends_with(".rs")
/// }
/// ```
/// Use instead:
/// ```rust
/// fn is_rust_file(filename: &str) -> bool {
/// let filename = std::path::Path::new(filename);
/// filename.extension()
/// .map_or(false, |ext| ext.eq_ignore_ascii_case("rs"))
/// }
/// ```
#[clippy::version = "1.51.0"]
pub CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
pedantic,
"Checks for calls to ends_with with case-sensitive file extensions"
}
declare_lint_pass!(CaseSensitiveFileExtensionComparisons => [CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS]);
fn check_case_sensitive_file_extension_comparison(ctx: &LateContext<'_>, expr: &Expr<'_>) -> Option<Span> {
if_chain! {
if let ExprKind::MethodCall(PathSegment { ident, .. }, [obj, extension, ..], span) = expr.kind;
if ident.as_str() == "ends_with";
if let ExprKind::Lit(Spanned { node: LitKind::Str(ext_literal, ..), ..}) = extension.kind;
if (2..=6).contains(&ext_literal.as_str().len());
if ext_literal.as_str().starts_with('.');
if ext_literal.as_str().chars().skip(1).all(|c| c.is_uppercase() || c.is_ascii_digit())
|| ext_literal.as_str().chars().skip(1).all(|c| c.is_lowercase() || c.is_ascii_digit());
then {
let mut ty = ctx.typeck_results().expr_ty(obj);
ty = match ty.kind() {
ty::Ref(_, ty, ..) => *ty,
_ => ty
};
match ty.kind() {
ty::Str => {
return Some(span);
},
ty::Adt(def, _) => {
if ctx.tcx.is_diagnostic_item(sym::String, def.did()) {
return Some(span);
}
},
_ => { return None; }
}
}
}
None
}
impl<'tcx> LateLintPass<'tcx> for CaseSensitiveFileExtensionComparisons {
fn check_expr(&mut self, ctx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if let Some(span) = check_case_sensitive_file_extension_comparison(ctx, expr) {
span_lint_and_help(
ctx,
CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
span,
"case-sensitive file extension comparison",
None,
"consider using a case-insensitive comparison instead",
);
}
}
}

View file

@ -64,7 +64,6 @@ store.register_lints(&[
cargo::NEGATIVE_FEATURE_NAMES,
cargo::REDUNDANT_FEATURE_NAMES,
cargo::WILDCARD_DEPENDENCIES,
case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
casts::AS_UNDERSCORE,
casts::BORROW_AS_PTR,
casts::CAST_ABS_TO_UNSIGNED,
@ -285,6 +284,7 @@ store.register_lints(&[
methods::BIND_INSTEAD_OF_MAP,
methods::BYTES_COUNT_TO_LEN,
methods::BYTES_NTH,
methods::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
methods::CHARS_LAST_CMP,
methods::CHARS_NEXT_CMP,
methods::CLONED_INSTEAD_OF_COPIED,

View file

@ -4,7 +4,6 @@
store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
LintId::of(attrs::INLINE_ALWAYS),
LintId::of(case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS),
LintId::of(casts::BORROW_AS_PTR),
LintId::of(casts::CAST_LOSSLESS),
LintId::of(casts::CAST_POSSIBLE_TRUNCATION),
@ -56,6 +55,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
LintId::of(matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS),
LintId::of(matches::MATCH_WILD_ERR_ARM),
LintId::of(matches::SINGLE_MATCH_ELSE),
LintId::of(methods::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS),
LintId::of(methods::CLONED_INSTEAD_OF_COPIED),
LintId::of(methods::FILTER_MAP_NEXT),
LintId::of(methods::FLAT_MAP_OPTION),

View file

@ -181,7 +181,6 @@ mod bool_assert_comparison;
mod booleans;
mod borrow_deref_ref;
mod cargo;
mod case_sensitive_file_extension_comparisons;
mod casts;
mod checked_conversions;
mod cognitive_complexity;
@ -852,9 +851,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(strings::StringToString));
store.register_late_pass(|| Box::new(zero_sized_map_values::ZeroSizedMapValues));
store.register_late_pass(|| Box::new(vec_init_then_push::VecInitThenPush::default()));
store.register_late_pass(|| {
Box::new(case_sensitive_file_extension_comparisons::CaseSensitiveFileExtensionComparisons)
});
store.register_late_pass(|| Box::new(redundant_slicing::RedundantSlicing));
store.register_late_pass(|| Box::new(from_str_radix_10::FromStrRadix10));
store.register_late_pass(move || Box::new(if_then_some_else_none::IfThenSomeElseNone::new(msrv)));

View file

@ -0,0 +1,41 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::ty::is_type_diagnostic_item;
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_span::{source_map::Spanned, symbol::sym, Span};
use super::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
call_span: Span,
recv: &'tcx Expr<'_>,
arg: &'tcx Expr<'_>,
) {
if_chain! {
if let Some(method_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if let Some(impl_id) = cx.tcx.impl_of_method(method_id);
if cx.tcx.type_of(impl_id).is_str();
if let ExprKind::Lit(Spanned { node: LitKind::Str(ext_literal, ..), ..}) = arg.kind;
if (2..=6).contains(&ext_literal.as_str().len());
let ext_str = ext_literal.as_str();
if ext_str.starts_with('.');
if ext_str.chars().skip(1).all(|c| c.is_uppercase() || c.is_ascii_digit())
|| ext_str.chars().skip(1).all(|c| c.is_lowercase() || c.is_ascii_digit());
let recv_ty = cx.typeck_results().expr_ty(recv).peel_refs();
if recv_ty.is_str() || is_type_diagnostic_item(cx, recv_ty, sym::String);
then {
span_lint_and_help(
cx,
CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
call_span,
"case-sensitive file extension comparison",
None,
"consider using a case-insensitive comparison instead",
);
}
}
}

View file

@ -2,6 +2,7 @@ mod bind_instead_of_map;
mod bytecount;
mod bytes_count_to_len;
mod bytes_nth;
mod case_sensitive_file_extension_comparisons;
mod chars_cmp;
mod chars_cmp_with_unwrap;
mod chars_last_cmp;
@ -2428,6 +2429,34 @@ declare_clippy_lint! {
"Using `bytes().count()` when `len()` performs the same functionality"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for calls to `ends_with` with possible file extensions
/// and suggests to use a case-insensitive approach instead.
///
/// ### Why is this bad?
/// `ends_with` is case-sensitive and may not detect files with a valid extension.
///
/// ### Example
/// ```rust
/// fn is_rust_file(filename: &str) -> bool {
/// filename.ends_with(".rs")
/// }
/// ```
/// Use instead:
/// ```rust
/// fn is_rust_file(filename: &str) -> bool {
/// let filename = std::path::Path::new(filename);
/// filename.extension()
/// .map_or(false, |ext| ext.eq_ignore_ascii_case("rs"))
/// }
/// ```
#[clippy::version = "1.51.0"]
pub CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
pedantic,
"Checks for calls to ends_with with case-sensitive file extensions"
}
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>,
@ -2534,6 +2563,7 @@ impl_lint_pass!(Methods => [
ITER_ON_EMPTY_COLLECTIONS,
NAIVE_BYTECOUNT,
BYTES_COUNT_TO_LEN,
CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -2801,6 +2831,10 @@ impl Methods {
("drain", [arg]) => {
iter_with_drain::check(cx, expr, recv, span, arg);
},
("ends_with", [arg]) => {
if let ExprKind::MethodCall(_, _, span) = expr.kind {
case_sensitive_file_extension_comparisons::check(cx, expr, span, recv, arg);
},
("expect", [_]) => match method_call(recv) {
Some(("ok", [recv], _)) => ok_expect::check(cx, expr, recv),
Some(("err", [recv], err_span)) => err_expect::check(cx, expr, recv, self.msrv, span, err_span),