Skip to content

Commit 52deee2

Browse files
borsbors
committed
Auto merge of #11902 - GuillaumeGomez:write-and-append, r=llogiq
Add `ineffective_open_options` lint Fixes rust-lang/rust-clippy#9628. For `OpenOptions`, in case you call both `write(true)` and `append(true)` on `OpenOptions`, then `write(true)` is actually useless since `append(true)` does it. r? `@flip1995` changelog: Add `ineffective_open_options` lint
2 parents 2a1645d + ded94ec commit 52deee2

8 files changed

+199
-1
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5146,6 +5146,7 @@ Released 2018-09-13
51465146
[`index_refutable_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#index_refutable_slice
51475147
[`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing
51485148
[`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask
5149+
[`ineffective_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_open_options
51495150
[`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string
51505151
[`infallible_destructuring_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_destructuring_match
51515152
[`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
215215
crate::index_refutable_slice::INDEX_REFUTABLE_SLICE_INFO,
216216
crate::indexing_slicing::INDEXING_SLICING_INFO,
217217
crate::indexing_slicing::OUT_OF_BOUNDS_INDEXING_INFO,
218+
crate::ineffective_open_options::INEFFECTIVE_OPEN_OPTIONS_INFO,
218219
crate::infinite_iter::INFINITE_ITER_INFO,
219220
crate::infinite_iter::MAYBE_INFINITE_ITER_INFO,
220221
crate::inherent_impl::MULTIPLE_INHERENT_IMPL_INFO,

‎clippy_lints/src/ineffective_open_options.rs

+95
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
use crate::methods::method_call;
2+
use clippy_utils::diagnostics::span_lint_and_sugg;
3+
use clippy_utils::peel_blocks;
4+
use rustc_ast::LitKind;
5+
use rustc_errors::Applicability;
6+
use rustc_hir::{Expr, ExprKind};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_middle::ty;
9+
use rustc_session::declare_lint_pass;
10+
use rustc_span::{sym, BytePos, Span};
11+
12+
declare_clippy_lint! {
13+
/// ### What it does
14+
/// Checks if both `.write(true)` and `.append(true)` methods are called
15+
/// on a same `OpenOptions`.
16+
///
17+
/// ### Why is this bad?
18+
/// `.append(true)` already enables `write(true)`, making this one
19+
/// superflous.
20+
///
21+
/// ### Example
22+
/// ```no_run
23+
/// # use std::fs::OpenOptions;
24+
/// let _ = OpenOptions::new()
25+
/// .write(true)
26+
/// .append(true)
27+
/// .create(true)
28+
/// .open("file.json");
29+
/// ```
30+
/// Use instead:
31+
/// ```no_run
32+
/// # use std::fs::OpenOptions;
33+
/// let _ = OpenOptions::new()
34+
/// .append(true)
35+
/// .create(true)
36+
/// .open("file.json");
37+
/// ```
38+
#[clippy::version = "1.76.0"]
39+
pub INEFFECTIVE_OPEN_OPTIONS,
40+
suspicious,
41+
"usage of both `write(true)` and `append(true)` on same `OpenOptions`"
42+
}
43+
44+
declare_lint_pass!(IneffectiveOpenOptions => [INEFFECTIVE_OPEN_OPTIONS]);
45+
46+
fn index_if_arg_is_boolean(args: &[Expr<'_>], call_span: Span) -> Option<Span> {
47+
if let [arg] = args
48+
&& let ExprKind::Lit(lit) = peel_blocks(arg).kind
49+
&& lit.node == LitKind::Bool(true)
50+
{
51+
// The `.` is not included in the span so we cheat a little bit to include it as well.
52+
Some(call_span.with_lo(call_span.lo() - BytePos(1)))
53+
} else {
54+
None
55+
}
56+
}
57+
58+
impl<'tcx> LateLintPass<'tcx> for IneffectiveOpenOptions {
59+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
60+
let Some(("open", mut receiver, [_arg], _, _)) = method_call(expr) else {
61+
return;
62+
};
63+
let receiver_ty = cx.typeck_results().expr_ty(receiver);
64+
match receiver_ty.peel_refs().kind() {
65+
ty::Adt(adt, _) if cx.tcx.is_diagnostic_item(sym::FsOpenOptions, adt.did()) => {},
66+
_ => return,
67+
}
68+
69+
let mut append = None;
70+
let mut write = None;
71+
72+
while let Some((name, recv, args, _, span)) = method_call(receiver) {
73+
if name == "append" {
74+
append = index_if_arg_is_boolean(args, span);
75+
} else if name == "write" {
76+
write = index_if_arg_is_boolean(args, span);
77+
}
78+
receiver = recv;
79+
}
80+
81+
if let Some(write_span) = write
82+
&& append.is_some()
83+
{
84+
span_lint_and_sugg(
85+
cx,
86+
INEFFECTIVE_OPEN_OPTIONS,
87+
write_span,
88+
"unnecessary use of `.write(true)` because there is `.append(true)`",
89+
"remove `.write(true)`",
90+
String::new(),
91+
Applicability::MachineApplicable,
92+
);
93+
}
94+
}
95+
}

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ mod implied_bounds_in_impls;
153153
mod inconsistent_struct_constructor;
154154
mod index_refutable_slice;
155155
mod indexing_slicing;
156+
mod ineffective_open_options;
156157
mod infinite_iter;
157158
mod inherent_impl;
158159
mod inherent_to_string;
@@ -1073,6 +1074,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
10731074
store.register_late_pass(|_| Box::new(impl_hash_with_borrow_str_and_bytes::ImplHashWithBorrowStrBytes));
10741075
store.register_late_pass(|_| Box::new(repeat_vec_with_capacity::RepeatVecWithCapacity));
10751076
store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences));
1077+
store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions));
10761078
// add lints here, do not remove this comment, it's used in `new_lint`
10771079
}
10781080

‎clippy_lints/src/methods/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3906,7 +3906,7 @@ impl_lint_pass!(Methods => [
39063906
]);
39073907

39083908
/// Extracts a method call name, args, and `Span` of the method name.
3909-
fn method_call<'tcx>(
3909+
pub fn method_call<'tcx>(
39103910
recv: &'tcx hir::Expr<'tcx>,
39113911
) -> Option<(&'tcx str, &'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>], Span, Span)> {
39123912
if let ExprKind::MethodCall(path, receiver, args, call_span) = recv.kind {

‎tests/ui/ineffective_open_options.fixed

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
#![warn(clippy::ineffective_open_options)]
2+
3+
use std::fs::OpenOptions;
4+
5+
fn main() {
6+
let file = OpenOptions::new()
7+
.create(true)
8+
//~ ERROR: unnecessary use of `.write(true)`
9+
.append(true)
10+
.open("dump.json")
11+
.unwrap();
12+
13+
let file = OpenOptions::new()
14+
.create(true)
15+
.append(true)
16+
//~ ERROR: unnecessary use of `.write(true)`
17+
.open("dump.json")
18+
.unwrap();
19+
20+
// All the next calls are ok.
21+
let file = OpenOptions::new()
22+
.create(true)
23+
.write(false)
24+
.append(true)
25+
.open("dump.json")
26+
.unwrap();
27+
let file = OpenOptions::new()
28+
.create(true)
29+
.write(true)
30+
.append(false)
31+
.open("dump.json")
32+
.unwrap();
33+
let file = OpenOptions::new()
34+
.create(true)
35+
.write(false)
36+
.append(false)
37+
.open("dump.json")
38+
.unwrap();
39+
let file = OpenOptions::new().create(true).append(true).open("dump.json").unwrap();
40+
let file = OpenOptions::new().create(true).write(true).open("dump.json").unwrap();
41+
}

‎tests/ui/ineffective_open_options.rs

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
#![warn(clippy::ineffective_open_options)]
2+
3+
use std::fs::OpenOptions;
4+
5+
fn main() {
6+
let file = OpenOptions::new()
7+
.create(true)
8+
.write(true) //~ ERROR: unnecessary use of `.write(true)`
9+
.append(true)
10+
.open("dump.json")
11+
.unwrap();
12+
13+
let file = OpenOptions::new()
14+
.create(true)
15+
.append(true)
16+
.write(true) //~ ERROR: unnecessary use of `.write(true)`
17+
.open("dump.json")
18+
.unwrap();
19+
20+
// All the next calls are ok.
21+
let file = OpenOptions::new()
22+
.create(true)
23+
.write(false)
24+
.append(true)
25+
.open("dump.json")
26+
.unwrap();
27+
let file = OpenOptions::new()
28+
.create(true)
29+
.write(true)
30+
.append(false)
31+
.open("dump.json")
32+
.unwrap();
33+
let file = OpenOptions::new()
34+
.create(true)
35+
.write(false)
36+
.append(false)
37+
.open("dump.json")
38+
.unwrap();
39+
let file = OpenOptions::new().create(true).append(true).open("dump.json").unwrap();
40+
let file = OpenOptions::new().create(true).write(true).open("dump.json").unwrap();
41+
}

‎tests/ui/ineffective_open_options.stderr

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
error: unnecessary use of `.write(true)` because there is `.append(true)`
2+
--> $DIR/ineffective_open_options.rs:8:9
3+
|
4+
LL | .write(true)
5+
| ^^^^^^^^^^^^ help: remove `.write(true)`
6+
|
7+
= note: `-D clippy::ineffective-open-options` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::ineffective_open_options)]`
9+
10+
error: unnecessary use of `.write(true)` because there is `.append(true)`
11+
--> $DIR/ineffective_open_options.rs:16:9
12+
|
13+
LL | .write(true)
14+
| ^^^^^^^^^^^^ help: remove `.write(true)`
15+
16+
error: aborting due to 2 previous errors
17+

0 commit comments

Comments
 (0)