fix false positive in suspicious_open_options, make paths work

Author: y21

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);
     /// ```

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");
             },
         );
     }

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,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"];

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");
 }

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");
    |                                         ^^^^^^^^^^^

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)]`