eclipse-theia / theia

Eclipse Theia is a cloud & desktop IDE framework implemented in TypeScript.
http://theia-ide.org
Eclipse Public License 2.0
19.29k stars 2.45k forks source link

Fix data race in problem view tree #13841

Closed msujew closed 1 week ago

msujew commented 1 week ago

What it does

Closes https://github.com/eclipse-theia/theia/issues/13750 Closes https://github.com/eclipse-theia/theia/issues/13835

Whenever we encounter a new diagnostics collection for a given file (i.e. the diagnostics have been updated/replaced), we now simply replace the still waiting collection in the map. This ensures that the problem view tree stays consistent and outdated diagnostics get correctly replaced.

How to test

Run the test steps from https://github.com/eclipse-theia/theia/issues/13835 and assert that everything works as expected.

Review checklist

Reminder for reviewers

tsmaeder commented 1 week ago

@msujew When I open the directory with the rust test code ("Open Folder"), I get an error in the browser log (Electron)

logger-protocol.ts:117 2024-06-25T09:11:07.994Z root ERROR TypeError: Cannot set property root of #<ProblemTree> which has only a getter
    at new MarkerTree (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/packages_markers_lib_browser_problem_problem-widget_js.js:104:19)
    at new ProblemTree (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/packages_markers_lib_browser_problem_problem-widget_js.js:442:9)
    at createInstanceWithInjections (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256717:20)
    at _createInstance (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256707:22)
    at resolveInstance (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256802:18)
    at _getResolvedFromBinding (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256920:85)
    at file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256938:22
    at _resolveInScope (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256932:14)
    at _resolveBinding (file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256937:12)
    at file:///C:/Users/thomas/code/theia/examples/electron/lib/frontend/bundle.js:256899:20
tsmaeder commented 1 week ago

Playwright tests have the same problem.

msujew commented 1 week ago

@tsmaeder Yep, sorry. Wanted to be too smart about type checking, which led to surprising runtime errors. Weird that TypeScript didn't catch that actually.

tsmaeder commented 1 week ago

@msujew I get an error dialog when starting up. Is this relevent?

Unfortunately we don't ship binaries for your platform yet. You need to manually clone the rust-analyzer repository and run cargo xtask install --server to build the language server from sources. If you feel that your platform should be supported, please create an issue about that [here](https://github.com/rust-lang/rust-analyzer/issues) and we will consider it.
msujew commented 1 week ago

@tsmaeder This should be fixed with https://github.com/eclipse-theia/theia/pull/13825, which isn't included in this PR yet. Let me quickly rebase this. Alternatively you can just manually download the correct version for your OS from open-vsx.

tsmaeder commented 1 week ago

@msujew the rust language server still complains about some toolchain not being setup correctly:

2024-06-25T13:31:29.769174Z ERROR rust_analyzer::main_loop: FetchWorkspaceError:
rust-analyzer failed to load workspace: Failed to load the project at C:\Users\thomas\Downloads\test1\test1\Cargo.toml: Failed to query rust toolchain version at C:\Users\thomas\Downloads\test1\test1, is your toolchain setup correctly?: "cargo" "--version" failed: program not found

Is there some additional setup required?

msujew commented 1 week ago

@tsmaeder The rust analyzer requires the cargo CLI (the rust build tools) to work correctly. See https://doc.rust-lang.org/cargo/getting-started/installation.html with instructions on how to install on Windows.