Skip to content

Commit

Permalink
Merge pull request #2871 from flip1995/gen_gen_nightly
Browse files Browse the repository at this point in the history
The Great Generics Generalisation: Clippy edition
  • Loading branch information
oli-obk authored Jun 25, 2018
2 parents 5f5fa08 + 203ad28 commit d4618e0
Show file tree
Hide file tree
Showing 23 changed files with 179 additions and 104 deletions.
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
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

0 comments on commit d4618e0

Please sign in to comment.