internal: make xtask lighter

Moving tests to `rust-analyzer` crate allows removing walkdir dependency
from `xtask`. It does seem more reasonable to keep tidy tests outside of
the "build system" and closer to other integration tests.

Cargo.lock

@@ -1343,6 +1343,7 @@ dependencies = [
  "vfs-notify",
  "winapi",
  "xflags",
+ "xshell",
 ]
 
 [[package]]
@@ -1957,7 +1958,6 @@ version = "0.1.0"
 dependencies = [
  "anyhow",
  "flate2",
- "walkdir",
  "write-json",
  "xflags",
  "xshell",

crates/rust-analyzer/Cargo.toml

@@ -66,6 +66,7 @@ jemallocator = { version = "0.4.1", package = "tikv-jemallocator", optional = tr
 
 [dev-dependencies]
 expect-test = "1.1"
+xshell = "0.1"
 
 test_utils = { path = "../test_utils" }
 sourcegen = { path = "../sourcegen" }

crates/rust-analyzer/tests/slow-tests/main.rs

@@ -9,6 +9,7 @@
 //! be sure without a real client anyway.
 
 mod sourcegen;
+mod tidy;
 mod testdir;
 mod support;
 

crates/rust-analyzer/tests/slow-tests/tidy.rs

@@ -3,14 +3,11 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use walkdir::{DirEntry, WalkDir};
 use xshell::{cmd, pushd, pushenv, read_file};
 
-use crate::project_root;
-
 #[test]
 fn check_code_formatting() {
-    let _dir = pushd(project_root()).unwrap();
+    let _dir = pushd(sourcegen::project_root()).unwrap();
     let _e = pushenv("RUSTUP_TOOLCHAIN", "stable");
 
     let out = cmd!("rustfmt --version").read().unwrap();
@@ -32,13 +29,14 @@ fn check_code_formatting() {
 fn check_lsp_extensions_docs() {
     let expected_hash = {
         let lsp_ext_rs =
-            read_file(project_root().join("crates/rust-analyzer/src/lsp_ext.rs")).unwrap();
+            read_file(sourcegen::project_root().join("crates/rust-analyzer/src/lsp_ext.rs"))
+                .unwrap();
         stable_hash(lsp_ext_rs.as_str())
     };
 
     let actual_hash = {
         let lsp_extensions_md =
-            read_file(project_root().join("docs/dev/lsp-extensions.md")).unwrap();
+            read_file(sourcegen::project_root().join("docs/dev/lsp-extensions.md")).unwrap();
         let text = lsp_extensions_md
             .lines()
             .find_map(|line| line.strip_prefix("lsp_ext.rs hash:"))
@@ -63,68 +61,78 @@ Please adjust docs/dev/lsp-extensions.md.
 }
 
 #[test]
-fn rust_files_are_tidy() {
+fn files_are_tidy() {
+    let files = sourcegen::list_files(&sourcegen::project_root().join("crates"));
+
     let mut tidy_docs = TidyDocs::default();
     let mut tidy_marks = TidyMarks::default();
-    for path in rust_files() {
+    for path in files {
-        let text = read_file(&path).unwrap();
+        let extension = path.extension().unwrap_or_default().to_str().unwrap_or_default();
-        check_todo(&path, &text);
+        match extension {
-        check_dbg(&path, &text);
+            "rs" => {
-        check_test_attrs(&path, &text);
+                let text = read_file(&path).unwrap();
-        check_trailing_ws(&path, &text);
+                check_todo(&path, &text);
-        deny_clippy(&path, &text);
+                check_dbg(&path, &text);
-        tidy_docs.visit(&path, &text);
+                check_test_attrs(&path, &text);
-        tidy_marks.visit(&path, &text);
+                check_trailing_ws(&path, &text);
+                deny_clippy(&path, &text);
+                tidy_docs.visit(&path, &text);
+                tidy_marks.visit(&path, &text);
+            }
+            "toml" => {
+                let text = read_file(&path).unwrap();
+                check_cargo_toml(&path, text);
+            }
+            _ => (),
+        }
     }
+
     tidy_docs.finish();
     tidy_marks.finish();
 }
 
-#[test]
+fn check_cargo_toml(path: &Path, text: String) -> () {
-fn cargo_files_are_tidy() {
+    let mut section = None;
-    for cargo in cargo_files() {
+    for (line_no, text) in text.lines().enumerate() {
-        let mut section = None;
+        let text = text.trim();
-        for (line_no, text) in read_file(&cargo).unwrap().lines().enumerate() {
+        if text.starts_with('[') {
-            let text = text.trim();
+            if !text.ends_with(']') {
-            if text.starts_with('[') {
+                panic!(
-                if !text.ends_with(']') {
+                    "\nplease don't add comments or trailing whitespace in section lines.\n\
+                        {}:{}\n",
+                    path.display(),
+                    line_no + 1
+                )
+            }
+            section = Some(text);
+            continue;
+        }
+        let text: String = text.split_whitespace().collect();
+        if !text.contains("path=") {
+            continue;
+        }
+        match section {
+            Some(s) if s.contains("dev-dependencies") => {
+                if text.contains("version") {
                     panic!(
-                        "\nplease don't add comments or trailing whitespace in section lines.\n\
+                        "\ncargo internal dev-dependencies should not have a version.\n\
-                            {}:{}\n",
+                        {}:{}\n",
-                        cargo.display(),
+                        path.display(),
                         line_no + 1
-                    )
+                    );
                 }
-                section = Some(text);
-                continue;
             }
-            let text: String = text.split_whitespace().collect();
+            Some(s) if s.contains("dependencies") => {
-            if !text.contains("path=") {
+                if !text.contains("version") {
-                continue;
+                    panic!(
-            }
+                        "\ncargo internal dependencies should have a version.\n\
-            match section {
+                        {}:{}\n",
-                Some(s) if s.contains("dev-dependencies") => {
+                        path.display(),
-                    if text.contains("version") {
+                        line_no + 1
-                        panic!(
+                    );
-                            "\ncargo internal dev-dependencies should not have a version.\n\
-                            {}:{}\n",
-                            cargo.display(),
-                            line_no + 1
-                        );
-                    }
                 }
-                Some(s) if s.contains("dependencies") => {
-                    if !text.contains("version") {
-                        panic!(
-                            "\ncargo internal dependencies should have a version.\n\
-                            {}:{}\n",
-                            cargo.display(),
-                            line_no + 1
-                        );
-                    }
-                }
-                _ => {}
             }
+            _ => {}
         }
     }
 }
@@ -293,7 +301,7 @@ fn check_todo(path: &Path, text: &str) {
 fn check_dbg(path: &Path, text: &str) {
     let need_dbg = &[
         // This file itself obviously needs to use dbg.
-        "tests/tidy.rs",
+        "slow-tests/tidy.rs",
         // Assists to remove `dbg!()`
         "handlers/remove_dbg.rs",
         // We have .dbg postfix
@@ -320,7 +328,9 @@ fn check_test_attrs(path: &Path, text: &str) {
     let ignore_rule =
         "https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/style.md#ignore";
     let need_ignore: &[&str] = &[
-        // Special case to run `#[ignore]` tests
+        // This file.
+        "slow-tests/tidy.rs",
+        // Special case to run `#[ignore]` tests.
         "ide/src/runnables.rs",
         // A legit test which needs to be ignored, as it takes too long to run
         // :(
@@ -338,7 +348,11 @@ fn check_test_attrs(path: &Path, text: &str) {
 
     let panic_rule =
         "https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/style.md#should_panic";
-    let need_panic: &[&str] = &["test_utils/src/fixture.rs"];
+    let need_panic: &[&str] = &[
+        // This file.
+        "slow-tests/tidy.rs",
+        "test_utils/src/fixture.rs",
+    ];
     if text.contains("#[should_panic") && !need_panic.iter().any(|p| path.ends_with(p)) {
         panic!(
             "\ndon't add `#[should_panic]` tests, see:\n\n    {}\n\n   {}\n",
@@ -422,7 +436,7 @@ impl TidyDocs {
 }
 
 fn is_exclude_dir(p: &Path, dirs_to_exclude: &[&str]) -> bool {
-    p.strip_prefix(project_root())
+    p.strip_prefix(sourcegen::project_root())
         .unwrap()
         .components()
         .rev()
@@ -481,31 +495,3 @@ fn find_mark<'a>(text: &'a str, mark: &'static str) -> Option<&'a str> {
     let text = &text[..idx];
     Some(text)
 }
-
-fn rust_files() -> impl Iterator<Item = PathBuf> {
-    rust_files_in(&project_root().join("crates"))
-}
-
-fn cargo_files() -> impl Iterator<Item = PathBuf> {
-    files_in(&project_root(), "toml")
-        .filter(|path| path.file_name().map(|it| it == "Cargo.toml").unwrap_or(false))
-}
-
-fn rust_files_in(path: &Path) -> impl Iterator<Item = PathBuf> {
-    files_in(path, "rs")
-}
-
-fn files_in(path: &Path, ext: &'static str) -> impl Iterator<Item = PathBuf> {
-    let iter = WalkDir::new(path);
-    return iter
-        .into_iter()
-        .filter_entry(|e| !is_hidden(e))
-        .map(|e| e.unwrap())
-        .filter(|e| !e.file_type().is_dir())
-        .map(|e| e.into_path())
-        .filter(move |path| path.extension().map(|it| it == ext).unwrap_or(false));
-
-    fn is_hidden(entry: &DirEntry) -> bool {
-        entry.file_name().to_str().map(|s| s.starts_with('.')).unwrap_or(false)
-    }
-}

xtask/Cargo.toml

@@ -9,7 +9,6 @@ license = "MIT OR Apache-2.0"
 [dependencies]
 anyhow = "1.0.26"
 flate2 = "1.0"
-walkdir = "2.3.1"
 write-json = "0.1.0"
 xshell = "0.1"
 xflags = "0.2.1"

xtask/src/main.rs

@@ -9,9 +9,6 @@
 //! `.cargo/config`.
 mod flags;
 
-#[cfg(test)]
-mod tidy;
-
 mod install;
 mod release;
 mod dist;