loco-rs / loco

🚂 🦀 The one-person framework for Rust for side-projects and startups
https://loco.rs
Apache License 2.0
4.41k stars 175 forks source link

Rust analyzer shows errors with loco-rs #490

Closed assapir closed 1 month ago

assapir commented 7 months ago

Description

When trying to open the loco-rs in VSCode with rust-analyzer extension, this extension fails to do its job, with the following errors:

2024-03-08T11:49:25.833538Z ERROR project_model::workspace: cyclic deps: loco_rs(Idx::<CrateData>(197)) -> loco_rs(Idx::<CrateData>(197)), alternative path: loco_rs(Idx::<CrateData>(197))
2024-03-08T11:49:26.301369Z ERROR project_model::workspace: cyclic deps: loco_rs(Idx::<CrateData>(197)) -> loco_rs(Idx::<CrateData>(197)), alternative path: loco_rs(Idx::<CrateData>(197))
2024-03-08T11:49:27.030212Z ERROR project_model::workspace: cyclic deps: loco_rs(Idx::<CrateData>(197)) -> loco_rs(Idx::<CrateData>(197)), alternative path: loco_rs(Idx::<CrateData>(197))

To Reproduce

Open the project in VSCode with rust-analyzer installed on a Linux (x64)

Expected Behavior

rust-analyzer should work and give all it's expected functionality, like "go to definitions", etc.

Environment:

rust-analyzer version details (from it's logs):

INFO [08/03/2024, 13:49:25]: /home/assaf/.vscode/extensions/rust-lang.rust-analyzer-0.3.1868-linux-x64/server/rust-analyzer --version: {
  status: 0,
  signal: null,
  output: [ null, 'rust-analyzer 0.3.1868-standalone\n', '' ],
  pid: 984450,
  stdout: 'rust-analyzer 0.3.1868-standalone\n',
  stderr: ''
}

Additional Context Probably related to https://github.com/rust-lang/rust-analyzer/issues/14167

I can help debug on my machine if needed. This doesn't happen if one is only opening an internal crate like the loco-cli

jondot commented 7 months ago

Never had that one, I'm using pre-release usually, which only failed me once in 3-4 years.

rust-analyzer 0.4.1870-standalone (767d5d3ea 2024-03-05)

@kaplanelad are you using pre-release or release?

assapir commented 7 months ago

Never had that one, I'm using pre-release usually, which only failed me once in 3-4 years.

rust-analyzer 0.4.1870-standalone (767d5d3ea 2024-03-05)

@kaplanelad are you using pre-release or release?

I tried to switch to pre-release, still happens:

INFO [08/03/2024, 14:25:48]: /home/assaf/.vscode/extensions/rust-lang.rust-analyzer-0.4.1873-linux-x64/server/rust-analyzer --version: {
  status: 0,
  signal: null,
  output: [ null, 'rust-analyzer 0.4.1873-standalone\n', '' ],
  pid: 1033877,
  stdout: 'rust-analyzer 0.4.1873-standalone\n',
  stderr: ''
}

The errors are shown inside OUTPUT -> Rust Analyzer Language Server

jondot commented 7 months ago

yes, now i get it! very strange, i just upgraded everything and it might started now (but I might have missed it earlier) i'll try figuring this out

assapir commented 7 months ago

yes, now i get it! very strange, i just upgraded everything and it might started now (but I might have missed it earlier) i'll try figuring this out

From the attached discussion in the rust-analyzer repo and from my testing, this is the offensive line: https://github.com/loco-rs/loco/blob/c3597abc116603eafbcbc2e400bcb5bc586ab085/Cargo.toml#L151

jondot commented 7 months ago

Yep I got this as well. And in your experience, rust-analyzer isn't working well?

assapir commented 7 months ago

Yep I got this as well. And in your experience, rust-analyzer isn't working well?

I am not sure, I thought it didn't work as expected, but it mostly does? 🤷🏻 I don't think it's urgent in any way, for sure.

jondot commented 7 months ago

Yea, I didn't notice anything but I can now document this like so:

Rust analyzer has trouble dealing with Loco requiring itself (and maybe rightly so?), because it does not deal with these kind of cycles well.

Options to resolve:

  1. Include testing by default, which will obsolete "re-requiring" in Loco and in starters
  2. Extract all testing related code into a separate crate loco-testing and depend on that in dev-dependencies, then, enable relevant testing code in Loco based on a required feature flag
  3. Ignore this, as there is some indication that rust-analyzer can still function well with this error

I'd look at (3) more deeply now that we're aware of it. If rust-analyzer isn't performing well, I'm leaning towards (1), and then having users build "for production" without testing. The disadvantage here is to force users to explicitly say what their feature flags are for production.

@kaplanelad WDYT?

jondot commented 7 months ago

...One more option is to not add testing as default, and require devs to say cargo test --features testing all the time (maybe we can hide this pesky typing under an alias cargo loco test)

UPDATE: this issue is relevant only for Loco testing Loco. Starters work fine (as intended, requiring the testing feature from Loco)

jondot commented 7 months ago

I came up with a minimal, golf solution. nice catch @assapir , looks like this started to appear this week as we added the new query DSL