in which inferable outlives-requirements are linted

RFC 2093 (tracking issue rust-lang#44493) lets us leave off
commonsensically inferable `T: 'a` outlives requirements. (A separate
feature-gate was split off for the case of 'static lifetimes, for
which questions still remain.) Detecting these was requested as an
idioms-2018 lint.

It turns out that issuing a correct, autofixable suggestion here is
somewhat subtle in the presence of other bounds and generic
parameters. Basically, we want to handle these three cases:

 • One outlives-bound. We want to drop the bound altogether, including
   the colon—

   MyStruct<'a, T: 'a>
                 ^^^^ help: remove this bound

 • An outlives bound first, followed by a trait bound. We want to
   delete the outlives bound and the following plus sign (and
   hopefully get the whitespace right, too)—

   MyStruct<'a, T: 'a + MyTrait>
                   ^^^^^ help: remove this bound

 • An outlives bound after a trait bound. We want to delete the
   outlives lifetime and the preceding plus sign—

   MyStruct<'a, T: MyTrait + 'a>
                          ^^^^^ help: remove this bound

This gets (slightly) even more complicated in the case of where
clauses, where we want to drop the where clause altogether if there's
just the one bound. Hopefully the comments are enough to explain
what's going on!

A script (in Python, sorry) was used to generate the
hopefully-sufficiently-exhaustive UI test input. Some of these are
split off into a different file because rust-lang/rustfix#141
(and, causally upstream of that, rust-lang#53934) prevents them from being
`run-rustfix`-tested.

We also make sure to include a UI test of a case (copied from RFC
2093) where the outlives-bound can't be inferred. Special thanks to
Niko Matsakis for pointing out the `inferred_outlives_of` query,
rather than blindly stripping outlives requirements as if we weren't a
production compiler and didn't care.

This concerns rust-lang#52042.

src/librustc/lint/builtin.rs

@@ -338,6 +338,12 @@ declare_lint! {
      cannot be referred to by absolute paths"
 }
 
+declare_lint! {
+    pub EXPLICIT_OUTLIVES_REQUIREMENTS,
+    Allow,
+    "outlives requirements can be inferred"
+}
+
 /// Some lints that are buffered from `libsyntax`. See `syntax::early_buffered_lints`.
 pub mod parser {
     declare_lint! {

src/librustc_lint/builtin.rs

@@ -1999,3 +1999,233 @@ impl EarlyLintPass for KeywordIdents {
         lint.emit()
     }
 }
+
+
+pub struct ExplicitOutlivesRequirements;
+
+impl LintPass for ExplicitOutlivesRequirements {
+    fn get_lints(&self) -> LintArray {
+        lint_array![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;
+                }
+
+                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()));
+                }
+            }
+        }
+        bound_spans
+    }
+
+    fn consolidate_outlives_bound_spans(
+        &self,
+        lo: Span,
+        bounds: &hir::GenericBounds,
+        bound_spans: Vec<(usize, Span)>
+    ) -> Vec<Span> {
+        if bounds.is_empty() {
+            return Vec::new();
+        }
+        if bound_spans.len() == bounds.len() {
+            let (_, last_bound_span) = bound_spans[bound_spans.len()-1];
+            // If all bounds are inferable, we want to delete the colon, so
+            // start from just after the parameter (span passed as argument)
+            vec![lo.to(last_bound_span)]
+        } else {
+            let mut merged = Vec::new();
+            let mut last_merged_i = None;
+
+            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 `+`
+                    None if i == 0 => {
+                        merged.push(bound_span.to(bounds[1].span().shrink_to_lo()));
+                        last_merged_i = Some(0);
+                    },
+                    // If consecutive bounds are inferable, merge their spans
+                    Some(h) if i == h+1 => {
+                        if let Some(tail) = merged.last_mut() {
+                            // Also eat the trailing `+` if the first
+                            // more-than-one bound is inferable
+                            let to_span = if from_start && i < bounds.len() {
+                                bounds[i+1].span().shrink_to_lo()
+                            } else {
+                                bound_span
+                            };
+                            *tail = tail.to(to_span);
+                            last_merged_i = Some(i);
+                        } else {
+                            bug!("another bound-span visited earlier");
+                        }
+                    },
+                    _ => {
+                        // When we find a non-inferable bound, subsequent inferable bounds
+                        // won't be consecutive from the start (and we'll eat the leading
+                        // `+` rather than the trailing one)
+                        from_start = false;
+                        merged.push(bounds[i-1].span().shrink_to_hi().to(bound_span));
+                        last_merged_i = Some(i);
+                    }
+                }
+            }
+            merged
+        }
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExplicitOutlivesRequirements {
+    fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) {
+        let infer_static = cx.tcx.features().infer_static_outlives_requirements;
+        let def_id = cx.tcx.hir.local_def_id(item.id);
+        if let hir::ItemKind::Struct(_, ref generics) = item.node {
+            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::Plain(name) => name.to_string()
+                        }
+                    }
+                };
+                let bound_spans = self.collect_outlives_bound_spans(
+                    cx, def_id, &param_name, &param.bounds, infer_static
+                );
+                bound_count += bound_spans.len();
+                lint_spans.extend(
+                    self.consolidate_outlives_bound_spans(
+                        param.span.shrink_to_hi(), &param.bounds, bound_spans
+                    )
+                );
+            }
+
+            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(),
+                                &predicate.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"));
+                lint_spans.push(
+                    full_where_span
+                );
+            } else {
+                lint_spans.extend(where_lint_spans);
+            }
+
+            if !lint_spans.is_empty() {
+                let mut err = cx.struct_span_lint(
+                    EXPLICIT_OUTLIVES_REQUIREMENTS,
+                    lint_spans.clone(),
+                    "outlives requirements can be inferred"
+                );
+                err.multipart_suggestion_with_applicability(
+                    if bound_count == 1 {
+                        "remove this bound"
+                    } else {
+                        "remove these bounds"
+                    },
+                    lint_spans.into_iter().map(|span| (span, "".to_owned())).collect::<Vec<_>>(),
+                    Applicability::MachineApplicable
+                );
+                err.emit();
+            }
+
+        }
+    }
+
+}

src/librustc_lint/lib.rs

@@ -48,6 +48,7 @@ use rustc::lint::builtin::{
     BARE_TRAIT_OBJECTS,
     ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
     ELIDED_LIFETIMES_IN_PATHS,
+    EXPLICIT_OUTLIVES_REQUIREMENTS,
     parser::QUESTION_MARK_MACRO_SEP
 };
 use rustc::session;
@@ -157,6 +158,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
         TypeLimits: TypeLimits::new(),
         MissingDoc: MissingDoc::new(),
         MissingDebugImplementations: MissingDebugImplementations::new(),
+        ExplicitOutlivesRequirements: ExplicitOutlivesRequirements,
     ]], ['tcx]);
 
     store.register_late_pass(sess, false, box BuiltinCombinedLateLintPass::new());
@@ -199,7 +201,8 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
                     BARE_TRAIT_OBJECTS,
                     UNUSED_EXTERN_CRATES,
                     ELLIPSIS_INCLUSIVE_RANGE_PATTERNS,
-                    ELIDED_LIFETIMES_IN_PATHS
+                    ELIDED_LIFETIMES_IN_PATHS,
+                    EXPLICIT_OUTLIVES_REQUIREMENTS
 
                     // FIXME(#52665, #47816) not always applicable and not all
                     // macros are ready for this yet.

src/test/ui/rust-2018/edition-lint-infer-outlives-multispan.rs

@@ -0,0 +1,85 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![allow(unused)]
+#![deny(explicit_outlives_requirements)]
+
+use std::fmt::{Debug, Display};
+
+// These examples should live in edition-lint-infer-outlives.rs, but are split
+// into this separate file because they can't be `rustfix`'d (and thus, can't
+// be part of a `run-rustfix` test file) until rust-lang-nursery/rustfix#141
+// is solved
+
+struct TeeOutlivesAyIsDebugBee<'a, 'b, T: 'a + Debug + 'b> {
+    //~^ ERROR outlives requirements can be inferred
+    tee: &'a &'b T
+}
+
+struct TeeWhereOutlivesAyIsDebugBee<'a, 'b, T> where T: 'a + Debug + 'b {
+    //~^ ERROR outlives requirements can be inferred
+    tee: &'a &'b T
+}
+
+struct TeeYooOutlivesAyIsDebugBee<'a, 'b, T, U: 'a + Debug + 'b> {
+    //~^ ERROR outlives requirements can be inferred
+    tee: T,
+    yoo: &'a &'b U
+}
+
+struct TeeOutlivesAyYooBeeIsDebug<'a, 'b, T: 'a, U: 'b + Debug> {
+    //~^ ERROR outlives requirements can be inferred
+    tee: &'a T,
+    yoo: &'b U
+}
+
+struct TeeOutlivesAyYooIsDebugBee<'a, 'b, T: 'a, U: Debug + 'b> {
+    //~^ ERROR outlives requirements can be inferred
+    tee: &'a T,
+    yoo: &'b U
+}
+
+struct TeeOutlivesAyYooWhereBee<'a, 'b, T: 'a, U> where U: 'b {
+    //~^ ERROR outlives requirements can be inferred
+    tee: &'a T,
+    yoo: &'b U
+}
+
+struct TeeYooWhereOutlivesAyIsDebugBee<'a, 'b, T, U> where U: 'a + Debug + 'b {
+    //~^ ERROR outlives requirements can be inferred
+    tee: T,
+    yoo: &'a &'b U
+}
+
+struct TeeOutlivesAyYooWhereBeeIsDebug<'a, 'b, T: 'a, U> where U: 'b + Debug {
+    //~^ ERROR outlives requirements can be inferred
+    tee: &'a T,
+    yoo: &'b U
+}
+
+struct TeeOutlivesAyYooWhereIsDebugBee<'a, 'b, T: 'a, U> where U: Debug + 'b {
+    //~^ ERROR outlives requirements can be inferred
+    tee: &'a T,
+    yoo: &'b U
+}
+
+struct TeeWhereOutlivesAyYooWhereBeeIsDebug<'a, 'b, T, U> where T: 'a, U: 'b + Debug {
+    //~^ ERROR outlives requirements can be inferred
+    tee: &'a T,
+    yoo: &'b U
+}
+
+struct TeeWhereOutlivesAyYooWhereIsDebugBee<'a, 'b, T, U> where T: 'a, U: Debug + 'b {
+    //~^ ERROR outlives requirements can be inferred
+    tee: &'a T,
+    yoo: &'b U
+}
+
+fn main() {}