Skip to content

Commit fad295b

Browse files
borsbors
committed
Auto merge of #87271 - flip1995:clippyup, r=Manishearth
Update Clippy This is an out-of-cycle Clippy update, to fix 3 ICEs before the release (This should be merged before beta is branched): rust-lang/rust-clippy#7470 rust-lang/rust-clippy#7471 rust-lang/rust-clippy#7473 cc `@jackh726` `@JohnTitor` rust-lang/rust-clippy#7470 was caused by #86867. I saw the same ICE in the last rustup for Clippy though, so this might be a more general problem. Is there something we should check before calling `layout_of`? Should we always check for `ty.has_escaping_bound_vars()` before calling `layout_of`? Or is this overkill? r? `@Manishearth`
2 parents b543e0d + b10966b commit fad295b

33 files changed

+468
-298
lines changed

Diff for: ‎src/tools/clippy/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2772,6 +2772,7 @@ Released 2018-09-13
27722772
[`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push
27732773
[`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
27742774
[`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment
2775+
[`self_named_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructor
27752776
[`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned
27762777
[`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
27772778
[`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse

Diff for: ‎src/tools/clippy/clippy_lints/src/assign_ops.rs

+28-65
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::source::snippet_opt;
33
use clippy_utils::ty::implements_trait;
4-
use clippy_utils::{eq_expr_value, get_trait_def_id, trait_ref_of_method};
5-
use clippy_utils::{higher, paths, sugg};
4+
use clippy_utils::{binop_traits, sugg};
5+
use clippy_utils::{eq_expr_value, trait_ref_of_method};
66
use if_chain::if_chain;
77
use rustc_errors::Applicability;
88
use rustc_hir as hir;
@@ -85,71 +85,34 @@ impl<'tcx> LateLintPass<'tcx> for AssignOps {
8585
let lint = |assignee: &hir::Expr<'_>, rhs: &hir::Expr<'_>| {
8686
let ty = cx.typeck_results().expr_ty(assignee);
8787
let rty = cx.typeck_results().expr_ty(rhs);
88-
macro_rules! ops {
89-
($op:expr,
90-
$cx:expr,
91-
$ty:expr,
92-
$rty:expr,
93-
$($trait_name:ident),+) => {
94-
match $op {
95-
$(hir::BinOpKind::$trait_name => {
96-
let [krate, module] = paths::OPS_MODULE;
97-
let path: [&str; 3] = [krate, module, concat!(stringify!($trait_name), "Assign")];
98-
let trait_id = if let Some(trait_id) = get_trait_def_id($cx, &path) {
99-
trait_id
100-
} else {
101-
return; // useless if the trait doesn't exist
102-
};
103-
// check that we are not inside an `impl AssignOp` of this exact operation
104-
let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id);
105-
if_chain! {
106-
if let Some(trait_ref) = trait_ref_of_method(cx, parent_fn);
107-
if trait_ref.path.res.def_id() == trait_id;
108-
then { return; }
88+
if_chain! {
89+
if let Some((_, lang_item)) = binop_traits(op.node);
90+
if let Ok(trait_id) = cx.tcx.lang_items().require(lang_item);
91+
let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id);
92+
if trait_ref_of_method(cx, parent_fn)
93+
.map_or(true, |t| t.path.res.def_id() != trait_id);
94+
if implements_trait(cx, ty, trait_id, &[rty.into()]);
95+
then {
96+
span_lint_and_then(
97+
cx,
98+
ASSIGN_OP_PATTERN,
99+
expr.span,
100+
"manual implementation of an assign operation",
101+
|diag| {
102+
if let (Some(snip_a), Some(snip_r)) =
103+
(snippet_opt(cx, assignee.span), snippet_opt(cx, rhs.span))
104+
{
105+
diag.span_suggestion(
106+
expr.span,
107+
"replace it with",
108+
format!("{} {}= {}", snip_a, op.node.as_str(), snip_r),
109+
Applicability::MachineApplicable,
110+
);
109111
}
110-
implements_trait($cx, $ty, trait_id, &[$rty])
111-
},)*
112-
_ => false,
113-
}
112+
},
113+
);
114114
}
115115
}
116-
if ops!(
117-
op.node,
118-
cx,
119-
ty,
120-
rty.into(),
121-
Add,
122-
Sub,
123-
Mul,
124-
Div,
125-
Rem,
126-
And,
127-
Or,
128-
BitAnd,
129-
BitOr,
130-
BitXor,
131-
Shr,
132-
Shl
133-
) {
134-
span_lint_and_then(
135-
cx,
136-
ASSIGN_OP_PATTERN,
137-
expr.span,
138-
"manual implementation of an assign operation",
139-
|diag| {
140-
if let (Some(snip_a), Some(snip_r)) =
141-
(snippet_opt(cx, assignee.span), snippet_opt(cx, rhs.span))
142-
{
143-
diag.span_suggestion(
144-
expr.span,
145-
"replace it with",
146-
format!("{} {}= {}", snip_a, op.node.as_str(), snip_r),
147-
Applicability::MachineApplicable,
148-
);
149-
}
150-
},
151-
);
152-
}
153116
};
154117

155118
let mut visitor = ExprVisitor {
@@ -206,7 +169,7 @@ fn lint_misrefactored_assign_op(
206169
if let (Some(snip_a), Some(snip_r)) = (snippet_opt(cx, assignee.span), snippet_opt(cx, rhs_other.span)) {
207170
let a = &sugg::Sugg::hir(cx, assignee, "..");
208171
let r = &sugg::Sugg::hir(cx, rhs, "..");
209-
let long = format!("{} = {}", snip_a, sugg::make_binop(higher::binop(op.node), a, r));
172+
let long = format!("{} = {}", snip_a, sugg::make_binop(op.node.into(), a, r));
210173
diag.span_suggestion(
211174
expr.span,
212175
&format!(

Diff for: ‎src/tools/clippy/clippy_lints/src/eq_op.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
102102
if macro_with_not_op(&left.kind) || macro_with_not_op(&right.kind) {
103103
return;
104104
}
105-
if is_useless_with_eq_exprs(higher::binop(op.node)) && eq_expr_value(cx, left, right) {
105+
if is_useless_with_eq_exprs(op.node.into()) && eq_expr_value(cx, left, right) {
106106
span_lint(
107107
cx,
108108
EQ_OP,

Diff for: ‎src/tools/clippy/clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ mod regex;
330330
mod repeat_once;
331331
mod returns;
332332
mod self_assignment;
333+
mod self_named_constructor;
333334
mod semicolon_if_nothing_returned;
334335
mod serde_api;
335336
mod shadow;
@@ -900,6 +901,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
900901
returns::LET_AND_RETURN,
901902
returns::NEEDLESS_RETURN,
902903
self_assignment::SELF_ASSIGNMENT,
904+
self_named_constructor::SELF_NAMED_CONSTRUCTOR,
903905
semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED,
904906
serde_api::SERDE_API_MISUSE,
905907
shadow::SHADOW_REUSE,
@@ -1406,6 +1408,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
14061408
LintId::of(returns::LET_AND_RETURN),
14071409
LintId::of(returns::NEEDLESS_RETURN),
14081410
LintId::of(self_assignment::SELF_ASSIGNMENT),
1411+
LintId::of(self_named_constructor::SELF_NAMED_CONSTRUCTOR),
14091412
LintId::of(serde_api::SERDE_API_MISUSE),
14101413
LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
14111414
LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT),
@@ -1559,6 +1562,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15591562
LintId::of(redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
15601563
LintId::of(returns::LET_AND_RETURN),
15611564
LintId::of(returns::NEEDLESS_RETURN),
1565+
LintId::of(self_named_constructor::SELF_NAMED_CONSTRUCTOR),
15621566
LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
15631567
LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
15641568
LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME),
@@ -2101,6 +2105,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
21012105
let scripts = conf.allowed_scripts.clone();
21022106
store.register_early_pass(move || box disallowed_script_idents::DisallowedScriptIdents::new(&scripts));
21032107
store.register_late_pass(|| box strlen_on_c_strings::StrlenOnCStrings);
2108+
store.register_late_pass(move || box self_named_constructor::SelfNamedConstructor);
21042109
}
21052110

21062111
#[rustfmt::skip]

Diff for: ‎src/tools/clippy/clippy_lints/src/matches.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ fn check_single_match_single_pattern(
721721
expr: &Expr<'_>,
722722
els: Option<&Expr<'_>>,
723723
) {
724-
if is_wild(&arms[1].pat) {
724+
if is_wild(arms[1].pat) {
725725
report_single_match_single_pattern(cx, ex, arms, expr, els);
726726
}
727727
}
@@ -1287,7 +1287,7 @@ fn find_matches_sugg(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr
12871287
if let Some((b1_arm, b0_arms)) = arms.split_last();
12881288
if let Some(b0) = find_bool_lit(&b0_arms[0].body.kind, desugared);
12891289
if let Some(b1) = find_bool_lit(&b1_arm.body.kind, desugared);
1290-
if is_wild(&b1_arm.pat);
1290+
if is_wild(b1_arm.pat);
12911291
if b0 != b1;
12921292
let if_guard = &b0_arms[0].guard;
12931293
if if_guard.is_none() || b0_arms.len() == 1;

Diff for: ‎src/tools/clippy/clippy_lints/src/self_named_constructor.rs

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
use clippy_utils::diagnostics::span_lint;
2+
use clippy_utils::return_ty;
3+
use clippy_utils::ty::{contains_adt_constructor, contains_ty};
4+
use rustc_hir::{Impl, ImplItem, ImplItemKind, ItemKind, Node};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_session::{declare_lint_pass, declare_tool_lint};
7+
8+
declare_clippy_lint! {
9+
/// **What it does:** Warns when constructors have the same name as their types.
10+
///
11+
/// **Why is this bad?** Repeating the name of the type is redundant.
12+
///
13+
/// **Known problems:** None.
14+
///
15+
/// **Example:**
16+
///
17+
/// ```rust,ignore
18+
/// struct Foo {}
19+
///
20+
/// impl Foo {
21+
/// pub fn foo() -> Foo {
22+
/// Foo {}
23+
/// }
24+
/// }
25+
/// ```
26+
/// Use instead:
27+
/// ```rust,ignore
28+
/// struct Foo {}
29+
///
30+
/// impl Foo {
31+
/// pub fn new() -> Foo {
32+
/// Foo {}
33+
/// }
34+
/// }
35+
/// ```
36+
pub SELF_NAMED_CONSTRUCTOR,
37+
style,
38+
"method should not have the same name as the type it is implemented for"
39+
}
40+
41+
declare_lint_pass!(SelfNamedConstructor => [SELF_NAMED_CONSTRUCTOR]);
42+
43+
impl<'tcx> LateLintPass<'tcx> for SelfNamedConstructor {
44+
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
45+
match impl_item.kind {
46+
ImplItemKind::Fn(ref sig, _) => {
47+
if sig.decl.implicit_self.has_implicit_self() {
48+
return;
49+
}
50+
},
51+
_ => return,
52+
}
53+
54+
let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id());
55+
let item = cx.tcx.hir().expect_item(parent);
56+
let self_ty = cx.tcx.type_of(item.def_id);
57+
let ret_ty = return_ty(cx, impl_item.hir_id());
58+
59+
// Do not check trait impls
60+
if matches!(item.kind, ItemKind::Impl(Impl { of_trait: Some(_), .. })) {
61+
return;
62+
}
63+
64+
// Ensure method is constructor-like
65+
if let Some(self_adt) = self_ty.ty_adt_def() {
66+
if !contains_adt_constructor(ret_ty, self_adt) {
67+
return;
68+
}
69+
} else if !contains_ty(ret_ty, self_ty) {
70+
return;
71+
}
72+
73+
if_chain! {
74+
if let Some(self_def) = self_ty.ty_adt_def();
75+
if let Some(self_local_did) = self_def.did.as_local();
76+
let self_id = cx.tcx.hir().local_def_id_to_hir_id(self_local_did);
77+
if let Some(Node::Item(x)) = cx.tcx.hir().find(self_id);
78+
let type_name = x.ident.name.as_str().to_lowercase();
79+
if impl_item.ident.name.as_str() == type_name || impl_item.ident.name.as_str().replace("_", "") == type_name;
80+
81+
then {
82+
span_lint(
83+
cx,
84+
SELF_NAMED_CONSTRUCTOR,
85+
impl_item.span,
86+
&format!("constructor `{}` has the same name as the type", impl_item.ident.name),
87+
);
88+
}
89+
}
90+
}
91+
}

0 commit comments

Comments
 (0)