mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #13602 - lowr:fix/nameres-transitive-visibility, r=Veykril

Browse Source 
fix: check visibility of each path segment

Upon path resolution, we have not been checking if every def pointed to by each segment of the path is visible from the original module. This leads to incorrect import resolutions, in particular when one uses glob imports and names collide.

There is decent amount of changes in this PR because:
- some of our tests were not correct in terms of visibility
  - I left several basic nameres tests as-is (with expect test updated) since I thought it would be nice to ensure we don't resolve defs that are not visible.
- `fix_visibility` assist relied on `Semantics::resolve_path()`, which uses the name resolution procedure I'm fixing and wouldn't be able to "see through" the items with strict visibility with this patch

The first commit is the gist of the fix itself.

Fixes #10991
Fixes #11473
Fixes #13252
This commit is contained in:
bors 2022-11-11 12:32:21 +00:00
commit 57cc2a6e27
16 changed files with 145 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
crates/hir-def/src/nameres/collector.rs
View File

@ -212,6 +212,7 @@ impl Import {
#[derive(Debug, Eq, PartialEq)]
struct ImportDirective {
/// The module this import directive is in.
module_id: LocalModuleId,
import: Import,
status: PartialResolvedImport,
@ -963,8 +964,10 @@ impl DefCollector<'_> {
fn update(
&mut self,
// The module for which `resolutions` have been resolve
module_id: LocalModuleId,
resolutions: &[(Option<Name>, PerNs)],
// Visibility this import will have
vis: Visibility,
import_type: ImportType,
) {
@ -974,6 +977,7 @@ impl DefCollector<'_> {
fn update_recursive(
&mut self,
// The module for which `resolutions` have been resolve
module_id: LocalModuleId,
resolutions: &[(Option<Name>, PerNs)],
// All resolutions are imported with this visibility; the visibilities in

7
crates/hir-def/src/nameres/path_resolution.rs
View File

@ -73,7 +73,10 @@ impl DefMap {
pub(crate) fn resolve_visibility(
&self,
db: &dyn DefDatabase,
// module to import to
original_module: LocalModuleId,
// pub(path)
// ^^^^ this
visibility: &RawVisibility,
) -> Option<Visibility> {
let mut vis = match visibility {
@ -115,6 +118,7 @@ impl DefMap {
&self,
db: &dyn DefDatabase,
mode: ResolveMode,
// module to import to
mut original_module: LocalModuleId,
path: &ModPath,
shadow: BuiltinShadowMode,
@ -361,6 +365,9 @@ impl DefMap {
);
}
};
curr_per_ns = curr_per_ns
.filter_visibility(|vis| vis.is_visible_from_def_map(db, self, original_module));
}
ResolvePathResult::with(curr_per_ns, ReachedFixedPoint::Yes, None, Some(self.krate))

6
crates/hir-def/src/nameres/tests.rs
View File

@ -58,9 +58,9 @@ extern {
"#,
expect![[r#"
crate
E: t
E: _
S: t v
V: t v
V: _
foo: t
crate::foo
@ -307,7 +307,7 @@ pub struct FromLib;
Bar: t v
crate::foo
Bar: t v
Bar: _
FromLib: t v
"#]],
);

33
crates/hir-def/src/nameres/tests/globs.rs
View File

@ -119,7 +119,7 @@ use foo::*;
use foo::bar::*;
//- /foo/mod.rs
mod bar;
pub mod bar;
fn Foo() {};
pub struct Foo {};
@ -132,6 +132,7 @@ pub(crate) struct PubCrateStruct;
crate
Foo: t
PubCrateStruct: t v
bar: t
foo: t
crate::foo
@ -336,3 +337,33 @@ mod d {
"#]],
);
}
#[test]
fn glob_name_collision_check_visibility() {
check(
r#"
mod event {
mod serenity {
pub fn Event() {}
}
use serenity::*;
pub struct Event {}
}
use event::Event;
"#,
expect![[r#"
crate
Event: t
event: t
crate::event
Event: t v
serenity: t
crate::event::serenity
Event: v
"#]],
);
}

2
crates/hir-def/src/nameres/tests/mod_resolution.rs
View File

@ -580,7 +580,7 @@ fn module_resolution_decl_inside_inline_module_in_crate_root() {
//- /main.rs
mod foo {
#[path = "baz.rs"]
mod bar;
pub mod bar;
}
use self::foo::bar::Baz;

40
crates/hir-ty/src/tests/method_resolution.rs
View File

@ -164,16 +164,16 @@ fn infer_associated_method_with_modules() {
check_infer(
r#"
mod a {
struct A;
pub struct A;
impl A { pub fn thing() -> A { A {} }}
}
mod b {
struct B;
pub struct B;
impl B { pub fn thing() -> u32 { 99 }}
mod c {
struct C;
pub mod c {
pub struct C;
impl C { pub fn thing() -> C { C {} }}
}
}
@ -186,22 +186,22 @@ fn infer_associated_method_with_modules() {
}
"#,
expect![[r#"
55..63 '{ A {} }': A
57..61 'A {}': A
125..131 '{ 99 }': u32
127..129 '99': u32
201..209 '{ C {} }': C
203..207 'C {}': C
240..324 '{ ...g(); }': ()
250..251 'x': A
254..265 'a::A::thing': fn thing() -> A
254..267 'a::A::thing()': A
277..278 'y': u32
281..292 'b::B::thing': fn thing() -> u32
281..294 'b::B::thing()': u32
304..305 'z': C
308..319 'c::C::thing': fn thing() -> C
308..321 'c::C::thing()': C
59..67 '{ A {} }': A
61..65 'A {}': A
133..139 '{ 99 }': u32
135..137 '99': u32
217..225 '{ C {} }': C
219..223 'C {}': C
256..340 '{ ...g(); }': ()
266..267 'x': A
270..281 'a::A::thing': fn thing() -> A
270..283 'a::A::thing()': A
293..294 'y': u32
297..308 'b::B::thing': fn thing() -> u32
297..310 'b::B::thing()': u32
320..321 'z': C
324..335 'c::C::thing': fn thing() -> C
324..337 'c::C::thing()': C
"#]],
);
}

18
crates/hir-ty/src/tests/simple.rs
View File

@ -214,7 +214,7 @@ fn infer_paths() {
fn a() -> u32 { 1 }
mod b {
fn c() -> u32 { 1 }
pub fn c() -> u32 { 1 }
}
fn test() {
@ -225,13 +225,13 @@ fn test() {
expect![[r#"
14..19 '{ 1 }': u32
16..17 '1': u32
47..52 '{ 1 }': u32
49..50 '1': u32
66..90 '{ ...c(); }': ()
72..73 'a': fn a() -> u32
72..75 'a()': u32
81..85 'b::c': fn c() -> u32
81..87 'b::c()': u32
51..56 '{ 1 }': u32
53..54 '1': u32
70..94 '{ ...c(); }': ()
76..77 'a': fn a() -> u32
76..79 'a()': u32
85..89 'b::c': fn c() -> u32
85..91 'b::c()': u32
"#]],
);
}
@ -1856,7 +1856,7 @@ fn not_shadowing_module_by_primitive() {
check_types(
r#"
//- /str.rs
fn foo() -> u32 {0}
pub fn foo() -> u32 {0}
//- /main.rs
mod str;

36
crates/hir-ty/src/tests/traits.rs
View File

@ -1706,7 +1706,7 @@ fn where_clause_trait_in_scope_for_method_resolution() {
check_types(
r#"
mod foo {
trait Trait {
pub trait Trait {
fn foo(&self) -> u32 { 0 }
}
}
@ -1723,7 +1723,7 @@ fn super_trait_method_resolution() {
check_infer(
r#"
mod foo {
trait SuperTrait {
pub trait SuperTrait {
fn foo(&self) -> u32 {}
}
}
@ -1735,15 +1735,15 @@ fn test<T: Trait1, U: Trait2>(x: T, y: U) {
y.foo();
}"#,
expect![[r#"
49..53 'self': &Self
62..64 '{}': u32
181..182 'x': T
187..188 'y': U
193..222 '{ ...o(); }': ()
199..200 'x': T
199..206 'x.foo()': u32
212..213 'y': U
212..219 'y.foo()': u32
53..57 'self': &Self
66..68 '{}': u32
185..186 'x': T
191..192 'y': U
197..226 '{ ...o(); }': ()
203..204 'x': T
203..210 'x.foo()': u32
216..217 'y': U
216..223 'y.foo()': u32
"#]],
);
}
@ -1754,7 +1754,7 @@ fn super_trait_impl_trait_method_resolution() {
r#"
//- minicore: sized
mod foo {
trait SuperTrait {
pub trait SuperTrait {
fn foo(&self) -> u32 {}
}
}
@ -1764,12 +1764,12 @@ fn test(x: &impl Trait1) {
x.foo();
}"#,
expect![[r#"
49..53 'self': &Self
62..64 '{}': u32
115..116 'x': &impl Trait1
132..148 '{ ...o(); }': ()
138..139 'x': &impl Trait1
138..145 'x.foo()': u32
53..57 'self': &Self
66..68 '{}': u32
119..120 'x': &impl Trait1
136..152 '{ ...o(); }': ()
142..143 'x': &impl Trait1
142..149 'x.foo()': u32
"#]],
);
}

28
crates/ide-assists/src/handlers/add_missing_impl_members.rs
View File

@ -379,14 +379,14 @@ impl Foo for S {
r#"
mod foo {
pub struct Bar;
trait Foo { fn foo(&self, bar: Bar); }
pub trait Foo { fn foo(&self, bar: Bar); }
}
struct S;
impl foo::Foo for S { $0 }"#,
r#"
mod foo {
pub struct Bar;
trait Foo { fn foo(&self, bar: Bar); }
pub trait Foo { fn foo(&self, bar: Bar); }
}
struct S;
impl foo::Foo for S {
@ -439,14 +439,14 @@ impl bar::Foo for S {
r#"
mod foo {
pub struct Bar<T>;
trait Foo { fn foo(&self, bar: Bar<u32>); }
pub trait Foo { fn foo(&self, bar: Bar<u32>); }
}
struct S;
impl foo::Foo for S { $0 }"#,
r#"
mod foo {
pub struct Bar<T>;
trait Foo { fn foo(&self, bar: Bar<u32>); }
pub trait Foo { fn foo(&self, bar: Bar<u32>); }
}
struct S;
impl foo::Foo for S {
@ -464,14 +464,14 @@ impl foo::Foo for S {
r#"
mod foo {
pub struct Bar<T>;
trait Foo<T> { fn foo(&self, bar: Bar<T>); }
pub trait Foo<T> { fn foo(&self, bar: Bar<T>); }
}
struct S;
impl foo::Foo<u32> for S { $0 }"#,
r#"
mod foo {
pub struct Bar<T>;
trait Foo<T> { fn foo(&self, bar: Bar<T>); }
pub trait Foo<T> { fn foo(&self, bar: Bar<T>); }
}
struct S;
impl foo::Foo<u32> for S {
@ -489,7 +489,7 @@ impl foo::Foo<u32> for S {
add_missing_impl_members,
r#"
mod foo {
trait Foo<T> { fn foo(&self, bar: T); }
pub trait Foo<T> { fn foo(&self, bar: T); }
pub struct Param;
}
struct Param;
@ -497,7 +497,7 @@ struct S;
impl foo::Foo<Param> for S { $0 }"#,
r#"
mod foo {
trait Foo<T> { fn foo(&self, bar: T); }
pub trait Foo<T> { fn foo(&self, bar: T); }
pub struct Param;
}
struct Param;
@ -518,7 +518,7 @@ impl foo::Foo<Param> for S {
mod foo {
pub struct Bar<T>;
impl Bar<T> { type Assoc = u32; }
trait Foo { fn foo(&self, bar: Bar<u32>::Assoc); }
pub trait Foo { fn foo(&self, bar: Bar<u32>::Assoc); }
}
struct S;
impl foo::Foo for S { $0 }"#,
@ -526,7 +526,7 @@ impl foo::Foo for S { $0 }"#,
mod foo {
pub struct Bar<T>;
impl Bar<T> { type Assoc = u32; }
trait Foo { fn foo(&self, bar: Bar<u32>::Assoc); }
pub trait Foo { fn foo(&self, bar: Bar<u32>::Assoc); }
}
struct S;
impl foo::Foo for S {
@ -545,7 +545,7 @@ impl foo::Foo for S {
mod foo {
pub struct Bar<T>;
pub struct Baz;
trait Foo { fn foo(&self, bar: Bar<Baz>); }
pub trait Foo { fn foo(&self, bar: Bar<Baz>); }
}
struct S;
impl foo::Foo for S { $0 }"#,
@ -553,7 +553,7 @@ impl foo::Foo for S { $0 }"#,
mod foo {
pub struct Bar<T>;
pub struct Baz;
trait Foo { fn foo(&self, bar: Bar<Baz>); }
pub trait Foo { fn foo(&self, bar: Bar<Baz>); }
}
struct S;
impl foo::Foo for S {
@ -571,14 +571,14 @@ impl foo::Foo for S {
r#"
mod foo {
pub trait Fn<Args> { type Output; }
trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); }
pub trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); }
}
struct S;
impl foo::Foo for S { $0 }"#,
r#"
mod foo {
pub trait Fn<Args> { type Output; }
trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); }
pub trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); }
}
struct S;
impl foo::Foo for S {

20
crates/ide-assists/src/handlers/fix_visibility.rs
View File

@ -1,4 +1,4 @@
use hir::{db::HirDatabase, HasSource, HasVisibility, PathResolution};
use hir::{db::HirDatabase, HasSource, HasVisibility, ModuleDef, PathResolution, ScopeDef};
use ide_db::base_db::FileId;
use syntax::{
ast::{self, HasVisibility as _},
@ -18,7 +18,7 @@ use crate::{utils::vis_offset, AssistContext, AssistId, AssistKind, Assists};
// fn frobnicate() {}
// }
// fn main() {
// m::frobnicate$0() {}
// m::frobnicate$0();
// }
// ```
// ->
@ -27,7 +27,7 @@ use crate::{utils::vis_offset, AssistContext, AssistId, AssistKind, Assists};
// $0pub(crate) fn frobnicate() {}
// }
// fn main() {
// m::frobnicate() {}
// m::frobnicate();
// }
// ```
pub(crate) fn fix_visibility(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
@ -37,11 +37,15 @@ pub(crate) fn fix_visibility(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
fn add_vis_to_referenced_module_def(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let path: ast::Path = ctx.find_node_at_offset()?;
let path_res = ctx.sema.resolve_path(&path)?;
let def = match path_res {
PathResolution::Def(def) => def,
_ => return None,
};
let qualifier = path.qualifier()?;
let name_ref = path.segment()?.name_ref()?;
let qualifier_res = ctx.sema.resolve_path(&qualifier)?;
let PathResolution::Def(ModuleDef::Module(module)) = qualifier_res else { return None; };
let (_, def) = module
.scope(ctx.db(), None)
.into_iter()
.find(|(name, _)| name.to_smol_str() == name_ref.text().as_str())?;
let ScopeDef::ModuleDef(def) = def else { return None; };
let current_module = ctx.sema.scope(path.syntax())?.module();
let target_module = def.module(ctx.db())?;

12
crates/ide-assists/src/handlers/generate_enum_variant.rs
View File

@ -261,12 +261,12 @@ fn main() {
}
//- /foo.rs
enum Foo {
pub enum Foo {
Bar,
}
",
r"
enum Foo {
pub enum Foo {
Bar,
Baz,
}
@ -310,7 +310,7 @@ fn main() {
generate_enum_variant,
r"
mod m {
enum Foo {
pub enum Foo {
Bar,
}
}
@ -320,7 +320,7 @@ fn main() {
",
r"
mod m {
enum Foo {
pub enum Foo {
Bar,
Baz,
}
@ -516,10 +516,10 @@ mod foo;
use foo::Foo::Bar$0;
//- /foo.rs
enum Foo {}
pub enum Foo {}
",
r"
enum Foo {
pub enum Foo {
Bar,
}
",

4
crates/ide-assists/src/handlers/generate_function.rs
View File

@ -1324,7 +1324,7 @@ fn foo() {
generate_function,
r"
mod bar {
mod baz {}
pub mod baz {}
}
fn foo() {
@ -1333,7 +1333,7 @@ fn foo() {
",
r"
mod bar {
mod baz {
pub mod baz {
pub(crate) fn my_fn() {
${0:todo!()}
}

4
crates/ide-assists/src/tests/generated.rs
View File

@ -741,7 +741,7 @@ mod m {
fn frobnicate() {}
}
fn main() {
m::frobnicate$0() {}
m::frobnicate$0();
}
"#####,
r#####"
@ -749,7 +749,7 @@ mod m {
$0pub(crate) fn frobnicate() {}
}
fn main() {
m::frobnicate() {}
m::frobnicate();
}
"#####,
)

4
crates/ide-diagnostics/src/handlers/no_such_field.rs
View File

@ -268,12 +268,12 @@ fn main() {
foo::Foo { bar: 3, $0baz: false};
}
//- /foo.rs
struct Foo {
pub struct Foo {
bar: i32
}
"#,
r#"
struct Foo {
pub struct Foo {
bar: i32,
pub(crate) baz: bool
}

20
crates/ide-diagnostics/src/handlers/useless_braces.rs
View File

@ -71,9 +71,9 @@ use a;
use a::{c, d::e};
mod a {
mod c {}
mod d {
mod e {}
pub mod c {}
pub mod d {
pub mod e {}
}
}
"#,
@ -87,9 +87,9 @@ use a::{
};
mod a {
mod c {}
mod d {
mod e {}
pub mod c {}
pub mod d {
pub mod e {}
}
}
"#,
@ -116,11 +116,11 @@ use b;
);
check_fix(
r#"
mod a { mod c {} }
mod a { pub mod c {} }
use a::{c$0};
"#,
r#"
mod a { mod c {} }
mod a { pub mod c {} }
use a::c;
"#,
);
@ -136,11 +136,11 @@ use a;
);
check_fix(
r#"
mod a { mod c {} mod d { mod e {} } }
mod a { pub mod c {} pub mod d { pub mod e {} } }
use a::{c, d::{e$0}};
"#,
r#"
mod a { mod c {} mod d { mod e {} } }
mod a { pub mod c {} pub mod d { pub mod e {} } }
use a::{c, d::e};
"#,
);

6
crates/ide/src/goto_definition.rs
View File

@ -289,10 +289,10 @@ mod b;
enum E { X(Foo$0) }
//- /a.rs
struct Foo;
//^^^
pub struct Foo;
//^^^
//- /b.rs
struct Foo;
pub struct Foo;
"#,
);
}