Skip to content

Commit 8012178

Browse files
committed
Auto merge of rust-lang#5868 - flip1995:rollup-5g8vft5, r=flip1995
Rollup of 5 pull requests Successful merges: - rust-lang#5837 (needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...) - rust-lang#5846 (Handle mapping to Option in `map_flatten` lint) - rust-lang#5848 (Add derive_ord_xor_partial_ord lint) - rust-lang#5852 (Add lint for duplicate methods of trait bounds) - rust-lang#5856 (Remove old Symbol reexport) Failed merges: r? @ghost changelog: rollup
2 parents 1968aed + fb7ad95 commit 8012178

20 files changed

+770
-67
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -1454,6 +1454,7 @@ Released 2018-09-13
14541454
[`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver
14551455
[`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof
14561456
[`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq
1457+
[`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
14571458
[`diverging_sub_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression
14581459
[`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
14591460
[`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons
@@ -1723,6 +1724,7 @@ Released 2018-09-13
17231724
[`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
17241725
[`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
17251726
[`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg
1727+
[`trait_duplication_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds
17261728
[`transmute_bytes_to_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_bytes_to_str
17271729
[`transmute_float_to_int`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_float_to_int
17281730
[`transmute_int_to_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_bool

clippy_lints/src/attrs.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//! checks for attributes
22
3-
use crate::reexport::Name;
43
use crate::utils::{
54
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_help,
65
span_lint_and_sugg, span_lint_and_then, without_block_comments,
@@ -517,7 +516,7 @@ fn is_relevant_expr(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>
517516
}
518517
}
519518

520-
fn check_attrs(cx: &LateContext<'_>, span: Span, name: Name, attrs: &[Attribute]) {
519+
fn check_attrs(cx: &LateContext<'_>, span: Span, name: Symbol, attrs: &[Attribute]) {
521520
if span.from_expansion() {
522521
return;
523522
}

clippy_lints/src/derive.rs

+114-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::utils::paths;
22
use crate::utils::{
3-
is_automatically_derived, is_copy, match_path, span_lint_and_help, span_lint_and_note, span_lint_and_then,
3+
get_trait_def_id, is_automatically_derived, is_copy, match_path, span_lint_and_help, span_lint_and_note,
4+
span_lint_and_then,
45
};
56
use if_chain::if_chain;
67
use rustc_hir::def_id::DefId;
@@ -43,6 +44,57 @@ declare_clippy_lint! {
4344
"deriving `Hash` but implementing `PartialEq` explicitly"
4445
}
4546

47+
declare_clippy_lint! {
48+
/// **What it does:** Checks for deriving `Ord` but implementing `PartialOrd`
49+
/// explicitly or vice versa.
50+
///
51+
/// **Why is this bad?** The implementation of these traits must agree (for
52+
/// example for use with `sort`) so it’s probably a bad idea to use a
53+
/// default-generated `Ord` implementation with an explicitly defined
54+
/// `PartialOrd`. In particular, the following must hold for any type
55+
/// implementing `Ord`:
56+
///
57+
/// ```text
58+
/// k1.cmp(&k2) == k1.partial_cmp(&k2).unwrap()
59+
/// ```
60+
///
61+
/// **Known problems:** None.
62+
///
63+
/// **Example:**
64+
///
65+
/// ```rust,ignore
66+
/// #[derive(Ord, PartialEq, Eq)]
67+
/// struct Foo;
68+
///
69+
/// impl PartialOrd for Foo {
70+
/// ...
71+
/// }
72+
/// ```
73+
/// Use instead:
74+
/// ```rust,ignore
75+
/// #[derive(PartialEq, Eq)]
76+
/// struct Foo;
77+
///
78+
/// impl PartialOrd for Foo {
79+
/// fn partial_cmp(&self, other: &Foo) -> Option<Ordering> {
80+
/// Some(self.cmp(other))
81+
/// }
82+
/// }
83+
///
84+
/// impl Ord for Foo {
85+
/// ...
86+
/// }
87+
/// ```
88+
/// or, if you don't need a custom ordering:
89+
/// ```rust,ignore
90+
/// #[derive(Ord, PartialOrd, PartialEq, Eq)]
91+
/// struct Foo;
92+
/// ```
93+
pub DERIVE_ORD_XOR_PARTIAL_ORD,
94+
correctness,
95+
"deriving `Ord` but implementing `PartialOrd` explicitly"
96+
}
97+
4698
declare_clippy_lint! {
4799
/// **What it does:** Checks for explicit `Clone` implementations for `Copy`
48100
/// types.
@@ -103,7 +155,12 @@ declare_clippy_lint! {
103155
"deriving `serde::Deserialize` on a type that has methods using `unsafe`"
104156
}
105157

106-
declare_lint_pass!(Derive => [EXPL_IMPL_CLONE_ON_COPY, DERIVE_HASH_XOR_EQ, UNSAFE_DERIVE_DESERIALIZE]);
158+
declare_lint_pass!(Derive => [
159+
EXPL_IMPL_CLONE_ON_COPY,
160+
DERIVE_HASH_XOR_EQ,
161+
DERIVE_ORD_XOR_PARTIAL_ORD,
162+
UNSAFE_DERIVE_DESERIALIZE
163+
]);
107164

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

118175
check_hash_peq(cx, item.span, trait_ref, ty, is_automatically_derived);
176+
check_ord_partial_ord(cx, item.span, trait_ref, ty, is_automatically_derived);
119177

120178
if is_automatically_derived {
121179
check_unsafe_derive_deserialize(cx, item, trait_ref, ty);
@@ -180,6 +238,60 @@ fn check_hash_peq<'tcx>(
180238
}
181239
}
182240

241+
/// Implementation of the `DERIVE_ORD_XOR_PARTIAL_ORD` lint.
242+
fn check_ord_partial_ord<'tcx>(
243+
cx: &LateContext<'tcx>,
244+
span: Span,
245+
trait_ref: &TraitRef<'_>,
246+
ty: Ty<'tcx>,
247+
ord_is_automatically_derived: bool,
248+
) {
249+
if_chain! {
250+
if let Some(ord_trait_def_id) = get_trait_def_id(cx, &paths::ORD);
251+
if let Some(partial_ord_trait_def_id) = cx.tcx.lang_items().partial_ord_trait();
252+
if let Some(def_id) = &trait_ref.trait_def_id();
253+
if *def_id == ord_trait_def_id;
254+
then {
255+
// Look for the PartialOrd implementations for `ty`
256+
cx.tcx.for_each_relevant_impl(partial_ord_trait_def_id, ty, |impl_id| {
257+
let partial_ord_is_automatically_derived = is_automatically_derived(&cx.tcx.get_attrs(impl_id));
258+
259+
if partial_ord_is_automatically_derived == ord_is_automatically_derived {
260+
return;
261+
}
262+
263+
let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation");
264+
265+
// Only care about `impl PartialOrd<Foo> for Foo`
266+
// For `impl PartialOrd<B> for A, input_types is [A, B]
267+
if trait_ref.substs.type_at(1) == ty {
268+
let mess = if partial_ord_is_automatically_derived {
269+
"you are implementing `Ord` explicitly but have derived `PartialOrd`"
270+
} else {
271+
"you are deriving `Ord` but have implemented `PartialOrd` explicitly"
272+
};
273+
274+
span_lint_and_then(
275+
cx,
276+
DERIVE_ORD_XOR_PARTIAL_ORD,
277+
span,
278+
mess,
279+
|diag| {
280+
if let Some(local_def_id) = impl_id.as_local() {
281+
let hir_id = cx.tcx.hir().as_local_hir_id(local_def_id);
282+
diag.span_note(
283+
cx.tcx.hir().span(hir_id),
284+
"`PartialOrd` implemented here"
285+
);
286+
}
287+
}
288+
);
289+
}
290+
});
291+
}
292+
}
293+
}
294+
183295
/// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
184296
fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &TraitRef<'_>, ty: Ty<'tcx>) {
185297
if match_path(&trait_ref.path, &paths::CLONE_TRAIT) {

clippy_lints/src/lib.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -322,10 +322,6 @@ mod zero_div_zero;
322322

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

325-
mod reexport {
326-
pub use rustc_span::Symbol as Name;
327-
}
328-
329325
/// Register all pre expansion lints
330326
///
331327
/// Pre-expansion lints run before any macro expansion has happened.
@@ -513,6 +509,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
513509
&default_trait_access::DEFAULT_TRAIT_ACCESS,
514510
&dereference::EXPLICIT_DEREF_METHODS,
515511
&derive::DERIVE_HASH_XOR_EQ,
512+
&derive::DERIVE_ORD_XOR_PARTIAL_ORD,
516513
&derive::EXPL_IMPL_CLONE_ON_COPY,
517514
&derive::UNSAFE_DERIVE_DESERIALIZE,
518515
&doc::DOC_MARKDOWN,
@@ -786,6 +783,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
786783
&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS,
787784
&temporary_assignment::TEMPORARY_ASSIGNMENT,
788785
&to_digit_is_some::TO_DIGIT_IS_SOME,
786+
&trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS,
789787
&trait_bounds::TYPE_REPETITION_IN_BOUNDS,
790788
&transmute::CROSSPOINTER_TRANSMUTE,
791789
&transmute::TRANSMUTE_BYTES_TO_STR,
@@ -1174,6 +1172,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11741172
LintId::of(&ranges::RANGE_PLUS_ONE),
11751173
LintId::of(&shadow::SHADOW_UNRELATED),
11761174
LintId::of(&strings::STRING_ADD_ASSIGN),
1175+
LintId::of(&trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS),
11771176
LintId::of(&trait_bounds::TYPE_REPETITION_IN_BOUNDS),
11781177
LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF),
11791178
LintId::of(&types::CAST_LOSSLESS),
@@ -1230,6 +1229,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12301229
LintId::of(&copies::IFS_SAME_COND),
12311230
LintId::of(&copies::IF_SAME_THEN_ELSE),
12321231
LintId::of(&derive::DERIVE_HASH_XOR_EQ),
1232+
LintId::of(&derive::DERIVE_ORD_XOR_PARTIAL_ORD),
12331233
LintId::of(&doc::MISSING_SAFETY_DOC),
12341234
LintId::of(&doc::NEEDLESS_DOCTEST_MAIN),
12351235
LintId::of(&double_comparison::DOUBLE_COMPARISONS),
@@ -1648,6 +1648,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16481648
LintId::of(&copies::IFS_SAME_COND),
16491649
LintId::of(&copies::IF_SAME_THEN_ELSE),
16501650
LintId::of(&derive::DERIVE_HASH_XOR_EQ),
1651+
LintId::of(&derive::DERIVE_ORD_XOR_PARTIAL_ORD),
16511652
LintId::of(&drop_bounds::DROP_BOUNDS),
16521653
LintId::of(&drop_forget_ref::DROP_COPY),
16531654
LintId::of(&drop_forget_ref::DROP_REF),

clippy_lints/src/lifetimes.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,8 @@ use rustc_lint::{LateContext, LateLintPass};
1313
use rustc_middle::hir::map::Map;
1414
use rustc_session::{declare_lint_pass, declare_tool_lint};
1515
use rustc_span::source_map::Span;
16-
use rustc_span::symbol::kw;
16+
use rustc_span::symbol::{kw, Symbol};
1717

18-
use crate::reexport::Name;
1918
use crate::utils::{in_macro, last_path_segment, span_lint, trait_ref_of_method};
2019

2120
declare_clippy_lint! {
@@ -113,7 +112,7 @@ impl<'tcx> LateLintPass<'tcx> for Lifetimes {
113112
enum RefLt {
114113
Unnamed,
115114
Static,
116-
Named(Name),
115+
Named(Symbol),
117116
}
118117

119118
fn check_fn_inner<'tcx>(
@@ -456,7 +455,7 @@ fn has_where_lifetimes<'tcx>(cx: &LateContext<'tcx>, where_clause: &'tcx WhereCl
456455
}
457456

458457
struct LifetimeChecker {
459-
map: FxHashMap<Name, Span>,
458+
map: FxHashMap<Symbol, Span>,
460459
}
461460

462461
impl<'tcx> Visitor<'tcx> for LifetimeChecker {

0 commit comments

Comments
 (0)