diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e7ac644b9f39..2d7bda27e4fcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 8b96dfd5981be..800e3a8767133 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -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, diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs index 9edced28408f1..f62d57af5b47f 100644 --- a/clippy_lints/src/lib.register_restriction.rs +++ b/clippy_lints/src/lib.register_restriction.rs @@ -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), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b54ecd12313aa..8723eb7943cf3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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; @@ -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` } diff --git a/clippy_lints/src/missing_trait_methods.rs b/clippy_lints/src/missing_trait_methods.rs new file mode 100644 index 0000000000000..68af8a672f6ae --- /dev/null +++ b/clippy_lints/src/missing_trait_methods.rs @@ -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", + ); + } + } + } +} diff --git a/src/docs.rs b/src/docs.rs index 0a9fb1ecd840c..c033ad294a38d 100644 --- a/src/docs.rs +++ b/src/docs.rs @@ -316,6 +316,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", diff --git a/src/docs/missing_trait_methods.txt b/src/docs/missing_trait_methods.txt new file mode 100644 index 0000000000000..788ad764f8c39 --- /dev/null +++ b/src/docs/missing_trait_methods.txt @@ -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() { /* ... */ } +} +``` \ No newline at end of file diff --git a/tests/ui/missing_trait_methods.rs b/tests/ui/missing_trait_methods.rs new file mode 100644 index 0000000000000..8df885919a3e6 --- /dev/null +++ b/tests/ui/missing_trait_methods.rs @@ -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>(a: &T) -> &[u8] { + a.as_ref() + } +} + +fn main() {} diff --git a/tests/ui/missing_trait_methods.stderr b/tests/ui/missing_trait_methods.stderr new file mode 100644 index 0000000000000..0c5205e196572 --- /dev/null +++ b/tests/ui/missing_trait_methods.stderr @@ -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 +