Rewrite import insertion

[Veykril]

This is my attempt at refactoring the import insertion #3947. I hope what I created here is somewhat in line with what was requested, it wouldn't surprise me .

common_prefix is a copy from merge_imports.rs so those should be unified somewhere, try_merge_trees is also copied from there but slighly modified to take the MergeBehaviour enum into account.
MergeBehaviour should in the end become a configuration option, and the order if ImportGroup probably as well?

I'm not too familiar with the assist stuff and the like which is why I dont know what i have to do with insert_use_statement and find_insert_use_container for now.

I will most likely add more test cases in the end as well as I currently only tried to hit every path in find_insert_position.
Some of the merge tests also fail atm due to them not sorting what they insert. There is also this test case I'm not sure if we want to support it. I would assume we want to? https://github.com/rust-analyzer/rust-analyzer/pull/5935/files#diff-6923916dd8bdd2f1ab4b984adacd265fR540-R547

The entire module was rewritten so looking at the the file itself is probably better than looking at the diff.

Regarding the sub issues of #3947:

• Auto-import should respect std/extern/crate order #3301: This is fixed with the rewrite, what this implementation does is that it scans through the first occurence of groupings and picks the appropriate one out. This means the user can actually rearrange the groupings on a per file basis to their liking. If a group isnt being found it is inserted according to the ImportGroup variant order(Would be nice if this was configurable I imagine).
• Auto-import without nested imports #3831: This should be fixed with the introduced MergeBehaviour enum and it's Last variant.
• Autoimport adds import to pub use items #3946: This should also be fixed
• "Import" assist places new "use" after the last existing one even if it's burried in the middle of a program #5795: This is fixed in the sense that the grouping search picks the first group that is of the same kind as the import that is being added. So if there is a random import in the middle of the program it should only be considered if there is no group of the same kind in the file already present.
• the last point in the list I havent checked yet, tho I got the feeling that it's not gonna be too simple as that will require knowledge of whether in this example ast is a crate or the module that is already imported.

[kjeremy]

Does this fix #5913?

[Veykril]

It seems to fix it, instead of erroring it turns the import into use token::TokenKind::{self::*, self};

[matklad]

Yes, this looks exactly how I would imagine it! Regarding find_insert_use_container and insert_use_statement, I think ideally we should just remove this functions from this module. Nothing here should mention AssistContext or TextEditBuilder. But it is OK to leave them in temporary, if that eases migration.

Similar point about code duplication -- it is fine if it make the rewrite easier, removing duplication can be done in a separate PR.

Basically, I think we should merge this as soon as the tests are green!

Excellent work @Veykril !

[matklad]

make::tokens::blank_line() maybe?

[Veykril]

I'm honestly not sure how I would implement that function, I'm not too used to how rowan and the like works yet. So I'm unsure how I would fetch two newlines as a single SyntaxToken.

[matklad]

I think something like this should work:

diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs
index c2c938ad1..aac4a066f 100644
--- a/crates/syntax/src/ast/make.rs
+++ b/crates/syntax/src/ast/make.rs
@@ -339,7 +339,7 @@ pub mod tokens {
     use crate::{ast, AstNode, Parse, SourceFile, SyntaxKind::*, SyntaxToken};
 
     pub(super) static SOURCE_FILE: Lazy<Parse<SourceFile>> =
-        Lazy::new(|| SourceFile::parse("const C: <()>::Item = (1 != 1, 2 == 2, !true)\n;"));
+        Lazy::new(|| SourceFile::parse("const C: <()>::Item = (1 != 1, 2 == 2, !true)\n;\n\n"));
 
     pub fn single_space() -> SyntaxToken {
         SOURCE_FILE
@@ -379,6 +379,16 @@ pub mod tokens {
             .unwrap()
     }
 
+    pub fn blank_newline() -> SyntaxToken {
+        SOURCE_FILE
+            .tree()
+            .syntax()
+            .descendants_with_tokens()
+            .filter_map(|it| it.into_token())
+            .find(|it| it.kind() == WHITESPACE && it.text().as_str() == "\n\n")
+            .unwrap()
+    }
+
     pub struct WsBuilder(SourceFile);
 
     impl WsBuilder {

[matklad]

There is also this test case I'm not sure if we want to support it. I would assume we want to?

It's OK to not support weird cases initially, if we don't do something completely crazy.

[matklad]

For config, I don't think we should handle it in this PR necessary, but it should work like this:

• define

struct InsertUseConfig { ... }

• add it as a field to AssistConfig

• add required user-visible options in package.json and ConfigData

• thread them to AssistConfig in https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/rust-analyzer/src/config.rs#L23

[Veykril]

Almost all of the current test failures are due to the tests using check_assist which panic on an Option::unwrap in https://github.com/rust-analyzer/rust-analyzer/blob/7f79dbc84f5d69ec6f48fa8d67bdfa0525c5dd34/crates/assists/src/tests.rs#L84 which I assume comes from me not using the AssistContext/TextEditBuilder anymore.

I've also started a Zulip topic, since its probably better to talk there about these things

[matklad]

bors r+

🎉

[bors]

Build succeeded:

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