Auto merge of rust-lang#53013 - zackmdavis:infer_outlints, r=nikomatsakis

in which inferable outlives-requirements are linted

RFC 2093 (tracking issue rust-lang#44493) lets us leave off these
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.

Resolves rust-lang#52042, an item under the fabulous metaïssue rust-lang#52047.

It's plausible that this shouldn't land until after `infer_outlives_requirements` has been stabilized ([final comment period started](rust-lang#44493 (comment)) 4 days ago), but I think there's also a strong case to not-wait in order to maximize the time that [Edition Preview 2](https://internals.rust-lang.org/t/rust-2018-release-schedule-and-extended-beta/8076) users have to kick at it. (It's allow by default, so there's no impact unless you explicitly turn it or the rust-2018-idioms group up to `warn` or higher.)

Questions—

 * Is `explicit-outlives-requirements` a good name? (I chose it as an [RFC 344](https://github.com/rust-lang/rfcs/blob/master/text/0344-conventions-galore.md#lints)-compliant "inversion" of the feature-gate name, `infer_outlives_requirements`, but I could imagine someone arguing that the word `struct` should be part of the name somewhere, for specificity.)

 * Are there any false-positives or false-negatives? @nikomatsakis [said that](rust-lang#52042 (comment)) getting this right would be "fairly hard", which makes me nervous that I'm missing something. The UI test in the initial submission of this pull request just exercises the examples [given in the Edition Guide](https://rust-lang-nursery.github.io/edition-guide/2018/transitioning/ownership-and-lifetimes/struct-inference.html).

![infer_outlints](https://user-images.githubusercontent.com/1076988/43625740-6bf43dca-96a3-11e8-9dcf-793ac83d424d.png)

r? @alexcrichton

Author: bors

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() {}