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

include! concat! env! OUT_DIR not auto completing in cargo workspace #5040

Closed
chinedufn opened this issue Jun 24, 2020 · 19 comments
Closed

include! concat! env! OUT_DIR not auto completing in cargo workspace #5040

chinedufn opened this issue Jun 24, 2020 · 19 comments
Labels
A-macro macro expansion S-actionable Someone could pick this issue up and work on it right now

Comments

@chinedufn
Copy link
Contributor

chinedufn commented Jun 24, 2020

Related to #4482, I've never had the OUT_DIR support work in my project.

I've created a new repository to demonstrate the issue.

https://github.com/chinedufn/rust-analyzer-issue-5040

In the screenshot below I expected crate1:: to autocomplete with HELLO, the name of the generated constant from the build.rs in the bottom left.

Instead there are only crate1::{super,self} as options for completion.

image

image

Load out dirs is checked

image

\cc @flodiebold @edwin0cheng

@bjorn3
Copy link
Member

bjorn3 commented Jun 24, 2020

Do you have "rust-analyzer.cargo.loadOutDirsFromCheck": true set?

@chinedufn
Copy link
Contributor Author

Yup! Just updated my issue comment to explicitly show that, thanks!

@lnicola

This comment has been minimized.

@chinedufn
Copy link
Contributor Author

I’m on nightly-2020-06-22

@lnicola lnicola added the A-macro macro expansion label Jun 24, 2020
@kiljacken
Copy link
Contributor

From my brief run in with how we do VFS glob matching, the issue might be that generated does not have the .rs file extension.

@chinedufn
Copy link
Contributor Author

I've updated the example repo to include .rs chinedufn/rust-analyzer-issue-5040@e159f6d

Still doesn't work unfortunately

@seunlanlege
Copy link

I also have this problem, are there any means of obtaining logs from the server so we can attach?

@seunlanlege
Copy link

Also it used to work for me, then I think I updated and it didn't work anymore

@matklad matklad added the E-unknown It's unclear if the issue is E-hard or E-easy without digging in label Jul 11, 2020
@ndarilek
Copy link

I'm experiencing this outside of a workspace and definitely have the setting enabled. Checking out this project, highlighting Node on line 29, and pressing F12 brings me to the lib.rs in the Rust gdnative crate, not to its definition.

Not sure if that is a different issue given the absence of a workspace, but figured it might be at least another data point.

Thanks.

@edwin0cheng
Copy link
Member

edwin0cheng commented Jan 13, 2021

@chinedufn I think I know what's going on in your example. Note that your example project structure is :

root
  Cargo.toml
  src/lib.rs
  crates/crate1
  crates/crate2

And the root Cargo.toml is :

[package]
name = "rust-analyzer-cargo-workspace"
version = "0.1.0"
authors = ["Chinedu Francis Nwafili <frankie.nwafili@gmail.com>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

[workspace]
members = [
	"crates/crate1",
	"crates/crate2",
]

Note that the workspace session in root Cargo.toml is redundant, because the root crate exists. if you changed to :

[package]
name = "rust-analyzer-cargo-workspace"
version = "0.1.0"
authors = ["Chinedu Francis Nwafili <frankie.nwafili@gmail.com>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
crate1 = {path="crates/crate1"}
crate2 = {path="crates/crate2"}

Then the problem you mentioned will be solved.

@edwin0cheng edwin0cheng added S-actionable Someone could pick this issue up and work on it right now and removed E-unknown It's unclear if the issue is E-hard or E-easy without digging in labels Jan 13, 2021
@edwin0cheng
Copy link
Member

OTHO, I think it is some bug in cargo-metadata for this situation or it is by design ?

@edwin0cheng
Copy link
Member

@ndarilek I think it is working now, I remember I fixed some bugs in previous weeks related on that.

@ndarilek
Copy link

ndarilek commented Jan 13, 2021 via email

@chinedufn
Copy link
Contributor Author

@edwin0cheng thanks for looking into this!

The use case for members instead of dependencies is so that the root crate does not have a bunch of dependencies that it does not need.

You could get around this by just not using the root crate so that you can give it bogus dependencies, as I think you're proposing, but then if you use something like cargo build --all you're building and linking a potentially massive root crate that you don't actually use for anything.

With that in mind I think it would be better if this worked with a crate that uses [members]. That would lead to less surprises, I think.

Is this possible? I'm not so familiar with how all of this go-to definition and completion stuff works.

Thanks!

@edwin0cheng
Copy link
Member

@ndarilek :

goto-definition is for include! macro is yet to implement , but it is another issue #3767

@chinedufn :

yeah, It definitely is a bug of RA , and the bug is related to how we treat the root crate dependencies. IIRC, we use "cargo check" on the workspace to fetch the OUTDIR (which is used for include! macro). Here is the related code :

https://github.com/rust-analyzer/rust-analyzer/blob/dab210d9b2d877ca9ed02bd7e0c8952d133965d3/crates/project_model/src/cargo_workspace.rs#L374-L405

Maybe 'cargo check' didn't check the members in this case ?

@chinedufn
Copy link
Contributor Author

@edwin0cheng would cargo check --workspace do the trick, or is there much more to it than that?

@edwin0cheng
Copy link
Member

edwin0cheng commented Jan 14, 2021

IIUC, it should do the trick, something like this will work :

cmd.args(&["check", "--message-format=json", "--manifest-path", "--workspace"]).arg(cargo_toml); 

For the reference It is the doc from cargo help check :

By default, when no package selection options are given, the packages selected depend on the selected
manifest file (based on the current working directory if --manifest-path is not given). If the manifest is
the root of a workspace then the workspaces default members are selected, otherwise only the package defined
by the manifest will be selected.

The default members of a workspace can be set explicitly with the workspace.default-members key in the root
manifest. If this is not set, a virtual workspace will include all workspace members (equivalent to passing
--workspace), and a non-virtual workspace will include only the root crate itself.

And in your case, it is a non-virtual workspace .

chinedufn added a commit to chinedufn/rust-analyzer that referenced this issue Jan 14, 2021
bors bot added a commit that referenced this issue Jan 14, 2021
7264: Use --workspace when loading extern resources r=edwin0cheng a=chinedufn

#5040 (comment)

Co-authored-by: Chinedu Francis Nwafili <frankie.nwafili@gmail.com>
@chinedufn
Copy link
Contributor Author

chinedufn commented Jan 14, 2021

Closed via #7264

ivan770 pushed a commit to ivan770/rust-analyzer that referenced this issue Jan 22, 2021
Matthias-Fauconneau pushed a commit to Matthias-Fauconneau/rust-analyzer that referenced this issue Feb 7, 2021
@ilyvion
Copy link

ilyvion commented Jul 7, 2021

I'm having an issue with a dependency that has a file that is just include!(concat!(env!("OUT_DIR"), "/timezones.rs")); (Source here) and rust-analyzer does not work with the types in this file:
image
image
etc.

I can't find any "Load out dirs" setting as mentioned in the OP. Cargo/Rust itself has no trouble using these types/functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

9 participants