Auto merge of #37196 - tamird:fix-tidy-features, r=brson

tidy/features: fix checking of lang features

r? @brson

See the commit messages - the actual "fixes" here are strawmen; I'm happy to adjust them if you have suggestions.

src/libcore/cmp.rs

@@ -706,24 +706,24 @@ mod impls {
 
     ord_impl! { char usize u8 u16 u32 u64 isize i8 i16 i32 i64 }
 
-    #[unstable(feature = "never_type", issue = "35121")]
+    #[unstable(feature = "never_type_impls", issue = "35121")]
     impl PartialEq for ! {
         fn eq(&self, _: &!) -> bool {
             *self
         }
     }
 
-    #[unstable(feature = "never_type", issue = "35121")]
+    #[unstable(feature = "never_type_impls", issue = "35121")]
     impl Eq for ! {}
 
-    #[unstable(feature = "never_type", issue = "35121")]
+    #[unstable(feature = "never_type_impls", issue = "35121")]
     impl PartialOrd for ! {
         fn partial_cmp(&self, _: &!) -> Option<Ordering> {
             *self
         }
     }
 
-    #[unstable(feature = "never_type", issue = "35121")]
+    #[unstable(feature = "never_type_impls", issue = "35121")]
     impl Ord for ! {
         fn cmp(&self, _: &!) -> Ordering {
             *self

src/libcore/fmt/mod.rs

@@ -1356,14 +1356,14 @@ macro_rules! fmt_refs {
 
 fmt_refs! { Debug, Display, Octal, Binary, LowerHex, UpperHex, LowerExp, UpperExp }
 
-#[unstable(feature = "never_type", issue = "35121")]
+#[unstable(feature = "never_type_impls", issue = "35121")]
 impl Debug for ! {
     fn fmt(&self, _: &mut Formatter) -> Result {
         *self
     }
 }
 
-#[unstable(feature = "never_type", issue = "35121")]
+#[unstable(feature = "never_type_impls", issue = "35121")]
 impl Display for ! {
     fn fmt(&self, _: &mut Formatter) -> Result {
         *self

src/libcore/macros.rs

@@ -553,7 +553,7 @@ pub mod builtin {
     /// For more information, see the documentation for [`std::concat_idents!`].
     ///
     /// [`std::concat_idents!`]: ../std/macro.concat_idents.html
-    #[unstable(feature = "concat_idents", issue = "29599")]
+    #[unstable(feature = "concat_idents_macro", issue = "29599")]
     #[macro_export]
     macro_rules! concat_idents {
         ($($e:ident),*) => ({ /* compiler built-in */ })

src/libstd/macros.rs

@@ -286,7 +286,7 @@ pub mod builtin {
     /// // fn concat_idents!(new, fun, name) { } // not usable in this way!
     /// # }
     /// ```
-    #[unstable(feature = "concat_idents", issue = "29599")]
+    #[unstable(feature = "concat_idents_macro", issue = "29599")]
     #[macro_export]
     macro_rules! concat_idents {
         ($($e:ident),*) => ({ /* compiler built-in */ })

src/tools/tidy/src/bins.rs

@@ -44,28 +44,27 @@ pub fn check(path: &Path, bad: &mut bool) {
         let filename = file.file_name().unwrap().to_string_lossy();
         let extensions = [".py", ".sh"];
         if extensions.iter().any(|e| filename.ends_with(e)) {
-            return
+            return;
         }
 
         let metadata = t!(fs::symlink_metadata(&file), &file);
         if metadata.mode() & 0o111 != 0 {
             let rel_path = file.strip_prefix(path).unwrap();
             let git_friendly_path = rel_path.to_str().unwrap().replace("\\", "/");
-            let ret_code = Command::new("git")
-                                        .arg("ls-files")
-                                        .arg(&git_friendly_path)
-                                        .current_dir(path)
-                                        .stdout(Stdio::null())
-                                        .stderr(Stdio::null())
-                                        .status()
-                                        .unwrap_or_else(|e| {
-                                            panic!("could not run git ls-files: {}", e);
-                                        });
-            if ret_code.success() {
+            let output = Command::new("git")
+                .arg("ls-files")
+                .arg(&git_friendly_path)
+                .current_dir(path)
+                .stderr(Stdio::null())
+                .output()
+                .unwrap_or_else(|e| {
+                    panic!("could not run git ls-files: {}", e);
+                });
+            let path_bytes = rel_path.as_os_str().as_bytes();
+            if output.status.success() && output.stdout.starts_with(path_bytes) {
                 println!("binary checked into source: {}", file.display());
                 *bad = true;
             }
         }
     })
 }
-

src/tools/tidy/src/features.rs

@@ -18,27 +18,42 @@
 //! * Library features have at most one `since` value
 
 use std::collections::HashMap;
+use std::fmt;
 use std::fs::File;
 use std::io::prelude::*;
 use std::path::Path;
 
-const STATUSES: &'static [&'static str] = &[
-    "Active", "Deprecated", "Removed", "Accepted",
-];
+#[derive(PartialEq)]
+enum Status {
+    Stable,
+    Unstable,
+}
+
+impl fmt::Display for Status {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        let as_str = match *self {
+            Status::Stable => "stable",
+            Status::Unstable => "unstable",
+        };
+        fmt::Display::fmt(as_str, f)
+    }
+}
+
 
 struct Feature {
     name: String,
+    level: Status,
     since: String,
-    status: String,
 }
 
 struct LibFeature {
-    level: String,
+    level: Status,
     since: String,
 }
 
 pub fn check(path: &Path, bad: &mut bool) {
     let features = collect_lang_features(&path.join("libsyntax/feature_gate.rs"));
+    assert!(!features.is_empty());
     let mut lib_features = HashMap::<String, LibFeature>::new();
 
     let mut contents = String::new();
@@ -48,7 +63,7 @@ pub fn check(path: &Path, bad: &mut bool) {
         let filename = file.file_name().unwrap().to_string_lossy();
         if !filename.ends_with(".rs") || filename == "features.rs" ||
            filename == "diagnostic_list.rs" {
-            return
+            return;
         }
 
         contents.truncate(0);
@@ -60,24 +75,24 @@ pub fn check(path: &Path, bad: &mut bool) {
                 *bad = true;
             };
             let level = if line.contains("[unstable(") {
-                "unstable"
+                Status::Unstable
             } else if line.contains("[stable(") {
-                "stable"
+                Status::Stable
             } else {
-                continue
+                continue;
             };
             let feature_name = match find_attr_val(line, "feature") {
                 Some(name) => name,
                 None => {
                     err("malformed stability attribute");
-                    continue
+                    continue;
                 }
             };
             let since = match find_attr_val(line, "since") {
                 Some(name) => name,
-                None if level == "stable" => {
+                None if level == Status::Stable => {
                     err("malformed stability attribute");
-                    continue
+                    continue;
                 }
                 None => "None",
             };
@@ -92,27 +107,34 @@ pub fn check(path: &Path, bad: &mut bool) {
                 if s.since != since {
                     err("different `since` than before");
                 }
-                continue
+                continue;
             }
-            lib_features.insert(feature_name.to_owned(), LibFeature {
-                level: level.to_owned(),
-                since: since.to_owned(),
-            });
+            lib_features.insert(feature_name.to_owned(),
+                                LibFeature {
+                                    level: level,
+                                    since: since.to_owned(),
+                                });
         }
     });
 
     if *bad {
-        return
+        return;
     }
 
     let mut lines = Vec::new();
     for feature in features {
         lines.push(format!("{:<32} {:<8} {:<12} {:<8}",
-                           feature.name, "lang", feature.status, feature.since));
+                           feature.name,
+                           "lang",
+                           feature.level,
+                           feature.since));
     }
     for (name, feature) in lib_features {
         lines.push(format!("{:<32} {:<8} {:<12} {:<8}",
-                           name, "lib", feature.level, feature.since));
+                           name,
+                           "lib",
+                           feature.level,
+                           feature.since));
     }
 
     lines.sort();
@@ -122,39 +144,32 @@ pub fn check(path: &Path, bad: &mut bool) {
 }
 
 fn find_attr_val<'a>(line: &'a str, attr: &str) -> Option<&'a str> {
-    line.find(attr).and_then(|i| {
-        line[i..].find("\"").map(|j| i + j + 1)
-    }).and_then(|i| {
-        line[i..].find("\"").map(|j| (i, i + j))
-    }).map(|(i, j)| {
-        &line[i..j]
-    })
+    line.find(attr)
+        .and_then(|i| line[i..].find('"').map(|j| i + j + 1))
+        .and_then(|i| line[i..].find('"').map(|j| (i, i + j)))
+        .map(|(i, j)| &line[i..j])
 }
 
 fn collect_lang_features(path: &Path) -> Vec<Feature> {
     let mut contents = String::new();
     t!(t!(File::open(path)).read_to_string(&mut contents));
 
-    let mut features = Vec::new();
-    for line in contents.lines().map(|l| l.trim()) {
-        if !STATUSES.iter().any(|s| line.starts_with(&format!("({}", s))) {
-            continue
-        }
-        let mut parts = line.split(",");
-        let status = match &parts.next().unwrap().trim().replace("(", "")[..] {
-            "active"   => "unstable",
-            "removed"  => "unstable",
-            "accepted" => "stable",
-            s => panic!("unknown status: {}", s),
-        };
-        let name = parts.next().unwrap().trim().to_owned();
-        let since = parts.next().unwrap().trim().replace("\"", "");
-
-        features.push(Feature {
-            name: name,
-            since: since,
-            status: status.to_owned(),
-        });
-    }
-    return features
+    contents.lines()
+        .filter_map(|line| {
+            let mut parts = line.trim().split(",");
+            let level = match parts.next().map(|l| l.trim().trim_left_matches('(')) {
+                Some("active") => Status::Unstable,
+                Some("removed") => Status::Unstable,
+                Some("accepted") => Status::Stable,
+                _ => return None,
+            };
+            let name = parts.next().unwrap().trim();
+            let since = parts.next().unwrap().trim().trim_matches('"');
+            Some(Feature {
+                name: name.to_owned(),
+                level: level,
+                since: since.to_owned(),
+            })
+        })
+        .collect()
 }