starkware-libs / cairo

Cairo is the first Turing-complete language for creating provable programs for general computation.
Apache License 2.0
1.54k stars 474 forks source link

bug: cairo-language-server unable to convert URI to filepath #3985

Open 0xLucqs opened 1 year ago

0xLucqs commented 1 year ago

Bug Report

Cairo version:

v2.2.0

Current behavior:

When trying to jump to the definition of a function or a trait defined with the #[generate_trait] macro the language server fails and outputs unable to convert URI to filepath: vfs://201/generate_trait.cairo

Expected behavior:

It shouldn't fail and jump to the definition.

Steps to reproduce:

Related code:

use bar::{A, DummyTrait};

fn main() {
    let a = A { b: 1 };
    a.dummy()
}
mod bar {
    #[derive(Drop)]
    struct A {
        b: u8
    }
    #[generate_trait]
    impl Dummy of DummyTrait {
        fn dummy(self: A) {}
    }
}

Now when trying to jump to the definition of a.dummy() or DummyTrait it fails

Other information:

xJonathanLEI commented 1 year ago

(All code links in this comment were taken from commit https://github.com/starkware-libs/cairo/commit/2fbb7b142c96d9007886ddacc0a10889c951cd2a)

Note that this issue only happens on non-VS-Code editors (e.g. Helix) but not VS Code. I originally thought that this is a universal problem, and thus submitted https://github.com/helix-editor/helix/pull/6000 to Helix to ignore vfs URLs altogether. However, @LucasLvy reminded me that it works in VS Code so I took a look and it does indeed work. Good news right? Not really...

Upon further debugging, it turns out that the Cairo LSP defines a custom method vfs_provide:

https://github.com/starkware-libs/cairo/blob/2fbb7b142c96d9007886ddacc0a10889c951cd2a/crates/cairo-lang-language-server/src/lib.rs#L89-L91

How the heck does VS Code know about this method then?? Well, in the extension:

https://github.com/starkware-libs/cairo/blob/2fbb7b142c96d9007886ddacc0a10889c951cd2a/vscode-cairo/src/extension.ts#L357-L363

So... basically the Cairo LSP is proprietary... Any other editors that expect a compliant LSP wouldn't work 100% with the Cairo LSP.

IMO, the ideal way to handle ephemeral files is to simply use a directory like target to actually generate the file and serve the actual path (instead of vfs) to clients.

mkaput commented 1 month ago

Writing down my proposed solution to this:

Instead of employing a custom vfs/provide method, CairoLS should lazily emit virtual files as temporary files in /tmp (follow platform conventions). These files should only be written whenever an URL for them is being generated. Implementor should pay great attention that unused files are deleted by OS (for example on system shutdown).