Skip to content

Commit 928501e

Browse files
atwamatwam
committed
Add suggestion/fix to suspicious_open_options
Also rebase and fix conflicts
1 parent 8e4475c commit 928501e

File tree

6 files changed

+56
-52
lines changed

6 files changed

+56
-52
lines changed

‎clippy_lints/src/methods/open_options.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,24 @@ use rustc_data_structures::fx::FxHashMap;
22

33
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
44
use clippy_utils::paths;
5-
use clippy_utils::ty::is_type_diagnostic_item;
5+
use clippy_utils::ty::{is_type_diagnostic_item, match_type};
66
use rustc_ast::ast::LitKind;
77
use rustc_hir::{Expr, ExprKind};
88
use rustc_lint::LateContext;
9+
use rustc_middle::ty::Ty;
910
use rustc_span::source_map::{Span, Spanned};
1011
use rustc_span::sym;
1112

1213
use super::{NONSENSICAL_OPEN_OPTIONS, SUSPICIOUS_OPEN_OPTIONS};
1314

15+
fn is_open_options(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
16+
is_type_diagnostic_item(cx, ty, sym::FsOpenOptions) || match_type(cx, ty, &paths::TOKIO_IO_OPEN_OPTIONS)
17+
}
18+
1419
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) {
1520
if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
1621
&& let Some(impl_id) = cx.tcx.impl_of_method(method_id)
17-
&& (
18-
is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id).instantiate_identity(), sym::FsOpenOptions) ||
19-
match_type(cx, cx.tcx.type_of(impl_id).instantiate_identity(), &paths::TOKIO_IO_OPEN_OPTIONS)
20-
)
21-
22-
{
22+
&& is_open_options(cx, cx.tcx.type_of(impl_id).instantiate_identity()) {
2323
let mut options = Vec::new();
2424
get_open_options(cx, recv, &mut options);
2525
check_open_options(cx, &options, e.span);
@@ -59,7 +59,7 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec
5959
let obj_ty = cx.typeck_results().expr_ty(receiver).peel_refs();
6060

6161
// Only proceed if this is a call on some object of type std::fs::OpenOptions
62-
if !arguments.is_empty() && (is_type_diagnostic_item(cx, obj_ty, sym::FsOpenOptions)) {
62+
if !arguments.is_empty() && is_open_options(cx, obj_ty) {
6363
let argument_option = match arguments[0].kind {
6464
ExprKind::Lit(span) => {
6565
if let Spanned {
@@ -144,13 +144,16 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, S
144144
if let Some((Argument::Set(true), create_span)) = options.get(&OpenOption::Create)
145145
&& let None = options.get(&OpenOption::Truncate)
146146
{
147-
span_lint_and_help(
147+
span_lint_and_then(
148148
cx,
149149
SUSPICIOUS_OPEN_OPTIONS,
150150
*create_span,
151151
"file opened with `create`, but `truncate` behavior not defined",
152-
Some(span),
153-
"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)`"
152+
|diag| {
153+
diag
154+
.span_suggestion(create_span.shrink_to_hi(), "add", ".truncate(true)".to_string(), rustc_errors::Applicability::MaybeIncorrect)
155+
.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)`");
156+
},
154157
);
155158
}
156159
}

‎tests/ui/open_options.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use std::fs::OpenOptions;
22
#[allow(unused_must_use)]
33
#[warn(clippy::nonsensical_open_options)]
4-
#[warn(clippy::suspicious_open_options)]
54
fn main() {
65
OpenOptions::new().read(true).truncate(true).open("foo.txt");
76
//~^ ERROR: file opened with `truncate` and `read`
@@ -11,14 +10,16 @@ fn main() {
1110

1211
OpenOptions::new().read(true).read(false).open("foo.txt");
1312
//~^ ERROR: the method `read` is called more than once
14-
OpenOptions::new().create(true).create(false).open("foo.txt");
15-
//~^ ERROR: the method `create` is called more than once
13+
OpenOptions::new()
14+
.create(true)
15+
.truncate(true) // Ensure we don't trigger suspicious open options by having create without truncate
16+
.create(false)
17+
//~^ ERROR: the method `create` is called more than once
18+
.open("foo.txt");
1619
OpenOptions::new().write(true).write(false).open("foo.txt");
1720
//~^ ERROR: the method `write` is called more than once
1821
OpenOptions::new().append(true).append(false).open("foo.txt");
1922
//~^ ERROR: the method `append` is called more than once
2023
OpenOptions::new().truncate(true).truncate(false).open("foo.txt");
2124
//~^ ERROR: the method `truncate` is called more than once
22-
OpenOptions::new().create(true).open("foo.txt");
23-
//~^ ERROR: file opened with `create`, but `truncate` behavior not defined
2425
}

‎tests/ui/open_options.stderr

Lines changed: 10 additions & 36 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:6:5
2+
--> $DIR/open_options.rs:5:5
33
|
44
LL | OpenOptions::new().read(true).truncate(true).open("foo.txt");
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -8,66 +8,40 @@ 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:9:5
11+
--> $DIR/open_options.rs:8: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:12:35
17+
--> $DIR/open_options.rs:11:35
1818
|
1919
LL | OpenOptions::new().read(true).read(false).open("foo.txt");
2020
| ^^^^^^^^^^^
2121

2222
error: the method `create` is called more than once
23-
--> $DIR/open_options.rs:14:37
23+
--> $DIR/open_options.rs:16:10
2424
|
25-
LL | OpenOptions::new().create(true).create(false).open("foo.txt");
26-
| ^^^^^^^^^^^^^
27-
28-
error: file opened with `create`, but `truncate` behavior not defined
29-
--> $DIR/open_options.rs:14:24
30-
|
31-
LL | OpenOptions::new().create(true).create(false).open("foo.txt");
32-
| ^^^^^^^^^^^^
33-
|
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)`
35-
--> $DIR/open_options.rs:14:5
36-
|
37-
LL | OpenOptions::new().create(true).create(false).open("foo.txt");
38-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
39-
= note: `-D clippy::suspicious-open-options` implied by `-D warnings`
40-
= help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]`
25+
LL | .create(false)
26+
| ^^^^^^^^^^^^^
4127

4228
error: the method `write` is called more than once
43-
--> $DIR/open_options.rs:16:36
29+
--> $DIR/open_options.rs:19:36
4430
|
4531
LL | OpenOptions::new().write(true).write(false).open("foo.txt");
4632
| ^^^^^^^^^^^^
4733

4834
error: the method `append` is called more than once
49-
--> $DIR/open_options.rs:18:37
35+
--> $DIR/open_options.rs:21:37
5036
|
5137
LL | OpenOptions::new().append(true).append(false).open("foo.txt");
5238
| ^^^^^^^^^^^^^
5339

5440
error: the method `truncate` is called more than once
55-
--> $DIR/open_options.rs:20:39
41+
--> $DIR/open_options.rs:23:39
5642
|
5743
LL | OpenOptions::new().truncate(true).truncate(false).open("foo.txt");
5844
| ^^^^^^^^^^^^^^^
5945

60-
error: file opened with `create`, but `truncate` behavior not defined
61-
--> $DIR/open_options.rs:22:24
62-
|
63-
LL | OpenOptions::new().create(true).open("foo.txt");
64-
| ^^^^^^^^^^^^
65-
|
66-
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)`
67-
--> $DIR/open_options.rs:22:5
68-
|
69-
LL | OpenOptions::new().create(true).open("foo.txt");
70-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
71-
72-
error: aborting due to 9 previous errors
46+
error: aborting due to 7 previous errors
7347

‎tests/ui/open_options_fixable.fixed

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
use std::fs::OpenOptions;
2+
#[allow(unused_must_use)]
3+
#[warn(clippy::suspicious_open_options)]
4+
fn main() {
5+
OpenOptions::new().create(true).truncate(true).open("foo.txt");
6+
//~^ ERROR: file opened with `create`, but `truncate` behavior not defined
7+
}

‎tests/ui/open_options_fixable.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
use std::fs::OpenOptions;
2+
#[allow(unused_must_use)]
3+
#[warn(clippy::suspicious_open_options)]
4+
fn main() {
5+
OpenOptions::new().create(true).open("foo.txt");
6+
//~^ ERROR: file opened with `create`, but `truncate` behavior not defined
7+
}

‎tests/ui/open_options_fixable.stderr

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
error: file opened with `create`, but `truncate` behavior not defined
2+
--> $DIR/open_options_fixable.rs:5:24
3+
|
4+
LL | OpenOptions::new().create(true).open("foo.txt");
5+
| ^^^^^^^^^^^^- help: add: `.truncate(true)`
6+
|
7+
= 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)`
8+
= note: `-D clippy::suspicious-open-options` implied by `-D warnings`
9+
= help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]`
10+
11+
error: aborting due to previous error
12+

0 commit comments

Comments
 (0)