grammers icon indicating copy to clipboard operation
grammers copied to clipboard

Make rust-analyzer work with generated code

Open quetz opened this issue 3 years ago • 5 comments

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?

quetz avatar Jul 20 '22 04:07 quetz

(Conversation split off #121.)

Regarding 1, re-stating what I said in https://github.com/Lonami/grammers/pull/121#issuecomment-1186997095:

[...] 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.

Lonami avatar Jul 20 '22 06:07 Lonami

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.

mkpankov avatar Jul 20 '22 06:07 mkpankov

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.

Lonami avatar Jul 20 '22 12:07 Lonami

  • 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 https://github.com/rust-lang/rust-analyzer/issues/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.

quetz avatar Jul 20 '22 15:07 quetz

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 avatar Jul 20 '22 19:07 Lonami