From 6c201db005c546bb16ce19c81d6b68f262b0203a Mon Sep 17 00:00:00 2001 From: atwam Date: Wed, 4 Oct 2023 13:56:18 +0100 Subject: [PATCH 1/6] Add suspicious_open_options lint. Checks for the suspicious use of OpenOptions::create() without an explicit OpenOptions::truncate(). create() alone will either create a new file or open an existing file. If the file already exists, it will be overwritten when written to, but the file will not be truncated by default. If less data is written to the file than it already contains, the remainder of the file will remain unchanged, and the end of the file will contain old data. In most cases, one should either use `create_new` to ensure the file is created from scratch, or ensure `truncate` is called so that the truncation behaviour is explicit. `truncate(true)` will ensure the file is entirely overwritten with new data, whereas `truncate(false)` will explicitely keep the default behavior. ```rust use std::fs::OpenOptions; OpenOptions::new().create(true).truncate(true); ``` --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/mod.rs | 32 ++++ clippy_lints/src/methods/open_options.rs | 156 +++++++----------- tests/ui/open_options.rs | 3 + tests/ui/open_options.stderr | 31 +++- .../ui/seek_to_start_instead_of_rewind.fixed | 3 + tests/ui/seek_to_start_instead_of_rewind.rs | 3 + .../ui/seek_to_start_instead_of_rewind.stderr | 2 +- 9 files changed, 131 insertions(+), 101 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d32bbec914a..db4b4c2aef60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5606,6 +5606,7 @@ Released 2018-09-13 [`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting [`suspicious_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_map [`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl +[`suspicious_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_open_options [`suspicious_operation_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_operation_groupings [`suspicious_splitn`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_splitn [`suspicious_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_to_owned diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 20230106d536..639edd8da301 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -439,6 +439,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::STR_SPLIT_AT_NEWLINE_INFO, crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO, crate::methods::SUSPICIOUS_MAP_INFO, + crate::methods::SUSPICIOUS_OPEN_OPTIONS_INFO, crate::methods::SUSPICIOUS_SPLITN_INFO, crate::methods::SUSPICIOUS_TO_OWNED_INFO, crate::methods::TYPE_ID_ON_BOX_INFO, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 89ea3597dc0f..84fb583595b9 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2827,6 +2827,37 @@ declare_clippy_lint! { "nonsensical combination of options for opening a file" } +declare_clippy_lint! { + /// ### What it does + /// Checks for the suspicious use of OpenOptions::create() + /// without an explicit OpenOptions::truncate(). + /// + /// ### Why is this bad? + /// create() alone will either create a new file or open an + /// existing file. If the file already exists, it will be + /// overwritten when written to, but the file will not be + /// truncated by default. If less data is written to the file + /// than it already contains, the remainder of the file will + /// remain unchanged, and the end of the file will contain old + /// data. + /// In most cases, one should either use `create_new` to ensure + /// the file is created from scratch, or ensure `truncate` is + /// called so that the truncation behaviour is explicit. `truncate(true)` + /// will ensure the file is entirely overwritten with new data, whereas + /// `truncate(false)` will explicitely keep the default behavior. + /// + /// ### Example + /// ```rust + /// use std::fs::OpenOptions; + /// + /// OpenOptions::new().create(true).truncate(true); + /// ``` + #[clippy::version = "1.73.0"] + pub SUSPICIOUS_OPEN_OPTIONS, + suspicious, + "suspicious combination of options for opening a file" +} + declare_clippy_lint! { /// ### What it does ///* Checks for [push](https://doc.rust-lang.org/std/path/struct.PathBuf.html#method.push) @@ -4033,6 +4064,7 @@ impl_lint_pass!(Methods => [ MAP_ERR_IGNORE, MUT_MUTEX_LOCK, NONSENSICAL_OPEN_OPTIONS, + SUSPICIOUS_OPEN_OPTIONS, PATH_BUF_PUSH_OVERWRITE, RANGE_ZIP_WITH_LEN, REPEAT_ONCE, diff --git a/clippy_lints/src/methods/open_options.rs b/clippy_lints/src/methods/open_options.rs index 65c986dcacce..81508a5cf65a 100644 --- a/clippy_lints/src/methods/open_options.rs +++ b/clippy_lints/src/methods/open_options.rs @@ -1,3 +1,5 @@ +use rustc_data_structures::fx::FxHashMap; + use clippy_utils::diagnostics::span_lint; use clippy_utils::ty::is_type_diagnostic_item; use rustc_ast::ast::LitKind; @@ -6,7 +8,7 @@ use rustc_lint::LateContext; use rustc_span::source_map::Spanned; use rustc_span::{sym, Span}; -use super::NONSENSICAL_OPEN_OPTIONS; +use super::{NONSENSICAL_OPEN_OPTIONS, SUSPICIOUS_OPEN_OPTIONS}; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) { if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) @@ -19,20 +21,32 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx } } -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Eq, PartialEq, Clone)] enum Argument { - True, - False, + Set(bool), Unknown, } -#[derive(Debug)] +#[derive(Debug, Eq, PartialEq, Hash, Clone)] enum OpenOption { - Write, + Append, + Create, + CreateNew, Read, Truncate, - Create, - Append, + Write, +} +impl std::fmt::Display for OpenOption { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + OpenOption::Append => write!(f, "append"), + OpenOption::Create => write!(f, "create"), + OpenOption::CreateNew => write!(f, "create_new"), + OpenOption::Read => write!(f, "read"), + OpenOption::Truncate => write!(f, "truncate"), + OpenOption::Write => write!(f, "write"), + } + } } fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec<(OpenOption, Argument)>) { @@ -48,7 +62,7 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec .. } = span { - if *lit { Argument::True } else { Argument::False } + Argument::Set(*lit) } else { // The function is called with a literal which is not a boolean literal. // This is theoretically possible, but not very likely. @@ -62,6 +76,9 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec "create" => { options.push((OpenOption::Create, argument_option)); }, + "create_new" => { + options.push((OpenOption::CreateNew, argument_option)); + }, "append" => { options.push((OpenOption::Append, argument_option)); }, @@ -82,97 +99,52 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec } } -fn check_open_options(cx: &LateContext<'_>, options: &[(OpenOption, Argument)], span: Span) { - let (mut create, mut append, mut truncate, mut read, mut write) = (false, false, false, false, false); - let (mut create_arg, mut append_arg, mut truncate_arg, mut read_arg, mut write_arg) = - (false, false, false, false, false); - // This code is almost duplicated (oh, the irony), but I haven't found a way to - // unify it. +fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument)], span: Span) { + // The args passed to these methods, if they have been called + let mut options = FxHashMap::default(); + for (option, arg) in settings { + if options.insert(option.clone(), arg.clone()).is_some() { + span_lint( + cx, + NONSENSICAL_OPEN_OPTIONS, + span, + &format!("the method `{}` is called more than once", &option), + ); + } + } - for option in options { - match *option { - (OpenOption::Create, arg) => { - if create { - span_lint( - cx, - NONSENSICAL_OPEN_OPTIONS, - span, - "the method `create` is called more than once", - ); - } else { - create = true; - } - create_arg = create_arg || (arg == Argument::True); - }, - (OpenOption::Append, arg) => { - if append { - span_lint( - cx, - NONSENSICAL_OPEN_OPTIONS, - span, - "the method `append` is called more than once", - ); - } else { - append = true; - } - append_arg = append_arg || (arg == Argument::True); - }, - (OpenOption::Truncate, arg) => { - if truncate { - span_lint( - cx, - NONSENSICAL_OPEN_OPTIONS, - span, - "the method `truncate` is called more than once", - ); - } else { - truncate = true; - } - truncate_arg = truncate_arg || (arg == Argument::True); - }, - (OpenOption::Read, arg) => { - if read { - span_lint( - cx, - NONSENSICAL_OPEN_OPTIONS, - span, - "the method `read` is called more than once", - ); - } else { - read = true; - } - read_arg = read_arg || (arg == Argument::True); - }, - (OpenOption::Write, arg) => { - if write { - span_lint( - cx, - NONSENSICAL_OPEN_OPTIONS, - span, - "the method `write` is called more than once", - ); - } else { - write = true; - } - write_arg = write_arg || (arg == Argument::True); - }, + if let (Some(Argument::Set(true)), Some(Argument::Set(true))) = + (options.get(&OpenOption::Read), options.get(&OpenOption::Truncate)) + { + if options.get(&OpenOption::Write).unwrap_or(&Argument::Set(false)) == &Argument::Set(false) { + span_lint( + cx, + NONSENSICAL_OPEN_OPTIONS, + span, + "file opened with `truncate` and `read`", + ); } } - if read && truncate && read_arg && truncate_arg && !(write && write_arg) { - span_lint( - cx, - NONSENSICAL_OPEN_OPTIONS, - span, - "file opened with `truncate` and `read`", - ); + if let (Some(Argument::Set(true)), Some(Argument::Set(true))) = + (options.get(&OpenOption::Append), options.get(&OpenOption::Truncate)) + { + if options.get(&OpenOption::Write).unwrap_or(&Argument::Set(false)) == &Argument::Set(false) { + span_lint( + cx, + NONSENSICAL_OPEN_OPTIONS, + span, + "file opened with `append` and `truncate`", + ); + } } - if append && truncate && append_arg && truncate_arg { + + if let (Some(Argument::Set(true)), None) = (options.get(&OpenOption::Create), options.get(&OpenOption::Truncate)) { span_lint( cx, - NONSENSICAL_OPEN_OPTIONS, + SUSPICIOUS_OPEN_OPTIONS, span, - "file opened with `append` and `truncate`", + "file opened with `create`, but `truncate` behavior not defined", ); } } diff --git a/tests/ui/open_options.rs b/tests/ui/open_options.rs index 0cdc5bf2bb59..dd380f4101ad 100644 --- a/tests/ui/open_options.rs +++ b/tests/ui/open_options.rs @@ -2,6 +2,7 @@ use std::fs::OpenOptions; #[allow(unused_must_use)] #[warn(clippy::nonsensical_open_options)] +#[warn(clippy::suspicious_open_options)] fn main() { OpenOptions::new().read(true).truncate(true).open("foo.txt"); //~^ ERROR: file opened with `truncate` and `read` @@ -19,4 +20,6 @@ fn main() { //~^ ERROR: the method `append` is called more than once OpenOptions::new().truncate(true).truncate(false).open("foo.txt"); //~^ ERROR: the method `truncate` is called more than once + OpenOptions::new().create(true).open("foo.txt"); + //~^ ERROR: file opened with `create`, but `truncate` behavior not defined } diff --git a/tests/ui/open_options.stderr b/tests/ui/open_options.stderr index 7ac826f52fa9..be6be250f22a 100644 --- a/tests/ui/open_options.stderr +++ b/tests/ui/open_options.stderr @@ -1,5 +1,5 @@ error: file opened with `truncate` and `read` - --> $DIR/open_options.rs:6:5 + --> $DIR/open_options.rs:7:5 | LL | OpenOptions::new().read(true).truncate(true).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,40 +8,55 @@ LL | OpenOptions::new().read(true).truncate(true).open("foo.txt"); = help: to override `-D warnings` add `#[allow(clippy::nonsensical_open_options)]` error: file opened with `append` and `truncate` - --> $DIR/open_options.rs:9:5 + --> $DIR/open_options.rs:10:5 | LL | OpenOptions::new().append(true).truncate(true).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: the method `read` is called more than once - --> $DIR/open_options.rs:12:5 + --> $DIR/open_options.rs:13:5 | LL | OpenOptions::new().read(true).read(false).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: the method `create` is called more than once - --> $DIR/open_options.rs:14:5 + --> $DIR/open_options.rs:15:5 | LL | OpenOptions::new().create(true).create(false).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +error: file opened with `create`, but `truncate` behavior not defined + --> $DIR/open_options.rs:15:5 + | +LL | OpenOptions::new().create(true).create(false).open("foo.txt"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::suspicious-open-options` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]` + error: the method `write` is called more than once - --> $DIR/open_options.rs:16:5 + --> $DIR/open_options.rs:17:5 | LL | OpenOptions::new().write(true).write(false).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: the method `append` is called more than once - --> $DIR/open_options.rs:18:5 + --> $DIR/open_options.rs:19:5 | LL | OpenOptions::new().append(true).append(false).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: the method `truncate` is called more than once - --> $DIR/open_options.rs:20:5 + --> $DIR/open_options.rs:21:5 | LL | OpenOptions::new().truncate(true).truncate(false).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error: file opened with `create`, but `truncate` behavior not defined + --> $DIR/open_options.rs:23:5 + | +LL | OpenOptions::new().create(true).open("foo.txt"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 9 previous errors diff --git a/tests/ui/seek_to_start_instead_of_rewind.fixed b/tests/ui/seek_to_start_instead_of_rewind.fixed index 15cc8d54faab..8859a68320f0 100644 --- a/tests/ui/seek_to_start_instead_of_rewind.fixed +++ b/tests/ui/seek_to_start_instead_of_rewind.fixed @@ -80,6 +80,7 @@ fn main() { .write(true) .read(true) .create(true) + .truncate(true) .open("foo.txt") .unwrap(); @@ -104,6 +105,7 @@ fn msrv_1_54() { .write(true) .read(true) .create(true) + .truncate(true) .open("foo.txt") .unwrap(); @@ -124,6 +126,7 @@ fn msrv_1_55() { .write(true) .read(true) .create(true) + .truncate(true) .open("foo.txt") .unwrap(); diff --git a/tests/ui/seek_to_start_instead_of_rewind.rs b/tests/ui/seek_to_start_instead_of_rewind.rs index 197225ffbd5c..7b72efb34ff8 100644 --- a/tests/ui/seek_to_start_instead_of_rewind.rs +++ b/tests/ui/seek_to_start_instead_of_rewind.rs @@ -80,6 +80,7 @@ fn main() { .write(true) .read(true) .create(true) + .truncate(true) .open("foo.txt") .unwrap(); @@ -104,6 +105,7 @@ fn msrv_1_54() { .write(true) .read(true) .create(true) + .truncate(true) .open("foo.txt") .unwrap(); @@ -124,6 +126,7 @@ fn msrv_1_55() { .write(true) .read(true) .create(true) + .truncate(true) .open("foo.txt") .unwrap(); diff --git a/tests/ui/seek_to_start_instead_of_rewind.stderr b/tests/ui/seek_to_start_instead_of_rewind.stderr index 05c11cf7f8c6..b6b0d2effa81 100644 --- a/tests/ui/seek_to_start_instead_of_rewind.stderr +++ b/tests/ui/seek_to_start_instead_of_rewind.stderr @@ -14,7 +14,7 @@ LL | t.seek(SeekFrom::Start(0)); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `rewind()` error: used `seek` to go to the start of the stream - --> $DIR/seek_to_start_instead_of_rewind.rs:133:7 + --> $DIR/seek_to_start_instead_of_rewind.rs:136:7 | LL | f.seek(SeekFrom::Start(0)); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `rewind()` From 6fb471d646045ce7c3e81483bc1db2cbb4c9f69c Mon Sep 17 00:00:00 2001 From: atwam Date: Tue, 10 Oct 2023 12:08:17 +0100 Subject: [PATCH 2/6] More helpful text, small style changes. --- clippy_lints/src/methods/mod.rs | 8 ++- clippy_lints/src/methods/open_options.rs | 75 ++++++++++++++---------- clippy_utils/src/paths.rs | 2 + tests/ui/open_options.rs | 1 - tests/ui/open_options.stderr | 35 ++++++----- 5 files changed, 71 insertions(+), 50 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 84fb583595b9..3d6a16f9ac61 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2829,8 +2829,8 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for the suspicious use of OpenOptions::create() - /// without an explicit OpenOptions::truncate(). + /// Checks for the suspicious use of `OpenOptions::create()` + /// without an explicit `OpenOptions::truncate()`. /// /// ### Why is this bad? /// create() alone will either create a new file or open an @@ -2850,9 +2850,11 @@ declare_clippy_lint! { /// ```rust /// use std::fs::OpenOptions; /// + /// OpenOptions::new().create(true); + /// /// OpenOptions::new().create(true).truncate(true); /// ``` - #[clippy::version = "1.73.0"] + #[clippy::version = "1.75.0"] pub SUSPICIOUS_OPEN_OPTIONS, suspicious, "suspicious combination of options for opening a file" diff --git a/clippy_lints/src/methods/open_options.rs b/clippy_lints/src/methods/open_options.rs index 81508a5cf65a..c54a75068366 100644 --- a/clippy_lints/src/methods/open_options.rs +++ b/clippy_lints/src/methods/open_options.rs @@ -1,6 +1,6 @@ use rustc_data_structures::fx::FxHashMap; -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; use clippy_utils::ty::is_type_diagnostic_item; use rustc_ast::ast::LitKind; use rustc_hir::{Expr, ExprKind}; @@ -13,7 +13,11 @@ use super::{NONSENSICAL_OPEN_OPTIONS, SUSPICIOUS_OPEN_OPTIONS}; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) { if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) && let Some(impl_id) = cx.tcx.impl_of_method(method_id) - && is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id).instantiate_identity(), sym::FsOpenOptions) + && ( + is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id).instantiate_identity(), sym::FsOpenOptions) || + match_type(cx, cx.tcx.type_of(impl_id).instantiate_identity(), &paths::TOKIO_IO_OPEN_OPTIONS) + ) + { let mut options = Vec::new(); get_open_options(cx, recv, &mut options); @@ -49,12 +53,12 @@ impl std::fmt::Display for OpenOption { } } -fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec<(OpenOption, Argument)>) { - if let ExprKind::MethodCall(path, receiver, arguments, _) = argument.kind { +fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec<(OpenOption, Argument, Span)>) { + if let ExprKind::MethodCall(path, receiver, arguments, span) = argument.kind { let obj_ty = cx.typeck_results().expr_ty(receiver).peel_refs(); // Only proceed if this is a call on some object of type std::fs::OpenOptions - if is_type_diagnostic_item(cx, obj_ty, sym::FsOpenOptions) && !arguments.is_empty() { + if !arguments.is_empty() && (is_type_diagnostic_item(cx, obj_ty, sym::FsOpenOptions)) { let argument_option = match arguments[0].kind { ExprKind::Lit(span) => { if let Spanned { @@ -74,22 +78,22 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec match path.ident.as_str() { "create" => { - options.push((OpenOption::Create, argument_option)); + options.push((OpenOption::Create, argument_option, span)); }, "create_new" => { - options.push((OpenOption::CreateNew, argument_option)); + options.push((OpenOption::CreateNew, argument_option, span)); }, "append" => { - options.push((OpenOption::Append, argument_option)); + options.push((OpenOption::Append, argument_option, span)); }, "truncate" => { - options.push((OpenOption::Truncate, argument_option)); + options.push((OpenOption::Truncate, argument_option, span)); }, "read" => { - options.push((OpenOption::Read, argument_option)); + options.push((OpenOption::Read, argument_option, span)); }, "write" => { - options.push((OpenOption::Write, argument_option)); + options.push((OpenOption::Write, argument_option, span)); }, _ => (), } @@ -99,24 +103,25 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec } } -fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument)], span: Span) { +fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, Span)], span: Span) { // The args passed to these methods, if they have been called let mut options = FxHashMap::default(); - for (option, arg) in settings { - if options.insert(option.clone(), arg.clone()).is_some() { + for (option, arg, sp) in settings { + if let Some((_, prev_span)) = options.insert(option.clone(), (arg.clone(), *sp)) { span_lint( cx, NONSENSICAL_OPEN_OPTIONS, - span, + prev_span, &format!("the method `{}` is called more than once", &option), ); } } - if let (Some(Argument::Set(true)), Some(Argument::Set(true))) = - (options.get(&OpenOption::Read), options.get(&OpenOption::Truncate)) - { - if options.get(&OpenOption::Write).unwrap_or(&Argument::Set(false)) == &Argument::Set(false) { + if_chain! { + if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Read); + if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate); + if let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Write); + then { span_lint( cx, NONSENSICAL_OPEN_OPTIONS, @@ -126,10 +131,11 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument)], } } - if let (Some(Argument::Set(true)), Some(Argument::Set(true))) = - (options.get(&OpenOption::Append), options.get(&OpenOption::Truncate)) - { - if options.get(&OpenOption::Write).unwrap_or(&Argument::Set(false)) == &Argument::Set(false) { + if_chain! { + if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Append); + if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate); + if let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Write); + then { span_lint( cx, NONSENSICAL_OPEN_OPTIONS, @@ -139,12 +145,21 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument)], } } - if let (Some(Argument::Set(true)), None) = (options.get(&OpenOption::Create), options.get(&OpenOption::Truncate)) { - span_lint( - cx, - SUSPICIOUS_OPEN_OPTIONS, - span, - "file opened with `create`, but `truncate` behavior not defined", - ); + if_chain! { + if let Some((Argument::Set(true), create_span)) = options.get(&OpenOption::Create); + if let None = options.get(&OpenOption::Truncate); + then { + span_lint_and_then( + cx, + SUSPICIOUS_OPEN_OPTIONS, + *create_span, + "file opened with `create`, but `truncate` behavior not defined", + |diag| { + diag + //.span_suggestion(create_span.shrink_to_hi(), "add", ".truncate(true)".to_string(), rustc_errors::Applicability::MaybeIncorrect) + .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)`"); + }, + ); + } } } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 0a820a1754ca..2771e49736e6 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -50,6 +50,8 @@ pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"]; pub const LATE_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "LateLintPass"]; pub const LINT: [&str; 2] = ["rustc_lint_defs", "Lint"]; pub const MSRV: [&str; 3] = ["clippy_config", "msrvs", "Msrv"]; +#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates +pub const TOKIO_IO_OPEN_OPTIONS: [&str; 3] = ["tokio", "fs", "OpenOptions"]; pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", "as_os_str"]; pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"]; pub const PARKING_LOT_MUTEX_GUARD: [&str; 3] = ["lock_api", "mutex", "MutexGuard"]; diff --git a/tests/ui/open_options.rs b/tests/ui/open_options.rs index dd380f4101ad..3f486b76e723 100644 --- a/tests/ui/open_options.rs +++ b/tests/ui/open_options.rs @@ -1,5 +1,4 @@ use std::fs::OpenOptions; - #[allow(unused_must_use)] #[warn(clippy::nonsensical_open_options)] #[warn(clippy::suspicious_open_options)] diff --git a/tests/ui/open_options.stderr b/tests/ui/open_options.stderr index be6be250f22a..ecab243069d3 100644 --- a/tests/ui/open_options.stderr +++ b/tests/ui/open_options.stderr @@ -1,5 +1,5 @@ error: file opened with `truncate` and `read` - --> $DIR/open_options.rs:7:5 + --> $DIR/open_options.rs:6:5 | LL | OpenOptions::new().read(true).truncate(true).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,55 +8,58 @@ LL | OpenOptions::new().read(true).truncate(true).open("foo.txt"); = help: to override `-D warnings` add `#[allow(clippy::nonsensical_open_options)]` error: file opened with `append` and `truncate` - --> $DIR/open_options.rs:10:5 + --> $DIR/open_options.rs:9:5 | LL | OpenOptions::new().append(true).truncate(true).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: the method `read` is called more than once - --> $DIR/open_options.rs:13:5 + --> $DIR/open_options.rs:12:35 | LL | OpenOptions::new().read(true).read(false).open("foo.txt"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^ error: the method `create` is called more than once - --> $DIR/open_options.rs:15:5 + --> $DIR/open_options.rs:14:37 | LL | OpenOptions::new().create(true).create(false).open("foo.txt"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ error: file opened with `create`, but `truncate` behavior not defined - --> $DIR/open_options.rs:15:5 + --> $DIR/open_options.rs:14:24 | LL | OpenOptions::new().create(true).create(false).open("foo.txt"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^ | + = 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)` = note: `-D clippy::suspicious-open-options` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]` error: the method `write` is called more than once - --> $DIR/open_options.rs:17:5 + --> $DIR/open_options.rs:16:36 | LL | OpenOptions::new().write(true).write(false).open("foo.txt"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^ error: the method `append` is called more than once - --> $DIR/open_options.rs:19:5 + --> $DIR/open_options.rs:18:37 | LL | OpenOptions::new().append(true).append(false).open("foo.txt"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ error: the method `truncate` is called more than once - --> $DIR/open_options.rs:21:5 + --> $DIR/open_options.rs:20:39 | LL | OpenOptions::new().truncate(true).truncate(false).open("foo.txt"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ error: file opened with `create`, but `truncate` behavior not defined - --> $DIR/open_options.rs:23:5 + --> $DIR/open_options.rs:22:24 | LL | OpenOptions::new().create(true).open("foo.txt"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^ + | + = 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)` error: aborting due to 9 previous errors From 2ec8729962aae2fc3c72f388a8249e13f241bc20 Mon Sep 17 00:00:00 2001 From: atwam Date: Tue, 10 Oct 2023 13:30:24 +0100 Subject: [PATCH 3/6] PR Fixes --- clippy_lints/src/methods/open_options.rs | 37 +++++++++--------------- tests/ui/open_options.stderr | 12 ++++++-- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/clippy_lints/src/methods/open_options.rs b/clippy_lints/src/methods/open_options.rs index c54a75068366..c006ea192ee5 100644 --- a/clippy_lints/src/methods/open_options.rs +++ b/clippy_lints/src/methods/open_options.rs @@ -1,6 +1,7 @@ use rustc_data_structures::fx::FxHashMap; use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; +use clippy_utils::paths; use clippy_utils::ty::is_type_diagnostic_item; use rustc_ast::ast::LitKind; use rustc_hir::{Expr, ExprKind}; @@ -117,11 +118,10 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, S } } - if_chain! { - if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Read); - if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate); - if let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Write); - then { + if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Read) + && let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate) + && let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Write) + { span_lint( cx, NONSENSICAL_OPEN_OPTIONS, @@ -129,37 +129,28 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, S "file opened with `truncate` and `read`", ); } - } - if_chain! { - if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Append); - if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate); - if let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Write); - then { + if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Append) + && let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate) + { span_lint( cx, NONSENSICAL_OPEN_OPTIONS, span, "file opened with `append` and `truncate`", ); - } } - if_chain! { - if let Some((Argument::Set(true), create_span)) = options.get(&OpenOption::Create); - if let None = options.get(&OpenOption::Truncate); - then { - span_lint_and_then( + if let Some((Argument::Set(true), create_span)) = options.get(&OpenOption::Create) + && let None = options.get(&OpenOption::Truncate) + { + span_lint_and_help( cx, SUSPICIOUS_OPEN_OPTIONS, *create_span, "file opened with `create`, but `truncate` behavior not defined", - |diag| { - diag - //.span_suggestion(create_span.shrink_to_hi(), "add", ".truncate(true)".to_string(), rustc_errors::Applicability::MaybeIncorrect) - .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)`"); - }, + Some(span), + "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)`" ); - } } } diff --git a/tests/ui/open_options.stderr b/tests/ui/open_options.stderr index ecab243069d3..11ba7444c97c 100644 --- a/tests/ui/open_options.stderr +++ b/tests/ui/open_options.stderr @@ -31,7 +31,11 @@ error: file opened with `create`, but `truncate` behavior not defined LL | OpenOptions::new().create(true).create(false).open("foo.txt"); | ^^^^^^^^^^^^ | - = 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)` +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)` + --> $DIR/open_options.rs:14:5 + | +LL | OpenOptions::new().create(true).create(false).open("foo.txt"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: `-D clippy::suspicious-open-options` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]` @@ -59,7 +63,11 @@ error: file opened with `create`, but `truncate` behavior not defined LL | OpenOptions::new().create(true).open("foo.txt"); | ^^^^^^^^^^^^ | - = 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)` +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)` + --> $DIR/open_options.rs:22:5 + | +LL | OpenOptions::new().create(true).open("foo.txt"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 9 previous errors From 84588a8815641a445a29f5fb0ea21767e6e4bbdf Mon Sep 17 00:00:00 2001 From: atwam Date: Thu, 26 Oct 2023 11:30:12 +0100 Subject: [PATCH 4/6] Add suggestion/fix to suspicious_open_options Also rebase and fix conflicts --- clippy_lints/src/methods/open_options.rs | 66 +++++++++++++----------- tests/ui/open_options.rs | 19 +++++-- tests/ui/open_options.stderr | 48 +++++------------ tests/ui/open_options_fixable.fixed | 7 +++ tests/ui/open_options_fixable.rs | 7 +++ tests/ui/open_options_fixable.stderr | 12 +++++ 6 files changed, 89 insertions(+), 70 deletions(-) create mode 100644 tests/ui/open_options_fixable.fixed create mode 100644 tests/ui/open_options_fixable.rs create mode 100644 tests/ui/open_options_fixable.stderr diff --git a/clippy_lints/src/methods/open_options.rs b/clippy_lints/src/methods/open_options.rs index c006ea192ee5..cbe504a9c638 100644 --- a/clippy_lints/src/methods/open_options.rs +++ b/clippy_lints/src/methods/open_options.rs @@ -2,23 +2,24 @@ use rustc_data_structures::fx::FxHashMap; use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; use clippy_utils::paths; -use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::ty::{is_type_diagnostic_item, match_type}; use rustc_ast::ast::LitKind; use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; +use rustc_middle::ty::Ty; use rustc_span::source_map::Spanned; use rustc_span::{sym, Span}; use super::{NONSENSICAL_OPEN_OPTIONS, SUSPICIOUS_OPEN_OPTIONS}; +fn is_open_options(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + is_type_diagnostic_item(cx, ty, sym::FsOpenOptions) || match_type(cx, ty, &paths::TOKIO_IO_OPEN_OPTIONS) +} + pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) { if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) && let Some(impl_id) = cx.tcx.impl_of_method(method_id) - && ( - is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id).instantiate_identity(), sym::FsOpenOptions) || - match_type(cx, cx.tcx.type_of(impl_id).instantiate_identity(), &paths::TOKIO_IO_OPEN_OPTIONS) - ) - + && is_open_options(cx, cx.tcx.type_of(impl_id).instantiate_identity()) { let mut options = Vec::new(); get_open_options(cx, recv, &mut options); @@ -59,7 +60,7 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec let obj_ty = cx.typeck_results().expr_ty(receiver).peel_refs(); // Only proceed if this is a call on some object of type std::fs::OpenOptions - if !arguments.is_empty() && (is_type_diagnostic_item(cx, obj_ty, sym::FsOpenOptions)) { + if !arguments.is_empty() && is_open_options(cx, obj_ty) { let argument_option = match arguments[0].kind { ExprKind::Lit(span) => { if let Spanned { @@ -121,36 +122,39 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, S if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Read) && let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate) && let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Write) - { - span_lint( - cx, - NONSENSICAL_OPEN_OPTIONS, - span, - "file opened with `truncate` and `read`", - ); - } + { + span_lint( + cx, + NONSENSICAL_OPEN_OPTIONS, + span, + "file opened with `truncate` and `read`", + ); + } if let Some((Argument::Set(true), _)) = options.get(&OpenOption::Append) && let Some((Argument::Set(true), _)) = options.get(&OpenOption::Truncate) - { - span_lint( - cx, - NONSENSICAL_OPEN_OPTIONS, - span, - "file opened with `append` and `truncate`", - ); + { + span_lint( + cx, + NONSENSICAL_OPEN_OPTIONS, + span, + "file opened with `append` and `truncate`", + ); } if let Some((Argument::Set(true), create_span)) = options.get(&OpenOption::Create) && let None = options.get(&OpenOption::Truncate) - { - span_lint_and_help( - cx, - SUSPICIOUS_OPEN_OPTIONS, - *create_span, - "file opened with `create`, but `truncate` behavior not defined", - Some(span), - "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)`" - ); + { + span_lint_and_then( + cx, + SUSPICIOUS_OPEN_OPTIONS, + *create_span, + "file opened with `create`, but `truncate` behavior not defined", + |diag| { + diag + .span_suggestion(create_span.shrink_to_hi(), "add", ".truncate(true)".to_string(), rustc_errors::Applicability::MaybeIncorrect) + .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)`"); + }, + ); } } diff --git a/tests/ui/open_options.rs b/tests/ui/open_options.rs index 3f486b76e723..726ec1b3ab04 100644 --- a/tests/ui/open_options.rs +++ b/tests/ui/open_options.rs @@ -1,7 +1,6 @@ use std::fs::OpenOptions; #[allow(unused_must_use)] #[warn(clippy::nonsensical_open_options)] -#[warn(clippy::suspicious_open_options)] fn main() { OpenOptions::new().read(true).truncate(true).open("foo.txt"); //~^ ERROR: file opened with `truncate` and `read` @@ -11,14 +10,24 @@ fn main() { OpenOptions::new().read(true).read(false).open("foo.txt"); //~^ ERROR: the method `read` is called more than once - OpenOptions::new().create(true).create(false).open("foo.txt"); - //~^ ERROR: the method `create` is called more than once + OpenOptions::new() + .create(true) + .truncate(true) // Ensure we don't trigger suspicious open options by having create without truncate + .create(false) + //~^ ERROR: the method `create` is called more than once + .open("foo.txt"); OpenOptions::new().write(true).write(false).open("foo.txt"); //~^ ERROR: the method `write` is called more than once OpenOptions::new().append(true).append(false).open("foo.txt"); //~^ ERROR: the method `append` is called more than once OpenOptions::new().truncate(true).truncate(false).open("foo.txt"); //~^ ERROR: the method `truncate` is called more than once - OpenOptions::new().create(true).open("foo.txt"); - //~^ ERROR: file opened with `create`, but `truncate` behavior not defined + + std::fs::File::options().read(true).read(false).open("foo.txt"); + //~^ ERROR: the method `read` is called more than once + + let mut options = std::fs::OpenOptions::new(); + options.read(true); + options.read(false); + //~^ ERROR: the method `read` is called more than once } diff --git a/tests/ui/open_options.stderr b/tests/ui/open_options.stderr index 11ba7444c97c..3e7d5a3499d3 100644 --- a/tests/ui/open_options.stderr +++ b/tests/ui/open_options.stderr @@ -1,5 +1,5 @@ error: file opened with `truncate` and `read` - --> $DIR/open_options.rs:6:5 + --> $DIR/open_options.rs:5:5 | LL | OpenOptions::new().read(true).truncate(true).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,66 +8,46 @@ LL | OpenOptions::new().read(true).truncate(true).open("foo.txt"); = help: to override `-D warnings` add `#[allow(clippy::nonsensical_open_options)]` error: file opened with `append` and `truncate` - --> $DIR/open_options.rs:9:5 + --> $DIR/open_options.rs:8:5 | LL | OpenOptions::new().append(true).truncate(true).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: the method `read` is called more than once - --> $DIR/open_options.rs:12:35 + --> $DIR/open_options.rs:11:35 | LL | OpenOptions::new().read(true).read(false).open("foo.txt"); | ^^^^^^^^^^^ error: the method `create` is called more than once - --> $DIR/open_options.rs:14:37 + --> $DIR/open_options.rs:16:10 | -LL | OpenOptions::new().create(true).create(false).open("foo.txt"); - | ^^^^^^^^^^^^^ - -error: file opened with `create`, but `truncate` behavior not defined - --> $DIR/open_options.rs:14:24 - | -LL | OpenOptions::new().create(true).create(false).open("foo.txt"); - | ^^^^^^^^^^^^ - | -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)` - --> $DIR/open_options.rs:14:5 - | -LL | OpenOptions::new().create(true).create(false).open("foo.txt"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: `-D clippy::suspicious-open-options` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]` +LL | .create(false) + | ^^^^^^^^^^^^^ error: the method `write` is called more than once - --> $DIR/open_options.rs:16:36 + --> $DIR/open_options.rs:19:36 | LL | OpenOptions::new().write(true).write(false).open("foo.txt"); | ^^^^^^^^^^^^ error: the method `append` is called more than once - --> $DIR/open_options.rs:18:37 + --> $DIR/open_options.rs:21:37 | LL | OpenOptions::new().append(true).append(false).open("foo.txt"); | ^^^^^^^^^^^^^ error: the method `truncate` is called more than once - --> $DIR/open_options.rs:20:39 + --> $DIR/open_options.rs:23:39 | LL | OpenOptions::new().truncate(true).truncate(false).open("foo.txt"); | ^^^^^^^^^^^^^^^ -error: file opened with `create`, but `truncate` behavior not defined - --> $DIR/open_options.rs:22:24 - | -LL | OpenOptions::new().create(true).open("foo.txt"); - | ^^^^^^^^^^^^ - | -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)` - --> $DIR/open_options.rs:22:5 +error: the method `read` is called more than once + --> $DIR/open_options.rs:26:41 | -LL | OpenOptions::new().create(true).open("foo.txt"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | std::fs::File::options().read(true).read(false).open("foo.txt"); + | ^^^^^^^^^^^ -error: aborting due to 9 previous errors +error: aborting due to 8 previous errors diff --git a/tests/ui/open_options_fixable.fixed b/tests/ui/open_options_fixable.fixed new file mode 100644 index 000000000000..90a129a9bdff --- /dev/null +++ b/tests/ui/open_options_fixable.fixed @@ -0,0 +1,7 @@ +use std::fs::OpenOptions; +#[allow(unused_must_use)] +#[warn(clippy::suspicious_open_options)] +fn main() { + OpenOptions::new().create(true).truncate(true).open("foo.txt"); + //~^ ERROR: file opened with `create`, but `truncate` behavior not defined +} diff --git a/tests/ui/open_options_fixable.rs b/tests/ui/open_options_fixable.rs new file mode 100644 index 000000000000..3a9e522ba150 --- /dev/null +++ b/tests/ui/open_options_fixable.rs @@ -0,0 +1,7 @@ +use std::fs::OpenOptions; +#[allow(unused_must_use)] +#[warn(clippy::suspicious_open_options)] +fn main() { + OpenOptions::new().create(true).open("foo.txt"); + //~^ ERROR: file opened with `create`, but `truncate` behavior not defined +} diff --git a/tests/ui/open_options_fixable.stderr b/tests/ui/open_options_fixable.stderr new file mode 100644 index 000000000000..1ef49c54afc2 --- /dev/null +++ b/tests/ui/open_options_fixable.stderr @@ -0,0 +1,12 @@ +error: file opened with `create`, but `truncate` behavior not defined + --> $DIR/open_options_fixable.rs:5:24 + | +LL | OpenOptions::new().create(true).open("foo.txt"); + | ^^^^^^^^^^^^- help: add: `.truncate(true)` + | + = 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)` + = note: `-D clippy::suspicious-open-options` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]` + +error: aborting due to previous error + From 515fe65ba8f1006bec5e4e906aaeb345b05834bc Mon Sep 17 00:00:00 2001 From: atwam Date: Thu, 11 Jan 2024 12:49:49 +0000 Subject: [PATCH 5/6] Fix conflicts - New ineffective_open_options had to be fixed. - Now not raising an issue on missing `truncate` when `append(true)` makes the intent clear. - Try implementing more advanced tests for non-chained operations. Fail --- clippy_lints/src/methods/open_options.rs | 31 ++++++++++++++++++------ tests/ui/ineffective_open_options.fixed | 9 ++++++- tests/ui/ineffective_open_options.rs | 9 ++++++- tests/ui/open_options.rs | 3 ++- tests/ui/open_options_fixable.stderr | 4 +-- 5 files changed, 44 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/methods/open_options.rs b/clippy_lints/src/methods/open_options.rs index cbe504a9c638..d45ba2a77285 100644 --- a/clippy_lints/src/methods/open_options.rs +++ b/clippy_lints/src/methods/open_options.rs @@ -1,8 +1,8 @@ use rustc_data_structures::fx::FxHashMap; use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; -use clippy_utils::paths; use clippy_utils::ty::{is_type_diagnostic_item, match_type}; +use clippy_utils::{match_any_def_paths, paths}; use rustc_ast::ast::LitKind; use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; @@ -19,7 +19,7 @@ fn is_open_options(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) { if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) && let Some(impl_id) = cx.tcx.impl_of_method(method_id) - && is_open_options(cx, cx.tcx.type_of(impl_id).instantiate_identity()) + //&& is_open_options(cx, cx.tcx.type_of(impl_id).instantiate_identity()) { let mut options = Vec::new(); get_open_options(cx, recv, &mut options); @@ -27,7 +27,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx } } -#[derive(Eq, PartialEq, Clone)] +#[derive(Eq, PartialEq, Clone, Debug)] enum Argument { Set(bool), Unknown, @@ -55,7 +55,11 @@ impl std::fmt::Display for OpenOption { } } -fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec<(OpenOption, Argument, Span)>) { +fn get_open_options( + cx: &LateContext<'_>, + argument: &Expr<'_>, + options: &mut Vec<(OpenOption, Argument, Span)>, +) -> bool { if let ExprKind::MethodCall(path, receiver, arguments, span) = argument.kind { let obj_ty = cx.typeck_results().expr_ty(receiver).peel_refs(); @@ -72,7 +76,8 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec } else { // The function is called with a literal which is not a boolean literal. // This is theoretically possible, but not very likely. - return; + // We'll ignore it for now + return get_open_options(cx, receiver, options); } }, _ => Argument::Unknown, @@ -100,8 +105,19 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec _ => (), } - get_open_options(cx, receiver, options); + get_open_options(cx, receiver, options) + } else { + false } + } else if let ExprKind::Call(callee, _) = argument.kind + && let ExprKind::Path(path) = callee.kind + && let Some(did) = cx.qpath_res(&path, callee.hir_id).opt_def_id() + { + match_any_def_paths(cx, did, &[&paths::TOKIO_IO_OPEN_OPTIONS]).is_some() + //is_type_diagnostic_item(cx, ty, sym::FsOpenOptions) || match_type(cx, ty, + // &paths::TOKIO_IO_OPEN_OPTIONS) + } else { + false } } @@ -144,6 +160,7 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, S if let Some((Argument::Set(true), create_span)) = options.get(&OpenOption::Create) && let None = options.get(&OpenOption::Truncate) + && let None | Some((Argument::Set(false), _)) = options.get(&OpenOption::Append) { span_lint_and_then( cx, @@ -153,7 +170,7 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, S |diag| { diag .span_suggestion(create_span.shrink_to_hi(), "add", ".truncate(true)".to_string(), rustc_errors::Applicability::MaybeIncorrect) - .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)`"); + .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)`. Alternatively, use `.append` to append to the file instead of overwriting it."); }, ); } diff --git a/tests/ui/ineffective_open_options.fixed b/tests/ui/ineffective_open_options.fixed index 3af8f3c5aaa4..88489dc46bbb 100644 --- a/tests/ui/ineffective_open_options.fixed +++ b/tests/ui/ineffective_open_options.fixed @@ -26,16 +26,23 @@ fn main() { .unwrap(); let file = OpenOptions::new() .create(true) + .truncate(true) .write(true) .append(false) .open("dump.json") .unwrap(); let file = OpenOptions::new() .create(true) + .truncate(true) .write(false) .append(false) .open("dump.json") .unwrap(); let file = OpenOptions::new().create(true).append(true).open("dump.json").unwrap(); - let file = OpenOptions::new().create(true).write(true).open("dump.json").unwrap(); + let file = OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open("dump.json") + .unwrap(); } diff --git a/tests/ui/ineffective_open_options.rs b/tests/ui/ineffective_open_options.rs index 4eaf6293c408..db8bca3e2acd 100644 --- a/tests/ui/ineffective_open_options.rs +++ b/tests/ui/ineffective_open_options.rs @@ -26,16 +26,23 @@ fn main() { .unwrap(); let file = OpenOptions::new() .create(true) + .truncate(true) .write(true) .append(false) .open("dump.json") .unwrap(); let file = OpenOptions::new() .create(true) + .truncate(true) .write(false) .append(false) .open("dump.json") .unwrap(); let file = OpenOptions::new().create(true).append(true).open("dump.json").unwrap(); - let file = OpenOptions::new().create(true).write(true).open("dump.json").unwrap(); + let file = OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open("dump.json") + .unwrap(); } diff --git a/tests/ui/open_options.rs b/tests/ui/open_options.rs index 726ec1b3ab04..873094dd7245 100644 --- a/tests/ui/open_options.rs +++ b/tests/ui/open_options.rs @@ -29,5 +29,6 @@ fn main() { let mut options = std::fs::OpenOptions::new(); options.read(true); options.read(false); - //~^ ERROR: the method `read` is called more than once + //#~^ ERROR: the method `read` is called more than once + options.open("foo.txt"); } diff --git a/tests/ui/open_options_fixable.stderr b/tests/ui/open_options_fixable.stderr index 1ef49c54afc2..de88af87cd41 100644 --- a/tests/ui/open_options_fixable.stderr +++ b/tests/ui/open_options_fixable.stderr @@ -4,9 +4,9 @@ error: file opened with `create`, but `truncate` behavior not defined LL | OpenOptions::new().create(true).open("foo.txt"); | ^^^^^^^^^^^^- help: add: `.truncate(true)` | - = 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)` + = 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)`. Alternatively, use `.append` to append to the file instead of overwriting it. = note: `-D clippy::suspicious-open-options` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]` -error: aborting due to previous error +error: aborting due to 1 previous error From f09cd88199e730e63a4ffb5df4ea13d64c834e2d Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sun, 14 Jan 2024 00:11:42 +0100 Subject: [PATCH 6/6] fix false positive in `suspicious_open_options`, make paths work --- clippy_lints/src/methods/mod.rs | 11 ++++-- clippy_lints/src/methods/open_options.rs | 49 +++++++++++++++++++----- clippy_utils/src/paths.rs | 10 ++++- tests/ui/open_options.rs | 24 ++++++++++-- tests/ui/open_options.stderr | 16 ++++---- tests/ui/open_options_fixable.stderr | 4 +- 6 files changed, 87 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 3d6a16f9ac61..03bcf108914a 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2833,10 +2833,11 @@ declare_clippy_lint! { /// without an explicit `OpenOptions::truncate()`. /// /// ### Why is this bad? - /// create() alone will either create a new file or open an + /// `create()` alone will either create a new file or open an /// existing file. If the file already exists, it will be /// overwritten when written to, but the file will not be - /// truncated by default. If less data is written to the file + /// truncated by default. + /// If less data is written to the file /// than it already contains, the remainder of the file will /// remain unchanged, and the end of the file will contain old /// data. @@ -2847,10 +2848,14 @@ declare_clippy_lint! { /// `truncate(false)` will explicitely keep the default behavior. /// /// ### Example - /// ```rust + /// ```rust,no_run /// use std::fs::OpenOptions; /// /// OpenOptions::new().create(true); + /// ``` + /// Use instead: + /// ```rust,no_run + /// use std::fs::OpenOptions; /// /// OpenOptions::new().create(true).truncate(true); /// ``` diff --git a/clippy_lints/src/methods/open_options.rs b/clippy_lints/src/methods/open_options.rs index d45ba2a77285..77484ab91a9d 100644 --- a/clippy_lints/src/methods/open_options.rs +++ b/clippy_lints/src/methods/open_options.rs @@ -19,11 +19,12 @@ fn is_open_options(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, recv: &'tcx Expr<'_>) { if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) && let Some(impl_id) = cx.tcx.impl_of_method(method_id) - //&& is_open_options(cx, cx.tcx.type_of(impl_id).instantiate_identity()) + && is_open_options(cx, cx.tcx.type_of(impl_id).instantiate_identity()) { let mut options = Vec::new(); - get_open_options(cx, recv, &mut options); - check_open_options(cx, &options, e.span); + if get_open_options(cx, recv, &mut options) { + check_open_options(cx, &options, e.span); + } } } @@ -55,6 +56,9 @@ impl std::fmt::Display for OpenOption { } } +/// Collects information about a method call chain on `OpenOptions`. +/// Returns false if an unexpected expression kind was found "on the way", +/// and linting should then be avoided. fn get_open_options( cx: &LateContext<'_>, argument: &Expr<'_>, @@ -102,7 +106,16 @@ fn get_open_options( "write" => { options.push((OpenOption::Write, argument_option, span)); }, - _ => (), + _ => { + // Avoid linting altogether if this method is from a trait. + // This might be a user defined extension trait with a method like `truncate_write` + // which would be a false positive + if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(argument.hir_id) + && cx.tcx.trait_of_item(method_def_id).is_some() + { + return false; + } + }, } get_open_options(cx, receiver, options) @@ -113,9 +126,17 @@ fn get_open_options( && let ExprKind::Path(path) = callee.kind && let Some(did) = cx.qpath_res(&path, callee.hir_id).opt_def_id() { - match_any_def_paths(cx, did, &[&paths::TOKIO_IO_OPEN_OPTIONS]).is_some() - //is_type_diagnostic_item(cx, ty, sym::FsOpenOptions) || match_type(cx, ty, - // &paths::TOKIO_IO_OPEN_OPTIONS) + match_any_def_paths( + cx, + did, + &[ + &paths::TOKIO_IO_OPEN_OPTIONS_NEW, + &paths::OPEN_OPTIONS_NEW, + &paths::FILE_OPTIONS, + &paths::TOKIO_FILE_OPTIONS, + ], + ) + .is_some() } else { false } @@ -168,9 +189,17 @@ fn check_open_options(cx: &LateContext<'_>, settings: &[(OpenOption, Argument, S *create_span, "file opened with `create`, but `truncate` behavior not defined", |diag| { - diag - .span_suggestion(create_span.shrink_to_hi(), "add", ".truncate(true)".to_string(), rustc_errors::Applicability::MaybeIncorrect) - .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)`. Alternatively, use `.append` to append to the file instead of overwriting it."); + diag.span_suggestion( + create_span.shrink_to_hi(), + "add", + ".truncate(true)".to_string(), + rustc_errors::Applicability::MaybeIncorrect, + ) + .help("if you intend to overwrite an existing file entirely, call `.truncate(true)`") + .help( + "if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`", + ) + .help("alternatively, use `.append(true)` to append to the file instead of overwriting it"); }, ); } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 2771e49736e6..0051f7845146 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -26,6 +26,7 @@ pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"]; pub const EARLY_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "EarlyLintPass"]; pub const F32_EPSILON: [&str; 4] = ["core", "f32", "", "EPSILON"]; pub const F64_EPSILON: [&str; 4] = ["core", "f64", "", "EPSILON"]; +pub const FILE_OPTIONS: [&str; 4] = ["std", "fs", "File", "options"]; #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const FUTURES_IO_ASYNCREADEXT: [&str; 3] = ["futures_util", "io", "AsyncReadExt"]; #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates @@ -50,8 +51,7 @@ pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"]; pub const LATE_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "LateLintPass"]; pub const LINT: [&str; 2] = ["rustc_lint_defs", "Lint"]; pub const MSRV: [&str; 3] = ["clippy_config", "msrvs", "Msrv"]; -#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates -pub const TOKIO_IO_OPEN_OPTIONS: [&str; 3] = ["tokio", "fs", "OpenOptions"]; +pub const OPEN_OPTIONS_NEW: [&str; 4] = ["std", "fs", "OpenOptions", "new"]; pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", "as_os_str"]; pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"]; pub const PARKING_LOT_MUTEX_GUARD: [&str; 3] = ["lock_api", "mutex", "MutexGuard"]; @@ -91,9 +91,15 @@ pub const SYMBOL_TO_IDENT_STRING: [&str; 4] = ["rustc_span", "symbol", "Symbol", pub const SYM_MODULE: [&str; 3] = ["rustc_span", "symbol", "sym"]; pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"]; #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates +pub const TOKIO_FILE_OPTIONS: [&str; 5] = ["tokio", "fs", "file", "File", "options"]; +#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const TOKIO_IO_ASYNCREADEXT: [&str; 5] = ["tokio", "io", "util", "async_read_ext", "AsyncReadExt"]; #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const TOKIO_IO_ASYNCWRITEEXT: [&str; 5] = ["tokio", "io", "util", "async_write_ext", "AsyncWriteExt"]; +#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates +pub const TOKIO_IO_OPEN_OPTIONS: [&str; 4] = ["tokio", "fs", "open_options", "OpenOptions"]; +#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates +pub const TOKIO_IO_OPEN_OPTIONS_NEW: [&str; 5] = ["tokio", "fs", "open_options", "OpenOptions", "new"]; pub const VEC_AS_MUT_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_mut_slice"]; pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"]; pub const VEC_DEQUE_ITER: [&str; 5] = ["alloc", "collections", "vec_deque", "VecDeque", "iter"]; diff --git a/tests/ui/open_options.rs b/tests/ui/open_options.rs index 873094dd7245..4acb3780df43 100644 --- a/tests/ui/open_options.rs +++ b/tests/ui/open_options.rs @@ -1,6 +1,18 @@ +#![allow(unused_must_use)] +#![warn(clippy::nonsensical_open_options)] + use std::fs::OpenOptions; -#[allow(unused_must_use)] -#[warn(clippy::nonsensical_open_options)] + +trait OpenOptionsExt { + fn truncate_write(&mut self, opt: bool) -> &mut Self; +} + +impl OpenOptionsExt for OpenOptions { + fn truncate_write(&mut self, opt: bool) -> &mut Self { + self.truncate(opt).write(opt) + } +} + fn main() { OpenOptions::new().read(true).truncate(true).open("foo.txt"); //~^ ERROR: file opened with `truncate` and `read` @@ -29,6 +41,12 @@ fn main() { let mut options = std::fs::OpenOptions::new(); options.read(true); options.read(false); - //#~^ ERROR: the method `read` is called more than once + // Possible future improvement: recognize open options method call chains across statements. options.open("foo.txt"); + + let mut options = std::fs::OpenOptions::new(); + options.truncate(true); + options.create(true).open("foo.txt"); + + OpenOptions::new().create(true).truncate_write(true).open("foo.txt"); } diff --git a/tests/ui/open_options.stderr b/tests/ui/open_options.stderr index 3e7d5a3499d3..4f84d1d09f98 100644 --- a/tests/ui/open_options.stderr +++ b/tests/ui/open_options.stderr @@ -1,5 +1,5 @@ error: file opened with `truncate` and `read` - --> $DIR/open_options.rs:5:5 + --> $DIR/open_options.rs:17:5 | LL | OpenOptions::new().read(true).truncate(true).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,43 +8,43 @@ LL | OpenOptions::new().read(true).truncate(true).open("foo.txt"); = help: to override `-D warnings` add `#[allow(clippy::nonsensical_open_options)]` error: file opened with `append` and `truncate` - --> $DIR/open_options.rs:8:5 + --> $DIR/open_options.rs:20:5 | LL | OpenOptions::new().append(true).truncate(true).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: the method `read` is called more than once - --> $DIR/open_options.rs:11:35 + --> $DIR/open_options.rs:23:35 | LL | OpenOptions::new().read(true).read(false).open("foo.txt"); | ^^^^^^^^^^^ error: the method `create` is called more than once - --> $DIR/open_options.rs:16:10 + --> $DIR/open_options.rs:28:10 | LL | .create(false) | ^^^^^^^^^^^^^ error: the method `write` is called more than once - --> $DIR/open_options.rs:19:36 + --> $DIR/open_options.rs:31:36 | LL | OpenOptions::new().write(true).write(false).open("foo.txt"); | ^^^^^^^^^^^^ error: the method `append` is called more than once - --> $DIR/open_options.rs:21:37 + --> $DIR/open_options.rs:33:37 | LL | OpenOptions::new().append(true).append(false).open("foo.txt"); | ^^^^^^^^^^^^^ error: the method `truncate` is called more than once - --> $DIR/open_options.rs:23:39 + --> $DIR/open_options.rs:35:39 | LL | OpenOptions::new().truncate(true).truncate(false).open("foo.txt"); | ^^^^^^^^^^^^^^^ error: the method `read` is called more than once - --> $DIR/open_options.rs:26:41 + --> $DIR/open_options.rs:38:41 | LL | std::fs::File::options().read(true).read(false).open("foo.txt"); | ^^^^^^^^^^^ diff --git a/tests/ui/open_options_fixable.stderr b/tests/ui/open_options_fixable.stderr index de88af87cd41..e327661713bf 100644 --- a/tests/ui/open_options_fixable.stderr +++ b/tests/ui/open_options_fixable.stderr @@ -4,7 +4,9 @@ error: file opened with `create`, but `truncate` behavior not defined LL | OpenOptions::new().create(true).open("foo.txt"); | ^^^^^^^^^^^^- help: add: `.truncate(true)` | - = 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)`. Alternatively, use `.append` to append to the file instead of overwriting it. + = help: if you intend to overwrite an existing file entirely, call `.truncate(true)` + = help: if you instead know that you may want to keep some parts of the old file, call `.truncate(false)` + = help: alternatively, use `.append(true)` to append to the file instead of overwriting it = note: `-D clippy::suspicious-open-options` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::suspicious_open_options)]`