Skip to content

Commit

Permalink
Auto merge of #5866 - flip1995:rollup-ovtjqh4, r=flip1995
Browse files Browse the repository at this point in the history 
Rollup of 6 pull requests

Successful merges:

 - #5725 (should_impl_trait - ignore methods with lifetime params)
 - #5837 (needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...)
 - #5846 (Handle mapping to Option in `map_flatten` lint)
 - #5848 (Add derive_ord_xor_partial_ord lint)
 - #5852 (Add lint for duplicate methods of trait bounds)
 - #5856 (Remove old Symbol reexport)

Failed merges:

r? @ghost

changelog: rollup
  • Loading branch information
@bors
bors committed Aug 4, 2020
2 parents bbbc973 + 59e901c commit 1358d4d
Show file tree
Hide file tree
Showing 27 changed files with 1,443 additions and 196 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1454,6 +1454,7 @@ Released 2018-09-13
[`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver
[`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof
[`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq
[`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
[`diverging_sub_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression
[`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
[`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons
Expand Down Expand Up @@ -1723,6 +1724,7 @@ Released 2018-09-13
[`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
[`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
[`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg
[`trait_duplication_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds
[`transmute_bytes_to_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_bytes_to_str
[`transmute_float_to_int`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_float_to_int
[`transmute_int_to_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_bool
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/attrs.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! checks for attributes
use crate::reexport::Name;
use crate::utils::{
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_help,
span_lint_and_sugg, span_lint_and_then, without_block_comments,
Expand Down Expand Up @@ -517,7 +516,7 @@ fn is_relevant_expr(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>
}
}

fn check_attrs(cx: &LateContext<'_>, span: Span, name: Name, attrs: &[Attribute]) {
fn check_attrs(cx: &LateContext<'_>, span: Span, name: Symbol, attrs: &[Attribute]) {
if span.from_expansion() {
return;
}
Expand Down
116 changes: 114 additions & 2 deletions clippy_lints/src/derive.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::utils::paths;
use crate::utils::{
is_automatically_derived, is_copy, match_path, span_lint_and_help, span_lint_and_note, span_lint_and_then,
get_trait_def_id, is_automatically_derived, is_copy, match_path, span_lint_and_help, span_lint_and_note,
span_lint_and_then,
};
use if_chain::if_chain;
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -43,6 +44,57 @@ declare_clippy_lint! {
"deriving `Hash` but implementing `PartialEq` explicitly"
}

declare_clippy_lint! {
/// **What it does:** Checks for deriving `Ord` but implementing `PartialOrd`
/// explicitly or vice versa.
///
/// **Why is this bad?** The implementation of these traits must agree (for
/// example for use with `sort`) so it’s probably a bad idea to use a
/// default-generated `Ord` implementation with an explicitly defined
/// `PartialOrd`. In particular, the following must hold for any type
/// implementing `Ord`:
///
/// ```text
/// k1.cmp(&k2) == k1.partial_cmp(&k2).unwrap()
/// ```
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust,ignore
/// #[derive(Ord, PartialEq, Eq)]
/// struct Foo;
///
/// impl PartialOrd for Foo {
/// ...
/// }
/// ```
/// Use instead:
/// ```rust,ignore
/// #[derive(PartialEq, Eq)]
/// struct Foo;
///
/// impl PartialOrd for Foo {
/// fn partial_cmp(&self, other: &Foo) -> Option<Ordering> {
/// Some(self.cmp(other))
/// }
/// }
///
/// impl Ord for Foo {
/// ...
/// }
/// ```
/// or, if you don't need a custom ordering:
/// ```rust,ignore
/// #[derive(Ord, PartialOrd, PartialEq, Eq)]
/// struct Foo;
/// ```
pub DERIVE_ORD_XOR_PARTIAL_ORD,
correctness,
"deriving `Ord` but implementing `PartialOrd` explicitly"
}

declare_clippy_lint! {
/// **What it does:** Checks for explicit `Clone` implementations for `Copy`
/// types.
Expand Down Expand Up @@ -103,7 +155,12 @@ declare_clippy_lint! {
"deriving `serde::Deserialize` on a type that has methods using `unsafe`"
}

declare_lint_pass!(Derive => [EXPL_IMPL_CLONE_ON_COPY, DERIVE_HASH_XOR_EQ, UNSAFE_DERIVE_DESERIALIZE]);
declare_lint_pass!(Derive => [
EXPL_IMPL_CLONE_ON_COPY,
DERIVE_HASH_XOR_EQ,
DERIVE_ORD_XOR_PARTIAL_ORD,
UNSAFE_DERIVE_DESERIALIZE
]);

impl<'tcx> LateLintPass<'tcx> for Derive {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
Expand All @@ -116,6 +173,7 @@ impl<'tcx> LateLintPass<'tcx> for Derive {
let is_automatically_derived = is_automatically_derived(&*item.attrs);

check_hash_peq(cx, item.span, trait_ref, ty, is_automatically_derived);
check_ord_partial_ord(cx, item.span, trait_ref, ty, is_automatically_derived);

if is_automatically_derived {
check_unsafe_derive_deserialize(cx, item, trait_ref, ty);
Expand Down Expand Up @@ -180,6 +238,60 @@ fn check_hash_peq<'tcx>(
}
}

/// Implementation of the `DERIVE_ORD_XOR_PARTIAL_ORD` lint.
fn check_ord_partial_ord<'tcx>(
cx: &LateContext<'tcx>,
span: Span,
trait_ref: &TraitRef<'_>,
ty: Ty<'tcx>,
ord_is_automatically_derived: bool,
) {
if_chain! {
if let Some(ord_trait_def_id) = get_trait_def_id(cx, &paths::ORD);
if let Some(partial_ord_trait_def_id) = cx.tcx.lang_items().partial_ord_trait();
if let Some(def_id) = &trait_ref.trait_def_id();
if *def_id == ord_trait_def_id;
then {
// Look for the PartialOrd implementations for `ty`
cx.tcx.for_each_relevant_impl(partial_ord_trait_def_id, ty, |impl_id| {
let partial_ord_is_automatically_derived = is_automatically_derived(&cx.tcx.get_attrs(impl_id));

if partial_ord_is_automatically_derived == ord_is_automatically_derived {
return;
}

let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation");

// Only care about `impl PartialOrd<Foo> for Foo`
// For `impl PartialOrd<B> for A, input_types is [A, B]
if trait_ref.substs.type_at(1) == ty {
let mess = if partial_ord_is_automatically_derived {
"you are implementing `Ord` explicitly but have derived `PartialOrd`"
} else {
"you are deriving `Ord` but have implemented `PartialOrd` explicitly"
};

span_lint_and_then(
cx,
DERIVE_ORD_XOR_PARTIAL_ORD,
span,
mess,
|diag| {
if let Some(local_def_id) = impl_id.as_local() {
let hir_id = cx.tcx.hir().as_local_hir_id(local_def_id);
diag.span_note(
cx.tcx.hir().span(hir_id),
"`PartialOrd` implemented here"
);
}
}
);
}
});
}
}
}

/// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &TraitRef<'_>, ty: Ty<'tcx>) {
if match_path(&trait_ref.path, &paths::CLONE_TRAIT) {
Expand Down
9 changes: 5 additions & 4 deletions clippy_lints/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,10 +322,6 @@ mod zero_div_zero;

pub use crate::utils::conf::Conf;

mod reexport {
pub use rustc_span::Symbol as Name;
}

/// Register all pre expansion lints
///
/// Pre-expansion lints run before any macro expansion has happened.
Expand Down Expand Up @@ -513,6 +509,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&default_trait_access::DEFAULT_TRAIT_ACCESS,
&dereference::EXPLICIT_DEREF_METHODS,
&derive::DERIVE_HASH_XOR_EQ,
&derive::DERIVE_ORD_XOR_PARTIAL_ORD,
&derive::EXPL_IMPL_CLONE_ON_COPY,
&derive::UNSAFE_DERIVE_DESERIALIZE,
&doc::DOC_MARKDOWN,
Expand Down Expand Up @@ -786,6 +783,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS,
&temporary_assignment::TEMPORARY_ASSIGNMENT,
&to_digit_is_some::TO_DIGIT_IS_SOME,
&trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS,
&trait_bounds::TYPE_REPETITION_IN_BOUNDS,
&transmute::CROSSPOINTER_TRANSMUTE,
&transmute::TRANSMUTE_BYTES_TO_STR,
Expand Down Expand Up @@ -1174,6 +1172,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&ranges::RANGE_PLUS_ONE),
LintId::of(&shadow::SHADOW_UNRELATED),
LintId::of(&strings::STRING_ADD_ASSIGN),
LintId::of(&trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS),
LintId::of(&trait_bounds::TYPE_REPETITION_IN_BOUNDS),
LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF),
LintId::of(&types::CAST_LOSSLESS),
Expand Down Expand Up @@ -1230,6 +1229,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&copies::IFS_SAME_COND),
LintId::of(&copies::IF_SAME_THEN_ELSE),
LintId::of(&derive::DERIVE_HASH_XOR_EQ),
LintId::of(&derive::DERIVE_ORD_XOR_PARTIAL_ORD),
LintId::of(&doc::MISSING_SAFETY_DOC),
LintId::of(&doc::NEEDLESS_DOCTEST_MAIN),
LintId::of(&double_comparison::DOUBLE_COMPARISONS),
Expand Down Expand Up @@ -1648,6 +1648,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&copies::IFS_SAME_COND),
LintId::of(&copies::IF_SAME_THEN_ELSE),
LintId::of(&derive::DERIVE_HASH_XOR_EQ),
LintId::of(&derive::DERIVE_ORD_XOR_PARTIAL_ORD),
LintId::of(&drop_bounds::DROP_BOUNDS),
LintId::of(&drop_forget_ref::DROP_COPY),
LintId::of(&drop_forget_ref::DROP_REF),
Expand Down
7 changes: 3 additions & 4 deletions clippy_lints/src/lifetimes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use rustc_span::symbol::kw;
use rustc_span::symbol::{kw, Symbol};

use crate::reexport::Name;
use crate::utils::{in_macro, last_path_segment, span_lint, trait_ref_of_method};

declare_clippy_lint! {
Expand Down Expand Up @@ -113,7 +112,7 @@ impl<'tcx> LateLintPass<'tcx> for Lifetimes {
enum RefLt {
Unnamed,
Static,
Named(Name),
Named(Symbol),
}

fn check_fn_inner<'tcx>(
Expand Down Expand Up @@ -456,7 +455,7 @@ fn has_where_lifetimes<'tcx>(cx: &LateContext<'tcx>, where_clause: &'tcx WhereCl
}

struct LifetimeChecker {
map: FxHashMap<Name, Span>,
map: FxHashMap<Symbol, Span>,
}

impl<'tcx> Visitor<'tcx> for LifetimeChecker {
Expand Down
Loading

0 comments on commit 1358d4d

Please sign in to comment.