Auto merge of rust-lang#10107 - tylerjw:suggest_path, r=Alexendoo

Suggest using Path for comparing extensions

fixes rust-lang#10042

changelog: Sugg: [`case_sensitive_file_extension_comparisons`]: Now displays a suggestion with `Path`
[rust-lang#10107](rust-lang/rust-clippy#10107)
<!-- changelog_checked -->

Author: bors

clippy_lints/src/methods/case_sensitive_file_extension_comparisons.rs

@@ -1,7 +1,10 @@
-use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::snippet_opt;
+use clippy_utils::source::{indent_of, reindent_multiline};
 use clippy_utils::ty::is_type_lang_item;
 use if_chain::if_chain;
 use rustc_ast::ast::LitKind;
+use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind, LangItem};
 use rustc_lint::LateContext;
 use rustc_span::{source_map::Spanned, Span};
@@ -15,6 +18,15 @@ pub(super) fn check<'tcx>(
     recv: &'tcx Expr<'_>,
     arg: &'tcx Expr<'_>,
 ) {
+    if let ExprKind::MethodCall(path_segment, ..) = recv.kind {
+        if matches!(
+            path_segment.ident.name.as_str(),
+            "to_lowercase" | "to_uppercase" | "to_ascii_lowercase" | "to_ascii_uppercase"
+        ) {
+            return;
+        }
+    }
+
     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);
@@ -28,13 +40,37 @@ pub(super) fn check<'tcx>(
         let recv_ty = cx.typeck_results().expr_ty(recv).peel_refs();
         if recv_ty.is_str() || is_type_lang_item(cx, recv_ty, LangItem::String);
         then {
-            span_lint_and_help(
+            span_lint_and_then(
                 cx,
                 CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
-                call_span,
+                recv.span.to(call_span),
                 "case-sensitive file extension comparison",
-                None,
-                "consider using a case-insensitive comparison instead",
+                |diag| {
+                    diag.help("consider using a case-insensitive comparison instead");
+                    if let Some(mut recv_source) = snippet_opt(cx, recv.span) {
+
+                        if !cx.typeck_results().expr_ty(recv).is_ref() {
+                            recv_source = format!("&{recv_source}");
+                        }
+
+                        let suggestion_source = reindent_multiline(
+                            format!(
+                                "std::path::Path::new({})
+                                    .extension()
+                                    .map_or(false, |ext| ext.eq_ignore_ascii_case(\"{}\"))",
+                                recv_source, ext_str.strip_prefix('.').unwrap()).into(),
+                            true,
+                            Some(indent_of(cx, call_span).unwrap_or(0) + 4)
+                        );
+
+                        diag.span_suggestion(
+                            recv.span.to(call_span),
+                            "use std::path::Path",
+                            suggestion_source,
+                            Applicability::MaybeIncorrect,
+                        );
+                    }
+                }
             );
         }
     }

tests/ui/case_sensitive_file_extension_comparisons.fixed

@@ -0,0 +1,67 @@
+// run-rustfix
+#![warn(clippy::case_sensitive_file_extension_comparisons)]
+
+use std::string::String;
+
+struct TestStruct;
+
+impl TestStruct {
+    fn ends_with(self, _arg: &str) {}
+}
+
+#[allow(dead_code)]
+fn is_rust_file(filename: &str) -> bool {
+    std::path::Path::new(filename)
+        .extension()
+        .map_or(false, |ext| ext.eq_ignore_ascii_case("rs"))
+}
+
+fn main() {
+    // std::string::String and &str should trigger the lint failure with .ext12
+    let _ = std::path::Path::new(&String::new())
+        .extension()
+        .map_or(false, |ext| ext.eq_ignore_ascii_case("ext12"));
+    let _ = std::path::Path::new("str")
+        .extension()
+        .map_or(false, |ext| ext.eq_ignore_ascii_case("ext12"));
+
+    // The fixup should preserve the indentation level
+    {
+        let _ = std::path::Path::new("str")
+            .extension()
+            .map_or(false, |ext| ext.eq_ignore_ascii_case("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 _ = std::path::Path::new(&String::new())
+        .extension()
+        .map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
+    let _ = std::path::Path::new("str")
+        .extension()
+        .map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
+
+    // Should not trigger the lint failure because of the calls to to_lowercase and to_uppercase
+    let _ = String::new().to_lowercase().ends_with(".EXT12");
+    let _ = String::new().to_uppercase().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::new().ends_with(".eXT12");
+    let _ = "str".ends_with(".eXT12");
+    TestStruct {}.ends_with(".eXT12");
+
+    // Should not trigger the lint failure with .EXT123 (too long)
+    let _ = String::new().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::new().ends_with("a.ext");
+    let _ = "str".ends_with("a.extA");
+    TestStruct {}.ends_with("a.ext");
+}

tests/ui/case_sensitive_file_extension_comparisons.rs

@@ -1,13 +1,15 @@
+// run-rustfix
 #![warn(clippy::case_sensitive_file_extension_comparisons)]
 
 use std::string::String;
 
 struct TestStruct;
 
 impl TestStruct {
-    fn ends_with(self, arg: &str) {}
+    fn ends_with(self, _arg: &str) {}
 }
 
+#[allow(dead_code)]
 fn is_rust_file(filename: &str) -> bool {
     filename.ends_with(".rs")
 }
@@ -17,13 +19,22 @@ fn main() {
     let _ = String::new().ends_with(".ext12");
     let _ = "str".ends_with(".ext12");
 
+    // The fixup should preserve the indentation level
+    {
+        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::new().ends_with(".EXT12");
     let _ = "str".ends_with(".EXT12");
 
+    // Should not trigger the lint failure because of the calls to to_lowercase and to_uppercase
+    let _ = String::new().to_lowercase().ends_with(".EXT12");
+    let _ = String::new().to_uppercase().ends_with(".EXT12");
+
     // The test struct should not trigger the lint failure with .EXT12
     TestStruct {}.ends_with(".EXT12");
 

tests/ui/case_sensitive_file_extension_comparisons.stderr

@@ -1,43 +1,87 @@
 error: case-sensitive file extension comparison
-  --> $DIR/case_sensitive_file_extension_comparisons.rs:12:14
+  --> $DIR/case_sensitive_file_extension_comparisons.rs:14:5
    |
 LL |     filename.ends_with(".rs")
-   |              ^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using a case-insensitive comparison instead
    = note: `-D clippy::case-sensitive-file-extension-comparisons` implied by `-D warnings`
+help: use std::path::Path
+   |
+LL ~     std::path::Path::new(filename)
+LL +         .extension()
+LL +         .map_or(false, |ext| ext.eq_ignore_ascii_case("rs"))
+   |
 
 error: case-sensitive file extension comparison
-  --> $DIR/case_sensitive_file_extension_comparisons.rs:17:27
+  --> $DIR/case_sensitive_file_extension_comparisons.rs:19:13
    |
 LL |     let _ = String::new().ends_with(".ext12");
-   |                           ^^^^^^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using a case-insensitive comparison instead
+help: use std::path::Path
+   |
+LL ~     let _ = std::path::Path::new(&String::new())
+LL +         .extension()
+LL ~         .map_or(false, |ext| ext.eq_ignore_ascii_case("ext12"));
+   |
 
 error: case-sensitive file extension comparison
-  --> $DIR/case_sensitive_file_extension_comparisons.rs:18:19
+  --> $DIR/case_sensitive_file_extension_comparisons.rs:20:13
    |
 LL |     let _ = "str".ends_with(".ext12");
-   |                   ^^^^^^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using a case-insensitive comparison instead
+help: use std::path::Path
+   |
+LL ~     let _ = std::path::Path::new("str")
+LL +         .extension()
+LL ~         .map_or(false, |ext| ext.eq_ignore_ascii_case("ext12"));
+   |
 
 error: case-sensitive file extension comparison
-  --> $DIR/case_sensitive_file_extension_comparisons.rs:24:27
+  --> $DIR/case_sensitive_file_extension_comparisons.rs:24:17
+   |
+LL |         let _ = "str".ends_with(".ext12");
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using a case-insensitive comparison instead
+help: use std::path::Path
+   |
+LL ~         let _ = std::path::Path::new("str")
+LL +             .extension()
+LL ~             .map_or(false, |ext| ext.eq_ignore_ascii_case("ext12"));
+   |
+
+error: case-sensitive file extension comparison
+  --> $DIR/case_sensitive_file_extension_comparisons.rs:31:13
    |
 LL |     let _ = String::new().ends_with(".EXT12");
-   |                           ^^^^^^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using a case-insensitive comparison instead
+help: use std::path::Path
+   |
+LL ~     let _ = std::path::Path::new(&String::new())
+LL +         .extension()
+LL ~         .map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
+   |
 
 error: case-sensitive file extension comparison
-  --> $DIR/case_sensitive_file_extension_comparisons.rs:25:19
+  --> $DIR/case_sensitive_file_extension_comparisons.rs:32:13
    |
 LL |     let _ = "str".ends_with(".EXT12");
-   |                   ^^^^^^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using a case-insensitive comparison instead
+help: use std::path::Path
+   |
+LL ~     let _ = std::path::Path::new("str")
+LL +         .extension()
+LL ~         .map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
+   |
 
-error: aborting due to 5 previous errors
+error: aborting due to 6 previous errors