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

goto-definition for include*!(expr), where expr is not a string literal #15836

Open
cyqsimon opened this issue Nov 4, 2023 · 9 comments
Open
Labels
A-vfs C-feature Category: feature request

Comments

@cyqsimon
Copy link

cyqsimon commented Nov 4, 2023

Related: #3767, #5871,#9149.

At the moment, rust-analyzer already supports goto-definition when including a file with a string literal. However the following case is not addressed:

// from https://docs.rs/phf_codegen/0.11.2/phf_codegen/#examples
include!(concat!(env!("OUT_DIR"), "/codegen.rs"));

Not having support for this case is especially inconvenient because OUT_DIR may change for each build, and finding the correct file in target/$PROFILE/build/$CRATE_NAME-$HASH/out is not the easiest thing in the world.

Right now I deliberately keep an unused item in the generated file, so that rust-analyzer marks it as containing a warning, allowing me to find it quickly in the file tree. Obviously this is a terrible hack and won't work for non Rust source files.

@cyqsimon cyqsimon added the C-feature Category: feature request label Nov 4, 2023
@wasd96040501
Copy link
Contributor

Hi @cyqsimon I think this feature is already supported.
Here is a gdnative example.

20231108-171058.mp4

@lnicola
Copy link
Member

lnicola commented Nov 8, 2023

The phf example also works for me:

image

Did you change any settings?

@cyqsimon
Copy link
Author

cyqsimon commented Nov 8, 2023

Ah yes thanks for showing this. I realised this is something I could do soon after I posted this issue too, but I don't think it fully solves the issue.

I believe this to be a workaround rather than actual support. It is reliant on the goto-definition on the included file content, rather than that on the include* macro itself. This means that it only works for Rust source code (and other formats rust-analyzer supports, if any). If for example, codegen generates a lua script that is included using include_str!, this method won't work because there is no "goto-definition" on the content.

I probably didn't explain myself with the best clarity. Please don't hesitate to ask for clarifications.

@lnicola
Copy link
Member

lnicola commented Nov 8, 2023

Do you want Go-to-definition on the include! to open the file? Does include!("file.rs") (with a string literal) behave the way you expect it to, or is it just easier to handle because you can copy-paste the file name?

Because you initially wrote about diagnostics, so that's clearly include!, not include_str!.

@cyqsimon
Copy link
Author

cyqsimon commented Nov 8, 2023

Do you want Go-to-definition on the include! to open the file?

Yes that would be ideal.

Does include!("file.rs") (with a string literal) behave the way you expect it to, or is it just easier to handle because you can copy-paste the file name?

I think currently include*!("literal") already supports direct jump to the file. Yes this is what I expected.

Because you initially wrote about diagnostics, so that's clearly include!, not include_str!.

I was thinking more about both. Because I saw in #9149 that the goto-definition impl for literals is shared across all include* macros.


Also I took a look at #9149. It seems like this is the only bit of code that needs changing:

let token = ast::String::cast(token)?;
let path = token.value()?.into_owned();

I can take a shot at this over the weekend.

@lnicola
Copy link
Member

lnicola commented Nov 8, 2023

I think currently include*!("literal") already supports direct jump to the file. Yes this is what I expected.

Well, not for me.. It works on include!("a.rs"), but not on include!("../build.rs").

@cyqsimon
Copy link
Author

cyqsimon commented Nov 8, 2023

Huh, curious. Maybe (I'm guessing here) it's because the current path resolution is too naive?

@Veykril
Copy link
Member

Veykril commented Nov 10, 2023

This is because of how our VFS handles source roots probably

@tamird
Copy link

tamird commented Oct 28, 2024

Hi folks, how can I help drive this forward? I am currently seeing this in linux where my source directory is ~/src/linux and the build directory is ~/src/linux/.kunit. rust-project.json is in the build directory, and vscode workspace config contains:

		"rust-analyzer.linkedProjects": [
			".kunit/rust-project.json",
		],

The errors are anchored on
https://github.com/torvalds/linux/blob/81983758430957d9a5cb3333fe324fd70cf63e7e/rust/bindings/lib.rs#L32
and
https://github.com/torvalds/linux/blob/81983758430957d9a5cb3333fe324fd70cf63e7e/rust/bindings/lib.rs#L45
and they are:

failed to load file `/Users/tamird/src/linux/.kunit/rust/bindings/bindings_generated.rs`
failed to load file `/Users/tamird/src/linux/.kunit/rust/bindings/bindings_helpers_generated.rs`

however those files clearly exist:

tamird@Tamirs-MBP linux % ls /Users/tamird/src/linux/.kunit/rust/bindings/bindings{,_helpers}_generated.rs
-rw-r--r--  1 tamird  staff   2.2M Oct 28 10:41 /Users/tamird/src/linux/.kunit/rust/bindings/bindings_generated.rs
-rw-r--r--  1 tamird  staff   7.3K Oct 28 10:41 /Users/tamird/src/linux/.kunit/rust/bindings/bindings_helpers_generated.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-vfs C-feature Category: feature request
Projects
None yet
Development

No branches or pull requests

5 participants