From 8f6efa01b2f549b13b702831dff5131777d2e79d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 17 Mar 2020 10:26:29 +0100 Subject: Tidier tidy checks --- xtask/tests/tidy-tests/cli.rs | 25 +------ xtask/tests/tidy-tests/docs.rs | 106 ------------------------------ xtask/tests/tidy-tests/main.rs | 145 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 145 insertions(+), 131 deletions(-) delete mode 100644 xtask/tests/tidy-tests/docs.rs (limited to 'xtask') diff --git a/xtask/tests/tidy-tests/cli.rs b/xtask/tests/tidy-tests/cli.rs index f9ca45292..f5b00a8b8 100644 --- a/xtask/tests/tidy-tests/cli.rs +++ b/xtask/tests/tidy-tests/cli.rs @@ -1,7 +1,6 @@ -use walkdir::WalkDir; use xtask::{ codegen::{self, Mode}, - project_root, run_rustfmt, + run_rustfmt, }; #[test] @@ -31,25 +30,3 @@ fn check_code_formatting() { panic!("{}. Please format the code by running `cargo format`", error); } } - -#[test] -fn no_todo() { - WalkDir::new(project_root().join("crates")).into_iter().for_each(|e| { - let e = e.unwrap(); - if e.path().extension().map(|it| it != "rs").unwrap_or(true) { - return; - } - if e.path().ends_with("tests/cli.rs") { - return; - } - let text = std::fs::read_to_string(e.path()).unwrap(); - if text.contains("TODO") || text.contains("TOOD") || text.contains("todo!") { - panic!( - "\nTODO markers should not be committed to the master branch,\n\ - use FIXME instead\n\ - {}\n", - e.path().display(), - ) - } - }) -} diff --git a/xtask/tests/tidy-tests/docs.rs b/xtask/tests/tidy-tests/docs.rs deleted file mode 100644 index 62c4f8441..000000000 --- a/xtask/tests/tidy-tests/docs.rs +++ /dev/null @@ -1,106 +0,0 @@ -use std::{collections::HashMap, fs, io::prelude::*, io::BufReader, path::Path}; - -use anyhow::Context; -use walkdir::{DirEntry, WalkDir}; -use xtask::project_root; - -fn is_exclude_dir(p: &Path) -> bool { - // Test hopefully don't really need comments, and for assists we already - // have special comments which are source of doc tests and user docs. - let exclude_dirs = ["tests", "test_data", "handlers"]; - let mut cur_path = p; - while let Some(path) = cur_path.parent() { - if exclude_dirs.iter().any(|dir| path.ends_with(dir)) { - return true; - } - cur_path = path; - } - - false -} - -fn is_exclude_file(d: &DirEntry) -> bool { - let file_names = ["tests.rs"]; - - d.file_name().to_str().map(|f_n| file_names.iter().any(|name| *name == f_n)).unwrap_or(false) -} - -fn is_hidden(entry: &DirEntry) -> bool { - entry.file_name().to_str().map(|s| s.starts_with('.')).unwrap_or(false) -} - -#[test] -fn no_docs_comments() { - let crates = project_root().join("crates"); - let iter = WalkDir::new(crates); - let mut missing_docs = Vec::new(); - let mut contains_fixme = Vec::new(); - for f in iter.into_iter().filter_entry(|e| !is_hidden(e)) { - let f = f.unwrap(); - if f.file_type().is_dir() { - continue; - } - if f.path().extension().map(|it| it != "rs").unwrap_or(false) { - continue; - } - if is_exclude_dir(f.path()) { - continue; - } - if is_exclude_file(&f) { - continue; - } - let mut reader = BufReader::new(fs::File::open(f.path()).unwrap()); - let mut line = String::new(); - reader - .read_line(&mut line) - .with_context(|| format!("Failed to read {}", f.path().display())) - .unwrap(); - - if line.starts_with("//!") { - if line.contains("FIXME") { - contains_fixme.push(f.path().to_path_buf()) - } - } else { - missing_docs.push(f.path().display().to_string()); - } - } - if !missing_docs.is_empty() { - panic!( - "\nMissing docs strings\n\n\ - modules:\n{}\n\n", - missing_docs.join("\n") - ) - } - - let whitelist = [ - "ra_db", - "ra_hir", - "ra_hir_expand", - "ra_ide", - "ra_mbe", - "ra_parser", - "ra_prof", - "ra_project_model", - "ra_syntax", - "ra_text_edit", - "ra_tt", - "ra_hir_ty", - ]; - - let mut has_fixmes = whitelist.iter().map(|it| (*it, false)).collect::>(); - 'outer: for path in contains_fixme { - for krate in whitelist.iter() { - if path.components().any(|it| it.as_os_str() == *krate) { - has_fixmes.insert(krate, true); - continue 'outer; - } - } - panic!("FIXME doc in a fully-documented crate: {}", path.display()) - } - - for (krate, has_fixme) in has_fixmes.iter() { - if !has_fixme { - panic!("crate {} is fully documented, remove it from the white list", krate) - } - } -} diff --git a/xtask/tests/tidy-tests/main.rs b/xtask/tests/tidy-tests/main.rs index 56d1318d6..2d2d88bec 100644 --- a/xtask/tests/tidy-tests/main.rs +++ b/xtask/tests/tidy-tests/main.rs @@ -1,2 +1,145 @@ mod cli; -mod docs; + +use std::{ + collections::HashMap, + path::{Path, PathBuf}, +}; + +use walkdir::{DirEntry, WalkDir}; +use xtask::{not_bash::fs2, project_root}; + +#[test] +fn rust_files_are_tidy() { + let mut tidy_docs = TidyDocs::default(); + for path in rust_files() { + let text = fs2::read_to_string(&path).unwrap(); + check_todo(&path, &text); + tidy_docs.visit(&path, &text); + } + tidy_docs.finish(); +} + +fn check_todo(path: &Path, text: &str) { + if path.ends_with("tests/cli.rs") { + return; + } + if text.contains("TODO") || text.contains("TOOD") || text.contains("todo!") { + panic!( + "\nTODO markers should not be committed to the master branch,\n\ + use FIXME instead\n\ + {}\n", + path.display(), + ) + } +} + +#[derive(Default)] +struct TidyDocs { + missing_docs: Vec, + contains_fixme: Vec, +} + +impl TidyDocs { + fn visit(&mut self, path: &Path, text: &str) { + if is_exclude_dir(path) || is_exclude_file(path) { + return; + } + + let first_line = match text.lines().next() { + Some(it) => it, + None => return, + }; + + if first_line.starts_with("//!") { + if first_line.contains("FIXME") { + self.contains_fixme.push(path.to_path_buf()) + } + } else { + self.missing_docs.push(path.display().to_string()); + } + + fn is_exclude_dir(p: &Path) -> bool { + // Test hopefully don't really need comments, and for assists we already + // have special comments which are source of doc tests and user docs. + let exclude_dirs = ["tests", "test_data", "handlers"]; + let mut cur_path = p; + while let Some(path) = cur_path.parent() { + if exclude_dirs.iter().any(|dir| path.ends_with(dir)) { + return true; + } + cur_path = path; + } + + false + } + + fn is_exclude_file(d: &Path) -> bool { + let file_names = ["tests.rs"]; + + d.file_name() + .unwrap_or_default() + .to_str() + .map(|f_n| file_names.iter().any(|name| *name == f_n)) + .unwrap_or(false) + } + } + + fn finish(self) { + if !self.missing_docs.is_empty() { + panic!( + "\nMissing docs strings\n\n\ + modules:\n{}\n\n", + self.missing_docs.join("\n") + ) + } + + let whitelist = [ + "ra_db", + "ra_hir", + "ra_hir_expand", + "ra_ide", + "ra_mbe", + "ra_parser", + "ra_prof", + "ra_project_model", + "ra_syntax", + "ra_text_edit", + "ra_tt", + "ra_hir_ty", + ]; + + let mut has_fixmes = + whitelist.iter().map(|it| (*it, false)).collect::>(); + 'outer: for path in self.contains_fixme { + for krate in whitelist.iter() { + if path.components().any(|it| it.as_os_str() == *krate) { + has_fixmes.insert(krate, true); + continue 'outer; + } + } + panic!("FIXME doc in a fully-documented crate: {}", path.display()) + } + + for (krate, has_fixme) in has_fixmes.iter() { + if !has_fixme { + panic!("crate {} is fully documented, remove it from the white list", krate) + } + } + } +} + +fn rust_files() -> impl Iterator { + let crates = project_root().join("crates"); + let iter = WalkDir::new(crates); + 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(|path| path.extension().map(|it| it == "rs").unwrap_or(false)); + + fn is_hidden(entry: &DirEntry) -> bool { + entry.file_name().to_str().map(|s| s.starts_with('.')).unwrap_or(false) + } +} -- cgit v1.2.3