Stream: t-compiler/wg-rls-2.0

Topic: Structural Search Replace


matklad (Nov 15 2019 at 20:46, on Zulip):

I've almost nerdsnipped myself with this, but decided to write up an issue with mentoring instructions in the end: https://github.com/rust-analyzer/rust-analyzer/issues/2267 :)

csmoe (Nov 16 2019 at 07:55, on Zulip):

@matklad https://github.com/google/rerast may help here, I'm transforming generated code with it these days.

matklad (Nov 16 2019 at 09:07, on Zulip):

@csmoe the idea is exactly to build rerast-like thin, but using rust-analyzer's types, which I believe I fundamentally more suitable for this kind of things in the limit (ie, when we finishing implementing all the things we yet have to add...)

Felix Kohlgrüber (Nov 20 2019 at 19:28, on Zulip):

How can I parse a String into an expr syntax tree node? And also, should ssr be performed on the concrete or abstract syntax tree?

matklad (Nov 20 2019 at 19:30, on Zulip):

@Felix Kohlgrüber SourceFile::parse in ra_syntax defines parsing of files

matklad (Nov 20 2019 at 19:31, on Zulip):

I think a similar method could be defined for ast::Expr, but you'll need parse_fragment function as an entry point

matklad (Nov 20 2019 at 19:31, on Zulip):

We don't have traditional AST (what is called ast is, in fact, a typed concrete syntax tree)

matklad (Nov 20 2019 at 19:32, on Zulip):

So I think it makes sense to run ssr on concrete syntax tree, with maybe some hacks for more abstract comparison (like, not taking the order of fields into account, etc)

matklad (Nov 20 2019 at 19:32, on Zulip):

One more interesting bit is that, because replace bit has to happen at the CST level, you sort-of need to run the search on the CST as well

Felix Kohlgrüber (Nov 20 2019 at 19:34, on Zulip):

Ok thx, got it!

matklad (Nov 20 2019 at 19:37, on Zulip):

@Felix Kohlgrüber actually, I thik you can also use make::expr_from_string -- it's hacky, but works

matklad (Nov 20 2019 at 19:38, on Zulip):

expr_from_text that is

Felix Kohlgrüber (Nov 20 2019 at 19:40, on Zulip):

ah ok, that should work. Thanks!

Felix Kohlgrüber (Nov 20 2019 at 19:44, on Zulip):

@matklad In the issue description, you wrote that writing a custom parser for the patterns might be better than using regex. Could you explain why you think so?

Felix Kohlgrüber (Nov 20 2019 at 19:44, on Zulip):

@matklad Ok so this is how mentions work. Sry, I'm new to zulip.

matklad (Nov 20 2019 at 19:46, on Zulip):

@Felix Kohlgrüber mainly because regex is a big dependency. We have it in the crate graph already (which is regrettable, I believe at least two of the three uses are completely unnecessary) , but core crates like ra_ide_api do not have this dependency

Felix Kohlgrüber (Nov 20 2019 at 19:48, on Zulip):

@matklad ok, sounds reasonable. I'll hand-roll my own ;-)

mikhail-m1 (Jan 20 2020 at 11:21, on Zulip):

@matklad I have several questions:

  1. You started fromSource { file_id: $file_id:expr, ast: $ast:expr } but later used

    replace $ident:fragment with __search_pattern_ident is file_id: $file_id:expr, should it be $<something>:expr?

  2. Is any way to sanitize temporary names except of using __search_pattern_<ident1>, __search_pattern_<ident2> and etc?

  3. I tried convert it to macros declaration and parse, it works but I get ast::TokenTree, may be there is a way to convert ast::TokenTree or use it during search?
  4. If I need to write code to replace $file_id:expr should it be done just by string functions or there is a better way,?
  5. Do you have any example how to compare trees?
mikhail-m1 (Jan 22 2020 at 11:59, on Zulip):

little bit more details for 3:

  1. convert a pattern to macro_rules! { ({}) => () }
  2. parse
  3. replace $file_id:expr by placeholders
  4. convert back to string
  5. parse again as expt

@matklad what do you think?

matklad (Jan 22 2020 at 12:23, on Zulip):

Don’t have time to really dig into this right now, but I don’t think we
need to parse anything as Macro here. I think just a text-based search for
patterns should be enough for the initial impl, and should be easier to
impl

mikhail-m1 (Feb 04 2020 at 11:26, on Zulip):

I've found that expr_from_text and TextEdit do not work together, because expr_from_text returns part of "const C: () = {};" with absolute offsets, so I need the full string back to make an edit. @matklad what do you think? I can just use ast_from_text same way as expr_from_text but it doesn't look right, or maybe there is a way to get relative offsets (TextRange)?

matklad (Feb 04 2020 at 11:31, on Zulip):

Hm, good question!

Let me see if I can quicly fix expr_from_text, this seems like a footgun

matklad (Feb 04 2020 at 12:24, on Zulip):

@mikhail-m1 should be fixed by https://github.com/rust-analyzer/rust-analyzer/pull/3009

mikhail-m1 (Feb 06 2020 at 13:56, on Zulip):

I little bit stuck with

Implement the driver, which visits all files in the current project and builds a SourceEdit

could you send me an example? I've tried to do something like: for krate in Crate::all(db) but looks like it iterates over some files outside of workspace.

matklad (Feb 06 2020 at 13:57, on Zulip):

https://github.com/rust-analyzer/rust-analyzer/blob/ea9d18ba836a7228f7310e1bc77c0918f0191a42/crates/ra_ide/src/symbol_index.rs#L94-L98

This is walks all local files for the purpose of index building

mikhail-m1 (Feb 07 2020 at 11:07, on Zulip):

Is any way to apply SourceChange without vs code?

matklad (Feb 07 2020 at 11:13, on Zulip):

Nope, there's no code for that

matklad (Feb 07 2020 at 11:13, on Zulip):

There's a way to apply text edit to a file though

mikhail-m1 (Feb 08 2020 at 15:36, on Zulip):

There's a way to apply text edit to a file though

next step, I have SourceFileEdit and it contains file_id, I can get file text and apply TextEdit but I need to write it back and for it I need full path, how can I get it? If I get it right, there are no full paths in Salsa, so I can do something similar to ra_cli/src/analysis_bench.rs or may be even build a map file_id -> full path, but may be I've missed something.

matklad (Feb 09 2020 at 12:29, on Zulip):

This generally should work like ra_analysis_cli.

matklad (Feb 09 2020 at 12:30, on Zulip):

Wait, I am confusing myself

matklad (Feb 09 2020 at 12:31, on Zulip):

I am thinking right now about a CLI tool for ssr, but I think the original plan was simply to expose this to VS Code?

Yeah, I guess we can do both a CLI and a VS Code action, and I would go with what's simpler. I think VS Code action might be simpler

mikhail-m1 (Feb 09 2020 at 17:00, on Zulip):

I created a working prototype for vscode only. It make changes in all files at once, but undo works only for current file. Rename's behavior is the same so I will leave it. I will try to clean it up and and create a PR soon. One more question, how should I call it? is Ssr enough or it's better to use StructuralSearchReplace everywhere?

std::Veetaha (Feb 09 2020 at 17:35, on Zulip):

SSR is server-side rendering ;)

matklad (Feb 09 2020 at 17:40, on Zulip):

I think structural search replace in IntelliJ existing before client side
rendering was a thing :-)

std::Veetaha (Feb 09 2020 at 17:42, on Zulip):

Well, I'd go better for SSR only if it is pervasive and we leave the comment on its meaning near the definition.

matklad (Feb 10 2020 at 09:12, on Zulip):

One more question, how should I call it? is Ssr enough or it's better to use StructuralSearchReplace everywhere?

Internally I would prefer ssr, for all user-visible strings we should use structural search replace

mikhail-m1 (Feb 11 2020 at 17:09, on Zulip):

@matklad considering your comment, have you put $<name>:expr to the second part as shown? In this case it should be related how expr_from_text handle incorrect code.

mikhail-m1 (Feb 11 2020 at 17:10, on Zulip):

As I understand expr_from_text is never fails, but output for incorrect code may be weird.

matklad (Feb 11 2020 at 17:10, on Zulip):

my bad, I indeed have done that

mikhail-m1 (Feb 11 2020 at 17:11, on Zulip):

I will check it later

matklad (Feb 11 2020 at 17:12, on Zulip):

I've checked it, it actually works great if you use the right syntax :sweat_smile:

mikhail-m1 (Feb 11 2020 at 17:18, on Zulip):

I mean I will check what is the output from expr_from_text for foo(name:expr)

matklad (Feb 11 2020 at 17:22, on Zulip):

Would you mind if I tweet about this work-in-progress today or tomorrow?

I've just tried it with the original InFile example, it works, and I am super excited!

mikhail-m1 (Feb 11 2020 at 19:32, on Zulip):

yeah, of course, feel free to share it

Last update: May 27 2020 at 22:55UTC