OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Add rc_buffer lint for Rc<String> and other buffer types

Browse Source
This commit is contained in:
Robin Schoonover 2020-09-13 21:52:25 -06:00
parent 231444d989
commit 1b5317f68b
7 changed files with 135 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -1775,6 +1775,7 @@ Released 2018-09-13
[`range_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_plus_one
[`range_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_step_by_zero
[`range_zip_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_zip_with_len
[`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer
[`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation
[`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
[`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

2
clippy_lints/src/lib.rs
View File

@ -837,6 +837,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&types::LET_UNIT_VALUE,
&types::LINKEDLIST,
&types::OPTION_OPTION,
&types::RC_BUFFER,
&types::REDUNDANT_ALLOCATION,
&types::TYPE_COMPLEXITY,
&types::UNIT_ARG,
@ -1804,6 +1805,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE),
LintId::of(&transmute::USELESS_TRANSMUTE),
LintId::of(&types::RC_BUFFER),
LintId::of(&use_self::USE_SELF),
]);
}

84
clippy_lints/src/types.rs
View File

@ -215,11 +215,41 @@ declare_clippy_lint! {
"redundant allocation"
}
declare_clippy_lint! {
/// **What it does:** Checks for Rc<T> and Arc<T> when T is a mutable buffer type such as String or Vec
///
/// **Why is this bad?** Expressions such as Rc<String> have no advantage over Rc<str>, since
/// it is larger and involves an extra level of indirection, and doesn't implement Borrow<str>.
///
/// While mutating a buffer type would still be possible with Rc::get_mut(), it only
/// works if there are no additional references yet, which defeats the purpose of
/// enclosing it in a shared ownership type. Instead, additionally wrapping the inner
/// type with an interior mutable container (such as RefCell or Mutex) would normally
/// be used.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// # use std::rc::Rc;
/// fn foo(interned: Rc<String>) { ... }
/// ```
///
/// Better:
///
/// ```rust,ignore
/// fn foo(interned: Rc<str>) { ... }
/// ```
pub RC_BUFFER,
nursery,
"shared ownership of a buffer type"
}
pub struct Types {
vec_box_size_threshold: u64,
}
impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION]);
impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER]);
impl<'tcx> LateLintPass<'tcx> for Types {
fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId) {
@ -272,6 +302,19 @@ fn match_type_parameter(cx: &LateContext<'_>, qpath: &QPath<'_>, path: &[&str])
None
}
fn match_buffer_type(cx: &LateContext<'_>, qpath: &QPath<'_>) -> Option<&'static str> {
if match_type_parameter(cx, qpath, &paths::STRING).is_some() {
return Some("str");
}
if match_type_parameter(cx, qpath, &paths::OS_STRING).is_some() {
return Some("std::ffi::OsStr");
}
if match_type_parameter(cx, qpath, &paths::PATH_BUF).is_some() {
return Some("std::path::Path");
}
None
}
fn match_borrows_parameter(_cx: &LateContext<'_>, qpath: &QPath<'_>) -> Option<Span> {
let last = last_path_segment(qpath);
if_chain! {
@ -385,6 +428,45 @@ impl Types {
);
return; // don't recurse into the type
}
if let Some(alternate) = match_buffer_type(cx, qpath) {
span_lint_and_sugg(
cx,
RC_BUFFER,
hir_ty.span,
"usage of `Rc<T>` when T is a buffer type",
"try",
format!("Rc<{}>", alternate),
Applicability::MachineApplicable,
);
return; // don't recurse into the type
}
if match_type_parameter(cx, qpath, &paths::VEC).is_some() {
let vec_ty = match &last_path_segment(qpath).args.unwrap().args[0] {
GenericArg::Type(ty) => match &ty.kind {
TyKind::Path(qpath) => qpath,
_ => return,
},
_ => return,
};
let inner_span = match &last_path_segment(&vec_ty).args.unwrap().args[0] {
GenericArg::Type(ty) => ty.span,
_ => return,
};
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
RC_BUFFER,
hir_ty.span,
"usage of `Rc<T>` when T is a buffer type",
"try",
format!(
"Rc<[{}]>",
snippet_with_applicability(cx, inner_span, "..", &mut applicability)
),
Applicability::MachineApplicable,
);
return; // don't recurse into the type
}
if let Some(span) = match_borrows_parameter(cx, qpath) {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(

1
clippy_lints/src/utils/paths.rs
View File

@ -113,6 +113,7 @@ pub const STD_CONVERT_IDENTITY: [&str; 3] = ["std", "convert", "identity"];
pub const STD_FS_CREATE_DIR: [&str; 3] = ["std", "fs", "create_dir"];
pub const STD_MEM_TRANSMUTE: [&str; 3] = ["std", "mem", "transmute"];
pub const STD_PTR_NULL: [&str; 3] = ["std", "ptr", "null"];
pub const STRING: [&str; 3] = ["alloc", "string", "String"];
pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"];
pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"];
pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"];

7
src/lintlist/mod.rs
View File

@ -1851,6 +1851,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "ranges",
},
Lint {
name: "rc_buffer",
group: "nursery",
desc: "shared ownership of a buffer type",
deprecation: None,
module: "types",
},
Lint {
name: "redundant_allocation",
group: "perf",

13
tests/ui/rc_buffer.rs Normal file
View File

@ -0,0 +1,13 @@
use std::ffi::OsString;
use std::path::PathBuf;
use std::rc::Rc;
#[warn(clippy::rc_buffer)]
struct S {
a: Rc<String>,
b: Rc<PathBuf>,
c: Rc<Vec<u8>>,
d: Rc<OsString>,
}
fn main() {}

28
tests/ui/rc_buffer.stderr Normal file
View File

@ -0,0 +1,28 @@
error: usage of `Rc<T>` when T is a buffer type
--> $DIR/rc_buffer.rs:7:8
|
LL | a: Rc<String>,
| ^^^^^^^^^^ help: try: `Rc<str>`
|
= note: `-D clippy::rc-buffer` implied by `-D warnings`
error: usage of `Rc<T>` when T is a buffer type
--> $DIR/rc_buffer.rs:8:8
|
LL | b: Rc<PathBuf>,
| ^^^^^^^^^^^ help: try: `Rc<std::path::Path>`
error: usage of `Rc<T>` when T is a buffer type
--> $DIR/rc_buffer.rs:9:8
|
LL | c: Rc<Vec<u8>>,
| ^^^^^^^^^^^ help: try: `Rc<[u8]>`
error: usage of `Rc<T>` when T is a buffer type
--> $DIR/rc_buffer.rs:10:8
|
LL | d: Rc<OsString>,
| ^^^^^^^^^^^^ help: try: `Rc<std::ffi::OsStr>`
error: aborting due to 4 previous errors