From 30e6e855088207a3fca8f1efb516f5b5dd43d1dd Mon Sep 17 00:00:00 2001 From: dboso Date: Fri, 28 Oct 2022 20:07:17 +0000 Subject: [PATCH 1/3] add [`permissions_set_readonly_false`] #9702 --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + .../src/permissions_set_readonly_false.rs | 48 +++++++++++++++++++ tests/ui/permissions_set_readonly_false.rs | 29 +++++++++++ .../ui/permissions_set_readonly_false.stderr | 11 +++++ 6 files changed, 92 insertions(+) create mode 100644 clippy_lints/src/permissions_set_readonly_false.rs create mode 100644 tests/ui/permissions_set_readonly_false.rs create mode 100644 tests/ui/permissions_set_readonly_false.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 17ff182c7be..52efa8db485 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4461,6 +4461,7 @@ Released 2018-09-13 [`partialeq_to_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_to_none [`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite [`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch +[`permissions_set_readonly_false`]: https://rust-lang.github.io/rust-clippy/master/index.html#permissions_set_readonly_false [`positional_named_format_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#positional_named_format_parameters [`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma [`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 480a65ac70c..980e128cba4 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -495,6 +495,7 @@ crate::pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE_INFO, crate::pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF_INFO, crate::pattern_type_mismatch::PATTERN_TYPE_MISMATCH_INFO, + crate::permissions_set_readonly_false::PERMISSIONS_SET_READONLY_FALSE_INFO, crate::precedence::PRECEDENCE_INFO, crate::ptr::CMP_NULL_INFO, crate::ptr::INVALID_NULL_PTR_USAGE_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 7f7d73339e4..676ef3946bf 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -235,6 +235,7 @@ mod partialeq_to_none; mod pass_by_ref_or_value; mod pattern_type_mismatch; +mod permissions_set_readonly_false; mod precedence; mod ptr; mod ptr_offset_with_cast; @@ -904,6 +905,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move |_| Box::new(manual_is_ascii_check::ManualIsAsciiCheck::new(msrv()))); store.register_late_pass(|_| Box::new(semicolon_block::SemicolonBlock)); store.register_late_pass(|_| Box::new(fn_null_check::FnNullCheck)); + store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/permissions_set_readonly_false.rs b/clippy_lints/src/permissions_set_readonly_false.rs new file mode 100644 index 00000000000..7d787ea4a4c --- /dev/null +++ b/clippy_lints/src/permissions_set_readonly_false.rs @@ -0,0 +1,48 @@ +use clippy_utils::diagnostics::span_lint_and_note; +use clippy_utils::paths; +use clippy_utils::ty::match_type; +use rustc_ast::ast::LitKind; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for calls to `std::fs::Permissions.set_readonly` with argument `false` + /// + /// ### Why is this bad? + /// On Unix platforms this results in the file being world writable + /// + /// ### Example + /// ```rust + /// use std::fs::File; + /// let f = File::create("foo.txt").unwrap(); + /// let metadata = f.metadata().unwrap(); + /// let mut permissions = metadata.permissions(); + /// permissions.set_readonly(false); + /// ``` + #[clippy::version = "1.66.0"] + pub PERMISSIONS_SET_READONLY_FALSE, + suspicious, + "Checks for calls to `std::fs::Permissions.set_readonly` with argument `false`" +} +declare_lint_pass!(PermissionsSetReadonlyFalse => [PERMISSIONS_SET_READONLY_FALSE]); + +impl<'tcx> LateLintPass<'tcx> for PermissionsSetReadonlyFalse { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if let ExprKind::MethodCall(path, receiver, [arg], _) = &expr.kind + && match_type(cx, cx.typeck_results().expr_ty(receiver), &paths::PERMISSIONS) + && path.ident.name == sym!(set_readonly) + && let ExprKind::Lit(lit) = &arg.kind + && LitKind::Bool(false) == lit.node { + span_lint_and_note( + cx, + PERMISSIONS_SET_READONLY_FALSE, + expr.span, + "call to `set_readonly` with argument `false`", + None, + "on Unix platforms this results in the file being world writable", + ); + } + } +} diff --git a/tests/ui/permissions_set_readonly_false.rs b/tests/ui/permissions_set_readonly_false.rs new file mode 100644 index 00000000000..28c00d10094 --- /dev/null +++ b/tests/ui/permissions_set_readonly_false.rs @@ -0,0 +1,29 @@ +#![allow(unused)] +#![warn(clippy::permissions_set_readonly_false)] + +use std::fs::File; + +struct A; + +impl A { + pub fn set_readonly(&mut self, b: bool) {} +} + +fn set_readonly(b: bool) {} + +fn main() { + let f = File::create("foo.txt").unwrap(); + let metadata = f.metadata().unwrap(); + let mut permissions = metadata.permissions(); + // lint here + permissions.set_readonly(false); + // no lint + permissions.set_readonly(true); + + let mut a = A; + // no lint here - a is not of type std::fs::Permissions + a.set_readonly(false); + + // no lint here - plain function + set_readonly(false); +} diff --git a/tests/ui/permissions_set_readonly_false.stderr b/tests/ui/permissions_set_readonly_false.stderr new file mode 100644 index 00000000000..106d2f793a4 --- /dev/null +++ b/tests/ui/permissions_set_readonly_false.stderr @@ -0,0 +1,11 @@ +error: call to `set_readonly` with argument `false` + --> $DIR/permissions_set_readonly_false.rs:19:5 + | +LL | permissions.set_readonly(false); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: on Unix platforms this results in the file being world writable + = note: `-D clippy::permissions-set-readonly-false` implied by `-D warnings` + +error: aborting due to previous error + From e1d3c1e8f1f2d7bd819fbaafe80de74e67ebb488 Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 17 Dec 2022 20:32:46 +0900 Subject: [PATCH 2/3] refactor: fix style --- .../src/permissions_set_readonly_false.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/permissions_set_readonly_false.rs b/clippy_lints/src/permissions_set_readonly_false.rs index 7d787ea4a4c..6c463bd8fce 100644 --- a/clippy_lints/src/permissions_set_readonly_false.rs +++ b/clippy_lints/src/permissions_set_readonly_false.rs @@ -34,15 +34,16 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { && match_type(cx, cx.typeck_results().expr_ty(receiver), &paths::PERMISSIONS) && path.ident.name == sym!(set_readonly) && let ExprKind::Lit(lit) = &arg.kind - && LitKind::Bool(false) == lit.node { - span_lint_and_note( - cx, - PERMISSIONS_SET_READONLY_FALSE, - expr.span, - "call to `set_readonly` with argument `false`", - None, - "on Unix platforms this results in the file being world writable", - ); + && LitKind::Bool(false) == lit.node + { + span_lint_and_note( + cx, + PERMISSIONS_SET_READONLY_FALSE, + expr.span, + "call to `set_readonly` with argument `false`", + None, + "on Unix platforms this results in the file being world writable", + ); } } } From b21cc36ee92a2cf3c387b47e40faa2aa4d39fd1a Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 17 Dec 2022 21:13:02 +0900 Subject: [PATCH 3/3] hotfix: add help dialog for `PermissionExt` --- .../src/permissions_set_readonly_false.rs | 33 ++++++++++--------- .../ui/permissions_set_readonly_false.stderr | 2 ++ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/permissions_set_readonly_false.rs b/clippy_lints/src/permissions_set_readonly_false.rs index 6c463bd8fce..e7095ec191f 100644 --- a/clippy_lints/src/permissions_set_readonly_false.rs +++ b/clippy_lints/src/permissions_set_readonly_false.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::span_lint_and_note; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::paths; use clippy_utils::ty::match_type; use rustc_ast::ast::LitKind; @@ -8,11 +8,11 @@ declare_clippy_lint! { /// ### What it does - /// Checks for calls to `std::fs::Permissions.set_readonly` with argument `false` + /// Checks for calls to `std::fs::Permissions.set_readonly` with argument `false`. /// /// ### Why is this bad? - /// On Unix platforms this results in the file being world writable - /// + /// On Unix platforms this results in the file being world writable, + /// equivalent to `chmod a+w `. /// ### Example /// ```rust /// use std::fs::File; @@ -32,18 +32,21 @@ impl<'tcx> LateLintPass<'tcx> for PermissionsSetReadonlyFalse { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if let ExprKind::MethodCall(path, receiver, [arg], _) = &expr.kind && match_type(cx, cx.typeck_results().expr_ty(receiver), &paths::PERMISSIONS) - && path.ident.name == sym!(set_readonly) - && let ExprKind::Lit(lit) = &arg.kind - && LitKind::Bool(false) == lit.node + && path.ident.name == sym!(set_readonly) + && let ExprKind::Lit(lit) = &arg.kind + && LitKind::Bool(false) == lit.node { - span_lint_and_note( - cx, - PERMISSIONS_SET_READONLY_FALSE, - expr.span, - "call to `set_readonly` with argument `false`", - None, - "on Unix platforms this results in the file being world writable", - ); + span_lint_and_then( + cx, + PERMISSIONS_SET_READONLY_FALSE, + expr.span, + "call to `set_readonly` with argument `false`", + |diag| { + diag.note("on Unix platforms this results in the file being world writable"); + diag.help("you can set the desired permissions using `PermissionsExt`. For more information, see\n\ + https://doc.rust-lang.org/std/os/unix/fs/trait.PermissionsExt.html"); + } + ); } } } diff --git a/tests/ui/permissions_set_readonly_false.stderr b/tests/ui/permissions_set_readonly_false.stderr index 106d2f793a4..e7a8ee6cb19 100644 --- a/tests/ui/permissions_set_readonly_false.stderr +++ b/tests/ui/permissions_set_readonly_false.stderr @@ -5,6 +5,8 @@ LL | permissions.set_readonly(false); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: on Unix platforms this results in the file being world writable + = help: you can set the desired permissions using `PermissionsExt`. For more information, see + https://doc.rust-lang.org/std/os/unix/fs/trait.PermissionsExt.html = note: `-D clippy::permissions-set-readonly-false` implied by `-D warnings` error: aborting due to previous error