Skip to content

Commit 2c79c4d

Browse files
tylerjwtylerjw
committed
Suggest using Path for comparing extensions
Signed-off-by: Tyler Weaver <[email protected]>
1 parent 4a09068 commit 2c79c4d

File tree

4 files changed

+175
-17
lines changed

4 files changed

+175
-17
lines changed

‎clippy_lints/src/methods/case_sensitive_file_extension_comparisons.rs

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
use clippy_utils::diagnostics::span_lint_and_help;
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::sugg::Sugg;
23
use clippy_utils::ty::is_type_lang_item;
34
use if_chain::if_chain;
45
use rustc_ast::ast::LitKind;
6+
use rustc_errors::Applicability;
57
use rustc_hir::{Expr, ExprKind, LangItem};
68
use rustc_lint::LateContext;
79
use rustc_span::{source_map::Spanned, Span};
@@ -28,13 +30,39 @@ pub(super) fn check<'tcx>(
2830
let recv_ty = cx.typeck_results().expr_ty(recv).peel_refs();
2931
if recv_ty.is_str() || is_type_lang_item(cx, recv_ty, LangItem::String);
3032
then {
31-
span_lint_and_help(
33+
span_lint_and_then(
3234
cx,
3335
CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
34-
call_span,
36+
recv.span.to(call_span),
3537
"case-sensitive file extension comparison",
36-
None,
37-
"consider using a case-insensitive comparison instead",
38+
|diag| {
39+
diag.help("consider using a case-insensitive comparison instead");
40+
let mut recv_str = Sugg::hir(cx, recv, "").to_string();
41+
42+
if is_type_lang_item(cx, recv_ty, LangItem::String) {
43+
recv_str = format!("&{recv_str}");
44+
}
45+
46+
if recv_str.contains(".to_lowercase()") {
47+
diag.note("to_lowercase allocates memory, this can be avoided by using Path");
48+
recv_str = recv_str.replace(".to_lowercase()", "");
49+
}
50+
51+
if recv_str.contains(".to_uppercase()") {
52+
diag.note("to_uppercase allocates memory, this can be avoided by using Path");
53+
recv_str = recv_str.replace(".to_uppercase()", "");
54+
}
55+
56+
diag.span_suggestion(
57+
recv.span.to(call_span),
58+
"use std::path::Path",
59+
format!("std::path::Path::new({})
60+
.extension()
61+
.map_or(false, |ext| ext.eq_ignore_ascii_case(\"{}\"))",
62+
recv_str, ext_str.strip_prefix('.').unwrap()),
63+
Applicability::MaybeIncorrect,
64+
);
65+
}
3866
);
3967
}
4068
}

‎tests/ui/case_sensitive_file_extension_comparisons.fixed

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// run-rustfix
2+
#![warn(clippy::case_sensitive_file_extension_comparisons)]
3+
4+
use std::string::String;
5+
6+
struct TestStruct;
7+
8+
impl TestStruct {
9+
fn ends_with(self, _arg: &str) {}
10+
}
11+
12+
#[allow(dead_code)]
13+
fn is_rust_file(filename: &str) -> bool {
14+
std::path::Path::new(filename)
15+
.extension()
16+
.map_or(false, |ext| ext.eq_ignore_ascii_case("rs"))
17+
}
18+
19+
fn main() {
20+
// std::string::String and &str should trigger the lint failure with .ext12
21+
let _ = std::path::Path::new(&String::new())
22+
.extension()
23+
.map_or(false, |ext| ext.eq_ignore_ascii_case("ext12"));
24+
let _ = std::path::Path::new("str")
25+
.extension()
26+
.map_or(false, |ext| ext.eq_ignore_ascii_case("ext12"));
27+
28+
// The test struct should not trigger the lint failure with .ext12
29+
TestStruct {}.ends_with(".ext12");
30+
31+
// std::string::String and &str should trigger the lint failure with .EXT12
32+
let _ = std::path::Path::new(&String::new())
33+
.extension()
34+
.map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
35+
let _ = std::path::Path::new("str")
36+
.extension()
37+
.map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
38+
39+
// This should print a note about how to_lowercase and to_uppercase allocates
40+
let _ = std::path::Path::new(&String::new())
41+
.extension()
42+
.map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
43+
let _ = std::path::Path::new(&String::new())
44+
.extension()
45+
.map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
46+
47+
// The test struct should not trigger the lint failure with .EXT12
48+
TestStruct {}.ends_with(".EXT12");
49+
50+
// Should not trigger the lint failure with .eXT12
51+
let _ = String::new().ends_with(".eXT12");
52+
let _ = "str".ends_with(".eXT12");
53+
TestStruct {}.ends_with(".eXT12");
54+
55+
// Should not trigger the lint failure with .EXT123 (too long)
56+
let _ = String::new().ends_with(".EXT123");
57+
let _ = "str".ends_with(".EXT123");
58+
TestStruct {}.ends_with(".EXT123");
59+
60+
// Shouldn't fail if it doesn't start with a dot
61+
let _ = String::new().ends_with("a.ext");
62+
let _ = "str".ends_with("a.extA");
63+
TestStruct {}.ends_with("a.ext");
64+
}

‎tests/ui/case_sensitive_file_extension_comparisons.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
1+
// run-rustfix
12
#![warn(clippy::case_sensitive_file_extension_comparisons)]
23

34
use std::string::String;
45

56
struct TestStruct;
67

78
impl TestStruct {
8-
fn ends_with(self, arg: &str) {}
9+
fn ends_with(self, _arg: &str) {}
910
}
1011

12+
#[allow(dead_code)]
1113
fn is_rust_file(filename: &str) -> bool {
1214
filename.ends_with(".rs")
1315
}
@@ -24,6 +26,10 @@ fn main() {
2426
let _ = String::new().ends_with(".EXT12");
2527
let _ = "str".ends_with(".EXT12");
2628

29+
// This should print a note about how to_lowercase and to_uppercase allocates
30+
let _ = String::new().to_lowercase().ends_with(".EXT12");
31+
let _ = String::new().to_uppercase().ends_with(".EXT12");
32+
2733
// The test struct should not trigger the lint failure with .EXT12
2834
TestStruct {}.ends_with(".EXT12");
2935

‎tests/ui/case_sensitive_file_extension_comparisons.stderr

Lines changed: 71 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,103 @@
11
error: case-sensitive file extension comparison
2-
--> $DIR/case_sensitive_file_extension_comparisons.rs:12:14
2+
--> $DIR/case_sensitive_file_extension_comparisons.rs:14:5
33
|
44
LL | filename.ends_with(".rs")
5-
| ^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= help: consider using a case-insensitive comparison instead
88
= note: `-D clippy::case-sensitive-file-extension-comparisons` implied by `-D warnings`
9+
help: use std::path::Path
10+
|
11+
LL ~ std::path::Path::new(filename)
12+
LL + .extension()
13+
LL + .map_or(false, |ext| ext.eq_ignore_ascii_case("rs"))
14+
|
915

1016
error: case-sensitive file extension comparison
11-
--> $DIR/case_sensitive_file_extension_comparisons.rs:17:27
17+
--> $DIR/case_sensitive_file_extension_comparisons.rs:19:13
1218
|
1319
LL | let _ = String::new().ends_with(".ext12");
14-
| ^^^^^^^^^^^^^^^^^^^
20+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1521
|
1622
= help: consider using a case-insensitive comparison instead
23+
help: use std::path::Path
24+
|
25+
LL ~ let _ = std::path::Path::new(&String::new())
26+
LL + .extension()
27+
LL ~ .map_or(false, |ext| ext.eq_ignore_ascii_case("ext12"));
28+
|
1729

1830
error: case-sensitive file extension comparison
19-
--> $DIR/case_sensitive_file_extension_comparisons.rs:18:19
31+
--> $DIR/case_sensitive_file_extension_comparisons.rs:20:13
2032
|
2133
LL | let _ = "str".ends_with(".ext12");
22-
| ^^^^^^^^^^^^^^^^^^^
34+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
2335
|
2436
= help: consider using a case-insensitive comparison instead
37+
help: use std::path::Path
38+
|
39+
LL ~ let _ = std::path::Path::new("str")
40+
LL + .extension()
41+
LL ~ .map_or(false, |ext| ext.eq_ignore_ascii_case("ext12"));
42+
|
2543

2644
error: case-sensitive file extension comparison
27-
--> $DIR/case_sensitive_file_extension_comparisons.rs:24:27
45+
--> $DIR/case_sensitive_file_extension_comparisons.rs:26:13
2846
|
2947
LL | let _ = String::new().ends_with(".EXT12");
30-
| ^^^^^^^^^^^^^^^^^^^
48+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3149
|
3250
= help: consider using a case-insensitive comparison instead
51+
help: use std::path::Path
52+
|
53+
LL ~ let _ = std::path::Path::new(&String::new())
54+
LL + .extension()
55+
LL ~ .map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
56+
|
3357

3458
error: case-sensitive file extension comparison
35-
--> $DIR/case_sensitive_file_extension_comparisons.rs:25:19
59+
--> $DIR/case_sensitive_file_extension_comparisons.rs:27:13
3660
|
3761
LL | let _ = "str".ends_with(".EXT12");
38-
| ^^^^^^^^^^^^^^^^^^^
62+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
63+
|
64+
= help: consider using a case-insensitive comparison instead
65+
help: use std::path::Path
66+
|
67+
LL ~ let _ = std::path::Path::new("str")
68+
LL + .extension()
69+
LL ~ .map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
70+
|
71+
72+
error: case-sensitive file extension comparison
73+
--> $DIR/case_sensitive_file_extension_comparisons.rs:30:13
74+
|
75+
LL | let _ = String::new().to_lowercase().ends_with(".EXT12");
76+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
77+
|
78+
= help: consider using a case-insensitive comparison instead
79+
= note: to_lowercase allocates memory, this can be avoided by using Path
80+
help: use std::path::Path
81+
|
82+
LL ~ let _ = std::path::Path::new(&String::new())
83+
LL + .extension()
84+
LL ~ .map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
85+
|
86+
87+
error: case-sensitive file extension comparison
88+
--> $DIR/case_sensitive_file_extension_comparisons.rs:31:13
89+
|
90+
LL | let _ = String::new().to_uppercase().ends_with(".EXT12");
91+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3992
|
4093
= help: consider using a case-insensitive comparison instead
94+
= note: to_uppercase allocates memory, this can be avoided by using Path
95+
help: use std::path::Path
96+
|
97+
LL ~ let _ = std::path::Path::new(&String::new())
98+
LL + .extension()
99+
LL ~ .map_or(false, |ext| ext.eq_ignore_ascii_case("EXT12"));
100+
|
41101

42-
error: aborting due to 5 previous errors
102+
error: aborting due to 7 previous errors
43103

0 commit comments

Comments
 (0)