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
This commit is contained in:
bors 2024-01-09 15:41:48 +00:00
commit da6f7e2c7b
3 changed files with 27 additions and 14 deletions

View File

@ -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;
@ -250,6 +254,18 @@ fn bar() -> i32 {
}
fn bar() -> i32 {
foo::goo::A_CONSTANT
}"#,
);
}
#[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
}"#,
);
}

View File

@ -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;

View File

@ -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() {