dotnet / vscode-csharp

Official C# support for Visual Studio Code
MIT License
2.86k stars 671 forks source link

Rename folder and class file breaks omnisharp lookup #3852

Open ghost opened 4 years ago

ghost commented 4 years ago

Issue Type: Bug

Whenever I change a folder name, immediately followed by changing a file name in that folder, OmniSharp begins to fail on lookup of that class (and lights up with "errors" anywhere that class is used).

Similar to this issue, but definitely happening on Windows, not just OSX: https://github.com/OmniSharp/omnisharp-vscode/issues/2040

Extension version: 1.22.0 VS Code version: Code 1.46.0 (a5d1cc28bb5da32ec67e86cc50f84c67cc690321, 2020-06-10T09:03:20.462Z) OS version: Windows_NT x64 10.0.18363

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz (8 x 2592)| |GPU Status|2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
protected_video_decode: unavailable_off
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off_ok
webgl: enabled
webgl2: enabled| |Load (avg)|undefined| |Memory (System)|15.88GB (6.86GB free)| |Process Argv|| |Screen Reader|no| |VM|0%|
cartermp commented 4 years ago

@scyentyfyc this should be resolved with the latest release, 1.22.1. Could you install that and report back?

ghost commented 4 years ago

It is partially fixed. I again renamed the folder, then renamed the file as previously reported, and references to the class in that file are now found \ can do the lookup, but NOW... I'm having using directive lookup problems, and here is now that manifests.

I have other files in the same folder (that was just renamed), containing classes in a namespace different from the file that was just renamed. The class in the renamed file, uses a class in that other namespace, and the using directive to that other namespace is now not resolving (unless I restart omnisharp).

ghost commented 4 years ago

This is weird. I also just noticed now, that if I only rename the folder, but don't rename the file in the folder, that the lookups to the class don't work after I rename the folder, but if I then proceed to rename the file, it "fixes" the lookups to the class. To be clear, this is a re-manifestation of the original problem, just in a slightly different way, i.e. only rename the folder. The other problem with the namespace, noted above, is a separate problem altogether. Just in case, here is the Omnisharp server startup output... pretty sure I'm up to the current version.

Starting OmniSharp server at 6/18/2020, 12:10:44 PM Target: [path to solution folder removed... this is me, not omnisharp!]

OmniSharp server started. Path: c:\Users\myusername.vscode\extensions\ ms-dotnettools.csharp-1.22.1 .omnisharp\1.35.3\OmniSharp.exe PID: 4320

    Starting OmniSharp on Windows 6.2.9200.0 (x64)
SirIntruder commented 4 years ago

I think the last update handled the "directory deleted" event, but not the "directory added" event.

So after renaming folder, documents get removed from the project model, but they don't get added under new path (until you give them specific "file changed" event, that is)

ghost commented 4 years ago

Today I got it again when I renamed a folder. OmniSharp highlighted everything red that depended on the classes inside that renamed folder. had to restart OmniSharp server

During OmniSharp server start, my file version looked like: .vscode\extensions\ms-dotnettools.csharp-1.22.1.omnisharp\1.35.3\OmniSharp.exe

SirIntruder commented 4 years ago

@savpek Could we fix this by iterating through "added" (new path after rename) directory and triggering OnFileAdded() code path for each .cs file? Only other alternative I can think of is to queue project rebuild for projects which include the new folder.

savpek commented 4 years ago

I don't know if this is releated to folder removal directly. */.* works with folders when they are added but it doesn't raise event on removed files if folder is removed.

I think this is releated to how renamed files are handled internally at omnisharp 🤔 Could be something to do with transient/non transient state after rename of file. But this seems relatively easy to reproduce so debugging through is probably easiest option.

SirIntruder commented 4 years ago

The issue happens after renaming folders, not renaming specific file. So that should raise "folder removed" event + "folder added" event? From my experience renaming folder does not raise a bunch of per-file events, but I will check.

edit - I attached debugger, and it seems o# is currently only getting one event after folder rename ("FolderDeleted"), and has no idea there are a bunch of files in the new folder. "Pinging" files in folder one-by-one with any change will restore o# awareness. I guess we should add event forwarding for "FolderAdded" and have some logic on server side fix this.

mariusmathisen commented 3 years ago

Any progress on this issue ? Since it is not fixed yet - does that mean that it works for most people ?