Auto merge of rust-lang#7001 - ebobrow:non-octal-file-permissions, r=Manishearth

Add non_octal_unix_permissions lint

fixes rust-lang#6934

changelog: add new lint that checks for non-octal values used to set unix file permissions

Author: bors

CHANGELOG.md

@@ -2382,6 +2382,7 @@ Released 2018-09-13
 [`new_without_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
 [`no_effect`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect
 [`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal
+[`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions
 [`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
 [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
 [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref

README.md

@@ -5,7 +5,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 400 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 450 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
 You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.

clippy_lints/src/lib.rs

@@ -297,6 +297,7 @@ mod new_without_default;
 mod no_effect;
 mod non_copy_const;
 mod non_expressive_names;
+mod non_octal_unix_permissions;
 mod open_options;
 mod option_env_unwrap;
 mod option_if_let_else;
@@ -873,6 +874,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &non_expressive_names::JUST_UNDERSCORES_AND_DIGITS,
         &non_expressive_names::MANY_SINGLE_CHAR_NAMES,
         &non_expressive_names::SIMILAR_NAMES,
+        &non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS,
         &open_options::NONSENSICAL_OPEN_OPTIONS,
         &option_env_unwrap::OPTION_ENV_UNWRAP,
         &option_if_let_else::OPTION_IF_LET_ELSE,
@@ -1057,6 +1059,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box implicit_saturating_sub::ImplicitSaturatingSub);
     store.register_late_pass(|| box default_numeric_fallback::DefaultNumericFallback);
     store.register_late_pass(|| box inconsistent_struct_constructor::InconsistentStructConstructor);
+    store.register_late_pass(|| box non_octal_unix_permissions::NonOctalUnixPermissions);
 
     let msrv = conf.msrv.as_ref().and_then(|s| {
         parse_msrv(s, None, None).or_else(|| {
@@ -1647,6 +1650,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
         LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
         LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
+        LintId::of(&non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
         LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
         LintId::of(&option_env_unwrap::OPTION_ENV_UNWRAP),
         LintId::of(&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
@@ -1987,6 +1991,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&misc::FLOAT_CMP),
         LintId::of(&misc::MODULO_ONE),
         LintId::of(&mut_key::MUTABLE_KEY_TYPE),
+        LintId::of(&non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
         LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
         LintId::of(&option_env_unwrap::OPTION_ENV_UNWRAP),
         LintId::of(&ptr::MUT_FROM_REF),

clippy_lints/src/non_octal_unix_permissions.rs

@@ -0,0 +1,106 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::{snippet_opt, snippet_with_applicability};
+use clippy_utils::ty::match_type;
+use clippy_utils::{match_def_path, paths};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+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 non-octal values used to set Unix file permissions.
+    ///
+    /// **Why is this bad?** They will be converted into octal, creating potentially
+    /// unintended file permissions.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust,ignore
+    /// use std::fs::OpenOptions;
+    /// use std::os::unix::fs::OpenOptionsExt;
+    ///
+    /// let mut options = OpenOptions::new();
+    /// options.mode(644);
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    /// use std::fs::OpenOptions;
+    /// use std::os::unix::fs::OpenOptionsExt;
+    ///
+    /// let mut options = OpenOptions::new();
+    /// options.mode(0o644);
+    /// ```
+    pub NON_OCTAL_UNIX_PERMISSIONS,
+    correctness,
+    "use of non-octal value to set unix file permissions, which will be translated into octal"
+}
+
+declare_lint_pass!(NonOctalUnixPermissions => [NON_OCTAL_UNIX_PERMISSIONS]);
+
+impl LateLintPass<'_> for NonOctalUnixPermissions {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        match &expr.kind {
+            ExprKind::MethodCall(path, _, [func, param], _) => {
+                let obj_ty = cx.typeck_results().expr_ty(&func).peel_refs();
+
+                if_chain! {
+                    if (path.ident.name == sym!(mode)
+                        && (match_type(cx, obj_ty, &paths::OPEN_OPTIONS)
+                            || match_type(cx, obj_ty, &paths::DIR_BUILDER)))
+                        || (path.ident.name == sym!(set_mode) && match_type(cx, obj_ty, &paths::PERMISSIONS));
+                    if let ExprKind::Lit(_) = param.kind;
+
+                    then {
+                        let snip = match snippet_opt(cx, param.span) {
+                            Some(s) => s,
+                            _ => return,
+                        };
+
+                        if !snip.starts_with("0o") {
+                            show_error(cx, param);
+                        }
+                    }
+                }
+            },
+            ExprKind::Call(ref func, [param]) => {
+                if_chain! {
+                    if let ExprKind::Path(ref path) = func.kind;
+                    if let Some(def_id) = cx.qpath_res(path, func.hir_id).opt_def_id();
+                    if match_def_path(cx, def_id, &paths::PERMISSIONS_FROM_MODE);
+                    if let ExprKind::Lit(_) = param.kind;
+
+                    then {
+                        let snip = match snippet_opt(cx, param.span) {
+                            Some(s) => s,
+                            _ => return,
+                        };
+
+                        if !snip.starts_with("0o") {
+                            show_error(cx, param);
+                        }
+                    }
+                }
+            },
+            _ => {},
+        };
+    }
+}
+
+fn show_error(cx: &LateContext<'_>, param: &Expr<'_>) {
+    let mut applicability = Applicability::MachineApplicable;
+    span_lint_and_sugg(
+        cx,
+        NON_OCTAL_UNIX_PERMISSIONS,
+        param.span,
+        "using a non-octal value to set unix file permissions",
+        "consider using an octal literal instead",
+        format!(
+            "0o{}",
+            snippet_with_applicability(cx, param.span, "0o..", &mut applicability,),
+        ),
+        applicability,
+    );
+}

clippy_utils/src/paths.rs

@@ -26,6 +26,7 @@ pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"];
 pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"];
 pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"];
 pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];
+pub const DIR_BUILDER: [&str; 3] = ["std", "fs", "DirBuilder"];
 pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"];
 pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"];
 pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"];
@@ -95,6 +96,8 @@ pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWri
 pub const PATH_BUF: [&str; 3] = ["std", "path", "PathBuf"];
 pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"];
 pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"];
+pub const PERMISSIONS: [&str; 3] = ["std", "fs", "Permissions"];
+pub const PERMISSIONS_FROM_MODE: [&str; 7] = ["std", "sys", "unix", "ext", "fs", "PermissionsExt", "from_mode"];
 pub const POLL: [&str; 4] = ["core", "task", "poll", "Poll"];
 pub const POLL_PENDING: [&str; 5] = ["core", "task", "poll", "Poll", "Pending"];
 pub const POLL_READY: [&str; 5] = ["core", "task", "poll", "Poll", "Ready"];

tests/ui/non_octal_unix_permissions.fixed

@@ -0,0 +1,33 @@
+// ignore-windows
+// run-rustfix
+#![warn(clippy::non_octal_unix_permissions)]
+use std::fs::{DirBuilder, File, OpenOptions, Permissions};
+use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt, PermissionsExt};
+
+fn main() {
+    let permissions = 0o760;
+
+    // OpenOptionsExt::mode
+    let mut options = OpenOptions::new();
+    options.mode(0o440);
+    options.mode(0o400);
+    options.mode(permissions);
+
+    // PermissionsExt::from_mode
+    let _permissions = Permissions::from_mode(0o647);
+    let _permissions = Permissions::from_mode(0o000);
+    let _permissions = Permissions::from_mode(permissions);
+
+    // PermissionsExt::set_mode
+    let f = File::create("foo.txt").unwrap();
+    let metadata = f.metadata().unwrap();
+    let mut permissions = metadata.permissions();
+
+    permissions.set_mode(0o644);
+    permissions.set_mode(0o704);
+
+    // DirBuilderExt::mode
+    let mut builder = DirBuilder::new();
+    builder.mode(0o755);
+    builder.mode(0o406);
+}

tests/ui/non_octal_unix_permissions.rs

@@ -0,0 +1,33 @@
+// ignore-windows
+// run-rustfix
+#![warn(clippy::non_octal_unix_permissions)]
+use std::fs::{DirBuilder, File, OpenOptions, Permissions};
+use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt, PermissionsExt};
+
+fn main() {
+    let permissions = 0o760;
+
+    // OpenOptionsExt::mode
+    let mut options = OpenOptions::new();
+    options.mode(440);
+    options.mode(0o400);
+    options.mode(permissions);
+
+    // PermissionsExt::from_mode
+    let _permissions = Permissions::from_mode(647);
+    let _permissions = Permissions::from_mode(0o000);
+    let _permissions = Permissions::from_mode(permissions);
+
+    // PermissionsExt::set_mode
+    let f = File::create("foo.txt").unwrap();
+    let metadata = f.metadata().unwrap();
+    let mut permissions = metadata.permissions();
+
+    permissions.set_mode(644);
+    permissions.set_mode(0o704);
+
+    // DirBuilderExt::mode
+    let mut builder = DirBuilder::new();
+    builder.mode(755);
+    builder.mode(0o406);
+}

tests/ui/non_octal_unix_permissions.stderr

@@ -0,0 +1,28 @@
+error: using a non-octal value to set unix file permissions
+  --> $DIR/non_octal_unix_permissions.rs:12:18
+   |
+LL |     options.mode(440);
+   |                  ^^^ help: consider using an octal literal instead: `0o440`
+   |
+   = note: `-D clippy::non-octal-unix-permissions` implied by `-D warnings`
+
+error: using a non-octal value to set unix file permissions
+  --> $DIR/non_octal_unix_permissions.rs:17:47
+   |
+LL |     let _permissions = Permissions::from_mode(647);
+   |                                               ^^^ help: consider using an octal literal instead: `0o647`
+
+error: using a non-octal value to set unix file permissions
+  --> $DIR/non_octal_unix_permissions.rs:26:26
+   |
+LL |     permissions.set_mode(644);
+   |                          ^^^ help: consider using an octal literal instead: `0o644`
+
+error: using a non-octal value to set unix file permissions
+  --> $DIR/non_octal_unix_permissions.rs:31:18
+   |
+LL |     builder.mode(755);
+   |                  ^^^ help: consider using an octal literal instead: `0o755`
+
+error: aborting due to 4 previous errors
+