about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-01-16 21:33:04 +0000
committerbors <bors@rust-lang.org>2024-01-16 21:33:04 +0000
commit5f3a06023aae5e7202381855fb30c7d759bd30a6 (patch)
tree87e724220cd44a91ddfae1c5252ac42c91787c5b
parentca7b54b085a291a8554648946c80c36f2279f837 (diff)
parentf09cd88199e730e63a4ffb5df4ea13d64c834e2d (diff)
downloadrust-5f3a06023aae5e7202381855fb30c7d759bd30a6.tar.gz
rust-5f3a06023aae5e7202381855fb30c7d759bd30a6.zip
Auto merge of #11608 - atwam:suspicious-open-options, r=y21
Add suspicious_open_options lint.

changelog: [`suspicious_open_options`]: Checks for the suspicious use of std::fs::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);
```

- [x] Followed [lint naming conventions][lint_naming]
- [x] Added passing UI tests (including committed `.stderr` file)
- [x] `cargo test` passes locally
- [x] Executed `cargo dev update_lints`
- [x] Added lint documentation
- [x] Run `cargo dev fmt`
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/methods/mod.rs39
-rw-r--r--clippy_lints/src/methods/open_options.rs230
-rw-r--r--clippy_utils/src/paths.rs8
-rw-r--r--tests/ui/ineffective_open_options.fixed9
-rw-r--r--tests/ui/ineffective_open_options.rs9
-rw-r--r--tests/ui/open_options.rs38
-rw-r--r--tests/ui/open_options.stderr34
-rw-r--r--tests/ui/open_options_fixable.fixed7
-rw-r--r--tests/ui/open_options_fixable.rs7
-rw-r--r--tests/ui/open_options_fixable.stderr14
-rw-r--r--tests/ui/seek_to_start_instead_of_rewind.fixed3
-rw-r--r--tests/ui/seek_to_start_instead_of_rewind.rs3
-rw-r--r--tests/ui/seek_to_start_instead_of_rewind.stderr2
15 files changed, 283 insertions, 122 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4d32bbec914..db4b4c2aef6 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 20230106d53..639edd8da30 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 89ea3597dc0..03bcf108914 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -2829,6 +2829,44 @@ declare_clippy_lint! {
 
 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)
     /// calls on `PathBuf` that can cause overwrites.
     ///
@@ -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 65c986dcacc..77484ab91a9 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 0a820a1754c..0051f784514 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", "<impl f32>", "EPSILON"];
 pub const F64_EPSILON: [&str; 4] = ["core", "f64", "<impl 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 3af8f3c5aaa..88489dc46bb 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 4eaf6293c40..db8bca3e2ac 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 0cdc5bf2bb5..4acb3780df4 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 7ac826f52fa..4f84d1d09f9 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 00000000000..90a129a9bdf
--- /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 00000000000..3a9e522ba15
--- /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 00000000000..e327661713b
--- /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 15cc8d54faa..8859a68320f 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 197225ffbd5..7b72efb34ff 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 05c11cf7f8c..b6b0d2effa8 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()`