Skip to content

Commit bbf65f0

Browse files
authored
add owned_cow lint (#13948)
Closes #13697. --- changelog: add [`owned_cow`] lint
2 parents 7c889ac + 83f5cba commit bbf65f0

12 files changed

+315
-96
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5984,6 +5984,7 @@ Released 2018-09-13
59845984
[`out_of_bounds_indexing`]: https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
59855985
[`overflow_check_conditional`]: https://rust-lang.github.io/rust-clippy/master/index.html#overflow_check_conditional
59865986
[`overly_complex_bool_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#overly_complex_bool_expr
5987+
[`owned_cow`]: https://rust-lang.github.io/rust-clippy/master/index.html#owned_cow
59875988
[`panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic
59885989
[`panic_in_result_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_in_result_fn
59895990
[`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params

book/src/lint_configuration.md

+1
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat
380380
* [`linkedlist`](https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist)
381381
* [`needless_pass_by_ref_mut`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut)
382382
* [`option_option`](https://rust-lang.github.io/rust-clippy/master/index.html#option_option)
383+
* [`owned_cow`](https://rust-lang.github.io/rust-clippy/master/index.html#owned_cow)
383384
* [`rc_buffer`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer)
384385
* [`rc_mutex`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex)
385386
* [`redundant_allocation`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation)

clippy_config/src/conf.rs

+1
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,7 @@ define_Conf! {
438438
linkedlist,
439439
needless_pass_by_ref_mut,
440440
option_option,
441+
owned_cow,
441442
rc_buffer,
442443
rc_mutex,
443444
redundant_allocation,

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
745745
crate::types::BOX_COLLECTION_INFO,
746746
crate::types::LINKEDLIST_INFO,
747747
crate::types::OPTION_OPTION_INFO,
748+
crate::types::OWNED_COW_INFO,
748749
crate::types::RC_BUFFER_INFO,
749750
crate::types::RC_MUTEX_INFO,
750751
crate::types::REDUNDANT_ALLOCATION_INFO,

clippy_lints/src/types/mod.rs

+61-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ mod borrowed_box;
22
mod box_collection;
33
mod linked_list;
44
mod option_option;
5+
mod owned_cow;
56
mod rc_buffer;
67
mod rc_mutex;
78
mod redundant_allocation;
@@ -355,13 +356,63 @@ declare_clippy_lint! {
355356
"usage of `Rc<Mutex<T>>`"
356357
}
357358

359+
declare_clippy_lint! {
360+
/// ### What it does
361+
/// Detects needlessly owned `Cow` types.
362+
///
363+
/// ### Why is this bad?
364+
/// The borrowed types are usually more flexible, in that e.g. a
365+
/// `Cow<'_, str>` can accept both `&str` and `String` while
366+
/// `Cow<'_, String>` can only accept `&String` and `String`. In
367+
/// particular, `&str` is more general, because it allows for string
368+
/// literals while `&String` can only be borrowed from a heap-owned
369+
/// `String`).
370+
///
371+
/// ### Known Problems
372+
/// The lint does not check for usage of the type. There may be external
373+
/// interfaces that require the use of an owned type.
374+
///
375+
/// At least the `CString` type also has a different API than `CStr`: The
376+
/// former has an `as_bytes` method which the latter calls `to_bytes`.
377+
/// There is no guarantee that other types won't gain additional methods
378+
/// leading to a similar mismatch.
379+
///
380+
/// In addition, the lint only checks for the known problematic types
381+
/// `String`, `Vec<_>`, `CString`, `OsString` and `PathBuf`. Custom types
382+
/// that implement `ToOwned` will not be detected.
383+
///
384+
/// ### Example
385+
/// ```no_run
386+
/// let wrogn: std::borrow::Cow<'_, Vec<u8>>;
387+
/// ```
388+
/// Use instead:
389+
/// ```no_run
390+
/// let right: std::borrow::Cow<'_, [u8]>;
391+
/// ```
392+
#[clippy::version = "1.85.0"]
393+
pub OWNED_COW,
394+
style,
395+
"needlessly owned Cow type"
396+
}
397+
358398
pub struct Types {
359399
vec_box_size_threshold: u64,
360400
type_complexity_threshold: u64,
361401
avoid_breaking_exported_api: bool,
362402
}
363403

364-
impl_lint_pass!(Types => [BOX_COLLECTION, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]);
404+
impl_lint_pass!(Types => [
405+
BOX_COLLECTION,
406+
VEC_BOX,
407+
OPTION_OPTION,
408+
LINKEDLIST,
409+
BORROWED_BOX,
410+
REDUNDANT_ALLOCATION,
411+
RC_BUFFER,
412+
RC_MUTEX,
413+
TYPE_COMPLEXITY,
414+
OWNED_COW
415+
]);
365416

366417
impl<'tcx> LateLintPass<'tcx> for Types {
367418
fn check_fn(
@@ -561,6 +612,7 @@ impl Types {
561612
triggered |= option_option::check(cx, hir_ty, qpath, def_id);
562613
triggered |= linked_list::check(cx, hir_ty, def_id);
563614
triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id);
615+
triggered |= owned_cow::check(cx, qpath, def_id);
564616

565617
if triggered {
566618
return;
@@ -612,6 +664,14 @@ impl Types {
612664
QPath::LangItem(..) => {},
613665
}
614666
},
667+
TyKind::Path(ref qpath) => {
668+
let res = cx.qpath_res(qpath, hir_ty.hir_id);
669+
if let Some(def_id) = res.opt_def_id()
670+
&& self.is_type_change_allowed(context)
671+
{
672+
owned_cow::check(cx, qpath, def_id);
673+
}
674+
},
615675
TyKind::Ref(lt, ref mut_ty) => {
616676
context.is_nested_call = true;
617677
if !borrowed_box::check(cx, hir_ty, lt, mut_ty) {

clippy_lints/src/types/owned_cow.rs

+66
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_opt;
3+
use rustc_errors::Applicability;
4+
use rustc_hir::def_id::DefId;
5+
use rustc_hir::{self as hir};
6+
use rustc_lint::LateContext;
7+
use rustc_span::{Span, sym};
8+
9+
pub(super) fn check(cx: &LateContext<'_>, qpath: &hir::QPath<'_>, def_id: DefId) -> bool {
10+
if cx.tcx.is_diagnostic_item(sym::Cow, def_id)
11+
&& let hir::QPath::Resolved(_, path) = qpath
12+
&& let [.., last_seg] = path.segments
13+
&& let Some(args) = last_seg.args
14+
&& let [_lt, carg] = args.args
15+
&& let hir::GenericArg::Type(cty) = carg
16+
&& let Some((span, repl)) = replacement(cx, cty.as_unambig_ty())
17+
{
18+
span_lint_and_sugg(
19+
cx,
20+
super::OWNED_COW,
21+
span,
22+
"needlessly owned Cow type",
23+
"use",
24+
repl,
25+
Applicability::Unspecified,
26+
);
27+
return true;
28+
}
29+
false
30+
}
31+
32+
fn replacement(cx: &LateContext<'_>, cty: &hir::Ty<'_>) -> Option<(Span, String)> {
33+
if clippy_utils::is_path_lang_item(cx, cty, hir::LangItem::String) {
34+
return Some((cty.span, "str".into()));
35+
}
36+
if clippy_utils::is_path_diagnostic_item(cx, cty, sym::Vec) {
37+
return if let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = cty.kind
38+
&& let [.., last_seg] = path.segments
39+
&& let Some(args) = last_seg.args
40+
&& let [t, ..] = args.args
41+
&& let Some(snip) = snippet_opt(cx, t.span())
42+
{
43+
Some((cty.span, format!("[{snip}]")))
44+
} else {
45+
None
46+
};
47+
}
48+
if clippy_utils::is_path_diagnostic_item(cx, cty, sym::cstring_type) {
49+
return Some((
50+
cty.span,
51+
(if clippy_utils::is_no_std_crate(cx) {
52+
"core::ffi::CStr"
53+
} else {
54+
"std::ffi::CStr"
55+
})
56+
.into(),
57+
));
58+
}
59+
// Neither OsString nor PathBuf are available outside std
60+
for (diag, repl) in [(sym::OsString, "std::ffi::OsStr"), (sym::PathBuf, "std::path::Path")] {
61+
if clippy_utils::is_path_diagnostic_item(cx, cty, diag) {
62+
return Some((cty.span, repl.into()));
63+
}
64+
}
65+
None
66+
}

tests/ui/owned_cow.fixed

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#![warn(clippy::owned_cow)]
2+
3+
use std::borrow::Cow;
4+
use std::ffi::{CString, OsString};
5+
use std::path::PathBuf;
6+
7+
fn main() {
8+
let x: Cow<'static, str> = Cow::Owned(String::from("Hi!"));
9+
//~^ ERROR: needlessly owned Cow type
10+
let y: Cow<'_, [u8]> = Cow::Owned(vec![]);
11+
//~^ ERROR: needlessly owned Cow type
12+
let z: Cow<'_, [_]> = Cow::Owned(vec![2_i32]);
13+
//~^ ERROR: needlessly owned Cow type
14+
let o: Cow<'_, std::ffi::OsStr> = Cow::Owned(OsString::new());
15+
//~^ ERROR: needlessly owned Cow type
16+
let c: Cow<'_, std::ffi::CStr> = Cow::Owned(CString::new("").unwrap());
17+
//~^ ERROR: needlessly owned Cow type
18+
let p: Cow<'_, std::path::Path> = Cow::Owned(PathBuf::new());
19+
//~^ ERROR: needlessly owned Cow type
20+
21+
// false positive: borrowed type
22+
let b: Cow<'_, str> = Cow::Borrowed("Hi!");
23+
}

tests/ui/owned_cow.rs

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#![warn(clippy::owned_cow)]
2+
3+
use std::borrow::Cow;
4+
use std::ffi::{CString, OsString};
5+
use std::path::PathBuf;
6+
7+
fn main() {
8+
let x: Cow<'static, String> = Cow::Owned(String::from("Hi!"));
9+
//~^ ERROR: needlessly owned Cow type
10+
let y: Cow<'_, Vec<u8>> = Cow::Owned(vec![]);
11+
//~^ ERROR: needlessly owned Cow type
12+
let z: Cow<'_, Vec<_>> = Cow::Owned(vec![2_i32]);
13+
//~^ ERROR: needlessly owned Cow type
14+
let o: Cow<'_, OsString> = Cow::Owned(OsString::new());
15+
//~^ ERROR: needlessly owned Cow type
16+
let c: Cow<'_, CString> = Cow::Owned(CString::new("").unwrap());
17+
//~^ ERROR: needlessly owned Cow type
18+
let p: Cow<'_, PathBuf> = Cow::Owned(PathBuf::new());
19+
//~^ ERROR: needlessly owned Cow type
20+
21+
// false positive: borrowed type
22+
let b: Cow<'_, str> = Cow::Borrowed("Hi!");
23+
}

tests/ui/owned_cow.stderr

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
error: needlessly owned Cow type
2+
--> tests/ui/owned_cow.rs:8:25
3+
|
4+
LL | let x: Cow<'static, String> = Cow::Owned(String::from("Hi!"));
5+
| ^^^^^^ help: use: `str`
6+
|
7+
= note: `-D clippy::owned-cow` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::owned_cow)]`
9+
10+
error: needlessly owned Cow type
11+
--> tests/ui/owned_cow.rs:10:20
12+
|
13+
LL | let y: Cow<'_, Vec<u8>> = Cow::Owned(vec![]);
14+
| ^^^^^^^ help: use: `[u8]`
15+
16+
error: needlessly owned Cow type
17+
--> tests/ui/owned_cow.rs:12:20
18+
|
19+
LL | let z: Cow<'_, Vec<_>> = Cow::Owned(vec![2_i32]);
20+
| ^^^^^^ help: use: `[_]`
21+
22+
error: needlessly owned Cow type
23+
--> tests/ui/owned_cow.rs:14:20
24+
|
25+
LL | let o: Cow<'_, OsString> = Cow::Owned(OsString::new());
26+
| ^^^^^^^^ help: use: `std::ffi::OsStr`
27+
28+
error: needlessly owned Cow type
29+
--> tests/ui/owned_cow.rs:16:20
30+
|
31+
LL | let c: Cow<'_, CString> = Cow::Owned(CString::new("").unwrap());
32+
| ^^^^^^^ help: use: `std::ffi::CStr`
33+
34+
error: needlessly owned Cow type
35+
--> tests/ui/owned_cow.rs:18:20
36+
|
37+
LL | let p: Cow<'_, PathBuf> = Cow::Owned(PathBuf::new());
38+
| ^^^^^^^ help: use: `std::path::Path`
39+
40+
error: aborting due to 6 previous errors
41+

tests/ui/unnecessary_to_owned.fixed

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
clippy::needless_borrows_for_generic_args,
44
clippy::ptr_arg,
55
clippy::manual_async_fn,
6-
clippy::needless_lifetimes
6+
clippy::needless_lifetimes,
7+
clippy::owned_cow
78
)]
89
#![warn(clippy::unnecessary_to_owned, clippy::redundant_clone)]
910

tests/ui/unnecessary_to_owned.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
clippy::needless_borrows_for_generic_args,
44
clippy::ptr_arg,
55
clippy::manual_async_fn,
6-
clippy::needless_lifetimes
6+
clippy::needless_lifetimes,
7+
clippy::owned_cow
78
)]
89
#![warn(clippy::unnecessary_to_owned, clippy::redundant_clone)]
910

0 commit comments

Comments
 (0)