Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make rust-analyzer work with generated code #122

Closed
quetz opened this issue Jul 20, 2022 · 6 comments
Closed

Make rust-analyzer work with generated code #122

quetz opened this issue Jul 20, 2022 · 6 comments

Comments

@quetz
Copy link
Contributor

quetz commented Jul 20, 2022

Right now .tl files are compiled to generated.rs that is put into cargo's OUT_DIR and then included from source code.
This breaks rust-analyzer's go-to-definition feature (as it jumps to file containing include! macro).
Proposed fixes:

  1. Make build script put generated.rs file directly into src/ (that can cause double-compilation and considered dirty practice)
  2. Produce generated.rs file outside of build process, put it into src/ (and add to git).

Personally I'd go with 2, as I see no real downsides (okay, more under version control but I'm sure git will manage).

Is there any better way?

@Lonami
Copy link
Owner

Lonami commented Jul 20, 2022

(Conversation split off #121.)

Regarding 1, re-stating what I said in #121 (comment):

[...] it's intended that build scripts generate files into $OUT_DIR and then they're included in the normal Rust source via:

include!(concat!(env!("OUT_DIR"), "/foo.rs"));]

...and the Cargo Book (emphasis mine):

Build scripts may save any output files or intermediate artifacts in the directory specified in the OUT_DIR environment variable. Scripts should not modify any files outside of that directory.

...seem to hint that we should not do that. Personally I agree that it's a bit dirty for a build script to write to the filesystem other than the controlled OUT_DIR location (even if it's still within the repository and we're not doing anything evil). I would love it if there was another way where the generated code could live in OUT_DIR but still be accessible by RA.

I don't feel too comfortable writing outside of OUT_DIR, and I'd like to avoid the bloat of comitting generated code to git.

Regarding 2, not only git bloat bothers me (although we're not committing a binary file, it's still a text file over 2MB which compresses to zip to 200KB, and it changes fairly regularly), but also the fact that, if some content can be generated from a versioned source, what should be versioned is said source and not the content, or it will very likely go out of sync eventually.


Actually, perhaps the reason I didn't do this was because 1. I don't want it committed to git and 2. cargo publish won't work if the staging area isn't clean (I don't clearly remember but this might've been it).

I wonder if it would be possible to teach RA at all about the include!() and open that file instead. This would be the ideal scenario.

@mkpankov
Copy link
Contributor

As a library user, I'm for 2.

We're reading its source and looking for definitions far more frequently than we update the schema file. I'd commit it right away and be done with it.

@Lonami
Copy link
Owner

Lonami commented Jul 20, 2022

There are some other experiences against this change in https://t.me/gramme_rs/14807.

  • The source file is still on disk if you need it (RA should let you navigate to the include!'d file directly).
  • cargo offers facilities to support this kind of code generation (OUT_DIR), so it makes sense to continue to use them. This way it will never get out of sync and can be tested.
  • "diff bloat is a real thing", and not checking in the generated code would get annoying when moving through history.

While this requires two windows open, I'd also recommend building the documentation offline and having it open to quickly navigate through the types.

@quetz
Copy link
Contributor Author

quetz commented Jul 20, 2022

  • The source file is still on disk if you need it (RA should let you navigate to the include!'d file directly).

File is there, but go-to-definition does not work. And instead of single click user has to search in 2.5mb text file.

  • cargo offers facilities to support this kind of code generation (OUT_DIR), so it makes sense to continue to use them. This way it will never get out of sync and can be tested.

YMMV, but for me working RA far outweights any inconveniences when updating layers (and that is done far less frequently than jumping to various structs in generated code).

  • "diff bloat is a real thing", and not checking in the generated code would get annoying when moving through history.

It is real, but this bloat is pretty local (one file) and can be ignored when moving through history.

While this requires two windows open, I'd also recommend building the documentation offline and having it open to quickly navigate through the types.

Documentation and working RA are separate issues. Having both is better 😄

There's an open issue in RA to support this case rust-lang/rust-analyzer#3767 but it's open for two years now and looks like it requires some deep changes in RA structures. Far more difficult than putting generated code in a place where RA can see it.

@Lonami
Copy link
Owner

Lonami commented Jul 20, 2022

File is there, but go-to-definition does not work. And instead of single click user has to search in 2.5mb text file.

What I mean is, "go to definition" on the include! path should open that generated file. The file size should not be a problem (whether it's in src/ or not, the file size is the same), and a text search should be quick.

Sorry but I'm still not sold on the idea, mainly because once it's done "there's no turning back" (the file will forever remain in git history, unless we do certain shenanigans I would rather not).

@Lonami
Copy link
Owner

Lonami commented Apr 27, 2024

I will close the issue as I'm still not willing to commit such a large blob of generated code to the repository. For me go to definition works in the generated code, but perhaps that's because I develop directly on the library (and maybe only path or git dependencies work, not from crates.io? I'm not sure.)

@Lonami Lonami closed this as completed Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants