[WIP] Warn when $name:matcher syntax is used on expansion side

src/libcore/fmt/mod.rs

@@ -1716,7 +1716,7 @@ macro_rules! tuple {
     () => ();
     ( $($name:ident,)+ ) => (
         #[stable(feature = "rust1", since = "1.0.0")]
-        impl<$($name:Debug),*> Debug for ($($name,)*) where last_type!($($name,)+): ?Sized {
+        impl<$($name: Debug),*> Debug for ($($name,)*) where last_type!($($name,)+): ?Sized {
             #[allow(non_snake_case, unused_assignments, deprecated)]
             fn fmt(&self, f: &mut Formatter) -> Result {
                 let mut builder = f.debug_tuple("");

src/libcore/tuple.rs

@@ -22,7 +22,7 @@ macro_rules! tuple_impls {
     )+) => {
         $(
             #[stable(feature = "rust1", since = "1.0.0")]
-            impl<$($T:PartialEq),+> PartialEq for ($($T,)+) where last_type!($($T,)+): ?Sized {
+            impl<$($T: PartialEq),+> PartialEq for ($($T,)+) where last_type!($($T,)+): ?Sized {
                 #[inline]
                 fn eq(&self, other: &($($T,)+)) -> bool {
                     $(self.$idx == other.$idx)&&+
@@ -34,10 +34,10 @@ macro_rules! tuple_impls {
             }
 
             #[stable(feature = "rust1", since = "1.0.0")]
-            impl<$($T:Eq),+> Eq for ($($T,)+) where last_type!($($T,)+): ?Sized {}
+            impl<$($T: Eq),+> Eq for ($($T,)+) where last_type!($($T,)+): ?Sized {}
 
             #[stable(feature = "rust1", since = "1.0.0")]
-            impl<$($T:PartialOrd + PartialEq),+> PartialOrd for ($($T,)+)
+            impl<$($T: PartialOrd + PartialEq),+> PartialOrd for ($($T,)+)
                     where last_type!($($T,)+): ?Sized {
                 #[inline]
                 fn partial_cmp(&self, other: &($($T,)+)) -> Option<Ordering> {
@@ -62,15 +62,15 @@ macro_rules! tuple_impls {
             }
 
             #[stable(feature = "rust1", since = "1.0.0")]
-            impl<$($T:Ord),+> Ord for ($($T,)+) where last_type!($($T,)+): ?Sized {
+            impl<$($T: Ord),+> Ord for ($($T,)+) where last_type!($($T,)+): ?Sized {
                 #[inline]
                 fn cmp(&self, other: &($($T,)+)) -> Ordering {
                     lexical_cmp!($(self.$idx, other.$idx),+)
                 }
             }
 
             #[stable(feature = "rust1", since = "1.0.0")]
-            impl<$($T:Default),+> Default for ($($T,)+) {
+            impl<$($T: Default),+> Default for ($($T,)+) {
                 #[inline]
                 fn default() -> ($($T,)+) {
                     ($({ let x: $T = Default::default(); x},)+)

src/libserialize/serialize.rs

@@ -679,7 +679,7 @@ macro_rules! count_idents {
 macro_rules! tuple {
     () => ();
     ( $($name:ident,)+ ) => (
-        impl<$($name:Decodable),*> Decodable for ($($name,)*) {
+        impl<$($name: Decodable),*> Decodable for ($($name,)*) {
             #[allow(non_snake_case)]
             fn decode<D: Decoder>(d: &mut D) -> Result<($($name,)*), D::Error> {
                 let len: usize = count_idents!($($name,)*);
@@ -693,7 +693,7 @@ macro_rules! tuple {
                 })
             }
         }
-        impl<$($name:Encodable),*> Encodable for ($($name,)*) {
+        impl<$($name: Encodable),*> Encodable for ($($name,)*) {
             #[allow(non_snake_case)]
             fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
                 let ($(ref $name,)*) = *self;

src/libsyntax/ext/tt/quoted.rs

@@ -190,31 +190,51 @@ pub fn parse(
         // Given the parsed tree, if there is a metavar and we are expecting matchers, actually
         // parse out the matcher (i.e. in `$id:ident` this would parse the `:` and `ident`).
         match tree {
-            TokenTree::MetaVar(start_sp, ident) if expect_matchers => {
-                let span = match trees.next() {
-                    Some(tokenstream::TokenTree::Token(span, token::Colon)) => match trees.next() {
-                        Some(tokenstream::TokenTree::Token(end_sp, ref tok)) => match tok.ident() {
-                            Some(kind) => {
-                                let span = end_sp.with_lo(start_sp.lo());
-                                result.push(TokenTree::MetaVarDecl(span, ident, kind));
-                                continue;
-                            }
-                            _ => end_sp,
-                        },
-                        tree => tree.as_ref()
-                            .map(tokenstream::TokenTree::span)
-                            .unwrap_or(span),
-                    },
-                    tree => tree.as_ref()
-                        .map(tokenstream::TokenTree::span)
-                        .unwrap_or(start_sp),
-                };
-                sess.missing_fragment_specifiers.borrow_mut().insert(span);
-                result.push(TokenTree::MetaVarDecl(
-                    span,
-                    ident,
-                    keywords::Invalid.ident(),
-                ));
+            TokenTree::MetaVar(start_sp, ident) => {
+                if expect_matchers {
+                    match parse_matcher(start_sp, ident, &mut trees) {
+                        Ok((meta_var_decl, _, _)) => result.push(meta_var_decl),
+                        Err(span) => {
+                            sess.missing_fragment_specifiers.borrow_mut().insert(span);
+                            result.push(TokenTree::MetaVarDecl(
+                                span,
+                                ident,
+                                keywords::Invalid.ident(),
+                            ));
+                        }
+                    }
+                } else {
+                    const VALID_SPECIFIERS: &[&str] = &[
+                        "ident", "block", "stmt", "expr", "pat", "ty", "path", "meta", "tt",
+                        "item", "vis",
+                    ];
+                    match parse_matcher(start_sp, ident, &mut trees.clone()) {
+                        Ok((TokenTree::MetaVarDecl(full_sp, _, kind), colon_sp, end_sp))
+                            if start_sp.hi() == colon_sp.lo() && colon_sp.hi() == end_sp.lo()
+                                && VALID_SPECIFIERS.contains(&&*kind.name.as_str()) =>
+                        {
+                            let specifier_sp = colon_sp.with_hi(end_sp.hi());
+                            sess.span_diagnostic
+                                .struct_span_warn(
+                                    specifier_sp,
+                                    "macro expansion includes fragment specifier",
+                                )
+                                .span_suggestion(
+                                    full_sp,
+                                    "to just use the macro argument, remove the fragment specifier",
+                                    format!("${}", ident),
+                                )
+                                .span_suggestion(
+                                    specifier_sp,
+                                    "to suppress this warning, add a space after the colon",
+                                    format!(": {}", kind),
+                                )
+                                .emit();
+                        }
+                        _ => {}
+                    }
+                    result.push(tree)
+                }
             }
 
             // Not a metavar or no matchers allowed, so just return the tree
@@ -224,6 +244,48 @@ pub fn parse(
     result
 }
 
+/// Parses a `:<ident>`. Doesn't report error on its own.
+///
+/// # Parameters
+///
+/// - `start_sp` - span of metavariable name preceding the `:`
+/// - `ident` - the name of metavariable
+/// - `trees` - iterator over trees
+///
+/// # Returns
+///
+/// On success, returns a parsed `MetaVarDecl`, span of `:` and span of matcher's type
+/// On error, returns a span for error.
+fn parse_matcher<I>(
+    start_sp: Span,
+    ident: ast::Ident,
+    trees: &mut I,
+) -> Result<(TokenTree, Span, Span), Span>
+where
+    I: Iterator<Item = tokenstream::TokenTree>,
+{
+    match trees.next() {
+        Some(tokenstream::TokenTree::Token(colon_sp, token::Colon)) => match trees.next() {
+            Some(tokenstream::TokenTree::Token(end_sp, ref tok)) => match tok.ident() {
+                Some(kind) => {
+                    let span = end_sp.with_lo(start_sp.lo());
+                    Ok((TokenTree::MetaVarDecl(span, ident, kind), colon_sp, end_sp))
+                }
+                _ => Err(end_sp),
+            },
+            tree => {
+                let span = tree.as_ref()
+                    .map(tokenstream::TokenTree::span)
+                    .unwrap_or(colon_sp);
+                Err(span)
+            }
+        },
+        tree => Err(tree.as_ref()
+            .map(tokenstream::TokenTree::span)
+            .unwrap_or(start_sp)),
+    }
+}
+
 /// Takes a `tokenstream::TokenTree` and returns a `self::TokenTree`. Specifically, this takes a
 /// generic `TokenTree`, such as is used in the rest of the compiler, and returns a `TokenTree`
 /// for use in parsing a macro.

src/test/compile-fail/hygiene/globs.rs

@@ -47,7 +47,7 @@ macro n($i:ident) {
             }
         }
 
-        macro n($j:ident) {
+        macro n($j: ident) {
             mod test {
                 use super::*;
                 fn g() {

src/test/compile-fail/hygiene/nested_macro_privacy.rs

@@ -14,7 +14,7 @@ macro n($foo:ident, $S:ident, $i:ident, $m:ident) {
     mod $foo {
         #[derive(Default)]
         pub struct $S { $i: u32 }
-        pub macro $m($e:expr) { $e.$i }
+        pub macro $m($e: expr) { $e.$i }
     }
 }
 

src/test/ui/fragment_specifier_in_expansion.rs

@@ -0,0 +1,30 @@
+// 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.
+
+// Tests the "fragment specifier in expansion" warning
+
+// must-compile-successfully
+
+#![allow(unused)]
+
+macro_rules! warn_me {
+    ( $thing:tt ) => { $thing:tt }
+    //~^ WARN fragment specifier
+}
+
+macro_rules! with_space {
+    ( $thing:tt ) => { $thing: tt }
+}
+
+macro_rules! unknown_specifier {
+    ( $thing:tt ) => { $thing:foobar }
+}
+
+fn main() {}

src/test/ui/fragment_specifier_in_expansion.stderr

@@ -0,0 +1,14 @@
+warning: macro expansion includes fragment specifier
+  --> $DIR/fragment_specifier_in_expansion.rs:18:30
+   |
+18 |     ( $thing:tt ) => { $thing:tt }
+   |                              ^^^
+help: to just use the macro argument, remove the fragment specifier
+   |
+18 |     ( $thing:tt ) => { $thing }
+   |                        ^^^^^^
+help: to suppress this warning, add a space after the colon
+   |
+18 |     ( $thing:tt ) => { $thing: tt }
+   |                              ^^^^
+