Improve the explicit_outlives_requirements lint

* Don't use Strings to compare parameters
* Extend the lint to lifetime bounds
* Extend the lint to enums and unions
* Use the correct span for where clauses in tuple structs
* Try to early-out where possible

Author: matthewjasper

src/librustc_lint/builtin.rs

@@ -23,7 +23,7 @@
 
 use rustc::hir::def::{Res, DefKind};
 use rustc::hir::def_id::{DefId, LOCAL_CRATE};
-use rustc::ty::{self, Ty};
+use rustc::ty::{self, Ty, TyCtxt};
 use rustc::{lint, util};
 use hir::Node;
 use util::nodemap::HirIdSet;
@@ -1494,58 +1494,107 @@ impl EarlyLintPass for KeywordIdents {
 declare_lint_pass!(ExplicitOutlivesRequirements => [EXPLICIT_OUTLIVES_REQUIREMENTS]);
 
 impl ExplicitOutlivesRequirements {
-    fn collect_outlives_bound_spans(
-        &self,
-        cx: &LateContext<'_, '_>,
-        item_def_id: DefId,
-        param_name: &str,
-        bounds: &hir::GenericBounds,
-        infer_static: bool
-    ) -> Vec<(usize, Span)> {
-        // For lack of a more elegant strategy for comparing the `ty::Predicate`s
-        // returned by this query with the params/bounds grabbed from the HIR—and
-        // with some regrets—we're going to covert the param/lifetime names to
-        // strings
-        let inferred_outlives = cx.tcx.inferred_outlives_of(item_def_id);
-
-        let ty_lt_names = inferred_outlives.iter().filter_map(|pred| {
-            let binder = match pred {
-                ty::Predicate::TypeOutlives(binder) => binder,
-                _ => { return None; }
-            };
-            let ty_outlives_pred = binder.skip_binder();
-            let ty_name = match ty_outlives_pred.0.sty {
-                ty::Param(param) => param.name.to_string(),
-                _ => { return None; }
-            };
-            let lt_name = match ty_outlives_pred.1 {
-                ty::RegionKind::ReEarlyBound(region) => {
-                    region.name.to_string()
-                },
-                _ => { return None; }
-            };
-            Some((ty_name, lt_name))
-        }).collect::<Vec<_>>();
-
-        let mut bound_spans = Vec::new();
-        for (i, bound) in bounds.iter().enumerate() {
-            if let hir::GenericBound::Outlives(lifetime) = bound {
-                let is_static = match lifetime.name {
-                    hir::LifetimeName::Static => true,
-                    _ => false
-                };
-                if is_static && !infer_static {
-                    // infer-outlives for 'static is still feature-gated (tracking issue #44493)
-                    continue;
+    fn lifetimes_outliving_lifetime<'tcx>(
+        inferred_outlives: &'tcx [ty::Predicate<'tcx>],
+        index: u32,
+    ) -> Vec<ty::Region<'tcx>> {
+        inferred_outlives.iter().filter_map(|pred| {
+            match pred {
+                ty::Predicate::RegionOutlives(outlives) => {
+                    let outlives = outlives.skip_binder();
+                    match outlives.0 {
+                        ty::ReEarlyBound(ebr) if ebr.index == index => {
+                            Some(outlives.1)
+                        }
+                        _ => None,
+                    }
                 }
+                _ => None
+            }
+        }).collect()
+    }
 
-                let lt_name = &lifetime.name.ident().to_string();
-                if ty_lt_names.contains(&(param_name.to_owned(), lt_name.to_owned())) {
-                    bound_spans.push((i, bound.span()));
+    fn lifetimes_outliving_type<'tcx>(
+        inferred_outlives: &'tcx [ty::Predicate<'tcx>],
+        index: u32,
+    ) -> Vec<ty::Region<'tcx>> {
+        inferred_outlives.iter().filter_map(|pred| {
+            match pred {
+                ty::Predicate::TypeOutlives(outlives) => {
+                    let outlives = outlives.skip_binder();
+                    if outlives.0.is_param(index) {
+                        Some(outlives.1)
+                    } else {
+                        None
+                    }
                 }
+                _ => None
+            }
+        }).collect()
+    }
+
+    fn collect_outlived_lifetimes<'tcx>(
+        &self,
+        param: &'tcx hir::GenericParam,
+        tcx: TyCtxt<'tcx>,
+        inferred_outlives: &'tcx [ty::Predicate<'tcx>],
+        ty_generics: &'tcx ty::Generics,
+    ) -> Vec<ty::Region<'tcx>> {
+        let index = ty_generics.param_def_id_to_index[
+            &tcx.hir().local_def_id_from_hir_id(param.hir_id)];
+
+        match param.kind {
+            hir::GenericParamKind::Lifetime { .. } => {
+                Self::lifetimes_outliving_lifetime(inferred_outlives, index)
+            }
+            hir::GenericParamKind::Type { .. } => {
+                Self::lifetimes_outliving_type(inferred_outlives, index)
             }
+            hir::GenericParamKind::Const { .. } => Vec::new(),
         }
-        bound_spans
+    }
+
+
+    fn collect_outlives_bound_spans<'tcx>(
+        &self,
+        tcx: TyCtxt<'tcx>,
+        bounds: &hir::GenericBounds,
+        inferred_outlives: &[ty::Region<'tcx>],
+        infer_static: bool,
+    ) -> Vec<(usize, Span)> {
+        use rustc::middle::resolve_lifetime::Region;
+
+        bounds
+            .iter()
+            .enumerate()
+            .filter_map(|(i, bound)| {
+                if let hir::GenericBound::Outlives(lifetime) = bound {
+                    let is_inferred = match tcx.named_region(lifetime.hir_id) {
+                        Some(Region::Static) if infer_static => {
+                            inferred_outlives.iter()
+                                .any(|r| if let ty::ReStatic = r { true } else { false })
+                        }
+                        Some(Region::EarlyBound(index, ..)) => inferred_outlives
+                            .iter()
+                            .any(|r| {
+                                if let ty::ReEarlyBound(ebr) = r {
+                                    ebr.index == index
+                                } else {
+                                    false
+                                }
+                            }),
+                        _ => false,
+                    };
+                    if is_inferred {
+                        Some((i, bound.span()))
+                    } else {
+                        None
+                    }
+                } else {
+                    None
+                }
+            })
+            .collect()
     }
 
     fn consolidate_outlives_bound_spans(
@@ -1569,7 +1618,7 @@ impl ExplicitOutlivesRequirements {
             let mut from_start = true;
             for (i, bound_span) in bound_spans {
                 match last_merged_i {
-                    // If the first bound is inferable, our span should also eat the trailing `+`
+                    // If the first bound is inferable, our span should also eat the leading `+`
                     None if i == 0 => {
                         merged.push(bound_span.to(bounds[1].span().shrink_to_lo()));
                         last_merged_i = Some(0);
@@ -1607,26 +1656,48 @@ impl ExplicitOutlivesRequirements {
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExplicitOutlivesRequirements {
     fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) {
+        use rustc::middle::resolve_lifetime::Region;
+
         let infer_static = cx.tcx.features().infer_static_outlives_requirements;
         let def_id = cx.tcx.hir().local_def_id_from_hir_id(item.hir_id);
-        if let hir::ItemKind::Struct(_, ref generics) = item.node {
+        if let hir::ItemKind::Struct(_, ref hir_generics)
+            | hir::ItemKind::Enum(_, ref hir_generics)
+            | hir::ItemKind::Union(_, ref hir_generics) = item.node
+        {
+            let inferred_outlives = cx.tcx.inferred_outlives_of(def_id);
+            if inferred_outlives.is_empty() {
+                return;
+            }
+
+            let ty_generics = cx.tcx.generics_of(def_id);
+
             let mut bound_count = 0;
             let mut lint_spans = Vec::new();
 
-            for param in &generics.params {
-                let param_name = match param.kind {
-                    hir::GenericParamKind::Lifetime { .. } => continue,
-                    hir::GenericParamKind::Type { .. } => {
-                        match param.name {
-                            hir::ParamName::Fresh(_) => continue,
-                            hir::ParamName::Error => continue,
-                            hir::ParamName::Plain(name) => name.to_string(),
-                        }
+            for param in &hir_generics.params {
+                let has_lifetime_bounds = param.bounds.iter().any(|bound| {
+                    if let hir::GenericBound::Outlives(_) = bound {
+                        true
+                    } else {
+                        false
                     }
-                    hir::GenericParamKind::Const { .. } => continue,
-                };
+                });
+                if !has_lifetime_bounds {
+                    continue;
+                }
+
+                let relevant_lifetimes = self.collect_outlived_lifetimes(
+                    param,
+                    cx.tcx,
+                    inferred_outlives,
+                    ty_generics,
+                );
+                if relevant_lifetimes.is_empty() {
+                    continue;
+                }
+
                 let bound_spans = self.collect_outlives_bound_spans(
-                    cx, def_id, &param_name, &param.bounds, infer_static
+                    cx.tcx, &param.bounds, &relevant_lifetimes, infer_static,
                 );
                 bound_count += bound_spans.len();
                 lint_spans.extend(
@@ -1638,54 +1709,92 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExplicitOutlivesRequirements {
 
             let mut where_lint_spans = Vec::new();
             let mut dropped_predicate_count = 0;
-            let num_predicates = generics.where_clause.predicates.len();
-            for (i, where_predicate) in generics.where_clause.predicates.iter().enumerate() {
-                if let hir::WherePredicate::BoundPredicate(predicate) = where_predicate {
-                    let param_name = match predicate.bounded_ty.node {
-                        hir::TyKind::Path(ref qpath) => {
-                            if let hir::QPath::Resolved(None, ty_param_path) = qpath {
-                                ty_param_path.segments[0].ident.to_string()
-                            } else {
-                                continue;
-                            }
-                        },
-                        _ => { continue; }
-                    };
-                    let bound_spans = self.collect_outlives_bound_spans(
-                        cx, def_id, &param_name, &predicate.bounds, infer_static
-                    );
-                    bound_count += bound_spans.len();
-
-                    let drop_predicate = bound_spans.len() == predicate.bounds.len();
-                    if drop_predicate {
-                        dropped_predicate_count += 1;
-                    }
-
-                    // If all the bounds on a predicate were inferable and there are
-                    // further predicates, we want to eat the trailing comma
-                    if drop_predicate && i + 1 < num_predicates {
-                        let next_predicate_span = generics.where_clause.predicates[i+1].span();
-                        where_lint_spans.push(
-                            predicate.span.to(next_predicate_span.shrink_to_lo())
-                        );
-                    } else {
-                        where_lint_spans.extend(
-                            self.consolidate_outlives_bound_spans(
-                                predicate.span.shrink_to_lo(),
+            let num_predicates = hir_generics.where_clause.predicates.len();
+            for (i, where_predicate) in hir_generics.where_clause.predicates.iter().enumerate() {
+                let (relevant_lifetimes, bounds, span) = match where_predicate {
+                    hir::WherePredicate::RegionPredicate(predicate) => {
+                        if let Some(Region::EarlyBound(index, ..))
+                            = cx.tcx.named_region(predicate.lifetime.hir_id)
+                        {
+                            (
+                                Self::lifetimes_outliving_lifetime(inferred_outlives, index),
                                 &predicate.bounds,
-                                bound_spans
+                                predicate.span,
                             )
-                        );
+                        } else {
+                            continue;
+                        }
                     }
+                    hir::WherePredicate::BoundPredicate(predicate) => {
+                        // FIXME we can also infer bounds on associated types,
+                        // and should check for them here.
+                        match predicate.bounded_ty.node {
+                            hir::TyKind::Path(hir::QPath::Resolved(
+                                None,
+                                ref path,
+                            )) => if let Res::Def(DefKind::TyParam, def_id) = path.res {
+                                let index = ty_generics.param_def_id_to_index[&def_id];
+                                (
+                                    Self::lifetimes_outliving_type(inferred_outlives, index),
+                                    &predicate.bounds,
+                                    predicate.span,
+                                )
+                            } else {
+                                continue
+                            },
+                            _ => { continue; }
+                        }
+                    }
+                    _ => continue,
+                };
+                if relevant_lifetimes.is_empty() {
+                    continue;
+                }
+
+                let bound_spans = self.collect_outlives_bound_spans(
+                    cx.tcx, bounds, &relevant_lifetimes, infer_static,
+                );
+                bound_count += bound_spans.len();
+
+                let drop_predicate = bound_spans.len() == bounds.len();
+                if drop_predicate {
+                    dropped_predicate_count += 1;
+                }
+
+                // If all the bounds on a predicate were inferable and there are
+                // further predicates, we want to eat the trailing comma
+                if drop_predicate && i + 1 < num_predicates {
+                    let next_predicate_span = hir_generics.where_clause.predicates[i+1].span();
+                    where_lint_spans.push(
+                        span.to(next_predicate_span.shrink_to_lo())
+                    );
+                } else {
+                    where_lint_spans.extend(
+                        self.consolidate_outlives_bound_spans(
+                            span.shrink_to_lo(),
+                            bounds,
+                            bound_spans
+                        )
+                    );
                 }
             }
 
             // If all predicates are inferable, drop the entire clause
             // (including the `where`)
             if num_predicates > 0 && dropped_predicate_count == num_predicates {
-                let full_where_span = generics.span.shrink_to_hi()
-                    .to(generics.where_clause.span()
-                    .expect("span of (nonempty) where clause should exist"));
+                let where_span = hir_generics.where_clause.span()
+                    .expect("span of (nonempty) where clause should exist");
+                // Extend the where clause back to the closing `>` of the
+                // generics, except for tuple struct, which have the `where`
+                // after the fields of the struct.
+                let full_where_span = match item.node {
+                    hir::ItemKind::Struct(hir::VariantData::Tuple(..), _) => {
+                        where_span
+                    }
+                    _ => {
+                        hir_generics.span.shrink_to_hi().to(where_span)
+                    }
+                };
                 lint_spans.push(
                     full_where_span
                 );