Add case_sensitive_file_extensions lint

Closes rust-lang#6425

Looks for ends_with methods calls with case sensitive extensions.

Author: Javier-varez

CHANGELOG.md

@@ -1963,6 +1963,7 @@ Released 2018-09-13
 [`mutable_key_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type
 [`mutex_atomic`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_atomic
 [`mutex_integer`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer
+[`case_sensitive_file_extensions`]: https://rust-lang.github.io/rust-clippy/master/index.html#case_sensitive_file_extensions
 [`naive_bytecount`]: https://rust-lang.github.io/rust-clippy/master/index.html#naive_bytecount
 [`needless_arbitrary_self_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_arbitrary_self_type
 [`needless_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_bool

clippy_lints/Cargo.toml

@@ -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 = []

clippy_lints/src/case_sensitive_file_extensions.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_EXTENSIONS,
+    pedantic,
+    "default lint description"
+}
+
+declare_lint_pass!(CaseSensitiveFileExtensions => [CASE_SENSITIVE_FILE_EXTENSIONS]);
+
+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 CaseSensitiveFileExtensions {
+    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_EXTENSIONS,
+                span,
+                "case-sensitive file extension comparison",
+                None,
+                "consider using a case-insensitive comparison instead",
+            );
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -170,6 +170,7 @@ mod blocks_in_if_conditions;
 mod booleans;
 mod bytecount;
 mod cargo_common_metadata;
+mod case_sensitive_file_extensions;
 mod checked_conversions;
 mod cognitive_complexity;
 mod collapsible_if;
@@ -792,6 +793,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL,
         &mutex_atomic::MUTEX_ATOMIC,
         &mutex_atomic::MUTEX_INTEGER,
+        &case_sensitive_file_extensions::CASE_SENSITIVE_FILE_EXTENSIONS,
         &needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE,
         &needless_bool::BOOL_COMPARISON,
         &needless_bool::NEEDLESS_BOOL,
@@ -1215,6 +1217,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box strings::StrToString);
     store.register_late_pass(|| box strings::StringToString);
     store.register_late_pass(|| box zero_sized_map_values::ZeroSizedMapValues);
+    store.register_late_pass(|| box case_sensitive_file_extensions::CaseSensitiveFileExtensions);
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1955,6 +1958,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&missing_const_for_fn::MISSING_CONST_FOR_FN),
         LintId::of(&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL),
         LintId::of(&mutex_atomic::MUTEX_INTEGER),
+        LintId::of(&case_sensitive_file_extensions::CASE_SENSITIVE_FILE_EXTENSIONS),
         LintId::of(&needless_borrow::NEEDLESS_BORROW),
         LintId::of(&path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
         LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE),

tests/ui/case_sensitive_file_extensions.rs

@@ -0,0 +1,44 @@
+#![warn(clippy::case_sensitive_file_extensions)]
+
+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");
+}

tests/ui/case_sensitive_file_extensions.stderr

@@ -0,0 +1,43 @@
+error: case-sensitive file extension comparison
+  --> $DIR/case_sensitive_file_extensions.rs:12:14
+   |
+LL |     filename.ends_with(".rs")
+   |              ^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::case-sensitive-file-extensions` implied by `-D warnings`
+   = help: consider using a case-insensitive comparison instead
+
+error: case-sensitive file extension comparison
+  --> $DIR/case_sensitive_file_extensions.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_extensions.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_extensions.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_extensions.rs:25:19
+   |
+LL |     let _ = "str".ends_with(".EXT12");
+   |                   ^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using a case-insensitive comparison instead
+
+error: aborting due to 5 previous errors
+