Skip to content

Commit

Permalink
add owned_cow lint
Browse files Browse the repository at this point in the history
  • Loading branch information
llogiq committed Jan 15, 2025
1 parent 0db6411 commit eaa33b6
Show file tree
Hide file tree
Showing 9 changed files with 205 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5934,6 +5934,7 @@ Released 2018-09-13
[`out_of_bounds_indexing`]: https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
[`overflow_check_conditional`]: https://rust-lang.github.io/rust-clippy/master/index.html#overflow_check_conditional
[`overly_complex_bool_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#overly_complex_bool_expr
[`owned_cow`]: https://rust-lang.github.io/rust-clippy/master/index.html#owned_cow
[`panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic
[`panic_in_result_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_in_result_fn
[`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
Expand Down
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat
* [`linkedlist`](https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist)
* [`needless_pass_by_ref_mut`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut)
* [`option_option`](https://rust-lang.github.io/rust-clippy/master/index.html#option_option)
* [`owned_cow`](https://rust-lang.github.io/rust-clippy/master/index.html#owned_cow)
* [`rc_buffer`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer)
* [`rc_mutex`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex)
* [`redundant_allocation`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation)
Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ define_Conf! {
linkedlist,
needless_pass_by_ref_mut,
option_option,
owned_cow,
rc_buffer,
rc_mutex,
redundant_allocation,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::types::BOX_COLLECTION_INFO,
crate::types::LINKEDLIST_INFO,
crate::types::OPTION_OPTION_INFO,
crate::types::OWNED_COW_INFO,
crate::types::RC_BUFFER_INFO,
crate::types::RC_MUTEX_INFO,
crate::types::REDUNDANT_ALLOCATION_INFO,
Expand Down
61 changes: 60 additions & 1 deletion clippy_lints/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ mod borrowed_box;
mod box_collection;
mod linked_list;
mod option_option;
mod owned_cow;
mod rc_buffer;
mod rc_mutex;
mod redundant_allocation;
Expand Down Expand Up @@ -355,13 +356,62 @@ declare_clippy_lint! {
"usage of `Rc<Mutex<T>>`"
}

declare_clippy_lint! {
/// ### What it does
/// Detects needlessly owned `Cow` types.
///
/// ### Why is this bad?
/// The borrowed types are usually more flexible, in that e.g. a
/// `Cow<'_, str>`can accept both `&str` and `String` while
/// `Cow<'_, String>` can only accept `&String` and `String`. In
/// particular, `&str` is more general, because it allows for string
/// literals while `&String` can only be borrowed from a heap-owned
/// `String`).
///
/// ### Known Problems:
/// The lint does not check for usage of the type. There may be external
/// interfaces that require the use of an owned type.
///
/// At least the `CString` type also has a richer API than `CStr`, there
/// is no guarantee that other types won't gain additional methods leading
/// to a similar mismatch whenever such a method gets called later.
///
/// In addition, the lint only checks for the known problematic types
/// `String`, `Vec<_>`, `CString`, `OsString` and `PathBuf`. Custom types
/// that implement `ToOwned` will not be detected.
///
/// ### Example
/// ```no_run
/// let wrogn: std::borrow::Cow<'_, Vec<u8>>;
/// ```
/// Use instead:
/// ```no_run
/// let right: std::borrow::Cow<'_, [u8]>;
/// ```
#[clippy::version = "1.85.0"]
pub OWNED_COW,
style,
"needlessly owned Cow type"
}

pub struct Types {
vec_box_size_threshold: u64,
type_complexity_threshold: u64,
avoid_breaking_exported_api: bool,
}

impl_lint_pass!(Types => [BOX_COLLECTION, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]);
impl_lint_pass!(Types => [
BOX_COLLECTION,
VEC_BOX,
OPTION_OPTION,
LINKEDLIST,
BORROWED_BOX,
REDUNDANT_ALLOCATION,
RC_BUFFER,
RC_MUTEX,
TYPE_COMPLEXITY,
OWNED_COW
]);

impl<'tcx> LateLintPass<'tcx> for Types {
fn check_fn(
Expand Down Expand Up @@ -541,6 +591,7 @@ impl Types {
triggered |= option_option::check(cx, hir_ty, qpath, def_id);
triggered |= linked_list::check(cx, hir_ty, def_id);
triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id);
triggered |= owned_cow::check(cx, qpath, def_id);

if triggered {
return;
Expand Down Expand Up @@ -592,6 +643,14 @@ impl Types {
QPath::LangItem(..) => {},
}
},
TyKind::Path(ref qpath) => {
let res = cx.qpath_res(qpath, hir_ty.hir_id);
if let Some(def_id) = res.opt_def_id()
&& self.is_type_change_allowed(context)
{
owned_cow::check(cx, qpath, def_id);
}
},
TyKind::Ref(lt, ref mut_ty) => {
context.is_nested_call = true;
if !borrowed_box::check(cx, hir_ty, lt, mut_ty) {
Expand Down
66 changes: 66 additions & 0 deletions clippy_lints/src/types/owned_cow.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_opt;
use rustc_errors::Applicability;
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir};
use rustc_lint::LateContext;
use rustc_span::{Span, sym};

pub(super) fn check(cx: &LateContext<'_>, qpath: &hir::QPath<'_>, def_id: DefId) -> bool {
if cx.tcx.is_diagnostic_item(sym::Cow, def_id)
&& let hir::QPath::Resolved(_, path) = qpath
&& let [.., last_seg] = path.segments
&& let Some(args) = last_seg.args
&& let [_lt, carg] = args.args
&& let hir::GenericArg::Type(cty) = carg
&& let Some((span, repl)) = replacement(cx, cty)
{
span_lint_and_sugg(
cx,
super::OWNED_COW,
span,
"needlessly owned Cow type",
"use",
repl,
Applicability::Unspecified,
);
return true;
}
false
}

fn replacement(cx: &LateContext<'_>, cty: &hir::Ty<'_>) -> Option<(Span, String)> {
if clippy_utils::is_path_lang_item(cx, cty, hir::LangItem::String) {
return Some((cty.span, "str".into()));
}
if clippy_utils::is_path_diagnostic_item(cx, cty, sym::Vec) {
return if let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = cty.kind
&& let [.., last_seg] = path.segments
&& let Some(args) = last_seg.args
&& let [t, ..] = args.args
&& let Some(snip) = snippet_opt(cx, t.span())
{
Some((cty.span, format!("[{snip}]")))
} else {
None
};
}
if clippy_utils::is_path_diagnostic_item(cx, cty, sym::cstring_type) {
return Some((
cty.span,
(if clippy_utils::is_no_std_crate(cx) {
"core::ffi::CStr"
} else {
"std::ffi::CStr"
})
.into(),
));
}
// Neither OsString nor PathBuf are available outside std
for (diag, repl) in [(sym::OsString, "std::ffi::OsStr"), (sym::PathBuf, "std::path::Path")] {
if clippy_utils::is_path_diagnostic_item(cx, cty, diag) {
return Some((cty.span, repl.into()));
}
}
None
}
17 changes: 17 additions & 0 deletions tests/ui/owned_cow.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![warn(clippy::owned_cow)]

use std::borrow::Cow;
use std::ffi::{CString, OsString};
use std::path::PathBuf;

fn main() {
let x: Cow<'static, str> = Cow::Owned(String::from("Hi!"));
let y: Cow<'_, [u8]> = Cow::Owned(vec![]);
let z: Cow<'_, [_]> = Cow::Owned(vec![2_i32]);
let o: Cow<'_, std::ffi::OsStr> = Cow::Owned(OsString::new());
let c: Cow<'_, std::ffi::CStr> = Cow::Owned(CString::new("").unwrap());
let p: Cow<'_, std::path::Path> = Cow::Owned(PathBuf::new());

// false positive: borrowed type
let b: Cow<'_, str> = Cow::Borrowed("Hi!");
}
17 changes: 17 additions & 0 deletions tests/ui/owned_cow.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![warn(clippy::owned_cow)]

use std::borrow::Cow;
use std::ffi::{CString, OsString};
use std::path::PathBuf;

fn main() {
let x: Cow<'static, String> = Cow::Owned(String::from("Hi!"));
let y: Cow<'_, Vec<u8>> = Cow::Owned(vec![]);
let z: Cow<'_, Vec<_>> = Cow::Owned(vec![2_i32]);
let o: Cow<'_, OsString> = Cow::Owned(OsString::new());
let c: Cow<'_, CString> = Cow::Owned(CString::new("").unwrap());
let p: Cow<'_, PathBuf> = Cow::Owned(PathBuf::new());

// false positive: borrowed type
let b: Cow<'_, str> = Cow::Borrowed("Hi!");
}
41 changes: 41 additions & 0 deletions tests/ui/owned_cow.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
error: needlessly owned Cow type
--> tests/ui/owned_cow.rs:8:25
|
LL | let x: Cow<'static, String> = Cow::Owned(String::from("Hi!"));
| ^^^^^^ help: use: `str`
|
= note: `-D clippy::owned-cow` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::owned_cow)]`

error: needlessly owned Cow type
--> tests/ui/owned_cow.rs:9:20
|
LL | let y: Cow<'_, Vec<u8>> = Cow::Owned(vec![]);
| ^^^^^^^ help: use: `[u8]`

error: needlessly owned Cow type
--> tests/ui/owned_cow.rs:10:20
|
LL | let z: Cow<'_, Vec<_>> = Cow::Owned(vec![2_i32]);
| ^^^^^^ help: use: `[_]`

error: needlessly owned Cow type
--> tests/ui/owned_cow.rs:11:20
|
LL | let o: Cow<'_, OsString> = Cow::Owned(OsString::new());
| ^^^^^^^^ help: use: `std::ffi::OsStr`

error: needlessly owned Cow type
--> tests/ui/owned_cow.rs:12:20
|
LL | let c: Cow<'_, CString> = Cow::Owned(CString::new("").unwrap());
| ^^^^^^^ help: use: `std::ffi::CStr`

error: needlessly owned Cow type
--> tests/ui/owned_cow.rs:13:20
|
LL | let p: Cow<'_, PathBuf> = Cow::Owned(PathBuf::new());
| ^^^^^^^ help: use: `std::path::Path`

error: aborting due to 6 previous errors

0 comments on commit eaa33b6

Please sign in to comment.