Skip to content

Commit

Permalink
Auto merge of rust-lang#13715 - feniljain:fix_completions, r=jonas-sc…
Browse files Browse the repository at this point in the history
…hievink

fix: breaking snippets on typed incomplete suggestions

Possible fix for rust-lang#7929

Fix the case where if a user types `&&42.o`, snippet completion was still applying &&Ok(42). Note this was fixed previously on `&&42.` but this still remained a problem for this case

Previous relevant PR: rust-lang#13517

### Points to help in review:

- The main problem why everything broke on adding an extra `o` was, earlier `dot_receiver` was `42.` which was a `LITERAL` but now `42.o` becomes a `FIELD_EXPR`

- Till now `include_references` was just checking for parent of `LITERAL` and if it was a `REF_EXPR`, but now we consider `FIELD_EXPR` and traverse all of them, finally to reach `REF_EXPR`. If `REF_EXPR` is not found we  just return the original `initial_element`

- We are constructing a new node during `include_references` because if we rely on `dot_receiver` solely we would get `&&42.o` to be replaced with, but we want `&&42` to be replaced with

### Output Video:

https://user-images.githubusercontent.com/49019259/205420166-efbdef78-5b3a-4aef-ab4b-d892dac056a0.mov

Hope everything I wrote makes sense 😅

Also interestingly previous PR's number was `13517` and this PR's number is `13715`, nicee
  • Loading branch information
bors committed Dec 12, 2022
2 parents e7dff74 + ec268c0 commit 15ff8a5
Showing 1 changed file with 77 additions and 22 deletions.
99 changes: 77 additions & 22 deletions crates/ide-completion/src/completions/postfix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ mod format_like;
use hir::{Documentation, HasAttrs};
use ide_db::{imports::insert_use::ImportScope, ty_filter::TryEnum, SnippetCap};
use syntax::{
ast::{self, AstNode, AstToken},
ast::{self, make, AstNode, AstToken},
SyntaxKind::{EXPR_STMT, STMT_LIST},
TextRange, TextSize,
};
Expand Down Expand Up @@ -129,8 +129,10 @@ pub(crate) fn complete_postfix(

// The rest of the postfix completions create an expression that moves an argument,
// so it's better to consider references now to avoid breaking the compilation
let dot_receiver = include_references(dot_receiver);
let receiver_text = get_receiver_text(&dot_receiver, receiver_is_ambiguous_float_literal);

let (dot_receiver, node_to_replace_with) = include_references(dot_receiver);
let receiver_text =
get_receiver_text(&node_to_replace_with, receiver_is_ambiguous_float_literal);
let postfix_snippet = match build_postfix_snippet_builder(ctx, cap, &dot_receiver) {
Some(it) => it,
None => return,
Expand Down Expand Up @@ -210,23 +212,43 @@ fn get_receiver_text(receiver: &ast::Expr, receiver_is_ambiguous_float_literal:
text.replace('\\', "\\\\").replace('$', "\\$")
}

fn include_references(initial_element: &ast::Expr) -> ast::Expr {
fn include_references(initial_element: &ast::Expr) -> (ast::Expr, ast::Expr) {
let mut resulting_element = initial_element.clone();
while let Some(parent_ref_element) =
resulting_element.syntax().parent().and_then(ast::RefExpr::cast)

while let Some(field_expr) = resulting_element.syntax().parent().and_then(ast::FieldExpr::cast)
{
resulting_element = ast::Expr::from(parent_ref_element);
resulting_element = ast::Expr::from(field_expr);
}
resulting_element

let mut new_element_opt = initial_element.clone();

if let Some(first_ref_expr) = resulting_element.syntax().parent().and_then(ast::RefExpr::cast) {
if let Some(expr) = first_ref_expr.expr() {
resulting_element = expr.clone();
}

while let Some(parent_ref_element) =
resulting_element.syntax().parent().and_then(ast::RefExpr::cast)
{
resulting_element = ast::Expr::from(parent_ref_element);

new_element_opt = make::expr_ref(new_element_opt, false);
}
} else {
// If we do not find any ref expressions, restore
// all the progress of tree climbing
resulting_element = initial_element.clone();
}

(resulting_element, new_element_opt)
}

fn build_postfix_snippet_builder<'ctx>(
ctx: &'ctx CompletionContext<'_>,
cap: SnippetCap,
receiver: &'ctx ast::Expr,
) -> Option<impl Fn(&str, &str, &str) -> Builder + 'ctx> {
let receiver_syntax = receiver.syntax();
let receiver_range = ctx.sema.original_range_opt(receiver_syntax)?.range;
let receiver_range = ctx.sema.original_range_opt(receiver.syntax())?.range;
if ctx.source_range().end() < receiver_range.start() {
// This shouldn't happen, yet it does. I assume this might be due to an incorrect token mapping.
return None;
Expand Down Expand Up @@ -616,22 +638,55 @@ fn main() {

#[test]
fn postfix_custom_snippets_completion_for_references() {
// https://github.com/rust-lang/rust-analyzer/issues/7929

let snippet = Snippet::new(
&[],
&["ok".into()],
&["Ok(${receiver})".into()],
"",
&[],
crate::SnippetScope::Expr,
)
.unwrap();

check_edit_with_config(
CompletionConfig {
snippets: vec![Snippet::new(
&[],
&["ok".into()],
&["Ok(${receiver})".into()],
"",
&[],
crate::SnippetScope::Expr,
)
.unwrap()],
..TEST_CONFIG
},
CompletionConfig { snippets: vec![snippet.clone()], ..TEST_CONFIG },
"ok",
r#"fn main() { &&42.o$0 }"#,
r#"fn main() { Ok(&&42) }"#,
);

check_edit_with_config(
CompletionConfig { snippets: vec![snippet.clone()], ..TEST_CONFIG },
"ok",
r#"fn main() { &&42.$0 }"#,
r#"fn main() { Ok(&&42) }"#,
);

check_edit_with_config(
CompletionConfig { snippets: vec![snippet], ..TEST_CONFIG },
"ok",
r#"
struct A {
a: i32,
}
fn main() {
let a = A {a :1};
&a.a.$0
}
"#,
r#"
struct A {
a: i32,
}
fn main() {
let a = A {a :1};
Ok(&a.a)
}
"#,
);
}
}

0 comments on commit 15ff8a5

Please sign in to comment.