rust-analyzer fails to resolve imports between circular dev-dependent crates

[oliver-giersch]

rust-analyzer version: 2022-01-31
rustc version: rustc 1.59.0-beta.6 (0426998f5 2022-02-02) [doesn't really matter]

Setup of minimal reproducible example:

• workspace with 2 members: base & ext
• ext has a dependency on base, uses functionality exported by base
• base has a dev-dependency on ext, uses functionality exported by ext only in tests

Issue

RA marks any imports from base in ext as "unresolved import", but both crates compile fine, tests succeed, etc. I've attached the example crate I used to reproduce the issue as a zip file:

circular-dev-deps.zip

Loading the example crate generates two error logs entries:

[ERROR project_model::workspace] cyclic deps: ext(CrateId(11)) -> base(CrateId(10)), alternative path: base(CrateId(10)) -> ext(CrateId(11))
[ERROR project_model::workspace] cyclic deps: ext(CrateId(11)) -> base(CrateId(10)), alternative path: base(CrateId(10)) -> ext(CrateId(11))

I traced the error to this function:

https://github.com/rust-analyzer/rust-analyzer/blob/d3aa2579ccddd1876bc71b8e8bc965d64a77d3c6/crates/base_db/src/input.rs#L302-L321

It is fairly easy to add a field like is_dev to the Dependency struct and set that accordingly when generating the crate graph in project_model/workspace.rs. It should then be possible to adjust CrateGraph::find_path so that it doesn't return a path if a circular dev dependency is found, although I haven't figured out yet how to do that.

Also, I don't know what other consequences might arise later, after the circular dependency is added.

Update

When naively adjusting CrateGraph::find_path to simply return None whenever a dev-dependency is encountered, leads to the following panic trace (so a fix might not be as straight-forward as I had hoped):

thread '<unnamed>' panicked at 'Internal error, cycle detected:

crate_def_map_query(CrateId(10))
crate_def_map_query(CrateId(11))
', /home/oliver/.cargo/registry/src/github.com-1ecc6299db9ec823/salsa-0.17.0-pre.2/src/lib.rs:422:35
stack backtrace:
   0: rust_begin_unwind
             at /rustc/0426998f5fc4b95dfec57853bf4bd3cf82feb4bf/library/std/src/panicking.rs:498:5
   1: core::panicking::panic_fmt
             at /rustc/0426998f5fc4b95dfec57853bf4bd3cf82feb4bf/library/core/src/panicking.rs:107:14
   2: salsa::QueryTable<Q>::get::{{closure}}
   3: core::result::Result<T,E>::unwrap_or_else
   4: salsa::QueryTable<Q>::get
   5: <DB as hir_def::db::DefDatabase>::crate_def_map_query::__shim
   6: <DB as hir_def::db::DefDatabase>::crate_def_map_query
   7: hir_def::db::crate_def_map_wait
   8: <DB as hir_def::db::DefDatabase>::crate_def_map
   9: hir_def::nameres::collector::collect_defs
  10: hir_def::nameres::DefMap::crate_def_map_query
  11: <hir_def::db::CrateDefMapQueryQuery as salsa::plumbing::QueryFunction>::execute
  12: salsa::derived::slot::Slot<Q,MP>::read_upgrade::{{closure}}
  13: salsa::runtime::Runtime::execute_query_implementation
  14: salsa::derived::slot::Slot<Q,MP>::read_upgrade
  15: salsa::derived::slot::Slot<Q,MP>::read
  16: <salsa::derived::DerivedStorage<Q,MP> as salsa::plumbing::QueryStorageOps<Q>>::try_fetch
  17: salsa::QueryTable<Q>::try_get
  18: salsa::QueryTable<Q>::get
  19: <DB as hir_def::db::DefDatabase>::crate_def_map_query::__shim
  20: <DB as hir_def::db::DefDatabase>::crate_def_map_query
  21: hir_def::db::crate_def_map_wait
  22: <DB as hir_def::db::DefDatabase>::crate_def_map
  23: hir_def::nameres::collector::collect_defs
  24: hir_def::nameres::DefMap::crate_def_map_query
  25: <hir_def::db::CrateDefMapQueryQuery as salsa::plumbing::QueryFunction>::execute
  26: salsa::derived::slot::Slot<Q,MP>::read_upgrade::{{closure}}
  27: salsa::runtime::Runtime::execute_query_implementation
  28: salsa::derived::slot::Slot<Q,MP>::read_upgrade
  29: salsa::derived::slot::Slot<Q,MP>::read
  30: <salsa::derived::DerivedStorage<Q,MP> as salsa::plumbing::QueryStorageOps<Q>>::try_fetch
  31: salsa::QueryTable<Q>::try_get
  32: salsa::QueryTable<Q>::get
  33: <DB as hir_def::db::DefDatabase>::crate_def_map_query::__shim
  34: <DB as hir_def::db::DefDatabase>::crate_def_map_query
  35: hir_def::db::crate_def_map_wait
  36: <DB as hir_def::db::DefDatabase>::crate_def_map
  37: hir::semantics::source_to_def::SourceToDefCtx::file_to_def
  38: hir::semantics::source_to_def::SourceToDefCtx::find_container
  39: hir::semantics::SemanticsImpl::analyze_impl::{{closure}}
  40: hir::semantics::SemanticsImpl::with_ctx
  41: hir::semantics::SemanticsImpl::analyze_impl
  42: hir::semantics::SemanticsImpl::analyze_no_infer
  43: hir::semantics::SemanticsImpl::scope
  44: hir::semantics::Semantics<DB>::scope
  45: ide::syntax_highlighting::highlight
  46: ide::Analysis::highlight_range::{{closure}}
  47: ide::Analysis::with_db::{{closure}}
  48: std::panicking::try::do_call
  49: __rust_try
  50: std::panicking::try
  51: std::panic::catch_unwind
  52: salsa::Cancelled::catch
  53: ide::Analysis::with_db
  54: ide::Analysis::highlight_range
  55: rust_analyzer::handlers::handle_semantic_tokens_range
  56: rust_analyzer::dispatch::RequestDispatcher::on::{{closure}}::{{closure}}
  57: std::panicking::try::do_call
  58: __rust_try
  59: std::panicking::try
  60: std::panic::catch_unwind
  61: rust_analyzer::dispatch::RequestDispatcher::on::{{closure}}
  62: rust_analyzer::thread_pool::TaskPool<T>::spawn::{{closure}}
  63: <F as threadpool::FnBox>::call_box
  64: threadpool::spawn_in_pool::{{closure}}
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
[Error - 1:34:44 PM] Request textDocument/semanticTokens/range failed.
  Message: server panicked: Internal error, cycle detected:

crate_def_map_query(CrateId(10))
crate_def_map_query(CrateId(11))

  Code: -32603 

[oliver-giersch]

This is likely a duplicate of #9574.

[gilescope]

Get this all over the place when rust analyzer tries to understand substrate. It's really annoying that it doesn't support dev dependencies yet. The code's definitely valid rust.

Given the false positives could we label it as a warning? At the moment the ui says "workspace reload required, click to reload" all the time, but when you click reload things never get any better...

[Veykril]

Closing in favor of #14167