Merge #2069

2069: auto-generate assists docs and tests r=matklad a=matklad



Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ra_assists/src/assists/early_return.rs

@@ -1,26 +1,3 @@
-//! Assist: `convert_to_guarded_return`
-//!
-//! Replace a large conditional with a guarded return.
-//!
-//! ```text
-//! fn <|>main() {
-//!     if cond {
-//!         foo();
-//!         bar();
-//!     }
-//! }
-//! ```
-//! ->
-//! ```text
-//! fn main() {
-//!     if !cond {
-//!         return;
-//!     }
-//!     foo();
-//!     bar();
-//! }
-//! ```
-
 use std::ops::RangeInclusive;
 
 use hir::db::HirDatabase;
@@ -36,6 +13,26 @@
     AssistId,
 };
 
+// Assist: convert_to_guarded_return
+// Replace a large conditional with a guarded return.
+// ```
+// fn main() {
+//     <|>if cond {
+//         foo();
+//         bar();
+//     }
+// }
+// ```
+// ->
+// ```
+// fn main() {
+//     if !cond {
+//         return;
+//     }
+//     foo();
+//     bar();
+// }
+// ```
 pub(crate) fn convert_to_guarded_return(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     let if_expr: ast::IfExpr = ctx.node_at_offset()?;
     let expr = if_expr.condition()?.expr()?;

crates/ra_assists/src/doc_tests.rs

@@ -0,0 +1,23 @@
+//! Each assist definition has a special comment, which specifies docs and
+//! example.
+//!
+//! We collect all the example and write the as tests in this module.
+
+mod generated;
+
+use hir::mock::MockDatabase;
+use ra_db::FileRange;
+use ra_syntax::TextRange;
+use test_utils::{assert_eq_text, extract_offset};
+
+fn check(assist_id: &str, before: &str, after: &str) {
+    let (before_cursor_pos, before) = extract_offset(before);
+    let (db, _source_root, file_id) = MockDatabase::with_single_file(&before);
+    let frange = FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) };
+
+    let (_assist_id, action) =
+        crate::assists(&db, frange).into_iter().find(|(id, _)| id.id.0 == assist_id).unwrap();
+
+    let actual = action.edit.apply(&before);
+    assert_eq_text!(after, &actual);
+}

crates/ra_assists/src/doc_tests/generated.rs

@@ -0,0 +1,27 @@
+//! Generated file, do not edit by hand, see `crate/ra_tools/src/codegen`
+
+use super::check;
+
+#[test]
+fn doctest_convert_to_guarded_return() {
+    check(
+        "convert_to_guarded_return",
+        r#####"
+fn main() {
+    <|>if cond {
+        foo();
+        bar();
+    }
+}
+"#####,
+        r#####"
+fn main() {
+    if !cond {
+        return;
+    }
+    foo();
+    bar();
+}
+"#####,
+    )
+}

crates/ra_assists/src/lib.rs

@@ -7,6 +7,8 @@
 
 mod assist_ctx;
 mod marks;
+#[cfg(test)]
+mod doc_tests;
 
 use hir::db::HirDatabase;
 use itertools::Itertools;
@@ -36,7 +38,7 @@ pub struct AssistAction {
     pub target: Option<TextRange>,
 }
 
-/// Return all the assists eapplicable at the given position.
+/// Return all the assists applicable at the given position.
 ///
 /// Assists are returned in the "unresolved" state, that is only labels are
 /// returned, without actual edits.

docs/user/assists.md

@@ -0,0 +1,24 @@
+# Assists
+
+## `convert_to_guarded_return`
+
+Replace a large conditional with a guarded return.
+
+```rust
+// BEFORE
+fn main() {
+    <|>if cond {
+        foo();
+        bar();
+    }
+}
+
+// AFTER
+fn main() {
+    if !cond {
+        return;
+    }
+    foo();
+    bar();
+}
+```

docs/user/features.md

@@ -97,11 +97,13 @@ Start `cargo watch` for live error highlighting. Will prompt to install if it's
 
 Stop `cargo watch`
 
-### Code Actions (Assists)
+### Assists (Code Actions)
 
 These are triggered in a particular context via light bulb. We use custom code on
 the VS Code side to be able to position cursor. `<|>` signifies cursor
 
+See [assists.md](./assists.md)
+
 - Add `#[derive]`
 
 ```rust

xtask/src/codegen.rs

@@ -7,12 +7,22 @@
 
 mod gen_syntax;
 mod gen_parser_tests;
+mod gen_assists_docs;
 
-use std::{fs, mem, path::Path};
+use std::{
+    fs,
+    io::Write,
+    mem,
+    path::Path,
+    process::{Command, Stdio},
+};
 
-use crate::Result;
+use crate::{project_root, Result};
 
-pub use self::{gen_parser_tests::generate_parser_tests, gen_syntax::generate_syntax};
+pub use self::{
+    gen_assists_docs::generate_assists_docs, gen_parser_tests::generate_parser_tests,
+    gen_syntax::generate_syntax,
+};
 
 pub const GRAMMAR: &str = "crates/ra_syntax/src/grammar.ron";
 const GRAMMAR_DIR: &str = "crates/ra_parser/src/grammar";
@@ -22,6 +32,10 @@
 pub const SYNTAX_KINDS: &str = "crates/ra_parser/src/syntax_kind/generated.rs";
 pub const AST: &str = "crates/ra_syntax/src/ast/generated.rs";
 
+const ASSISTS_DIR: &str = "crates/ra_assists/src/assists";
+const ASSISTS_TESTS: &str = "crates/ra_assists/src/doc_tests/generated.rs";
+const ASSISTS_DOCS: &str = "docs/user/assists.md";
+
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub enum Mode {
     Overwrite,
@@ -30,7 +44,7 @@ pub enum Mode {
 
 /// A helper to update file on disk if it has changed.
 /// With verify = false,
-pub fn update(path: &Path, contents: &str, mode: Mode) -> Result<()> {
+fn update(path: &Path, contents: &str, mode: Mode) -> Result<()> {
     match fs::read_to_string(path) {
         Ok(ref old_contents) if old_contents == contents => {
             return Ok(());
@@ -45,6 +59,20 @@ pub fn update(path: &Path, contents: &str, mode: Mode) -> Result<()> {
     Ok(())
 }
 
+fn reformat(text: impl std::fmt::Display) -> Result<String> {
+    let mut rustfmt = Command::new("rustfmt")
+        .arg("--config-path")
+        .arg(project_root().join("rustfmt.toml"))
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .spawn()?;
+    write!(rustfmt.stdin.take().unwrap(), "{}", text)?;
+    let output = rustfmt.wait_with_output()?;
+    let stdout = String::from_utf8(output.stdout)?;
+    let preamble = "Generated file, do not edit by hand, see `crate/ra_tools/src/codegen`";
+    Ok(format!("//! {}\n\n{}", preamble, stdout))
+}
+
 fn extract_comment_blocks(text: &str) -> Vec<Vec<String>> {
     let mut res = Vec::new();
 

xtask/src/codegen/gen_assists_docs.rs

@@ -0,0 +1,123 @@
+use std::{fs, path::Path};
+
+use crate::{
+    codegen::{self, extract_comment_blocks, Mode},
+    project_root, Result,
+};
+
+pub fn generate_assists_docs(mode: Mode) -> Result<()> {
+    let assists = collect_assists()?;
+    generate_tests(&assists, mode)?;
+    generate_docs(&assists, mode)?;
+    Ok(())
+}
+
+#[derive(Debug)]
+struct Assist {
+    id: String,
+    doc: String,
+    before: String,
+    after: String,
+}
+
+fn collect_assists() -> Result<Vec<Assist>> {
+    let mut res = Vec::new();
+    for entry in fs::read_dir(project_root().join(codegen::ASSISTS_DIR))? {
+        let entry = entry?;
+        let path = entry.path();
+        if path.is_file() {
+            collect_file(&mut res, path.as_path())?;
+        }
+    }
+    res.sort_by(|lhs, rhs| lhs.id.cmp(&rhs.id));
+    return Ok(res);
+
+    fn collect_file(acc: &mut Vec<Assist>, path: &Path) -> Result<()> {
+        let text = fs::read_to_string(path)?;
+        let comment_blocks = extract_comment_blocks(&text);
+
+        for block in comment_blocks {
+            // FIXME: doesn't support blank lines yet, need to tweak
+            // `extract_comment_blocks` for that.
+            let mut lines = block.iter();
+            let first_line = lines.next().unwrap();
+            if !first_line.starts_with("Assist: ") {
+                continue;
+            }
+            let id = first_line["Assist: ".len()..].to_string();
+            assert!(id.chars().all(|it| it.is_ascii_lowercase() || it == '_'));
+
+            let doc = take_until(lines.by_ref(), "```");
+            let before = take_until(lines.by_ref(), "```");
+
+            assert_eq!(lines.next().unwrap().as_str(), "->");
+            assert_eq!(lines.next().unwrap().as_str(), "```");
+            let after = take_until(lines.by_ref(), "```");
+            acc.push(Assist { id, doc, before, after })
+        }
+
+        fn take_until<'a>(lines: impl Iterator<Item = &'a String>, marker: &str) -> String {
+            let mut buf = Vec::new();
+            for line in lines {
+                if line == marker {
+                    break;
+                }
+                buf.push(line.clone());
+            }
+            buf.join("\n")
+        }
+        Ok(())
+    }
+}
+
+fn generate_tests(assists: &[Assist], mode: Mode) -> Result<()> {
+    let mut buf = String::from("use super::check;\n");
+
+    for assist in assists.iter() {
+        let test = format!(
+            r######"
+#[test]
+fn doctest_{}() {{
+    check(
+        "{}",
+r#####"
+{}
+"#####, r#####"
+{}
+"#####)
+}}
+"######,
+            assist.id, assist.id, assist.before, assist.after
+        );
+
+        buf.push_str(&test)
+    }
+    let buf = codegen::reformat(buf)?;
+    codegen::update(&project_root().join(codegen::ASSISTS_TESTS), &buf, mode)
+}
+
+fn generate_docs(assists: &[Assist], mode: Mode) -> Result<()> {
+    let mut buf = String::from("# Assists\n");
+
+    for assist in assists {
+        let docs = format!(
+            "
+## `{}`
+
+{}
+
+```rust
+// BEFORE
+{}
+
+// AFTER
+{}
+```
+",
+            assist.id, assist.doc, assist.before, assist.after
+        );
+        buf.push_str(&docs);
+    }
+
+    codegen::update(&project_root().join(codegen::ASSISTS_DOCS), &buf, mode)
+}

xtask/src/codegen/gen_syntax.rs

@@ -3,12 +3,7 @@
 //! Specifically, it generates the `SyntaxKind` enum and a number of newtype
 //! wrappers around `SyntaxNode` which implement `ra_syntax::AstNode`.
 
-use std::{
-    collections::BTreeMap,
-    fs,
-    io::Write,
-    process::{Command, Stdio},
-};
+use std::{collections::BTreeMap, fs};
 
 use proc_macro2::{Punct, Spacing};
 use quote::{format_ident, quote};
@@ -163,7 +158,7 @@ impl #name {
         #(#nodes)*
     };
 
-    let pretty = reformat(ast)?;
+    let pretty = codegen::reformat(ast)?;
     Ok(pretty)
 }
 
@@ -276,21 +271,7 @@ macro_rules! T {
         }
     };
 
-    reformat(ast)
-}
-
-fn reformat(text: impl std::fmt::Display) -> Result<String> {
-    let mut rustfmt = Command::new("rustfmt")
-        .arg("--config-path")
-        .arg(project_root().join("rustfmt.toml"))
-        .stdin(Stdio::piped())
-        .stdout(Stdio::piped())
-        .spawn()?;
-    write!(rustfmt.stdin.take().unwrap(), "{}", text)?;
-    let output = rustfmt.wait_with_output()?;
-    let stdout = String::from_utf8(output.stdout)?;
-    let preamble = "Generated file, do not edit by hand, see `crate/ra_tools/src/codegen`";
-    Ok(format!("//! {}\n\n{}", preamble, stdout))
+    codegen::reformat(ast)
 }
 
 #[derive(Deserialize, Debug)]

xtask/src/main.rs

@@ -64,6 +64,7 @@ fn main() -> Result<()> {
             }
             codegen::generate_syntax(Mode::Overwrite)?;
             codegen::generate_parser_tests(Mode::Overwrite)?;
+            codegen::generate_assists_docs(Mode::Overwrite)?;
         }
         "format" => {
             if matches.contains(["-h", "--help"]) {

xtask/tests/tidy-tests/cli.rs

@@ -18,6 +18,13 @@ fn generated_tests_are_fresh() {
     }
 }
 
+#[test]
+fn generated_assists_are_fresh() {
+    if let Err(error) = codegen::generate_assists_docs(Mode::Verify) {
+        panic!("{}. Please update assists by running `cargo xtask codegen`", error);
+    }
+}
+
 #[test]
 fn check_code_formatting() {
     if let Err(error) = run_rustfmt(Mode::Verify) {

xtask/tests/tidy-tests/docs.rs

@@ -8,7 +8,9 @@
 use xtask::project_root;
 
 fn is_exclude_dir(p: &Path) -> bool {
-    let exclude_dirs = ["tests", "test_data"];
+    // 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", "assists"];
     let mut cur_path = p;
     while let Some(path) = cur_path.parent() {
         if exclude_dirs.iter().any(|dir| path.ends_with(dir)) {