Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

rename did not change tests #190

Closed
sbeckeriv opened this issue Mar 1, 2017 · 13 comments
Closed

rename did not change tests #190

sbeckeriv opened this issue Mar 1, 2017 · 13 comments

Comments

@sbeckeriv
Copy link

sbeckeriv commented Mar 1, 2017

in my lib.rs file

struct HamlNode {
    pub childern: Vec<HamlCode>,
    ....
}

#[cfg(test)]
mod tests {
    //#![feature(trace_macros)]
    use nom::IResult;
    use super::*;
    #[test]
    fn it_parses_haml_line_haml_tag() {
        let node = HamlNode {
            children: vec![],
        };
   ...
  }
}

I want to remove the children from the struct. Naming things is hard and I used children as a variable name in many places. I figured why not rename it something crazy and just search for that. I call rename and it changes the struct and the few places I created my objects. In my test section of the file my children were not renamed.

Thoughts? I dont remember the commit I was on. Running rename on master right now is not working for me.

Thanks I am enjoying rls.
Becker

@booyaa
Copy link
Contributor

booyaa commented Mar 15, 2017

OS: macOS Sierra 10.12.3
VS Code: 1.10.2
Rust Extension: 0.3.10
RLS commit: e24fc84

I tried to reproduce the problem use the code (which doesn't compile and I'm also assuming the children field is typo'd in the struct) and yes rename doesn't work.

Further investigation reveals there's no symbols being generated for the code (presumably you don't generate any if the code fails to build?)

no_symbols

Once I got the code to build

struct HamlNode {
    pub children: Vec<HamlNode>,
}

#[cfg(test)]
mod tests {
    //#![feature(trace_macros)]
    // use nom::IResult;
    use super::*;
    #[test]
    fn it_parses_haml_line_haml_tag() {
        let node = HamlNode { children: vec![] };
    }
}

Symbols appeared and I was able to rename children.

symbols

Conclusion: RLS must be able build the code (complete it's analysis) successfully at least once to generate symbols.

Update: I did a further test, I tried to simulate what OP did by pasting each line of code one at time letting RLS complete it's analysis and generate symbols. In this scenario I was able to rename the symbol without problem

@sbeckeriv
Copy link
Author

Sorry for the typo in my example. I will try the latest master today.

Thank you
Becker

@sbeckeriv
Copy link
Author

@booyaa thanks for you help. I went back to a simple example project and ran rename and my setup worked fine. However in my fully compiling tested code I do not see rename working at all. My full code for reference https://github.com/sbeckeriv/nom_haml .

It could be the nom macos. I will try updating my example project see if nom is causing an issue.

Thanks again.
Becker

I am using nvim on OSX 10.11.6 (15G1217) with https://github.com/autozimu/LanguageClient-neovim this plugin.
My rename command and the response per dtruss log. I have tried many different spots with no luck.

read(0x0, "Content-Length: 211\r\n\r\n{\"params\": {\"newName\": \"attr\", \"textDocument\": {\"uri\": \"file:///Users/becker/trash/haml_parser/src/lib.rs\"}, \"position\": {\"character\": 8, \"line\": 42}}, \"id\": 4, \"jsonrpc\": \"2.0\", \"method\": \"textDocument/rename\"}\0", 0x2000)		 = 234 0
...
close(0x4)		 = 0 0
write(0x1, "Content-Length: 48\r\n\r\n\0", 0x16)		 = 22 0
write(0x1, "{\"jsonrpc\":\"2.0\",\"id\":4,\"result\":{\"changes\":{}}}\0", 0x30)		 = 48 0
sigaltstack(0x700003851EC0, 0x0, 0x30)		 = 0 0
munmap(0x1070A0000, 0x20000)		 = 0 0
__disable_threadsignal(0x1, 0x20000, 0x30)		 = 0 0

I can confirm that in the same project "textDocument/definition" is working.

read(0x0, "Content-Length: 197\r\n\r\n{\"params\": {\"textDocument\": {\"uri\": \"file:///Users/becker/trash/haml_parser/src/lib.rs\"}, \"position\": {\"character\": 34, \"line\": 55}}, \"id\": 5, \"jsonrpc\": \"2.0\", \"method\": \"textDocument/definition\"}\0", 0x2000)
......
write(0x1, "Content-Length: 171\r\n\r\n\0", 0x17)		 = 23 0
write(0x1, "{\"jsonrpc\":\"2.0\",\"id\":5,\"result\":[{\"uri\":\"file:///Users/becker/trash/haml_parser/src/lib.rs\",\"range\":{\"start\":{\"line\":42,\"character\":4},\"end\":{\"line\":42,\"character\":4}}}]}\0", 0xAB) = 171 0

@booyaa
Copy link
Contributor

booyaa commented Mar 16, 2017

@sbeckeriv could you post your ~/.config/nvim/init.vim? I followed the instructions and it doesn't seem to work.

Also what symbols did you try to rename? I guessed you tried to rename tag, I was able to rename this in the rust vscode extension (including in tests). I've also tested with the latest reference rls vscode extension.

@sbeckeriv
Copy link
Author

@booyaa I really appreciate the help debugging this. This is my current config file. I do feel like setting up nvim was pretty involved.
https://gist.github.com/sbeckeriv/0f97d3397e88d856841a4e7f1c17ee4c

I installed visual studio code and the rls plugin. I dont see a running rls process running. How do I know if rls is working vs normal ide features?

I tried renaming attributes on line 43, HamlNode on line 41, string on line 52 and HtmlDisplay on line 34.

Thanks again!
Becker

@sbeckeriv
Copy link
Author

sorry to respond to myself. So in nvim I tested hover. It works in my test code but not my parser. So I can get it to work. I start nvim. I run LanguageClientStart and wait till the cpu dies down. I then delete all but my use std commands and save. I then stop the server LanguageClientStop and run LanguageClientStart. I then undo my deletes and save. Now rename and hover will working again. Noting that rename did not work for the nom macro like code_run on line 92ish.

@booyaa
Copy link
Contributor

booyaa commented Mar 17, 2017

Re: vscode extension - I'd recommend you use @KalitaAlexey 's rust extension for day to day use. The reference extension is just that a reference/example plugin. I do use it to differentiate whether the problem I have relates to RLS or the rust vscode extension.

Re: I've finally got neovim to work. Renaming most symbols works, but trying to rename the nom macro code_run did not work. The same happens in the vscode reference extension. I never thought to use hover definition, but this definitely confirms you can't edit nom macros since they're not being recogised as symbols.

named!(code_run<CodeRun>,
       chain!(
           tag!("- ") ~
           data: many1!(anychar),
           || CodeRun::from(data.into_iter().collect::<String>())
           )
      ); 

I'll do some more digging as to why the nom macros aren't being recognise as macros.

@nrc
Copy link
Member

nrc commented Mar 19, 2017

You won't be able to rename macros yet - the RLS is not able to recognise macros properly (in terms of defs/refs) so we can't rename. Theoretically, there is support for this in the compiler, but it seems to have regressed. I think this should be fixed later this year.

@sbeckeriv
Copy link
Author

@booyaa Thanks again. It must be my setup. When I start nvim then start rls rename and hover do not work at all. I have found that if I comment out everything after this line https://github.com/sbeckeriv/nom_haml/blob/master/src/lib.rs#L171 that the first load will work fine. I am going to try and remove the attributes_list from my code and see if it starts working on first load.

my rust and rls:
rustc 1.17.0-nightly (be76056 2017-02-28) and rls e24fc84

@nrc is macro rename related to this ticket #216 ? should I close, rename or open a new tracking ticket for macros and renaming?

Thanks!
Becker

@nrc
Copy link
Member

nrc commented Mar 19, 2017

@sbeckeriv jump to def for macros and thus rename for macros is covered in #142, #216 is definitely an issue for this kind of thing too. Between the two, I think we have renaming macros covered, so we don't need to open another issue. It is probably worth tracking your issee with nom_haml in particular. Either by opening a new issue or re-purposing this one.

@sbeckeriv
Copy link
Author

I have found that these lines of code prevent me from using rls in my setup https://github.com/sbeckeriv/nom_haml/blob/51eda345c122282be54156826946cf3ce74cac62/src/lib.rs#L176-L179 I did this by commenting out lines as I went down the file. The project still compiles but when I start rls from nvim it does not respond to hover or rename calls.

I rewrote attribute_hash_key_value with no luck. I have looked at the expanded macro code and I got scared. It looks like my current rustfmt as does not know how to format that line of code as well.

rustc 1.17.0-nightly (be76056 2017-02-28)
rls commit e24fc84

I will update to the nightly rust and try again. If no one else has the same problem I will just close the issue. I am not sure how to debug from here.

Thanks again
Becker

@nrc
Copy link
Member

nrc commented Mar 23, 2017

@sbeckeriv a good next step to debug is to try and reproduce outside the RLS. Try compiling your project with RUSTFLAGS="-Zsave-analysis" cargo check and see if there is any unusual output. If so there is a compiler bug and the output should indicate what. If not, then there is a bug in the RLS. Examining the JSON file produced might give some insight into what is going wrong, but it can be tricky to read.

@sbeckeriv
Copy link
Author

Remove target and target-test folders, updated to rustc 1.17.0-nightly (8c4f2c6 2017-03-22) and rls 40da310.

RLS works on first run now. Unfortunately I do not know which of these fixed the issue.

Thanks again for the help.
Becker

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants