Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add non_octal_unix_permissions lint #7001

Merged
merged 1 commit into from
Mar 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(|| {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
106 changes: 106 additions & 0 deletions clippy_lints/src/non_octal_unix_permissions.rs
Original file line number Diff line number Diff line change
@@ -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;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only be linting if its parameter is a literal

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,
);
}
3 changes: 3 additions & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
Expand Down Expand Up @@ -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"];
Expand Down
33 changes: 33 additions & 0 deletions tests/ui/non_octal_unix_permissions.fixed
Original file line number Diff line number Diff line change
@@ -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);
}
33 changes: 33 additions & 0 deletions tests/ui/non_octal_unix_permissions.rs
Original file line number Diff line number Diff line change
@@ -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);
}
28 changes: 28 additions & 0 deletions tests/ui/non_octal_unix_permissions.stderr
Original file line number Diff line number Diff line change
@@ -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