add apply ssr assist

[JoshMcguigan]

This PR adds an Apply SSR assist which was briefly mentioned in #3186. It allows writing an ssr rule as a comment, and then applying that SSR via an assist. This workflow is much nicer than the default available via coc-rust-analyzer when iterating to find the proper replacement.

As currently implemented, this requires the ssr rule is written as a single line in the comment, and it doesn't require any kind of prefix. Anything which properly parses as a ssr rule will enable the assist. The benefit of requiring the ssr rule be on a single line is it allows for a workflow where the user has several rules written one after the other, possibly to be triggered in order, without having to try to parse multiple lines of text and determine where one rule ends and the next begins. The benefit of not requiring a prefix is less typing 😆 - plus, I think the chance of something accidentally parsing as an ssr rule is minimal.

I think a reasonable extension of this would be to allow either any ssr rule that fits on a single line, or any comment block which in its entirety makes up a single ssr rule (parsing a comment block containing multiple ssr rules and running them all would break the use case that currently works where a user writes multiple ssr rules then runs them each one by one in arbitrary order).

I've marked this as a draft because for some reason I am strugging to make the unit tests pass. It does work when I compile rust-analyzer and test it in my editor though, so I'm not sure what is going on.

[JoshMcguigan]

This is the error I am getting from the unit test. I confirmed it is properly parsing the ssr rule, but for some reason it seems to not return any edits. As noted above, this assist does actually work if I build rust-analyzer and run it with my editor, so the unit test errors are quite puzzling.

$ cargo test -p ide_assists handlers::apply_ssr
    Finished test [unoptimized] target(s) in 0.07s
     Running target/debug/deps/ide_assists-cfba0b9b7eebf6de

running 1 test
test handlers::apply_ssr::tests::in_file ... FAILED

failures:

---- handlers::apply_ssr::tests::in_file stdout ----
thread 'handlers::apply_ssr::tests::in_file' panicked at 'assertion failed: !source_change.source_
file_edits.is_empty()', crates/ide_assists/src/tests.rs:47:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    handlers::apply_ssr::tests::in_file

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 867 filtered out

error: test failed, to rerun pass '-p ide_assists --lib'

[JoshMcguigan]

    let mut local_roots = FxHashSet::default();
    local_roots.insert(ide_db::base_db::fixture::WORKSPACE);
    db.set_local_roots_with_durability(Arc::new(local_roots), Durability::HIGH);

It seems the test setup in ide_assists::tests::check was not setting any local root. I added these three lines and my test is now passing!

I'm leaving this as a draft for now as there are some additional tests I'd like to add. But I'm certainly open to feedback on this PR.

[kjeremy]

FWIW we can get previews for WorkspaceEdits in vscode.

…

On Tue, Mar 9, 2021, 7:23 AM Aleksey Kladov ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In crates/ide_ssr/src/from_comment.rs <#7874 (comment)> : > +//! This module allows building an SSR MatchFinder by parsing the SSR rule +//! from a comment. + +use ide_db::{ + base_db::{FilePosition, FileRange}, + RootDatabase, +}; +use syntax::ast::{self, AstToken}; + +use crate::{MatchFinder, SsrError}; + +pub fn ssr_from_comment<'db>( + db: &'db RootDatabase, + frange: FileRange, + comment: &ast::Comment, +) -> Result<MatchFinder<'db>, SsrError> { Yeah, I think it's better that the caller passes just the range. I don't think we need any kind of infrastructure here -- just a single test that it works would be enough. ------------------------------ In crates/ide/src/lib.rs <#7874 (comment)> : > @@ -579,6 +586,33 @@ impl Analysis { } } +fn add_ssr_assist( + db: &RootDatabase, + base: &mut Vec<Assist>, + resolve: bool, + frange: FileRange, +) -> Option<()> { + let comment = { + let file = db.parse(frange.file_id); + file.tree().syntax().token_at_offset(frange.range.start()).find_map(ast::Comment::cast) + }?; + let match_finder = ide_ssr::ssr_from_comment(db, frange, &comment).ok()?; + + let mut assist = Assist { + id: AssistId("ssr", AssistKind::RefactorRewrite), + label: Label::new("Apply SSR"), SGTM! Ideally, we should have some kind of live preview for SSR, but that seems pretty far ahead. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#7874 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBACRCI32TSTMV355P5ABDTCYHLTANCNFSM4YUOLFWA> .

[matklad]

LGTM now!

bors d+

[bors]

✌️ JoshMcguigan can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

[JoshMcguigan]

I started working on adding this into the doc generation infra for assists, but that looks to be a larger change, since when we generate user facing docs we also generate doc tests - and those doc tests assume the assist to be tested is in the ide_assists crate.

I'm going to punt on the doc infrastructure changes for now, as it isn't directly related to the creation of this assist and I'm not exactly sure how we'd want to handle it. But @matklad let me know if you have thoughts on how this should be done and I can submit a follow up PR if you'd like.

bors r+

[bors]

Build succeeded:

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript (ubuntu-latest)
• TypeScript (windows-latest)

[matklad]

@JoshMcguigan you can add a paragraph to the existing ssr documentation?

https://github.com/rust-analyzer/rust-analyzer/blob/a9b1e5cde1cc6e470be60cf2c230b99a62c2d2c3/crates/ide_ssr/src/lib.rs#L6-L28

[matklad]

The docs are rendered here:

https://rust-analyzer.github.io/manual.html#structural-search-and-replace

[lnicola]

[matklad]

Almost forgot, cc @davidlattimore , you'll like this ^ :)