Skip to content

Commit 3940e9a

Browse files
atwamatwam
committed
More helpful text, small style changes.
1 parent 5161394 commit 3940e9a

File tree

5 files changed

+75
-51
lines changed

5 files changed

+75
-51
lines changed

‎clippy_lints/src/methods/mod.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2820,8 +2820,8 @@ declare_clippy_lint! {
28202820

28212821
declare_clippy_lint! {
28222822
/// ### What it does
2823-
/// Checks for the suspicious use of OpenOptions::create()
2824-
/// without an explicit OpenOptions::truncate().
2823+
/// Checks for the suspicious use of `OpenOptions::create()`
2824+
/// without an explicit `OpenOptions::truncate()`.
28252825
///
28262826
/// ### Why is this bad?
28272827
/// create() alone will either create a new file or open an
@@ -2841,9 +2841,11 @@ declare_clippy_lint! {
28412841
/// ```rust
28422842
/// use std::fs::OpenOptions;
28432843
///
2844+
/// OpenOptions::new().create(true);
2845+
///
28442846
/// OpenOptions::new().create(true).truncate(true);
28452847
/// ```
2846-
#[clippy::version = "1.73.0"]
2848+
#[clippy::version = "1.75.0"]
28472849
pub SUSPICIOUS_OPEN_OPTIONS,
28482850
suspicious,
28492851
"suspicious combination of options for opening a file"

‎clippy_lints/src/methods/open_options.rs

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use rustc_data_structures::fx::FxHashMap;
22

3-
use clippy_utils::diagnostics::span_lint;
3+
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
44
use clippy_utils::paths;
55
use clippy_utils::ty::match_type;
66
use rustc_ast::ast::LitKind;
@@ -13,7 +13,11 @@ use super::{NONSENSICAL_OPEN_OPTIONS, SUSPICIOUS_OPEN_OPTIONS};
1313
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) {
1414
if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
1515
&& let Some(impl_id) = cx.tcx.impl_of_method(method_id)
16-
&& match_type(cx, cx.tcx.type_of(impl_id).instantiate_identity(), &paths::OPEN_OPTIONS)
16+
&& (
17+
match_type(cx, cx.tcx.type_of(impl_id).instantiate_identity(), &paths::OPEN_OPTIONS) ||
18+
match_type(cx, cx.tcx.type_of(impl_id).instantiate_identity(), &paths::TOKIO_IO_OPEN_OPTIONS)
19+
)
20+
1721
{
1822
let mut options = Vec::new();
1923
get_open_options(cx, recv, &mut options);
@@ -49,12 +53,15 @@ impl std::fmt::Display for OpenOption {
4953
}
5054
}
5155

52-
fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec<(OpenOption, Argument)>) {
53-
if let ExprKind::MethodCall(path, receiver, arguments, _) = argument.kind {
56+
fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec<(OpenOption, Argument, Span)>) {
57+
if let ExprKind::MethodCall(path, receiver, arguments, span) = argument.kind {
5458
let obj_ty = cx.typeck_results().expr_ty(receiver).peel_refs();
5559

56-
// Only proceed if this is a call on some object of type std::fs::OpenOptions
57-
if match_type(cx, obj_ty, &paths::OPEN_OPTIONS) && !arguments.is_empty() {
60+
// Only proceed if this is a call on some object of type std::fs::OpenOptions or
61+
// tokio::fs::OpenOptions
62+
if !arguments.is_empty()
63+
&& (match_type(cx, obj_ty, &paths::OPEN_OPTIONS) || match_type(cx, obj_ty, &paths::TOKIO_IO_OPEN_OPTIONS))
64+
{
5865
let argument_option = match arguments[0].kind {
5966
ExprKind::Lit(span) => {
6067
if let Spanned {
@@ -74,22 +81,22 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec
7481

7582
match path.ident.as_str() {
7683
"create" => {
77-
options.push((OpenOption::Create, argument_option));
84+
options.push((OpenOption::Create, argument_option, span));
7885
},
7986
"create_new" => {
80-
options.push((OpenOption::CreateNew, argument_option));
87+
options.push((OpenOption::CreateNew, argument_option, span));
8188
},
8289
"append" => {
83-
options.push((OpenOption::Append, argument_option));
90+
options.push((OpenOption::Append, argument_option, span));
8491
},
8592
"truncate" => {
86-
options.push((OpenOption::Truncate, argument_option));
93+
options.push((OpenOption::Truncate, argument_option, span));
8794
},
8895
"read" => {
89-
options.push((OpenOption::Read, argument_option));
96+
options.push((OpenOption::Read, argument_option, span));
9097
},
9198
"write" => {
92-
options.push((OpenOption::Write, argument_option));
99+
options.push((OpenOption::Write, argument_option, span));
93100
},
94101
_ => (),
95102
}
@@ -99,24 +106,25 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec
99106
}
100107
}
101108

102-
fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument)], span: Span) {
109+
fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, Span)], span: Span) {
103110
// The args passed to these methods, if they have been called
104111
let mut options = FxHashMap::default();
105-
for (option, arg) in settings {
106-
if options.insert(option.clone(), arg.clone()).is_some() {
112+
for (option, arg, sp) in settings {
113+
if let Some((_, prev_span)) = options.insert(option.clone(), (arg.clone(), *sp)) {
107114
span_lint(
108115
cx,
109116
NONSENSICAL_OPEN_OPTIONS,
110-
span,
117+
prev_span,
111118
&format!("the method `{}` is called more than once", &option),
112119
);
113120
}
114121
}
115122

116-
if let (Some(Argument::Set(true)), Some(Argument::Set(true))) =
117-
(options.get(&OpenOption::Read), options.get(&OpenOption::Truncate))
118-
{
119-
if options.get(&OpenOption::Write).unwrap_or(&Argument::Set(false)) == &Argument::Set(false) {
123+
if_chain! {
124+
if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Read);
125+
if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate);
126+
if let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Write);
127+
then {
120128
span_lint(
121129
cx,
122130
NONSENSICAL_OPEN_OPTIONS,
@@ -126,10 +134,11 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument)],
126134
}
127135
}
128136

129-
if let (Some(Argument::Set(true)), Some(Argument::Set(true))) =
130-
(options.get(&OpenOption::Append), options.get(&OpenOption::Truncate))
131-
{
132-
if options.get(&OpenOption::Write).unwrap_or(&Argument::Set(false)) == &Argument::Set(false) {
137+
if_chain! {
138+
if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Append);
139+
if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate);
140+
if let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Write);
141+
then {
133142
span_lint(
134143
cx,
135144
NONSENSICAL_OPEN_OPTIONS,
@@ -139,12 +148,21 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument)],
139148
}
140149
}
141150

142-
if let (Some(Argument::Set(true)), None) = (options.get(&OpenOption::Create), options.get(&OpenOption::Truncate)) {
143-
span_lint(
144-
cx,
145-
SUSPICIOUS_OPEN_OPTIONS,
146-
span,
147-
"file opened with `create`, but `truncate` behavior not defined",
148-
);
151+
if_chain! {
152+
if let Some((Argument::Set(true), create_span)) = options.get(&OpenOption::Create);
153+
if let None = options.get(&OpenOption::Truncate);
154+
then {
155+
span_lint_and_then(
156+
cx,
157+
SUSPICIOUS_OPEN_OPTIONS,
158+
*create_span,
159+
"file opened with `create`, but `truncate` behavior not defined",
160+
|diag| {
161+
diag
162+
//.span_suggestion(create_span.shrink_to_hi(), "add", ".truncate(true)".to_string(), rustc_errors::Applicability::MaybeIncorrect)
163+
.help("if you intend to overwrite an existing file entirely, call `.truncate(true)`. if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`");
164+
},
165+
);
166+
}
149167
}
150168
}

‎clippy_utils/src/paths.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ pub const MEM_SWAP: [&str; 3] = ["core", "mem", "swap"];
6363
#[cfg(feature = "internal")]
6464
pub const MSRV: [&str; 3] = ["clippy_utils", "msrvs", "Msrv"];
6565
pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"];
66+
#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates
67+
pub const TOKIO_IO_OPEN_OPTIONS: [&str; 3] = ["tokio", "fs", "OpenOptions"];
6668
pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", "as_os_str"];
6769
pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"];
6870
pub const PARKING_LOT_MUTEX_GUARD: [&str; 3] = ["lock_api", "mutex", "MutexGuard"];

‎tests/ui/open_options.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use std::fs::OpenOptions;
2-
32
#[allow(unused_must_use)]
43
#[warn(clippy::nonsensical_open_options)]
54
#[warn(clippy::suspicious_open_options)]

‎tests/ui/open_options.stderr

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: file opened with `truncate` and `read`
2-
--> $DIR/open_options.rs:7:5
2+
--> $DIR/open_options.rs:6:5
33
|
44
LL | OpenOptions::new().read(true).truncate(true).open("foo.txt");
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -8,55 +8,58 @@ LL | OpenOptions::new().read(true).truncate(true).open("foo.txt");
88
= help: to override `-D warnings` add `#[allow(clippy::nonsensical_open_options)]`
99

1010
error: file opened with `append` and `truncate`
11-
--> $DIR/open_options.rs:10:5
11+
--> $DIR/open_options.rs:9:5
1212
|
1313
LL | OpenOptions::new().append(true).truncate(true).open("foo.txt");
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1515

1616
error: the method `read` is called more than once
17-
--> $DIR/open_options.rs:13:5
17+
--> $DIR/open_options.rs:12:35
1818
|
1919
LL | OpenOptions::new().read(true).read(false).open("foo.txt");
20-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
20+
| ^^^^^^^^^^^
2121

2222
error: the method `create` is called more than once
23-
--> $DIR/open_options.rs:15:5
23+
--> $DIR/open_options.rs:14:37
2424
|
2525
LL | OpenOptions::new().create(true).create(false).open("foo.txt");
26-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
26+
| ^^^^^^^^^^^^^
2727

2828
error: file opened with `create`, but `truncate` behavior not defined
29-
--> $DIR/open_options.rs:15:5
29+
--> $DIR/open_options.rs:14:24
3030
|
3131
LL | OpenOptions::new().create(true).create(false).open("foo.txt");
32-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
32+
| ^^^^^^^^^^^^
3333
|
34+
= help: if you intend to overwrite an existing file entirely, call `.truncate(true)`. if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`
3435
= note: `-D clippy::suspicious-open-options` implied by `-D warnings`
3536
= help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]`
3637

3738
error: the method `write` is called more than once
38-
--> $DIR/open_options.rs:17:5
39+
--> $DIR/open_options.rs:16:36
3940
|
4041
LL | OpenOptions::new().write(true).write(false).open("foo.txt");
41-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
42+
| ^^^^^^^^^^^^
4243

4344
error: the method `append` is called more than once
44-
--> $DIR/open_options.rs:19:5
45+
--> $DIR/open_options.rs:18:37
4546
|
4647
LL | OpenOptions::new().append(true).append(false).open("foo.txt");
47-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
48+
| ^^^^^^^^^^^^^
4849

4950
error: the method `truncate` is called more than once
50-
--> $DIR/open_options.rs:21:5
51+
--> $DIR/open_options.rs:20:39
5152
|
5253
LL | OpenOptions::new().truncate(true).truncate(false).open("foo.txt");
53-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
54+
| ^^^^^^^^^^^^^^^
5455

5556
error: file opened with `create`, but `truncate` behavior not defined
56-
--> $DIR/open_options.rs:23:5
57+
--> $DIR/open_options.rs:22:24
5758
|
5859
LL | OpenOptions::new().create(true).open("foo.txt");
59-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
60+
| ^^^^^^^^^^^^
61+
|
62+
= help: if you intend to overwrite an existing file entirely, call `.truncate(true)`. if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`
6063

6164
error: aborting due to 9 previous errors
6265

0 commit comments

Comments
 (0)