Rollup merge of rust-lang#48909 - RalfJung:type_alias_bounds, r=petro…

…chenkov

Improve lint for type alias bounds

First of all, I learned just today that I was wrong assuming that the bounds in type aliases are entirely ignored: It turns out they are used to resolve associated types in type aliases. So:
```rust
type T1<U: Bound> = U::Assoc; // compiles
type T2<U> = U::Assoc; // fails
type T3<U> = <U as Bound>::Assoc; // "correct" way to write this, maybe?
```
I am sorry for creating this mess.

This PR changes the wording of the lint accordingly. Moreover, since just removing the bound is no longer always a possible fix, I tried to detect cases like `T1` above and show a helpful message to the user:
```
warning: bounds on generic parameters are not enforced in type aliases
  --> $DIR/type-alias-bounds.rs:57:12
   |
LL | type T1<U: Bound> = U::Assoc; //~ WARN not enforced in type aliases
   |            ^^^^^
   |
   = help: the bound will not be checked when the type alias is used, and should be removed
help: use absolute paths (i.e., <T as Trait>::Assoc) to refer to associated types in type aliases
  --> $DIR/type-alias-bounds.rs:57:21
   |
LL | type T1<U: Bound> = U::Assoc; //~ WARN not enforced in type aliases
   |                     ^^^^^^^^
```
I am not sure if I got this entirely right. Ideally, we could provide a suggestion involving the correct trait and type name -- however, while I have access to the HIR in the lint, I do not know how to get access to the resolved name information, like which trait `Assoc` belongs to above. The lint does not even run if that resolution fails, so I assume that information is available *somewhere*...

This is a follow-up for (parts of) rust-lang#48326. Also see rust-lang#21903.

This changes the name of a lint, but that lint was just merged to master yesterday and has never even been on beta.

src/librustc/hir/mod.rs

@@ -395,6 +395,15 @@ pub enum TyParamBound {
     RegionTyParamBound(Lifetime),
 }
 
+impl TyParamBound {
+    pub fn span(&self) -> Span {
+        match self {
+            &TraitTyParamBound(ref t, ..) => t.span,
+            &RegionTyParamBound(ref l) => l.span,
+        }
+    }
+}
+
 /// A modifier on a bound, currently this is only used for `?Sized`, where the
 /// modifier is `Maybe`. Negative bounds should also be handled here.
 #[derive(Copy, Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
@@ -571,6 +580,16 @@ pub enum WherePredicate {
     EqPredicate(WhereEqPredicate),
 }
 
+impl WherePredicate {
+    pub fn span(&self) -> Span {
+        match self {
+            &WherePredicate::BoundPredicate(ref p) => p.span,
+            &WherePredicate::RegionPredicate(ref p) => p.span,
+            &WherePredicate::EqPredicate(ref p) => p.span,
+        }
+    }
+}
+
 /// A type bound, eg `for<'c> Foo: Send+Clone+'c`
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
 pub struct WhereBoundPredicate {

src/librustc_lint/builtin.rs

@@ -46,6 +46,7 @@ use syntax::attr;
 use syntax::feature_gate::{AttributeGate, AttributeType, Stability, deprecated_attributes};
 use syntax_pos::{BytePos, Span, SyntaxContext};
 use syntax::symbol::keywords;
+use syntax::errors::DiagnosticBuilder;
 
 use rustc::hir::{self, PatKind};
 use rustc::hir::intravisit::FnKind;
@@ -1316,48 +1317,115 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnreachablePub {
     }
 }
 
-/// Lint for trait and lifetime bounds that are (accidentally) accepted by the parser, but
-/// ignored later.
+/// Lint for trait and lifetime bounds in type aliases being mostly ignored:
+/// They are relevant when using associated types, but otherwise neither checked
+/// at definition site nor enforced at use site.
 
-pub struct IgnoredGenericBounds;
+pub struct TypeAliasBounds;
 
 declare_lint! {
-    IGNORED_GENERIC_BOUNDS,
+    TYPE_ALIAS_BOUNDS,
     Warn,
-    "these generic bounds are ignored"
+    "bounds in type aliases are not enforced"
 }
 
-impl LintPass for IgnoredGenericBounds {
+impl LintPass for TypeAliasBounds {
     fn get_lints(&self) -> LintArray {
-        lint_array!(IGNORED_GENERIC_BOUNDS)
+        lint_array!(TYPE_ALIAS_BOUNDS)
     }
 }
 
-impl EarlyLintPass for IgnoredGenericBounds {
-    fn check_item(&mut self, cx: &EarlyContext, item: &ast::Item) {
-        let type_alias_generics = match item.node {
-            ast::ItemKind::Ty(_, ref generics) => generics,
+impl TypeAliasBounds {
+    fn is_type_variable_assoc(qpath: &hir::QPath) -> bool {
+        match *qpath {
+            hir::QPath::TypeRelative(ref ty, _) => {
+                // If this is a type variable, we found a `T::Assoc`.
+                match ty.node {
+                    hir::TyPath(hir::QPath::Resolved(None, ref path)) => {
+                        match path.def {
+                            Def::TyParam(_) => true,
+                            _ => false
+                        }
+                    }
+                    _ => false
+                }
+            }
+            hir::QPath::Resolved(..) => false,
+        }
+    }
+
+    fn suggest_changing_assoc_types(ty: &hir::Ty, err: &mut DiagnosticBuilder) {
+        // Access to associates types should use `<T as Bound>::Assoc`, which does not need a
+        // bound.  Let's see if this type does that.
+
+        // We use a HIR visitor to walk the type.
+        use rustc::hir::intravisit::{self, Visitor};
+        use syntax::ast::NodeId;
+        struct WalkAssocTypes<'a, 'db> where 'db: 'a {
+            err: &'a mut DiagnosticBuilder<'db>
+        }
+        impl<'a, 'db, 'v> Visitor<'v> for WalkAssocTypes<'a, 'db> {
+            fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'v>
+            {
+                intravisit::NestedVisitorMap::None
+            }
+
+            fn visit_qpath(&mut self, qpath: &'v hir::QPath, id: NodeId, span: Span) {
+                if TypeAliasBounds::is_type_variable_assoc(qpath) {
+                    self.err.span_help(span,
+                        "use fully disambiguated paths (i.e., `<T as Trait>::Assoc`) to refer to \
+                         associated types in type aliases");
+                }
+                intravisit::walk_qpath(self, qpath, id, span)
+            }
+        }
+
+        // Let's go for a walk!
+        let mut visitor = WalkAssocTypes { err };
+        visitor.visit_ty(ty);
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypeAliasBounds {
+    fn check_item(&mut self, cx: &LateContext, item: &hir::Item) {
+        let (ty, type_alias_generics) = match item.node {
+            hir::ItemTy(ref ty, ref generics) => (&*ty, generics),
             _ => return,
         };
+        let mut suggested_changing_assoc_types = false;
         // There must not be a where clause
         if !type_alias_generics.where_clause.predicates.is_empty() {
             let spans : Vec<_> = type_alias_generics.where_clause.predicates.iter()
                 .map(|pred| pred.span()).collect();
-            cx.span_lint(IGNORED_GENERIC_BOUNDS, spans,
-                "where clauses are ignored in type aliases");
+            let mut err = cx.struct_span_lint(TYPE_ALIAS_BOUNDS, spans,
+                "where clauses are not enforced in type aliases");
+            err.help("the clause will not be checked when the type alias is used, \
+                      and should be removed");
+            if !suggested_changing_assoc_types {
+                TypeAliasBounds::suggest_changing_assoc_types(ty, &mut err);
+                suggested_changing_assoc_types = true;
+            }
+            err.emit();
         }
         // The parameters must not have bounds
         for param in type_alias_generics.params.iter() {
             let spans : Vec<_> = match param {
-                &ast::GenericParam::Lifetime(ref l) => l.bounds.iter().map(|b| b.span).collect(),
-                &ast::GenericParam::Type(ref ty) => ty.bounds.iter().map(|b| b.span()).collect(),
+                &hir::GenericParam::Lifetime(ref l) => l.bounds.iter().map(|b| b.span).collect(),
+                &hir::GenericParam::Type(ref ty) => ty.bounds.iter().map(|b| b.span()).collect(),
             };
             if !spans.is_empty() {
-                cx.span_lint(
-                    IGNORED_GENERIC_BOUNDS,
+                let mut err = cx.struct_span_lint(
+                    TYPE_ALIAS_BOUNDS,
                     spans,
-                    "bounds on generic parameters are ignored in type aliases",
+                    "bounds on generic parameters are not enforced in type aliases",
                 );
+                err.help("the bound will not be checked when the type alias is used, \
+                          and should be removed");
+                if !suggested_changing_assoc_types {
+                    TypeAliasBounds::suggest_changing_assoc_types(ty, &mut err);
+                    suggested_changing_assoc_types = true;
+                }
+                err.emit();
             }
         }
     }

src/librustc_lint/lib.rs

@@ -109,7 +109,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
                        UnusedImportBraces,
                        AnonymousParameters,
                        UnusedDocComment,
-                       IgnoredGenericBounds,
                        );
 
     add_early_builtin_with_new!(sess,
@@ -139,6 +138,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
                  MutableTransmutes,
                  UnionsWithDropFields,
                  UnreachablePub,
+                 TypeAliasBounds,
                  );
 
     add_builtin_with_new!(sess,

src/test/compile-fail/issue-17994.rs

@@ -10,5 +10,4 @@
 
 trait Tr {}
 type Huh<T> where T: Tr = isize; //~  ERROR type parameter `T` is unused
-                                 //~| WARNING where clauses are ignored in type aliases
 fn main() {}

src/test/compile-fail/private-in-public-warn.rs

@@ -58,7 +58,6 @@ mod traits {
     pub trait PubTr {}
 
     pub type Alias<T: PrivTr> = T; //~ ERROR private trait `traits::PrivTr` in public interface
-    //~^ WARNING bounds on generic parameters are ignored
     //~| WARNING hard error
     pub trait Tr1: PrivTr {} //~ ERROR private trait `traits::PrivTr` in public interface
     //~^ WARNING hard error
@@ -85,7 +84,6 @@ mod traits_where {
     pub type Alias<T> where T: PrivTr = T;
         //~^ ERROR private trait `traits_where::PrivTr` in public interface
         //~| WARNING hard error
-        //~| WARNING where clauses are ignored in type aliases
     pub trait Tr2<T> where T: PrivTr {}
         //~^ ERROR private trait `traits_where::PrivTr` in public interface
         //~| WARNING hard error

src/test/ui/param-bounds-ignored.rs → src/test/ui/higher-lifetime-bounds.rs

@@ -10,31 +10,7 @@
 
 #![allow(dead_code, non_camel_case_types)]
 
-use std::rc::Rc;
-
-type SVec<T: Send+Send> = Vec<T>;
-//~^ WARN bounds on generic parameters are ignored in type aliases
-type VVec<'b, 'a: 'b+'b> = Vec<&'a i32>;
-//~^ WARN bounds on generic parameters are ignored in type aliases
-type WVec<'b, T: 'b+'b> = Vec<T>;
-//~^ WARN bounds on generic parameters are ignored in type aliases
-type W2Vec<'b, T> where T: 'b, T: 'b = Vec<T>;
-//~^ WARN where clauses are ignored in type aliases
-
-fn foo<'a>(y: &'a i32) {
-    // If the bounds above would matter, the code below would be rejected.
-    let mut x : SVec<_> = Vec::new();
-    x.push(Rc::new(42));
-
-    let mut x : VVec<'static, 'a> = Vec::new();
-    x.push(y);
-
-    let mut x : WVec<'static, & 'a i32> = Vec::new();
-    x.push(y);
-
-    let mut x : W2Vec<'static, & 'a i32> = Vec::new();
-    x.push(y);
-}
+// Test that bounds on higher-kinded lifetime binders are rejected.
 
 fn bar1<'a, 'b>(
     x: &'a i32,

src/test/ui/param-bounds-ignored.stderr → src/test/ui/higher-lifetime-bounds.stderr

@@ -1,94 +1,68 @@
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:42:22
+  --> $DIR/higher-lifetime-bounds.rs:18:22
    |
 LL |     f: for<'xa, 'xb: 'xa+'xa> fn(&'xa i32, &'xb i32) -> &'xa i32)
    |                      ^^^ ^^^
 
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:50:34
+  --> $DIR/higher-lifetime-bounds.rs:26:34
    |
 LL | fn bar2<'a, 'b, F: for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32>(
    |                                  ^^^
 
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:65:28
+  --> $DIR/higher-lifetime-bounds.rs:41:28
    |
 LL |     where F: for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32
    |                            ^^^
 
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:77:25
+  --> $DIR/higher-lifetime-bounds.rs:53:25
    |
 LL |     where for<'xa, 'xb: 'xa> F: Fn(&'xa i32, &'xb i32) -> &'xa i32
    |                         ^^^
 
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:85:28
+  --> $DIR/higher-lifetime-bounds.rs:61:28
    |
 LL | struct S1<F: for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32>(F);
    |                            ^^^
 
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:87:40
+  --> $DIR/higher-lifetime-bounds.rs:63:40
    |
 LL | struct S2<F>(F) where F: for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32;
    |                                        ^^^
 
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:89:37
+  --> $DIR/higher-lifetime-bounds.rs:65:37
    |
 LL | struct S3<F>(F) where for<'xa, 'xb: 'xa> F: Fn(&'xa i32, &'xb i32) -> &'xa i32;
    |                                     ^^^
 
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:92:29
+  --> $DIR/higher-lifetime-bounds.rs:68:29
    |
 LL | struct S_fnty(for<'xa, 'xb: 'xa> fn(&'xa i32, &'xb i32) -> &'xa i32);
    |                             ^^^
 
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:95:29
+  --> $DIR/higher-lifetime-bounds.rs:71:29
    |
 LL | type T1 = Box<for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32>;
    |                             ^^^
 
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:99:34
+  --> $DIR/higher-lifetime-bounds.rs:75:34
    |
 LL |     let _ : Option<for<'xa, 'xb: 'xa> fn(&'xa i32, &'xb i32) -> &'xa i32> = None;
    |                                  ^^^
 
 error: lifetime bounds cannot be used in this context
-  --> $DIR/param-bounds-ignored.rs:101:38
+  --> $DIR/higher-lifetime-bounds.rs:77:38
    |
 LL |     let _ : Option<Box<for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32>> = None;
    |                                      ^^^
 
-warning: bounds on generic parameters are ignored in type aliases
-  --> $DIR/param-bounds-ignored.rs:15:14
-   |
-LL | type SVec<T: Send+Send> = Vec<T>;
-   |              ^^^^ ^^^^
-   |
-   = note: #[warn(ignored_generic_bounds)] on by default
-
-warning: bounds on generic parameters are ignored in type aliases
-  --> $DIR/param-bounds-ignored.rs:17:19
-   |
-LL | type VVec<'b, 'a: 'b+'b> = Vec<&'a i32>;
-   |                   ^^ ^^
-
-warning: bounds on generic parameters are ignored in type aliases
-  --> $DIR/param-bounds-ignored.rs:19:18
-   |
-LL | type WVec<'b, T: 'b+'b> = Vec<T>;
-   |                  ^^ ^^
-
-warning: where clauses are ignored in type aliases
-  --> $DIR/param-bounds-ignored.rs:21:25
-   |
-LL | type W2Vec<'b, T> where T: 'b, T: 'b = Vec<T>;
-   |                         ^^^^^  ^^^^^
-
 error: aborting due to 11 previous errors