Skip to content

Commit

Permalink
Add case_sensitive_file_extensions lint
Browse files Browse the repository at this point in the history
Closes #6425

Looks for ends_with methods calls with case sensitive extensions.
  • Loading branch information
Javier-varez committed Jan 5, 2021
1 parent a6b72d3 commit 61f3d9d
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -1878,6 +1878,7 @@ Released 2018-09-13
[`boxed_local`]: https://rust-lang.github.io/rust-clippy/master/index.html#boxed_local
[`builtin_type_shadow`]: https://rust-lang.github.io/rust-clippy/master/index.html#builtin_type_shadow
[`cargo_common_metadata`]: https://rust-lang.github.io/rust-clippy/master/index.html#cargo_common_metadata
[`case_sensitive_file_extension_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#case_sensitive_file_extension_comparisons
[`cast_lossless`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless
[`cast_possible_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation
[`cast_possible_wrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_wrap
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/Cargo.toml
Expand Up @@ -34,6 +34,8 @@ rustc-semver="1.1.0"
url = { version = "2.1.0", features = ["serde"] }
quote = "1"
syn = { version = "1", features = ["full"] }
regex = "1.4"
lazy_static = "1.4"

[features]
deny-warnings = []
Expand Down
88 changes: 88 additions & 0 deletions clippy_lints/src/case_sensitive_file_extension_comparisons.rs
@@ -0,0 +1,88 @@
use crate::utils::paths::STRING;
use crate::utils::{match_def_path, span_lint_and_help};
use if_chain::if_chain;
use lazy_static::lazy_static;
use regex::Regex;
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, 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.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// fn is_rust_file(filename: &str) -> bool {
/// filename.ends_with(".rs")
/// }
/// ```
/// Use instead:
/// ```rust
/// fn is_rust_file(filename: &str) -> bool {
/// filename.rsplit('.').next().map(|ext| ext.eq_ignore_ascii_case("rs")) == Some(true)
/// }
/// ```
pub CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
pedantic,
"default lint description"
}

declare_lint_pass!(CaseSensitiveFileExtensionComparisons => [CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS]);

fn check_case_sensitive_file_extension_comparison(ctx: &LateContext<'_>, expr: &Expr<'_>) -> Option<Span> {
lazy_static! {
static ref RE: Regex = Regex::new(r"^\.([a-z0-9]{1,5}|[A-Z0-9]{1,5})$").unwrap();
}
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 RE.is_match(&ext_literal.as_str());
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(&ty::AdtDef { did, .. }, _) => {
if match_def_path(ctx, did, &STRING) {
return Some(span);
}
},
_ => { return None; }
}
}
}
None
}

impl 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",
);
}
}
}
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Expand Up @@ -170,6 +170,7 @@ mod blocks_in_if_conditions;
mod booleans;
mod bytecount;
mod cargo_common_metadata;
mod case_sensitive_file_extension_comparisons;
mod checked_conversions;
mod cognitive_complexity;
mod collapsible_if;
Expand Down Expand Up @@ -556,6 +557,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&booleans::NONMINIMAL_BOOL,
&bytecount::NAIVE_BYTECOUNT,
&cargo_common_metadata::CARGO_COMMON_METADATA,
&case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
&checked_conversions::CHECKED_CONVERSIONS,
&cognitive_complexity::COGNITIVE_COMPLEXITY,
&collapsible_if::COLLAPSIBLE_ELSE_IF,
Expand Down Expand Up @@ -1224,6 +1226,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box zero_sized_map_values::ZeroSizedMapValues);
store.register_late_pass(|| box vec_init_then_push::VecInitThenPush::default());
store.register_late_pass(move || box types::PtrAsPtr::new(msrv));
store.register_late_pass(|| box case_sensitive_file_extension_comparisons::CaseSensitiveFileExtensionComparisons);

store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
Expand Down Expand Up @@ -1281,6 +1284,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&await_holding_invalid::AWAIT_HOLDING_LOCK),
LintId::of(&await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
LintId::of(&bit_mask::VERBOSE_BIT_MASK),
LintId::of(&case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS),
LintId::of(&checked_conversions::CHECKED_CONVERSIONS),
LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
LintId::of(&copy_iterator::COPY_ITERATOR),
Expand Down
44 changes: 44 additions & 0 deletions tests/ui/case_sensitive_file_extension_comparisons.rs
@@ -0,0 +1,44 @@
#![warn(clippy::case_sensitive_file_extension_comparisons)]

use std::string::String;

struct TestStruct {}

impl TestStruct {
fn ends_with(self, arg: &str) {}
}

fn is_rust_file(filename: &str) -> bool {
filename.ends_with(".rs")
}

fn main() {
// std::string::String and &str should trigger the lint failure with .ext12
let _ = String::from("").ends_with(".ext12");
let _ = "str".ends_with(".ext12");

// The test struct should not trigger the lint failure with .ext12
TestStruct {}.ends_with(".ext12");

// std::string::String and &str should trigger the lint failure with .EXT12
let _ = String::from("").ends_with(".EXT12");
let _ = "str".ends_with(".EXT12");

// The test struct should not trigger the lint failure with .EXT12
TestStruct {}.ends_with(".EXT12");

// Should not trigger the lint failure with .eXT12
let _ = String::from("").ends_with(".eXT12");
let _ = "str".ends_with(".eXT12");
TestStruct {}.ends_with(".eXT12");

// Should not trigger the lint failure with .EXT123 (too long)
let _ = String::from("").ends_with(".EXT123");
let _ = "str".ends_with(".EXT123");
TestStruct {}.ends_with(".EXT123");

// Shouldn't fail if it doesn't start with a dot
let _ = String::from("").ends_with("a.ext");
let _ = "str".ends_with("a.extA");
TestStruct {}.ends_with("a.ext");
}
43 changes: 43 additions & 0 deletions tests/ui/case_sensitive_file_extension_comparisons.stderr
@@ -0,0 +1,43 @@
error: case-sensitive file extension comparison
--> $DIR/case_sensitive_file_extension_comparisons.rs:12:14
|
LL | filename.ends_with(".rs")
| ^^^^^^^^^^^^^^^^
|
= note: `-D clippy::case-sensitive-file-extension-comparisons` implied by `-D warnings`
= help: consider using a case-insensitive comparison instead

error: case-sensitive file extension comparison
--> $DIR/case_sensitive_file_extension_comparisons.rs:17:30
|
LL | let _ = String::from("").ends_with(".ext12");
| ^^^^^^^^^^^^^^^^^^^
|
= help: consider using a case-insensitive comparison instead

error: case-sensitive file extension comparison
--> $DIR/case_sensitive_file_extension_comparisons.rs:18:19
|
LL | let _ = "str".ends_with(".ext12");
| ^^^^^^^^^^^^^^^^^^^
|
= help: consider using a case-insensitive comparison instead

error: case-sensitive file extension comparison
--> $DIR/case_sensitive_file_extension_comparisons.rs:24:30
|
LL | let _ = String::from("").ends_with(".EXT12");
| ^^^^^^^^^^^^^^^^^^^
|
= help: consider using a case-insensitive comparison instead

error: case-sensitive file extension comparison
--> $DIR/case_sensitive_file_extension_comparisons.rs:25:19
|
LL | let _ = "str".ends_with(".EXT12");
| ^^^^^^^^^^^^^^^^^^^
|
= help: consider using a case-insensitive comparison instead

error: aborting due to 5 previous errors

0 comments on commit 61f3d9d

Please sign in to comment.