-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
minor: Migrate (un)wrap_return_type
assists to use SyntaxEditor
#18524
base: master
Are you sure you want to change the base?
minor: Migrate (un)wrap_return_type
assists to use SyntaxEditor
#18524
Conversation
f2c9293
to
8542c04
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't add a comment to the file in question, but it would be good also add a test in syntax_editor::tests
testing that token edits in a parent node won't be considered ancestors of edits in the child node:
#[test]
fn test_replace_token_in_parent() {
let parent_fn = make::fn_(
None,
make::name("it"),
None,
None,
make::param_list(None, []),
make::block_expr([], Some(make::expr_unit())),
Some(make::ret_type(make::ty_unit())),
false,
false,
false,
false,
);
let mut editor = SyntaxEditor::new(parent_fn.syntax().clone());
if let Some(ret_ty) = parent_fn.ret_type() {
editor.delete(ret_ty.syntax().clone());
if let Some(SyntaxElement::Token(token)) = ret_ty.syntax().next_sibling_or_token() {
if token.kind().is_trivia() {
editor.delete(token);
}
}
}
if let Some(tail) = parent_fn.body().unwrap().tail_expr() {
editor.delete(tail.syntax().clone());
}
let edit = editor.finish();
let expect = expect![[r#"
fn it() {
}"#]];
expect.assert_eq(&edit.new_root.to_string());
}
Forgot to mention that the final tabstop snippet for |
842bcdc
to
9f5cc26
Compare
☔ The latest upstream changes (presumably #18483) made this pull request unmergeable. Please resolve the merge conflicts. |
21e9566
to
b9db9e9
Compare
This comment has been minimized.
This comment has been minimized.
97c1ee7
to
f62ef42
Compare
I noticed that in #18551, the following pub fn turbofish_generic_arg_list(
&self,
args: impl IntoIterator<Item = ast::GenericArg> + Clone,
) -> ast::GenericArgList {
let ast = make::turbofish_generic_arg_list(args.clone()).clone_for_update();
if let Some(mut mapping) = self.mappings() {
let mut builder = SyntaxMappingBuilder::new(ast.syntax().clone());
builder.map_children(
args.into_iter().map(|arg| arg.syntax().clone()),
ast.generic_args().map(|arg| arg.syntax().clone()),
);
builder.finish(&mut mapping);
}
ast
} But typically, I would have written this constructor as follows pub fn turbofish_generic_arg_list(
&self,
args: impl IntoIterator<Item = ast::GenericArg>,
) -> ast::GenericArgList {
let args: Vec<ast::GenericArg> = args.into_iter().collect();
let input: Vec<ast::SyntaxNode> = args.iter().map(|it| it.syntax().clone()).collect();
let ast = make::turbofish_generic_arg_list(args).clone_for_update();
if let Some(mut mapping) = self.mappings() {
let mut builder = SyntaxMappingBuilder::new(ast.syntax().clone());
builder.map_children(
input.into_iter(),
ast.generic_args().map(|arg| arg.syntax().clone()),
);
builder.finish(&mut mapping);
}
ast
} The difference being how |
f62ef42
to
355c4d8
Compare
☔ The latest upstream changes (presumably #18657) made this pull request unmergeable. Please resolve the merge conflicts. |
355c4d8
to
491dc94
Compare
☔ The latest upstream changes (presumably #18652) made this pull request unmergeable. Please resolve the merge conflicts. |
Also changes `make::expr_empty_block()` to return `ast::BlockExpr` instead of `ast::Expr`
491dc94
to
f82c46f
Compare
Part of #15710 and #18285
This PR also includes some small fixes to
SyntaxEditor
, in particular a fix to the problem withaffected_range
outlined here.I also updated
unwrap_return_type
to use placeholders when unwrappingNone
. I had tried to do this with the mutable tree API before and it was kind of a pain so it's nice to see improvements like this :)