Skip to content

Commit 6c65918

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

File tree

4 files changed

+63
-48
lines changed

4 files changed

+63
-48
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: 39 additions & 28 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;
@@ -49,8 +49,8 @@ impl std::fmt::Display for OpenOption {
4949
}
5050
}
5151

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

5656
// Only proceed if this is a call on some object of type std::fs::OpenOptions
@@ -74,22 +74,22 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec
7474

7575
match path.ident.as_str() {
7676
"create" => {
77-
options.push((OpenOption::Create, argument_option));
77+
options.push((OpenOption::Create, argument_option, span));
7878
},
7979
"create_new" => {
80-
options.push((OpenOption::CreateNew, argument_option));
80+
options.push((OpenOption::CreateNew, argument_option, span));
8181
},
8282
"append" => {
83-
options.push((OpenOption::Append, argument_option));
83+
options.push((OpenOption::Append, argument_option, span));
8484
},
8585
"truncate" => {
86-
options.push((OpenOption::Truncate, argument_option));
86+
options.push((OpenOption::Truncate, argument_option, span));
8787
},
8888
"read" => {
89-
options.push((OpenOption::Read, argument_option));
89+
options.push((OpenOption::Read, argument_option, span));
9090
},
9191
"write" => {
92-
options.push((OpenOption::Write, argument_option));
92+
options.push((OpenOption::Write, argument_option, span));
9393
},
9494
_ => (),
9595
}
@@ -99,24 +99,25 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec
9999
}
100100
}
101101

102-
fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument)], span: Span) {
102+
fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, Span)], span: Span) {
103103
// The args passed to these methods, if they have been called
104104
let mut options = FxHashMap::default();
105-
for (option, arg) in settings {
106-
if options.insert(option.clone(), arg.clone()).is_some() {
105+
for (option, arg, sp) in settings {
106+
if let Some((_, prev_span)) = options.insert(option.clone(), (arg.clone(), sp.clone())) {
107107
span_lint(
108108
cx,
109109
NONSENSICAL_OPEN_OPTIONS,
110-
span,
110+
prev_span,
111111
&format!("the method `{}` is called more than once", &option),
112112
);
113113
}
114114
}
115115

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) {
116+
if_chain! {
117+
if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Read);
118+
if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate);
119+
if let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Write);
120+
then {
120121
span_lint(
121122
cx,
122123
NONSENSICAL_OPEN_OPTIONS,
@@ -126,10 +127,11 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument)],
126127
}
127128
}
128129

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) {
130+
if_chain! {
131+
if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Append);
132+
if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate);
133+
if let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Write);
134+
then {
133135
span_lint(
134136
cx,
135137
NONSENSICAL_OPEN_OPTIONS,
@@ -139,12 +141,21 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument)],
139141
}
140142
}
141143

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-
);
144+
if_chain! {
145+
if let Some((Argument::Set(true), create_span)) = options.get(&OpenOption::Create);
146+
if let None = options.get(&OpenOption::Truncate);
147+
then {
148+
span_lint_and_then(
149+
cx,
150+
SUSPICIOUS_OPEN_OPTIONS,
151+
*create_span,
152+
"file opened with `create`, but `truncate` behavior not defined",
153+
|diag| {
154+
diag
155+
//.span_suggestion(create_span.shrink_to_hi(), "add", ".truncate(true)".to_string(), rustc_errors::Applicability::MaybeIncorrect)
156+
.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)`");
157+
},
158+
);
159+
}
149160
}
150161
}

‎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)