Skip to content
This repository has been archived by the owner on Nov 18, 2022. It is now read-only.

Refactoring Rename strange behaviour #244

Open
boozook opened this issue Feb 1, 2018 · 18 comments
Open

Refactoring Rename strange behaviour #244

boozook opened this issue Feb 1, 2018 · 18 comments
Milestone

Comments

@boozook
Copy link

boozook commented Feb 1, 2018

Rename works on saved buffer instead the current buffer, I think.

vsc-rls rename-issue mov opt

Steps to reproduce:

  • open any .rs in the vscode and paste following code:
  • let foo = ();
    println!("12345 {:?}", foo);
  • save
  • delete 12345
  • rename foo to xx
  • you'll get an unexpected result: foo);xx because you deleted 5 symbols and length of foo); is five too ;)

versions:

macOS: 10.12.6
vscode: 1.19.3 x64
rust-lang.rust: 0.3.2
rustup:
    default host: x86_64-apple-darwin
    toolchain: nightly-x86_64-apple-darwin
rls: rls-preview 0.124.0-nightly (299a6a9 2017-12-25)
@nrc nrc added the bug label Mar 29, 2018
@duriantang
Copy link
Contributor

os: fedora 27
vscode: 1.22.2
rustc: 1.25.0
rls-preview: 0.125.1-stable
rls-vscode: 0.4.1

rename_bug

I can't reproduce this bug, maybe it fixed.

@suhr
Copy link

suhr commented Jun 14, 2018

Still occurs on rls-preview 0.128.0-nightly.

@nrc nrc added the P-High label Jun 18, 2018
@doxxx
Copy link

doxxx commented Jul 17, 2018

I've seen a similar bug, but I made sure that my files were saved before performing the rename, so I suspect that it's in RLS.

Basically, I had something like this:

#[derive(Debug, Serialize, Deserialize)]
pub(crate) enum JobStatus {
    Queued,
    Running,
    Completed,
    Failed,
}

pub(crate) fn new_job_instance(state: &AppState, job: &JobDefinition) -> Result<String, SubmitJobError> {
    if pipeline_exists(state, &job.pipeline)? {
        insert_job_instance(state, job)
    } else {
        Err(SubmitJobError::UnknownPipelineError {
            name: job.pipeline.clone(),
        })
    }
}

When I renamed SubmitJobError to NewJobInstanceError, the return type in the function signature was mangled:

pub(crate) fn new_job_instance(state: &AppState, job: &JobDefinition) -> Result<StNewJobInstanceErrorbError> {
    if pipeline_exists(state, &job.pipeline)? {
        insert_job_instance(state, job)
    } else {
        Err(SubmitJobError::UnknownPipelineError {
            name: job.pipeline.clone(),
        })
    }
}

Notice that it also didn't rename references to variants of the enum.

@nrc nrc added this to the 1.0 milestone Jul 22, 2018
@nrc
Copy link
Member

nrc commented Jul 22, 2018

@suhr or @doxxx Can you reproduce this reliably or does it only sometimes happen? @doxx was the RLS still indexing (i.e., spinner was spinning) when you did the change?

Notice that it also didn't rename references to variants of the enum.

This is expected - the RLS doesn't know anything about the prefixes of paths (e.g., foo::bar in foo::bar::baz).

@suhr
Copy link

suhr commented Jul 22, 2018

Can you reproduce this reliably or does it only sometimes happen?

It happens sometimes.

@doxxx
Copy link

doxxx commented Jul 22, 2018

I was able to reproduce it multiple times for that specific case I described, as well as a few other cases. I'm fairly certain that the RLS was not indexing at the time.

@doxxx
Copy link

doxxx commented Jul 22, 2018

This is expected - the RLS doesn't know anything about the prefixes of paths (e.g., foo::bar in foo::bar::baz).

Is this rust-dev-tools/rls-analysis#109?

@nrc
Copy link
Member

nrc commented Jul 31, 2018

Is this rust-dev-tools/rls-analysis#109?

yes

@nrc
Copy link
Member

nrc commented Oct 31, 2018

@fzzr- @doxxx I could not reproduce this, is it still happening?

@doxxx
Copy link

doxxx commented Oct 31, 2018

@nrc

Renaming an enum now works and renames all usages of that enum including usages of its variants. However, I can only rename some enums and not others. The rename action simply has no effect on the source code with no errors reported. So far the pattern appears to be that for a library project I can rename enums defined in lib.rs but not in any other modules in the same crate. Additionally, I cannot rename functions or traits anywhere in the project. I can rename structs anywhere in the project.

I tested this with rust stable 1.30.0 and nightly 1.31.0 (1cf82fd9c 2018-10-30) installed. My VSCode was configured with rust-client.channel set to nightly. My project uses stable rust.

I also tried modifying my VSCode settings to remove the rust-client.channel override, reloaded the VSCode window and accepted the "RLS not found, install?" prompt. I wasn't expecting that prompt because I had rls-preview installed for both stable and nightly... However, it still exhibits the same rename behavior.

@doxxx
Copy link

doxxx commented Oct 31, 2018

After restarting VSCode with the channel overridden to nightly again, rename is not working anywhere in my project for any kind of symbol.

@doxxx
Copy link

doxxx commented Oct 31, 2018

I restarted VSCode with the env var RLS_LOG=debug set. I get RLS debug logging in the output panel now. After the project has loaded and RLS has finished initializing, attempting to rename a symbol only logs the following:

 INFO 2018-10-31T13:29:52Z: rls_analysis: find_all_refs: 0.000020389s

If I run RLS in CLI mode and try the rename there, I get the following:

> document src/lib.rs
2: [
    SymbolInformation {
        name: "",
        kind: Module,
        location: Location {
            uri: "file:///C:/Users/gordon.tyler/dev/rust/glia-rs/src/lib.rs",
            range: Range {
                start: Position {
                    line: 0,
                    character: 0
                },
                end: Position {
                    line: 13,
                    character: 1
                }
            }
        },
        container_name: None
    },
    SymbolInformation {
        name: "Bar",
        kind: EnumMember,
        location: Location {
            uri: "file:///C:/Users/gordon.tyler/dev/rust/glia-rs/src/lib.rs",
            range: Range {
                start: Position {
                    line: 9,
                    character: 4
                },
                end: Position {
                    line: 9,
                    character: 7
                }
            }
        },
        container_name: Some(
            "Foo"
        )
    },
    SymbolInformation {
        name: "Foo",
        kind: Enum,
        location: Location {
            uri: "file:///C:/Users/gordon.tyler/dev/rust/glia-rs/src/lib.rs",
            range: Range {
                start: Position {
                    line: 8,
                    character: 5
                },
                end: Position {
                    line: 8,
                    character: 8
                }
            }
        },
        container_name: Some(
            ""
        )
    },
    SymbolInformation {
        name: "Stuff",
        kind: Struct,
        location: Location {
            uri: "file:///C:/Users/gordon.tyler/dev/rust/glia-rs/src/lib.rs",
            range: Range {
                start: Position {
                    line: 12,
                    character: 7
                },
                end: Position {
                    line: 12,
                    character: 12
                }
            }
        },
        container_name: Some(
            ""
        )
    }
]
> rename src/lib.rs 12 7 Stuff2
 INFO 2018-10-31T13:35:37Z: rls_analysis: find_all_refs: 0.000006418s
3: WorkspaceEdit {
    changes: Some(
        {}
    ),
    document_changes: None
}

@doxxx
Copy link

doxxx commented Oct 31, 2018

Correction: Renaming does appear to be working for structs sometimes, but there appears to be some condition where it fails. I have not been able to determine what these conditions are yet.

@doxxx
Copy link

doxxx commented Oct 31, 2018

So it looks like I cannot rename a struct if it has no impl block or functions which use it. Once I add either an impl block or a function referencing it, renaming works for that struct.

Enums are also affected.

@nrc
Copy link
Member

nrc commented Nov 1, 2018

Currently the RLS is a bit broken and missing from nightly. Hopefully we'll have a new version tomorrow. On master I've added more messaging which should make this easier to debug.

@doxxx
Copy link

doxxx commented Nov 9, 2018

@nrc I updated to the latest rust nightly this morning, and renames are still not working in all cases. I tried setting RULST_LOG=debug but I don't see any additional messages in the RLS log to indicate why the renames are failing.

Checking the actual versions though it looks like RLS is still old:

>  rustup run nightly rustc --version
rustc 1.32.0-nightly (653da4fd0 2018-11-08)
>  rustup run nightly rls --version
rls-preview 0.130.5 (1c755ef 2018-10-20)

What's up with that?

@nrc
Copy link
Member

nrc commented Nov 11, 2018

There's a few odd behaviours around renames, I'm working on a fix...

@rcuhljr
Copy link

rcuhljr commented Dec 17, 2018

Just throwing in my two cents that I saw some strange behavior, it was reproducible in that I could ctrl+z undo the refactor and try and apply it again and it would generate the same erroneous result.

Nothing indexing, changing the name on a function in a struct and an unrelated function at the bottom of the file gets for line in raw_data.iter() { turned into for line in raw_data.iter(newFunctionName.

Xanewok pushed a commit to Xanewok/rls-vscode that referenced this issue Mar 27, 2019
Add sysroot configuration to rls.toml
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants