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 missing_trait_methods lint #9670

Merged
merged 1 commit into from
Oct 20, 2022
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 @@ -4049,6 +4049,7 @@ Released 2018-09-13
[`missing_panics_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc
[`missing_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
[`missing_spin_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_spin_loop
[`missing_trait_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_trait_methods
[`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
[`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
[`mixed_read_write_in_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_read_write_in_expression
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ store.register_lints(&[
missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS,
missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES,
missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS,
missing_trait_methods::MISSING_TRAIT_METHODS,
mixed_read_write_in_expression::DIVERGING_SUB_EXPRESSION,
mixed_read_write_in_expression::MIXED_READ_WRITE_IN_EXPRESSION,
module_style::MOD_MODULE_FILES,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_restriction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
LintId::of(missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS),
LintId::of(missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES),
LintId::of(missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS),
LintId::of(missing_trait_methods::MISSING_TRAIT_METHODS),
LintId::of(mixed_read_write_in_expression::MIXED_READ_WRITE_IN_EXPRESSION),
LintId::of(module_style::MOD_MODULE_FILES),
LintId::of(module_style::SELF_NAMED_MODULE_FILES),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ mod missing_const_for_fn;
mod missing_doc;
mod missing_enforced_import_rename;
mod missing_inline;
mod missing_trait_methods;
mod mixed_read_write_in_expression;
mod module_style;
mod multi_assignments;
Expand Down Expand Up @@ -916,6 +917,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(box_default::BoxDefault));
store.register_late_pass(|_| Box::new(implicit_saturating_add::ImplicitSaturatingAdd));
store.register_early_pass(|| Box::new(partial_pub_fields::PartialPubFields));
store.register_late_pass(|_| Box::new(missing_trait_methods::MissingTraitMethods));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
98 changes: 98 additions & 0 deletions clippy_lints/src/missing_trait_methods.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_lint_allowed;
use clippy_utils::macros::span_is_local;
use rustc_hir::def_id::DefIdMap;
use rustc_hir::{Impl, Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::AssocItem;
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// ### What it does
/// Checks if a provided method is used implicitly by a trait
/// implementation. A usage example would be a wrapper where every method
/// should perform some operation before delegating to the inner type's
/// implemenation.
///
/// This lint should typically be enabled on a specific trait `impl` item
/// rather than globally.
///
/// ### Why is this bad?
/// Indicates that a method is missing.
///
/// ### Example
/// ```rust
/// trait Trait {
/// fn required();
///
/// fn provided() {}
/// }
///
/// # struct Type;
/// #[warn(clippy::missing_trait_methods)]
/// impl Trait for Type {
/// fn required() { /* ... */ }
/// }
/// ```
/// Use instead:
/// ```rust
/// trait Trait {
/// fn required();
///
/// fn provided() {}
/// }
///
/// # struct Type;
/// #[warn(clippy::missing_trait_methods)]
/// impl Trait for Type {
/// fn required() { /* ... */ }
///
/// fn provided() { /* ... */ }
/// }
/// ```
#[clippy::version = "1.66.0"]
pub MISSING_TRAIT_METHODS,
restriction,
"trait implementation uses default provided method"
}
declare_lint_pass!(MissingTraitMethods => [MISSING_TRAIT_METHODS]);

impl<'tcx> LateLintPass<'tcx> for MissingTraitMethods {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if !is_lint_allowed(cx, MISSING_TRAIT_METHODS, item.hir_id())
&& span_is_local(item.span)
&& let ItemKind::Impl(Impl {
items,
of_trait: Some(trait_ref),
..
}) = item.kind
&& let Some(trait_id) = trait_ref.trait_def_id()
{
let mut provided: DefIdMap<&AssocItem> = cx
.tcx
.provided_trait_methods(trait_id)
.map(|assoc| (assoc.def_id, assoc))
.collect();

for impl_item in *items {
if let Some(def_id) = impl_item.trait_item_def_id {
provided.remove(&def_id);
}
}

for assoc in provided.values() {
let source_map = cx.tcx.sess.source_map();
let definition_span = source_map.guess_head_span(cx.tcx.def_span(assoc.def_id));

span_lint_and_help(
cx,
MISSING_TRAIT_METHODS,
source_map.guess_head_span(item.span),
&format!("missing trait method provided by default: `{}`", assoc.name),
Some(definition_span),
"implement the method",
);
}
}
}
}
1 change: 1 addition & 0 deletions src/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ docs! {
"missing_panics_doc",
"missing_safety_doc",
"missing_spin_loop",
"missing_trait_methods",
"mistyped_literal_suffixes",
"mixed_case_hex_literals",
"mixed_read_write_in_expression",
Expand Down
40 changes: 40 additions & 0 deletions src/docs/missing_trait_methods.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
### What it does
Checks if a provided method is used implicitly by a trait
implementation. A usage example would be a wrapper where every method
should perform some operation before delegating to the inner type's
implemenation.

This lint should typically be enabled on a specific trait `impl` item
rather than globally.

### Why is this bad?
Indicates that a method is missing.

### Example
```
trait Trait {
fn required();

fn provided() {}
}

#[warn(clippy::missing_trait_methods)]
impl Trait for Type {
fn required() { /* ... */ }
}
```
Use instead:
```
trait Trait {
fn required();

fn provided() {}
}

#[warn(clippy::missing_trait_methods)]
impl Trait for Type {
fn required() { /* ... */ }

fn provided() { /* ... */ }
}
```
50 changes: 50 additions & 0 deletions tests/ui/missing_trait_methods.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#![allow(unused, clippy::needless_lifetimes)]
#![warn(clippy::missing_trait_methods)]

trait A {
fn provided() {}
}

trait B {
fn required();

fn a(_: usize) -> usize {
1
}

fn b<'a, T: AsRef<[u8]>>(a: &'a T) -> &'a [u8] {
a.as_ref()
}
}

struct Partial;

impl A for Partial {}

impl B for Partial {
fn required() {}

fn a(_: usize) -> usize {
2
}
}

struct Complete;

impl A for Complete {
fn provided() {}
}

impl B for Complete {
fn required() {}

fn a(_: usize) -> usize {
2
}

fn b<T: AsRef<[u8]>>(a: &T) -> &[u8] {
a.as_ref()
}
}

fn main() {}
27 changes: 27 additions & 0 deletions tests/ui/missing_trait_methods.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error: missing trait method provided by default: `provided`
--> $DIR/missing_trait_methods.rs:22:1
|
LL | impl A for Partial {}
| ^^^^^^^^^^^^^^^^^^
|
help: implement the method
--> $DIR/missing_trait_methods.rs:5:5
|
LL | fn provided() {}
| ^^^^^^^^^^^^^
= note: `-D clippy::missing-trait-methods` implied by `-D warnings`

error: missing trait method provided by default: `b`
--> $DIR/missing_trait_methods.rs:24:1
|
LL | impl B for Partial {
| ^^^^^^^^^^^^^^^^^^
|
help: implement the method
--> $DIR/missing_trait_methods.rs:15:5
|
LL | fn b<'a, T: AsRef<[u8]>>(a: &'a T) -> &'a [u8] {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors