From 03b02e6bd0c61fc800c0694e1156690e9e2a142c Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 28 Feb 2024 16:17:44 +0100 Subject: [PATCH] internal: Move ide-assists codegen tests into an xtask codegen command --- .cargo/config.toml | 1 + .github/workflows/ci.yaml | 3 + Cargo.lock | 2 +- crates/ide-assists/Cargo.toml | 1 - crates/ide-assists/src/tests.rs | 2 - xtask/Cargo.toml | 3 +- xtask/src/codegen.rs | 211 ++++++++++++++++++ .../src/codegen/assists_doc_tests.rs | 26 ++- xtask/src/flags.rs | 29 +++ xtask/src/main.rs | 18 +- xtask/src/release.rs | 6 +- 11 files changed, 273 insertions(+), 29 deletions(-) create mode 100644 xtask/src/codegen.rs rename crates/ide-assists/src/tests/sourcegen.rs => xtask/src/codegen/assists_doc_tests.rs (89%) diff --git a/.cargo/config.toml b/.cargo/config.toml index c3cfda85517..070560dfbc3 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -3,6 +3,7 @@ xtask = "run --package xtask --bin xtask --" tq = "test -- -q" qt = "tq" lint = "clippy --all-targets -- --cap-lints warn" +codegen = "run --package xtask --bin xtask -- codegen" [target.x86_64-pc-windows-msvc] linker = "rust-lld" diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5a8b18e3fe1..2d8946520d5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -79,6 +79,9 @@ jobs: if: matrix.os == 'ubuntu-latest' run: sed -i '/\[profile.dev]/a opt-level=1' Cargo.toml + - name: Codegen checks (rust-analyzer) + run: cargo codegen --check + - name: Compile (tests) run: cargo test --no-run --locked ${{ env.USE_SYSROOT_ABI }} diff --git a/Cargo.lock b/Cargo.lock index 3c87291dbad..0bdd4821139 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -676,7 +676,6 @@ dependencies = [ "itertools", "profile", "smallvec", - "sourcegen", "stdx", "syntax", "test-fixture", @@ -2502,6 +2501,7 @@ version = "0.1.0" dependencies = [ "anyhow", "flate2", + "stdx", "time", "write-json", "xflags", diff --git a/crates/ide-assists/Cargo.toml b/crates/ide-assists/Cargo.toml index 98961a18de2..d84fa7dffc5 100644 --- a/crates/ide-assists/Cargo.toml +++ b/crates/ide-assists/Cargo.toml @@ -33,7 +33,6 @@ expect-test = "1.4.0" # local deps test-utils.workspace = true test-fixture.workspace = true -sourcegen.workspace = true [features] in-rust-tree = [] diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs index 9b6f7d018ee..32d69841020 100644 --- a/crates/ide-assists/src/tests.rs +++ b/crates/ide-assists/src/tests.rs @@ -1,6 +1,4 @@ mod generated; -#[cfg(not(feature = "in-rust-tree"))] -mod sourcegen; use expect_test::expect; use hir::Semantics; diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index 863a63ac82e..5e758e0190c 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -14,7 +14,8 @@ xshell.workspace = true xflags = "0.3.0" time = { version = "0.3", default-features = false } zip = { version = "0.6", default-features = false, features = ["deflate", "time"] } +stdx.workspace = true # Avoid adding more dependencies to this crate [lints] -workspace = true \ No newline at end of file +workspace = true diff --git a/xtask/src/codegen.rs b/xtask/src/codegen.rs new file mode 100644 index 00000000000..e579660ac9b --- /dev/null +++ b/xtask/src/codegen.rs @@ -0,0 +1,211 @@ +use std::{ + fmt, fs, mem, + path::{Path, PathBuf}, +}; + +use xshell::{cmd, Shell}; + +use crate::{flags, project_root}; + +pub(crate) mod assists_doc_tests; + +impl flags::Codegen { + pub(crate) fn run(self, _sh: &Shell) -> anyhow::Result<()> { + match self.codegen_type.unwrap_or_default() { + flags::CodegenType::All => { + assists_doc_tests::generate(self.check); + } + flags::CodegenType::AssistsDocTests => assists_doc_tests::generate(self.check), + } + Ok(()) + } +} + +fn list_rust_files(dir: &Path) -> Vec { + let mut res = list_files(dir); + res.retain(|it| { + it.file_name().unwrap_or_default().to_str().unwrap_or_default().ends_with(".rs") + }); + res +} + +fn list_files(dir: &Path) -> Vec { + let mut res = Vec::new(); + let mut work = vec![dir.to_path_buf()]; + while let Some(dir) = work.pop() { + for entry in dir.read_dir().unwrap() { + let entry = entry.unwrap(); + let file_type = entry.file_type().unwrap(); + let path = entry.path(); + let is_hidden = + path.file_name().unwrap_or_default().to_str().unwrap_or_default().starts_with('.'); + if !is_hidden { + if file_type.is_dir() { + work.push(path); + } else if file_type.is_file() { + res.push(path); + } + } + } + } + res +} + +#[derive(Clone)] +pub(crate) struct CommentBlock { + pub(crate) id: String, + pub(crate) line: usize, + pub(crate) contents: Vec, + is_doc: bool, +} + +impl CommentBlock { + fn extract(tag: &str, text: &str) -> Vec { + assert!(tag.starts_with(char::is_uppercase)); + + let tag = format!("{tag}:"); + let mut blocks = CommentBlock::extract_untagged(text); + blocks.retain_mut(|block| { + let first = block.contents.remove(0); + let Some(id) = first.strip_prefix(&tag) else { + return false; + }; + + if block.is_doc { + panic!("Use plain (non-doc) comments with tags like {tag}:\n {first}"); + } + + block.id = id.trim().to_owned(); + true + }); + blocks + } + + fn extract_untagged(text: &str) -> Vec { + let mut res = Vec::new(); + + let lines = text.lines().map(str::trim_start); + + let dummy_block = + CommentBlock { id: String::new(), line: 0, contents: Vec::new(), is_doc: false }; + let mut block = dummy_block.clone(); + for (line_num, line) in lines.enumerate() { + match line.strip_prefix("//") { + Some(mut contents) => { + if let Some('/' | '!') = contents.chars().next() { + contents = &contents[1..]; + block.is_doc = true; + } + if let Some(' ') = contents.chars().next() { + contents = &contents[1..]; + } + block.contents.push(contents.to_owned()); + } + None => { + if !block.contents.is_empty() { + let block = mem::replace(&mut block, dummy_block.clone()); + res.push(block); + } + block.line = line_num + 2; + } + } + } + if !block.contents.is_empty() { + res.push(block); + } + res + } +} + +#[derive(Debug)] +pub(crate) struct Location { + pub(crate) file: PathBuf, + pub(crate) line: usize, +} + +impl fmt::Display for Location { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let path = self.file.strip_prefix(project_root()).unwrap().display().to_string(); + let path = path.replace('\\', "/"); + let name = self.file.file_name().unwrap(); + write!( + f, + "https://github.com/rust-lang/rust-analyzer/blob/master/{}#L{}[{}]", + path, + self.line, + name.to_str().unwrap() + ) + } +} + +fn ensure_rustfmt(sh: &Shell) { + let version = cmd!(sh, "rustup run stable rustfmt --version").read().unwrap_or_default(); + if !version.contains("stable") { + panic!( + "Failed to run rustfmt from toolchain 'stable'. \ + Please run `rustup component add rustfmt --toolchain stable` to install it.", + ); + } +} + +fn reformat(text: String) -> String { + let sh = Shell::new().unwrap(); + ensure_rustfmt(&sh); + let rustfmt_toml = project_root().join("rustfmt.toml"); + let mut stdout = cmd!( + sh, + "rustup run stable rustfmt --config-path {rustfmt_toml} --config fn_single_line=true" + ) + .stdin(text) + .read() + .unwrap(); + if !stdout.ends_with('\n') { + stdout.push('\n'); + } + stdout +} + +fn add_preamble(generator: &'static str, mut text: String) -> String { + let preamble = format!("//! Generated by `{generator}`, do not edit by hand.\n\n"); + text.insert_str(0, &preamble); + text +} + +/// Checks that the `file` has the specified `contents`. If that is not the +/// case, updates the file and then fails the test. +#[allow(clippy::print_stderr)] +fn ensure_file_contents(file: &Path, contents: &str, check: bool) { + if let Ok(old_contents) = fs::read_to_string(file) { + if normalize_newlines(&old_contents) == normalize_newlines(contents) { + // File is already up to date. + return; + } + } + + let display_path = file.strip_prefix(project_root()).unwrap_or(file); + if check { + panic!( + "{} was not up-to-date{}", + file.display(), + if std::env::var("CI").is_ok() { + "\n NOTE: run `cargo codegen` locally and commit the updated files\n" + } else { + "" + } + ); + } else { + eprintln!( + "\n\x1b[31;1merror\x1b[0m: {} was not up-to-date, updating\n", + display_path.display() + ); + + if let Some(parent) = file.parent() { + let _ = fs::create_dir_all(parent); + } + fs::write(file, contents).unwrap(); + } +} + +fn normalize_newlines(s: &str) -> String { + s.replace("\r\n", "\n") +} diff --git a/crates/ide-assists/src/tests/sourcegen.rs b/xtask/src/codegen/assists_doc_tests.rs similarity index 89% rename from crates/ide-assists/src/tests/sourcegen.rs rename to xtask/src/codegen/assists_doc_tests.rs index 847cb1af51e..b2d89dde765 100644 --- a/crates/ide-assists/src/tests/sourcegen.rs +++ b/xtask/src/codegen/assists_doc_tests.rs @@ -3,10 +3,15 @@ use std::{fmt, fs, path::Path}; use stdx::format_to_acc; -use test_utils::project_root; -#[test] -fn sourcegen_assists_docs() { +use crate::{ + codegen::{ + add_preamble, ensure_file_contents, list_rust_files, reformat, CommentBlock, Location, + }, + project_root, +}; + +pub(crate) fn generate(check: bool) { let assists = Assist::collect(); { @@ -40,10 +45,11 @@ fn doctest_{}() {{ buf.push_str(&test) } } - let buf = sourcegen::add_preamble("sourcegen_assists_docs", sourcegen::reformat(buf)); - sourcegen::ensure_file_contents( + let buf = add_preamble("sourcegen_assists_docs", reformat(buf)); + ensure_file_contents( &project_root().join("crates/ide-assists/src/tests/generated.rs"), &buf, + check, ); } @@ -52,7 +58,7 @@ fn doctest_{}() {{ // git repo. Instead, `cargo xtask release` runs this test before making // a release. - let contents = sourcegen::add_preamble( + let contents = add_preamble( "sourcegen_assists_docs", assists.into_iter().map(|it| it.to_string()).collect::>().join("\n\n"), ); @@ -71,7 +77,7 @@ struct Section { #[derive(Debug)] struct Assist { id: String, - location: sourcegen::Location, + location: Location, sections: Vec
, } @@ -80,7 +86,7 @@ fn collect() -> Vec { let handlers_dir = project_root().join("crates/ide-assists/src/handlers"); let mut res = Vec::new(); - for path in sourcegen::list_rust_files(&handlers_dir) { + for path in list_rust_files(&handlers_dir) { collect_file(&mut res, path.as_path()); } res.sort_by(|lhs, rhs| lhs.id.cmp(&rhs.id)); @@ -88,7 +94,7 @@ fn collect() -> Vec { fn collect_file(acc: &mut Vec, path: &Path) { let text = fs::read_to_string(path).unwrap(); - let comment_blocks = sourcegen::CommentBlock::extract("Assist", &text); + let comment_blocks = CommentBlock::extract("Assist", &text); for block in comment_blocks { let id = block.id; @@ -97,7 +103,7 @@ fn collect_file(acc: &mut Vec, path: &Path) { "invalid assist id: {id:?}" ); let mut lines = block.contents.iter().peekable(); - let location = sourcegen::Location { file: path.to_path_buf(), line: block.line }; + let location = Location { file: path.to_path_buf(), line: block.line }; let mut assist = Assist { id, location, sections: Vec::new() }; while lines.peek().is_some() { diff --git a/xtask/src/flags.rs b/xtask/src/flags.rs index e234090a07c..f5827170b65 100644 --- a/xtask/src/flags.rs +++ b/xtask/src/flags.rs @@ -52,6 +52,11 @@ cmd bb { required suffix: String } + + cmd codegen { + optional codegen_type: CodegenType + optional --check + } } } @@ -73,8 +78,32 @@ pub enum XtaskCmd { PublishReleaseNotes(PublishReleaseNotes), Metrics(Metrics), Bb(Bb), + Codegen(Codegen), } +#[derive(Debug)] +pub struct Codegen { + pub check: bool, + pub codegen_type: Option, +} + +#[derive(Debug, Default)] +pub enum CodegenType { + #[default] + All, + AssistsDocTests, +} + +impl FromStr for CodegenType { + type Err = String; + fn from_str(s: &str) -> Result { + match s { + "all" => Ok(Self::All), + "assists-doc-tests" => Ok(Self::AssistsDocTests), + _ => Err("Invalid option".to_owned()), + } + } +} #[derive(Debug)] pub struct Install { pub client: bool, diff --git a/xtask/src/main.rs b/xtask/src/main.rs index df4d9810e6f..9418675a348 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -13,6 +13,7 @@ mod flags; +mod codegen; mod dist; mod install; mod metrics; @@ -20,10 +21,7 @@ mod release; use anyhow::bail; -use std::{ - env, - path::{Path, PathBuf}, -}; +use std::{env, path::PathBuf}; use xshell::{cmd, Shell}; fn main() -> anyhow::Result<()> { @@ -40,6 +38,7 @@ fn main() -> anyhow::Result<()> { flags::XtaskCmd::Dist(cmd) => cmd.run(sh), flags::XtaskCmd::PublishReleaseNotes(cmd) => cmd.run(sh), flags::XtaskCmd::Metrics(cmd) => cmd.run(sh), + flags::XtaskCmd::Codegen(cmd) => cmd.run(sh), flags::XtaskCmd::Bb(cmd) => { { let _d = sh.push_dir("./crates/rust-analyzer"); @@ -54,14 +53,11 @@ fn main() -> anyhow::Result<()> { } } +/// Returns the path to the root directory of `rust-analyzer` project. fn project_root() -> PathBuf { - Path::new( - &env::var("CARGO_MANIFEST_DIR").unwrap_or_else(|_| env!("CARGO_MANIFEST_DIR").to_owned()), - ) - .ancestors() - .nth(1) - .unwrap() - .to_path_buf() + let dir = + env::var("CARGO_MANIFEST_DIR").unwrap_or_else(|_| env!("CARGO_MANIFEST_DIR").to_owned()); + PathBuf::from(dir).parent().unwrap().to_owned() } fn run_fuzzer(sh: &Shell) -> anyhow::Result<()> { diff --git a/xtask/src/release.rs b/xtask/src/release.rs index 4a306914778..f99f9ecbc39 100644 --- a/xtask/src/release.rs +++ b/xtask/src/release.rs @@ -2,7 +2,7 @@ use xshell::{cmd, Shell}; -use crate::{date_iso, flags, is_release_tag, project_root}; +use crate::{codegen, date_iso, flags, is_release_tag, project_root}; impl flags::Release { pub(crate) fn run(self, sh: &Shell) -> anyhow::Result<()> { @@ -23,8 +23,8 @@ pub(crate) fn run(self, sh: &Shell) -> anyhow::Result<()> { } // Generates bits of manual.adoc. - cmd!(sh, "cargo test -p ide-assists -p ide-diagnostics -p rust-analyzer -- sourcegen_") - .run()?; + cmd!(sh, "cargo test -p ide-diagnostics -p rust-analyzer -- sourcegen_").run()?; + codegen::assists_doc_tests::generate(false); let website_root = project_root().join("../rust-analyzer.github.io"); {