Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #16275 - davidsemakula:ast-path-segments, r=Veykril
fix: Fix `ast::Path::segments` implementation calling `ast::Path::segments` on a qualifier currently returns all the segments of the top path instead of just the segments of the qualifier. The issue can be summarized by the simple failing test below: ```rust #[test] fn path_segments() { //use ra_ap_syntax::ast; let path: ast::Path = ...; // e.g. `ast::Path` for "foo::bar::item". let path_segments: Vec<_> = path.segments().collect(); let qualifier_segments: Vec<_> = path.qualifier().unwrap().segments().collect(); assert_eq!(path_segments.len(), qualifier_segments.len() + 1); // Fails because `LHS = RHS`. } ``` This PR: - Fixes the implementation of `ast::Path::segments` - Fixes `ast::Path::segments` callers that either implicitly relied on behavior of previous implementation or exhibited other "wrong" behavior directly related to the result of `ast::Path::segments` (all callers have been reviewed, only one required modification) - Removes unnecessary (and now unused) `ast::Path::segments` alternatives
bors 2024-01-09
parent bffafc4 · parent 89d6b01 · commit da6f7e2
-rw-r--r--crates/ide-assists/src/handlers/generate_constant.rs16
-rw-r--r--crates/ide-db/src/imports/import_assets.rs5
-rw-r--r--crates/syntax/src/ast/node_ext.rs20
3 files changed, 27 insertions, 14 deletions
diff --git a/crates/ide-assists/src/handlers/generate_constant.rs b/crates/ide-assists/src/handlers/generate_constant.rs
index a4e8e7388f..8b8c6ceee9 100644
--- a/crates/ide-assists/src/handlers/generate_constant.rs
+++ b/crates/ide-assists/src/handlers/generate_constant.rs
@@ -50,6 +50,10 @@ pub(crate) fn generate_constant(acc: &mut Assists, ctx: &AssistContext<'_>) -> O
ty.original().display_source_code(ctx.db(), constant_module.into(), false).ok()?;
let target = statement.syntax().parent()?.text_range();
let path = constant_token.syntax().ancestors().find_map(ast::Path::cast)?;
+ if path.parent_path().is_some() {
+ cov_mark::hit!(not_last_path_segment);
+ return None;
+ }
let name_refs = path.segments().map(|s| s.name_ref());
let mut outer_exists = false;
@@ -253,4 +257,16 @@ fn bar() -> i32 {
}"#,
);
}
+
+ #[test]
+ fn test_wont_apply_when_not_last_path_segment() {
+ cov_mark::check!(not_last_path_segment);
+ check_assist_not_applicable(
+ generate_constant,
+ r#"mod foo {}
+fn bar() -> i32 {
+ foo::A_CON$0STANT::invalid_segment
+}"#,
+ );
+ }
}
diff --git a/crates/ide-db/src/imports/import_assets.rs b/crates/ide-db/src/imports/import_assets.rs
index 652968d808..b834f517d4 100644
--- a/crates/ide-db/src/imports/import_assets.rs
+++ b/crates/ide-db/src/imports/import_assets.rs
@@ -681,11 +681,10 @@ fn path_import_candidate(
Some(qualifier) => match sema.resolve_path(&qualifier) {
None => {
if qualifier.first_qualifier().map_or(true, |it| sema.resolve_path(&it).is_none()) {
- let mut qualifier = qualifier
- .segments_of_this_path_only_rev()
+ let qualifier = qualifier
+ .segments()
.map(|seg| seg.name_ref().map(|name| SmolStr::new(name.text())))
.collect::<Option<Vec<_>>>()?;
- qualifier.reverse();
ImportCandidate::Path(PathImportCandidate { qualifier: Some(qualifier), name })
} else {
return None;
diff --git a/crates/syntax/src/ast/node_ext.rs b/crates/syntax/src/ast/node_ext.rs
index 8618018c0b..1c6157de55 100644
--- a/crates/syntax/src/ast/node_ext.rs
+++ b/crates/syntax/src/ast/node_ext.rs
@@ -285,14 +285,16 @@ impl ast::Path {
self.first_qualifier_or_self().segment()
}
- // FIXME: Check usages of Self::segments, they might be wrong because of the logic of the bloew function
- pub fn segments_of_this_path_only_rev(&self) -> impl Iterator<Item = ast::PathSegment> + Clone {
- self.qualifiers_and_self().filter_map(|it| it.segment())
- }
-
pub fn segments(&self) -> impl Iterator<Item = ast::PathSegment> + Clone {
- successors(self.first_segment(), |p| {
- p.parent_path().parent_path().and_then(|p| p.segment())
+ let path_range = self.syntax().text_range();
+ successors(self.first_segment(), move |p| {
+ p.parent_path().parent_path().and_then(|p| {
+ if path_range.contains_range(p.syntax().text_range()) {
+ p.segment()
+ } else {
+ None
+ }
+ })
})
}
@@ -300,10 +302,6 @@ impl ast::Path {
successors(self.qualifier(), |p| p.qualifier())
}
- pub fn qualifiers_and_self(&self) -> impl Iterator<Item = ast::Path> + Clone {
- successors(Some(self.clone()), |p| p.qualifier())
- }
-
pub fn top_path(&self) -> ast::Path {
let mut this = self.clone();
while let Some(path) = this.parent_path() {