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

The Great Generics Generalisation: Clippy edition #2871

Merged
merged 4 commits into from
Jun 25, 2018
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
2 changes: 1 addition & 1 deletion clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass {
}

fn is_relevant_item(tcx: TyCtxt, item: &Item) -> bool {
if let ItemFn(_, _, _, _, _, eid) = item.node {
if let ItemFn(_, _, _, eid) = item.node {
is_relevant_expr(tcx, tcx.body_tables(eid), &tcx.hir.body(eid).value)
} else {
true
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/eval_order_dependence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
impl<'a, 'tcx> Visitor<'tcx> for DivergenceVisitor<'a, 'tcx> {
fn visit_expr(&mut self, e: &'tcx Expr) {
match e.node {
ExprAgain(_) | ExprBreak(_, _) | ExprRet(_) => self.report_diverging_sub_expr(e),
ExprContinue(_) | ExprBreak(_, _) | ExprRet(_) => self.report_diverging_sub_expr(e),
ExprCall(ref func, _) => {
let typ = self.cx.tables.expr_ty(func);
match typ.sty {
Expand Down
12 changes: 6 additions & 6 deletions clippy_lints/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions {
};

let unsafety = match kind {
hir::intravisit::FnKind::ItemFn(_, _, unsafety, _, _, _, _) => unsafety,
hir::intravisit::FnKind::Method(_, sig, _, _) => sig.unsafety,
hir::intravisit::FnKind::ItemFn(_, _, hir::FnHeader { unsafety, .. }, _, _) => unsafety,
hir::intravisit::FnKind::Method(_, sig, _, _) => sig.header.unsafety,
hir::intravisit::FnKind::Closure(_) => return,
};

// don't warn for implementations, it's not their fault
if !is_impl {
// don't lint extern functions decls, it's not their fault either
match kind {
hir::intravisit::FnKind::Method(_, &hir::MethodSig { abi: Abi::Rust, .. }, _, _) |
hir::intravisit::FnKind::ItemFn(_, _, _, _, Abi::Rust, _, _) => self.check_arg_number(cx, decl, span),
hir::intravisit::FnKind::Method(_, &hir::MethodSig { header: hir::FnHeader { abi: Abi::Rust, .. }, .. }, _, _) |
hir::intravisit::FnKind::ItemFn(_, _, hir::FnHeader { abi: Abi::Rust, .. }, _, _) => self.check_arg_number(cx, decl, span),
_ => {},
}
}
Expand All @@ -113,13 +113,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions {
fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem) {
if let hir::TraitItemKind::Method(ref sig, ref eid) = item.node {
// don't lint extern functions decls, it's not their fault
if sig.abi == Abi::Rust {
if sig.header.abi == Abi::Rust {
self.check_arg_number(cx, &sig.decl, item.span);
}

if let hir::TraitMethod::Provided(eid) = *eid {
let body = cx.tcx.hir.body(eid);
self.check_raw_ptr(cx, sig.unsafety, &sig.decl, body, item.id);
self.check_raw_ptr(cx, sig.header.unsafety, &sig.decl, body, item.id);
}
}
}
Expand Down
48 changes: 31 additions & 17 deletions clippy_lints/src/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl LintPass for LifetimePass {

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LifetimePass {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
if let ItemFn(ref decl, _, _, _, ref generics, id) = item.node {
if let ItemFn(ref decl, _, ref generics, id) = item.node {
check_fn_inner(cx, decl, Some(id), generics, item.span);
}
}
Expand Down Expand Up @@ -101,23 +101,31 @@ fn check_fn_inner<'a, 'tcx>(
}

let mut bounds_lts = Vec::new();
for typ in generics.ty_params() {
let types = generics.params.iter().filter_map(|param| match param.kind {
GenericParamKind::Type { .. } => Some(param),
GenericParamKind::Lifetime { .. } => None,
});
for typ in types {
for bound in &typ.bounds {
let mut visitor = RefVisitor::new(cx);
walk_ty_param_bound(&mut visitor, bound);
walk_param_bound(&mut visitor, bound);
if visitor.lts.iter().any(|lt| matches!(lt, RefLt::Named(_))) {
return;
}
if let TraitTyParamBound(ref trait_ref, _) = *bound {
if let GenericBound::Trait(ref trait_ref, _) = *bound {
let params = &trait_ref
.trait_ref
.path
.segments
.last()
.expect("a path must have at least one segment")
.parameters;
.args;
if let Some(ref params) = *params {
for bound in &params.lifetimes {
let lifetimes = params.args.iter().filter_map(|arg| match arg {
GenericArg::Lifetime(lt) => Some(lt),
GenericArg::Type(_) => None,
});
for bound in lifetimes {
if bound.name.name() != "'static" && !bound.is_elided() {
return;
}
Expand Down Expand Up @@ -230,9 +238,9 @@ fn could_use_elision<'a, 'tcx: 'a>(
fn allowed_lts_from(named_generics: &[GenericParam]) -> HashSet<RefLt> {
let mut allowed_lts = HashSet::new();
for par in named_generics.iter() {
if let GenericParam::Lifetime(ref lt) = *par {
if lt.bounds.is_empty() {
allowed_lts.insert(RefLt::Named(lt.lifetime.name.name()));
if let GenericParamKind::Lifetime { .. } = par.kind {
if par.bounds.is_empty() {
allowed_lts.insert(RefLt::Named(par.name.name()));
}
}
}
Expand Down Expand Up @@ -295,8 +303,12 @@ impl<'v, 't> RefVisitor<'v, 't> {
}

fn collect_anonymous_lifetimes(&mut self, qpath: &QPath, ty: &Ty) {
if let Some(ref last_path_segment) = last_path_segment(qpath).parameters {
if !last_path_segment.parenthesized && last_path_segment.lifetimes.is_empty() {
if let Some(ref last_path_segment) = last_path_segment(qpath).args {
if !last_path_segment.parenthesized
&& !last_path_segment.args.iter().any(|arg| match arg {
GenericArg::Lifetime(_) => true,
GenericArg::Type(_) => false,
}) {
let hir_id = self.cx.tcx.hir.node_to_hir_id(ty.id);
match self.cx.tables.qpath_def(qpath, hir_id) {
Def::TyAlias(def_id) | Def::Struct(def_id) => {
Expand Down Expand Up @@ -335,7 +347,7 @@ impl<'a, 'tcx> Visitor<'tcx> for RefVisitor<'a, 'tcx> {
TyImplTraitExistential(exist_ty_id, _, _) => {
if let ItemExistential(ref exist_ty) = self.cx.tcx.hir.expect_item(exist_ty_id.id).node {
for bound in &exist_ty.bounds {
if let RegionTyParamBound(_) = *bound {
if let GenericBound::Outlives(_) = *bound {
self.record(&None);
}
}
Expand Down Expand Up @@ -377,7 +389,7 @@ fn has_where_lifetimes<'a, 'tcx: 'a>(cx: &LateContext<'a, 'tcx>, where_clause: &
let allowed_lts = allowed_lts_from(&pred.bound_generic_params);
// now walk the bounds
for bound in pred.bounds.iter() {
walk_ty_param_bound(&mut visitor, bound);
walk_param_bound(&mut visitor, bound);
}
// and check that all lifetimes are allowed
match visitor.into_vec() {
Expand Down Expand Up @@ -418,7 +430,7 @@ impl<'tcx> Visitor<'tcx> for LifetimeChecker {
// don't want to spuriously remove them
// `'b` in `'a: 'b` is useless unless used elsewhere in
// a non-lifetime bound
if param.is_type_param() {
if let GenericParamKind::Type { .. } = param.kind {
walk_generic_param(self, param)
}
}
Expand All @@ -428,9 +440,11 @@ impl<'tcx> Visitor<'tcx> for LifetimeChecker {
}

fn report_extra_lifetimes<'a, 'tcx: 'a>(cx: &LateContext<'a, 'tcx>, func: &'tcx FnDecl, generics: &'tcx Generics) {
let hs = generics
.lifetimes()
.map(|lt| (lt.lifetime.name.name(), lt.lifetime.span))
let hs = generics.params.iter()
.filter_map(|par| match par.kind {
GenericParamKind::Lifetime { .. } => Some((par.name.name(), par.span)),
_ => None,
})
.collect();
let mut checker = LifetimeChecker { map: hs };

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ fn never_loop_expr(expr: &Expr, main_loop_id: NodeId) -> NeverLoopResult {
}
},
ExprBlock(ref b, _) => never_loop_block(b, main_loop_id),
ExprAgain(d) => {
ExprContinue(d) => {
let id = d.target_id
.expect("target id can only be missing in the presence of compilation errors");
if id == main_loop_id {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/map_clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ fn expr_eq_name(expr: &Expr, id: ast::Name) -> bool {
let arg_segment = [
PathSegment {
name: id,
parameters: None,
args: None,
infer_types: true,
},
];
Expand Down
53 changes: 31 additions & 22 deletions clippy_lints/src/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::fmt;
use std::iter;
use syntax::ast;
use syntax::codemap::{Span, BytePos};
use crate::utils::{get_arg_name, get_trait_def_id, implements_trait, in_external_macro, in_macro, is_copy, is_expn_of, is_self,
use crate::utils::{get_arg_name, get_trait_def_id, implements_trait, in_external_macro, in_macro, is_copy, is_expn_of, is_self,
is_self_ty, iter_input_pats, last_path_segment, match_def_path, match_path, match_qpath, match_trait_method,
match_type, method_chain_args, match_var, return_ty, remove_blocks, same_tys, single_segment_path, snippet,
span_lint, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth};
Expand Down Expand Up @@ -336,7 +336,7 @@ declare_clippy_lint! {
///
/// **Known problems:** If the function has side-effects, not calling it will
/// change the semantic of the program, but you shouldn't rely on that anyway.
///
///
/// **Example:**
/// ```rust
/// foo.expect(&format("Err {}: {}", err_code, err_msg))
Expand Down Expand Up @@ -1020,7 +1020,7 @@ fn lint_expect_fun_call(cx: &LateContext, expr: &hir::Expr, method_span: Span, n
}
}
};

snippet(cx, a.span, "..").into_owned()
}

Expand Down Expand Up @@ -1077,7 +1077,7 @@ fn lint_expect_fun_call(cx: &LateContext, expr: &hir::Expr, method_span: Span, n
}

let sugg: Cow<_> = snippet(cx, arg.span, "..");

span_lint_and_sugg(
cx,
EXPECT_FUN_CALL,
Expand Down Expand Up @@ -2091,26 +2091,35 @@ impl SelfKind {

fn is_as_ref_or_mut_trait(ty: &hir::Ty, self_ty: &hir::Ty, generics: &hir::Generics, name: &[&str]) -> bool {
single_segment_ty(ty).map_or(false, |seg| {
generics.ty_params().any(|param| {
param.name == seg.name && param.bounds.iter().any(|bound| {
if let hir::TyParamBound::TraitTyParamBound(ref ptr, ..) = *bound {
let path = &ptr.trait_ref.path;
match_path(path, name) && path.segments.last().map_or(false, |s| {
if let Some(ref params) = s.parameters {
if params.parenthesized {
false
generics.params.iter().any(|param| match param.kind {
hir::GenericParamKind::Type { .. } => {
param.name.name() == seg.name && param.bounds.iter().any(|bound| {
if let hir::GenericBound::Trait(ref ptr, ..) = *bound {
let path = &ptr.trait_ref.path;
match_path(path, name) && path.segments.last().map_or(false, |s| {
if let Some(ref params) = s.args {
if params.parenthesized {
false
} else {
// FIXME(flip1995): messy, improve if there is a better option
// in the compiler
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is usually done with an iterator and matching on (it.next(), it.next()).

let types: Vec<_> = params.args.iter().filter_map(|arg| match arg {
hir::GenericArg::Type(ty) => Some(ty),
_ => None,
}).collect();
types.len() == 1
&& (is_self_ty(&types[0]) || is_ty(&*types[0], self_ty))
}
} else {
params.types.len() == 1
&& (is_self_ty(&params.types[0]) || is_ty(&*params.types[0], self_ty))
false
}
} else {
false
}
})
} else {
false
}
})
})
} else {
false
}
})
},
_ => false,
})
})
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ fn in_attributes_expansion(expr: &Expr) -> bool {
.ctxt()
.outer()
.expn_info()
.map_or(false, |info| matches!(info.callee.format, ExpnFormat::MacroAttribute(_)))
.map_or(false, |info| matches!(info.format, ExpnFormat::MacroAttribute(_)))
}

/// Test whether `def` is a variable defined outside a macro.
Expand Down
10 changes: 5 additions & 5 deletions clippy_lints/src/misc_early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,13 @@ impl LintPass for MiscEarly {
impl EarlyLintPass for MiscEarly {
fn check_generics(&mut self, cx: &EarlyContext, gen: &Generics) {
for param in &gen.params {
if let GenericParam::Type(ref ty) = *param {
let name = ty.ident.name.as_str();
if let GenericParamKind::Type { .. } = param.kind {
let name = param.ident.name.as_str();
if constants::BUILTIN_TYPES.contains(&&*name) {
span_lint(
cx,
BUILTIN_TYPE_SHADOW,
ty.ident.span,
param.ident.span,
&format!("This generic shadows the built-in type `{}`", name),
);
}
Expand Down Expand Up @@ -296,7 +296,7 @@ impl EarlyLintPass for MiscEarly {
}
match expr.node {
ExprKind::Call(ref paren, _) => if let ExprKind::Paren(ref closure) = paren.node {
if let ExprKind::Closure(_, _, ref decl, ref block, _) = closure.node {
if let ExprKind::Closure(_, _, _, ref decl, ref block, _) = closure.node {
span_lint_and_then(
cx,
REDUNDANT_CLOSURE_CALL,
Expand Down Expand Up @@ -327,7 +327,7 @@ impl EarlyLintPass for MiscEarly {
if_chain! {
if let StmtKind::Local(ref local) = w[0].node;
if let Option::Some(ref t) = local.init;
if let ExprKind::Closure(_, _, _, _, _) = t.node;
if let ExprKind::Closure(..) = t.node;
if let PatKind::Ident(_, ident, _) = local.pat.node;
if let StmtKind::Semi(ref second) = w[1].node;
if let ExprKind::Assign(_, ref call) = second.node;
Expand Down
11 changes: 7 additions & 4 deletions clippy_lints/src/needless_pass_by_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
}

match kind {
FnKind::ItemFn(.., abi, _, attrs) => {
if abi != Abi::Rust {
FnKind::ItemFn(.., header, _, attrs) => {
if header.abi != Abi::Rust {
return;
}
for a in attrs {
Expand Down Expand Up @@ -218,8 +218,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
if let TyPath(QPath::Resolved(_, ref path)) = input.node;
if let Some(elem_ty) = path.segments.iter()
.find(|seg| seg.name == "Vec")
.and_then(|ps| ps.parameters.as_ref())
.map(|params| &params.types[0]);
.and_then(|ps| ps.args.as_ref())
.map(|params| params.args.iter().find_map(|arg| match arg {
GenericArg::Type(ty) => Some(ty),
GenericArg::Lifetime(_) => None,
}).unwrap());
then {
let slice_ty = format!("&[{}]", snippet(cx, elem_ty.span, "_"));
db.span_suggestion(input.span,
Expand Down
7 changes: 5 additions & 2 deletions clippy_lints/src/new_without_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault {
if let hir::ImplItemKind::Method(ref sig, _) = impl_item.node {
let name = impl_item.name;
let id = impl_item.id;
if sig.constness == hir::Constness::Const {
if sig.header.constness == hir::Constness::Const {
// can't be implemented by default
return;
}
if impl_item.generics.params.iter().any(|gen| gen.is_type_param()) {
if impl_item.generics.params.iter().any(|gen| match gen.kind {
hir::GenericParamKind::Type { .. } => true,
_ => false
}) {
// when the result of `new()` depends on a type parameter we should not require
// an
// impl of `Default`
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/non_expressive_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ impl<'a, 'tcx> Visitor<'tcx> for SimilarNamesLocalVisitor<'a, 'tcx> {

impl EarlyLintPass for NonExpressiveNames {
fn check_item(&mut self, cx: &EarlyContext, item: &Item) {
if let ItemKind::Fn(ref decl, _, _, _, _, ref blk) = item.node {
if let ItemKind::Fn(ref decl, _, _, ref blk) = item.node {
do_check(self, cx, &item.attrs, decl, blk);
}
}
Expand Down
Loading