Auto merge of #6500 - Javier-varez:case_sensitive_file_extensions, r=llogiq

Case sensitive file extensions

Closes #6425

Looks for ends_with methods calls with case sensitive extension comparisons.

changelog: Add new lint that warns about case-sensitive file extension comparisons.

CHANGELOG.md

@@ -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

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_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,
+    "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> {
+    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",
+            );
+        }
+    }
+}

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_extension_comparisons;
 mod checked_conversions;
 mod cognitive_complexity;
 mod collapsible_if;
@@ -558,6 +559,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,
@@ -1226,6 +1228,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),
@@ -1283,6 +1286,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),

tests/compile-test.rs

@@ -44,7 +44,9 @@ fn third_party_crates() -> String {
         };
         if let Some(name) = path.file_name().and_then(OsStr::to_str) {
             for dep in CRATES {
-                if name.starts_with(&format!("lib{}-", dep)) && name.ends_with(".rlib") {
+                if name.starts_with(&format!("lib{}-", dep))
+                    && name.rsplit('.').next().map(|ext| ext.eq_ignore_ascii_case("rlib")) == Some(true)
+                {
                     if let Some(old) = crates.insert(dep, path.clone()) {
                         panic!("Found multiple rlibs for crate `{}`: `{:?}` and `{:?}", dep, old, path);
                     }

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");
+}

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
+