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..03bcf108914a 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2827,6 +2827,44 @@ 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,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); + /// ``` + #[clippy::version = "1.75.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 +4071,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..77484ab91a9d 100644 --- a/clippy_lints/src/methods/open_options.rs +++ b/clippy_lints/src/methods/open_options.rs @@ -1,46 +1,74 @@ -use clippy_utils::diagnostics::span_lint; -use clippy_utils::ty::is_type_diagnostic_item; +use rustc_data_structures::fx::FxHashMap; + +use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; +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; +use rustc_middle::ty::Ty; use rustc_span::source_map::Spanned; use rustc_span::{sym, Span}; -use super::NONSENSICAL_OPEN_OPTIONS; +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) + && 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); + } } } -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Eq, PartialEq, Clone, Debug)] 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)>) { - if let ExprKind::MethodCall(path, receiver, arguments, _) = argument.kind { +/// 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<'_>, + 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(); // 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_open_options(cx, obj_ty) { let argument_option = match arguments[0].kind { ExprKind::Lit(span) => { if let Spanned { @@ -48,11 +76,12 @@ 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. - return; + // We'll ignore it for now + return get_open_options(cx, receiver, options); } }, _ => Argument::Unknown, @@ -60,106 +89,77 @@ 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, 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)); + }, + _ => { + // 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); + 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_NEW, + &paths::OPEN_OPTIONS_NEW, + &paths::FILE_OPTIONS, + &paths::TOKIO_FILE_OPTIONS, + ], + ) + .is_some() + } else { + false } } -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. - - 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); - }, +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, sp) in settings { + if let Some((_, prev_span)) = options.insert(option.clone(), (arg.clone(), *sp)) { + span_lint( + cx, + NONSENSICAL_OPEN_OPTIONS, + prev_span, + &format!("the method `{}` is called more than once", &option), + ); } } - if read && truncate && read_arg && truncate_arg && !(write && write_arg) { + 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, @@ -167,7 +167,10 @@ fn check_open_options(cx: &LateContext<'_>, options: &[(OpenOption, Argument)], "file opened with `truncate` and `read`", ); } - if append && truncate && append_arg && truncate_arg { + + 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, @@ -175,4 +178,29 @@ fn check_open_options(cx: &LateContext<'_>, options: &[(OpenOption, Argument)], "file opened with `append` and `truncate`", ); } + + 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, + 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)`") + .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 0a820a1754ca..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,6 +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"]; +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"]; @@ -89,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/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 0cdc5bf2bb59..4acb3780df43 100644 --- a/tests/ui/open_options.rs +++ b/tests/ui/open_options.rs @@ -1,7 +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` @@ -11,12 +22,31 @@ 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 + + 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); + // 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 7ac826f52fa9..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:6:5 + --> $DIR/open_options.rs:17:5 | LL | OpenOptions::new().read(true).truncate(true).open("foo.txt"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,40 +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: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:12:5 + --> $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:14:5 + --> $DIR/open_options.rs:28:10 | -LL | OpenOptions::new().create(true).create(false).open("foo.txt"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | .create(false) + | ^^^^^^^^^^^^^ error: the method `write` is called more than once - --> $DIR/open_options.rs:16:5 + --> $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:18:5 + --> $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:20:5 + --> $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:38:41 + | +LL | std::fs::File::options().read(true).read(false).open("foo.txt"); + | ^^^^^^^^^^^ -error: aborting due to 7 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..e327661713bf --- /dev/null +++ b/tests/ui/open_options_fixable.stderr @@ -0,0 +1,14 @@ +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)` + = 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)]` + +error: aborting due to 1 previous error + 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()`