Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extract function broken the codes, add redundant slash #16867

Closed
xusd320 opened this issue Mar 18, 2024 · 12 comments
Closed

extract function broken the codes, add redundant slash #16867

xusd320 opened this issue Mar 18, 2024 · 12 comments
Labels
C-bug Category: bug

Comments

@xusd320
Copy link

xusd320 commented Mar 18, 2024

As you can see in blow gif, a redundant slash added before the end of extracted fn. With rust-analyzer 2024-02-12 version, no that redundant slash. I guess it may the related to #16618 according to the version timeline, not very sure.

iShot_2024-03-18_12 36 56

rust-analyzer version: 0.3.1850-standalone (68c506f 2024-02-19)

rustc version: 1.76.0 (07dca489a 2024-02-04)

relevant settings:

{
          ["rust-analyzer"] = {
            cachePriming = {
              enable = false,
            },
            cargo = {
              allFeatures = false,
            },
            check = {
              allTargets = false,
            },
            checkOnSave = {
              allFeatures = false,
              command = "clippy",
              extraArgs = { "--no-deps" },
            },
            completion = {
              fullFunctionSignatures = true,
            },
            diagnostics = {
              experimental = {
                enable = true,
              },
            },
            procMacro = {
              enable = true,
              ignored = {
                ["async-trait"] = { "async_trait" },
                ["napi-derive"] = { "napi" },
                ["async-recursion"] = { "async_recursion" },
              },
            },
          },
        }

repository link (if public, optional): None

code snippet to reproduce:

use tokio::runtime::Builder;
use tokio::time::{sleep, Duration};

fn main() {
    let runtime = Builder::new_multi_thread()
        .worker_threads(1)
        .enable_all()
        .build()
        .unwrap();

    let mut handles = Vec::with_capacity(10);
    for i in 0..10 {
        handles.push(runtime.spawn(my_bg_task(i)));
    }

    // Do something time-consuming while the background tasks execute.
    // std::thread::sleep(Duration::from_millis(750));
    println!("Finished time-consuming task.");

    // Wait for all of them to complete.
    for handle in handles {
        // The `spawn` method returns a `JoinHandle`. A `JoinHandle` is
        // a future, so we can wait for it using `block_on`.
        runtime.block_on(handle).unwrap();
    }
}

async fn my_bg_task(i: u64) {
    // By subtracting, the tasks with larger values of i sleep for a
    // shorter duration.
    let millis = 1000 - 50 * i;
    println!("Task {} sleeping for {} ms.", i, millis);

    sleep(Duration::from_millis(millis)).await;

    println!("Task {} stopping.", i);
}
@xusd320 xusd320 added the C-bug Category: bug label Mar 18, 2024
@roife
Copy link
Member

roife commented Mar 18, 2024

Hmm... The latest release does not seem to have this problem on my machine. It should have been fixed in May 11. Try upgrading.

@xusd320
Copy link
Author

xusd320 commented Mar 18, 2024

Hmm... The latest does not seem to have this problem on my machine.

With RA 2024-03-11 version, the problem exists. What's RA version your are using? @roife

@roife
Copy link
Member

roife commented Mar 18, 2024

rust-analyzer version: 0.3.1877-standalone (574e23e 2024-03-09)

@rami3l
Copy link
Member

rami3l commented Mar 20, 2024

I believe this is a dup of #16607.

However, I agree with @xusd320 that this problem is not completely resolved.

Coming from Neovim with rust-analyzer 0.3.1885-standalone (b6d1887bc 2024-03-17) installed, I get:

snip

The thing is that \{ is gone but \} is still there. Thus, #16618 did work, but I doubt whether

The } case is already fixed by #16475.

#16475 being a purely TypeScript update, didn't fix anything on the LSP side of the things. Or is there anything wrong with Neovim's interpretation of \}?

(@DropDemBits maybe you can provide more insights on this one?)

@xusd320
Copy link
Author

xusd320 commented Mar 20, 2024

I believe this is a dup of #16607.

However, I agree with @xusd320 that this problem is not completely resolved.

Coming from Neovim with rust-analyzer 0.3.1885-standalone (b6d1887bc 2024-03-17) installed, I get:

snip

The thing is that \{ is gone but \} is still there. Thus, #16618 did work, but I doubt whether

The } case is already fixed by #16475.

#16475 being a purely TypeScript update, didn't fix anything on the LSP side of the things. Or is there anything wrong with Neovim's interpretation of \}?

(@DropDemBits maybe you can provide more insights on this one?)

Yeah, I tried all versions after 2024-02-19, be sure the problem is not fixed.

@rami3l
Copy link
Member

rami3l commented Mar 20, 2024

Just tried again with the debug logs on the following snippet:

impl std::fmt::Display for SelfUpdateMode {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.write_str(match self {
            SelfUpdateMode::Enable => "enable",
            SelfUpdateMode::Disable => "disable",
            SelfUpdateMode::CheckOnly => "check-only",
        })
    }
}

https://github.com/rust-lang/rustup/blob/a05f8a5bbc9a028741babd18a2ff9ca197d30b3f/src/cli/self_update.rs#L135-L143

When I was making r-a generate a fn fmt() for me, I got this log:

[DEBUG][2024-03-20 13:02:32] .../vim/lsp/rpc.lua:387	"rpc.receive"	{  id = 61,  jsonrpc = "2.0",  result = {    edit = {      documentChanges = { {          edits = { {              insertTextFormat = 2,              newText = "\n    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {\n        ${0:todo!()}\n    \\}",              range = {                ["end"] = {                  character = 43,                  line = 134                },                start = {                  character = 43,                  line = 134                }              }            } },          textDocument = {            uri = "file://<SNIP>/rustup/src/cli/self_update.rs",            version = 5          }        } }    },    kind = "quickfix",    title = "Implement missing members"  }}
[DEBUG][2024-03-20 13:02:32] .../vim/lsp/rpc.lua:284	"rpc.send"	{  jsonrpc = "2.0",  method = "textDocument/didChange",  params = {    contentChanges = { {        range = {          ["end"] = {            character = 43,            line = 134          },          start = {            character = 43,            line = 134          }        },        rangeLength = 0,        text = "\n    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {\n        todo!()\n    \\}"      } },    textDocument = {      uri = "file:///<SNIP>/rustup/src/cli/self_update.rs",      version = 6    }  }}

I'm not sure where did this \\} thing come from.

@DropDemBits
Copy link
Contributor

DropDemBits commented Mar 20, 2024

#16475 being a purely TypeScript update, didn't fix anything on the LSP side of the things. Or is there anything wrong with Neovim's interpretation of \}?

Per the grammar section of the LSP Snippet Syntax page, \$, \\, and \} should be escaped as $, \ and } respectively:

Below is the EBNF (extended Backus-Naur form) for snippets. With \ (backslash), you can escape $, } and \.

So I believe this is a Neovim-side issue?

@rami3l
Copy link
Member

rami3l commented Mar 20, 2024

So I believe this is a Neovim-side issue?

@DropDemBits Thanks for the reply!

After some discussions with @roife I have the same feeling. That same RPC response above can be correctly received by VSCode so in theory there could be a problem in its parsing with Neovim. We'll try to do some more investigations.

@DropDemBits
Copy link
Contributor

Curious if you've tried the nightly version of Neovim, since it has this PR merged in which seems relevant 😁

@rami3l
Copy link
Member

rami3l commented Mar 20, 2024

@DropDemBits Thanks! We can confirm that neovim/neovim#25611 has fixed this. (I wonder why I failed to find any issue in the Neovim repo, maybe it was a keywords problem...) (Not quite, see #16867 (comment).)

Anyway, in that case I think it's okay for @xusd320 to close this issue as resolved 🤦 Sorry for all the trouble!

@xusd320
Copy link
Author

xusd320 commented Mar 20, 2024

@DropDemBits Thanks! We can confirm that neovim/neovim#25611 has fixed this. (I wonder why I failed to find any issue in the Neovim repo, maybe it was a keywords problem...)

Anyway, in that case I think it's okay for @xusd320 to close this issue as resolved 🤦 Sorry for all the trouble!

Great!Thanks for your investigatiing.

@xusd320 xusd320 closed this as completed Mar 20, 2024
@rami3l
Copy link
Member

rami3l commented Mar 20, 2024

@xusd320 I did even more research on this. My previous report on Neovim 0.10 wasn't quite precise, but I'm sure it's a problem on Neovim's side anyway, so still safe to keep it closed on r-a's side.

It's still about the experimental LSP flag server.capabilities.experimental.snippetTextEdit.
As a temporary workaround, if you're using rustaceanvim, you can set vim.g.rustaceanvim.server.capabilities.experimental.snippetTextEdit to false (mrcjkb/rustaceanvim#303 (comment)).
If you're using something else, you should be able to tweak the flag in a similar way in Lua.

bors added a commit that referenced this issue Apr 1, 2024
update: add editor/extension information to bug report template

When attempting to reproduce issues, I encounter difficulties due to differences in versions of LSP clients and editors (such as #16985, #16867, and more)

This sometimes consumes a lot of efforts from contributors to communicate the details about LSP client information. Therefore, I believe adding editor/extension information to the issue template would be helpful for problem reproduction.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

4 participants