diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-05-07 17:29:01 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2020-05-07 17:29:01 +0100 |
commit | fd84c31ff7f48a4cde05929869dd5810a068081a (patch) | |
tree | d662138b20b83dd129182a74c925f1471aba7b72 | |
parent | a9945137dc0e65ecb2ae8d0da4821afa80a367bd (diff) | |
parent | 210f0cbd27b0621c47e15c74bbb16ab47a642999 (diff) |
Merge #4346
4346: Fix rename of enum variant visible from module r=matklad a=montekki
Probably fixes #4237
It looks like the ref is found correctly in this case but it's visibility is not correctly determined. I took a stab at fixing that by adding an implementation of `HasVisibility` for `EnumVariant` so it works more or less the same way it does for struct fields.
In other words, the `search_range` here does not contain the ref since it's not considered visible:
https://github.com/rust-analyzer/rust-analyzer/blob/efd8e34c396f1524623a495e47111f1047cf2879/crates/ra_ide_db/src/search.rs#L209-L214
Before that I tried to populate `ItemScope` with visible enum variants but that ended up with breaking tests all over the place and also it looked illogical in the end: `ItemScope` is not populated with, say, public struct fields and the same should be true for `enum` variants.
I've added two more or less identical tests: one for the case with a struct field rename and one for enum variant rename; the test for struct should probably be removed and the names should be changed.
Co-authored-by: Fedor Sakharov <[email protected]>
-rw-r--r-- | crates/ra_ide/src/references/rename.rs | 62 | ||||
-rw-r--r-- | crates/ra_ide_db/src/defs.rs | 10 |
2 files changed, 70 insertions, 2 deletions
diff --git a/crates/ra_ide/src/references/rename.rs b/crates/ra_ide/src/references/rename.rs index 0398d53bc..2cbb82c1a 100644 --- a/crates/ra_ide/src/references/rename.rs +++ b/crates/ra_ide/src/references/rename.rs | |||
@@ -712,6 +712,68 @@ mod tests { | |||
712 | "###); | 712 | "###); |
713 | } | 713 | } |
714 | 714 | ||
715 | #[test] | ||
716 | fn test_enum_variant_from_module_1() { | ||
717 | test_rename( | ||
718 | r#" | ||
719 | mod foo { | ||
720 | pub enum Foo { | ||
721 | Bar<|>, | ||
722 | } | ||
723 | } | ||
724 | |||
725 | fn func(f: foo::Foo) { | ||
726 | match f { | ||
727 | foo::Foo::Bar => {} | ||
728 | } | ||
729 | } | ||
730 | "#, | ||
731 | "Baz", | ||
732 | r#" | ||
733 | mod foo { | ||
734 | pub enum Foo { | ||
735 | Baz, | ||
736 | } | ||
737 | } | ||
738 | |||
739 | fn func(f: foo::Foo) { | ||
740 | match f { | ||
741 | foo::Foo::Baz => {} | ||
742 | } | ||
743 | } | ||
744 | "#, | ||
745 | ); | ||
746 | } | ||
747 | |||
748 | #[test] | ||
749 | fn test_enum_variant_from_module_2() { | ||
750 | test_rename( | ||
751 | r#" | ||
752 | mod foo { | ||
753 | pub struct Foo { | ||
754 | pub bar<|>: uint, | ||
755 | } | ||
756 | } | ||
757 | |||
758 | fn foo(f: foo::Foo) { | ||
759 | let _ = f.bar; | ||
760 | } | ||
761 | "#, | ||
762 | "baz", | ||
763 | r#" | ||
764 | mod foo { | ||
765 | pub struct Foo { | ||
766 | pub baz: uint, | ||
767 | } | ||
768 | } | ||
769 | |||
770 | fn foo(f: foo::Foo) { | ||
771 | let _ = f.baz; | ||
772 | } | ||
773 | "#, | ||
774 | ); | ||
775 | } | ||
776 | |||
715 | fn test_rename(text: &str, new_name: &str, expected: &str) { | 777 | fn test_rename(text: &str, new_name: &str, expected: &str) { |
716 | let (analysis, position) = single_file_with_position(text); | 778 | let (analysis, position) = single_file_with_position(text); |
717 | let source_change = analysis.rename(position, new_name).unwrap(); | 779 | let source_change = analysis.rename(position, new_name).unwrap(); |
diff --git a/crates/ra_ide_db/src/defs.rs b/crates/ra_ide_db/src/defs.rs index 40d0e77b5..f990e3bb9 100644 --- a/crates/ra_ide_db/src/defs.rs +++ b/crates/ra_ide_db/src/defs.rs | |||
@@ -6,7 +6,7 @@ | |||
6 | // FIXME: this badly needs rename/rewrite (matklad, 2020-02-06). | 6 | // FIXME: this badly needs rename/rewrite (matklad, 2020-02-06). |
7 | 7 | ||
8 | use hir::{ | 8 | use hir::{ |
9 | Field, HasVisibility, ImplDef, Local, MacroDef, Module, ModuleDef, Name, PathResolution, | 9 | Adt, Field, HasVisibility, ImplDef, Local, MacroDef, Module, ModuleDef, Name, PathResolution, |
10 | Semantics, TypeParam, Visibility, | 10 | Semantics, TypeParam, Visibility, |
11 | }; | 11 | }; |
12 | use ra_prof::profile; | 12 | use ra_prof::profile; |
@@ -47,7 +47,13 @@ impl Definition { | |||
47 | match self { | 47 | match self { |
48 | Definition::Macro(_) => None, | 48 | Definition::Macro(_) => None, |
49 | Definition::Field(sf) => Some(sf.visibility(db)), | 49 | Definition::Field(sf) => Some(sf.visibility(db)), |
50 | Definition::ModuleDef(def) => module?.visibility_of(db, def), | 50 | Definition::ModuleDef(def) => match def { |
51 | ModuleDef::EnumVariant(id) => { | ||
52 | let parent = id.parent_enum(db); | ||
53 | module?.visibility_of(db, &ModuleDef::Adt(Adt::Enum(parent))) | ||
54 | } | ||
55 | _ => module?.visibility_of(db, def), | ||
56 | }, | ||
51 | Definition::SelfType(_) => None, | 57 | Definition::SelfType(_) => None, |
52 | Definition::Local(_) => None, | 58 | Definition::Local(_) => None, |
53 | Definition::TypeParam(_) => None, | 59 | Definition::TypeParam(_) => None, |