Add non_octal_unix_permissions lint #7001
Conversation
|
r? @Manishearth (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| 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); | ||
|
|
There was a problem hiding this comment.
We should only be linting if its parameter is a literal
| _ => return, | ||
| }; | ||
|
|
||
| if !snip.starts_with("0o") { |
There was a problem hiding this comment.
show_error should not be doing additional checks relevant to the lint IMO, these should be in the main lint
ebobrow
force-pushed
the
non-octal-file-permissions
branch
from
c5224f5
to
724ac87
Compare
|
@Manishearth Thanks for the review! I've applied the changes. Should I also be checking for a case where the parameter is a variable holding a non-octal value, like: let permissions = 440;
let mut options = OpenOptions::new();
options.mode(permissions);It seems to me like a pretty rare use case, but I could be wrong. Thoughts? |
|
I would rather not try to lint for that |
|
Okay, I won't do that then. Can you re-review my changes? |
|
@bors r+ |
|
📌 Commit 724ac87 has been approved by |
Add non_octal_unix_permissions lint fixes #6934 changelog: add new lint that checks for non-octal values used to set unix file permissions
|
💔 Test failed - checks-action_test |
ebobrow
force-pushed
the
non-octal-file-permissions
branch
from
724ac87
to
56a1aa1
Compare
|
Hopefully this fixes the error. |
ebobrow
force-pushed
the
non-octal-file-permissions
branch
from
56a1aa1
to
01b89aa
Compare
|
@bors r+ |
|
📌 Commit 01b89aa has been approved by |
Add non_octal_unix_permissions lint fixes #6934 changelog: add new lint that checks for non-octal values used to set unix file permissions
|
💔 Test failed - checks-action_test |
|
@Manishearth I might need some help with this. It looks like the test is failing on windows because of the unix-specific features. I thought that adding |
clippy_lints/src/lib.rs
Outdated
| @@ -1057,6 +1059,8 @@ 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); | |||
| #[cfg(target_family = "unix")] | |||
ebobrow
force-pushed
the
non-octal-file-permissions
branch
from
01b89aa
to
9f8cb2d
Compare
|
Thank you! |
|
@bors r+ |
|
📌 Commit 9f8cb2d has been approved by |
Add non_octal_unix_permissions lint fixes #6934 changelog: add new lint that checks for non-octal values used to set unix file permissions
|
💔 Test failed - checks-action_test |
|
Unsure why ignore-windows isn't working. @oli-obk any idea? |
|
It looks like it did work. This time the error is coming from the example in declare_clippy_lint. Should I just add |
|
I think we want to run the test on Linux at least |
ebobrow
force-pushed
the
non-octal-file-permissions
branch
from
9f8cb2d
to
81598f3
Compare
|
@bors try |
Add non_octal_unix_permissions lint fixes #6934 changelog: add new lint that checks for non-octal values used to set unix file permissions
|
💔 Test failed - checks-action_test |
|
|
|
Oh! that's a doctest! no, it won't work; I think you should just |
ebobrow
force-pushed
the
non-octal-file-permissions
branch
from
81598f3
to
7fcd155
Compare
|
@bors r+ |
|
📌 Commit 7fcd155 has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
fixes #6934
changelog: add new lint that checks for non-octal values used to set unix file permissions