Completion check_stability doesn't look at the attributes on the parent modules

[Kivooeo]

Why this suggest me nightly(?) feature on a stable version

It should suggest me println! not a place!

rust-analyzer --verstion output

rust-analyzer --version
rust-analyzer 1.73.0 (cc66ad46 2023-10-03)

Previously, about two weeks ago, it suggests a println! on first place, i dont think this is correct work.

[lnicola]

Until two weeks ago we didn't suggest place! at all. But I agree, it shouldn't be the first suggestion.

[Veykril]

rust-analyzer/crates/ide-completion/src/completions/flyimport.rs

Line 267 in 11a87c9

&& ctx.check_stability(original_item.attrs(ctx.db).as_deref())

and

rust-analyzer/crates/ide-completion/src/completions/flyimport.rs

Line 309 in 11a87c9

&& ctx.check_stability(original_item.attrs(ctx.db).as_deref())

are the two places where we need to look at the stability of all segments of the import.import_path.

[lnicola]

Tentative, non-working patch at lnicola@873d34b.

[Veykril]

Actually, after having worked on #15871 just now, I think we can forego looking at the attributes entirely for this in completions again (the linked PRs changes unrelated to the idea). We know the stability of an import path when calculating it in find_path

rust-analyzer/crates/hir-def/src/find_path.rs

Line 329 in ba61766

) -> Option<(ModPath, Stability)> {

, we just aren't exposing that from find_path_inner where we map the stability away. LocatedImport's import paths being looked for here

rust-analyzer/crates/ide-db/src/imports/import_assets.rs

Lines 603 to 613 in ba61766

	if let Some(prefix_kind) = prefixed {
	module_with_candidate.find_use_path_prefixed(
	db,
	item_to_search,
	prefix_kind,
	prefer_no_std,
	prefer_prelude,
	)
	} else {
	module_with_candidate.find_use_path(db, item_to_search, prefer_no_std, prefer_prelude)
	}

[lnicola]

But my proposed change (which doesn't actually work for place! for some reason) also applies outside of flyimport. For example (this won't compile, of course):

struct S;

mod m {
    #![unstable]

    impl super::S {
        pub fn foo(&self) {}
    }
}

fn main() {
    S.<|>;

We currently suggest foo here, but not under lnicola@873d34b.

[Veykril]

Right, my idea won't cover method calls. Aside from that I don't think there really is a point in checking the parent module of anything though, since any other completion would require the thing in question to already be in scope, and hence the user should have the feature enabled anyways?

Walking up the ancestors also has the problem of re-exports through unstable modules not being recognized as unstable actually. (I think rustc itself currently fails this as well, or used to iirc)