fix: make bool_to_enum assist create enum at top-level by rmehri01 · Pull Reques...
source link: https://github.com/rust-lang/rust-analyzer/pull/15667
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Conversation
Contributor
This pr makes the bool_to_enum assist create the enum at the next closest module block or at top-level, which fixes a few tricky cases such as with an associated const in a trait or module:
trait Foo {
const $0BOOL: bool;
}
impl Foo for usize {
const BOOL: bool = true;
}
fn main() {
if <usize as Foo>::BOOL {
println!("foo");
}
}Which now properly produces:
#[derive(PartialEq, Eq)]
enum Bool { True, False }
trait Foo {
const BOOL: Bool;
}
impl Foo for usize {
const BOOL: Bool = Bool::True;
}
fn main() {
if <usize as Foo>::BOOL == Bool::True {
println!("foo");
}
}I also think it's a bit nicer, especially for local variables, but didn't really know to do it in the first PR :)
added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label
Comment on lines
475 to 503
/// Finds where to put the new enum definition, at the nearest module or at top-level. |
||
fn node_to_insert_before(mut target_node: SyntaxNode) -> SyntaxNode { |
||
let mut ancestors = target_node.ancestors(); |
||
while let Some(ancestor) = ancestors.next() { |
||
match_ast! { |
||
match ancestor { |
||
ast::Item(item) => { |
||
if item |
||
.syntax() |
||
.parent() |
||
.and_then(|item_list| item_list.parent()) |
||
.and_then(ast::Module::cast) |
||
.is_some() |
||
{ |
||
return ancestor; |
||
} |
||
}, |
||
ast::SourceFile(_) => break, |
||
_ => (), |
||
} |
||
} |
||
target_node = ancestor; |
||
} |
||
target_node |
||
} |
||
This can be simplified to
| /// Finds where to put the new enum definition, at the nearest module or at top-level. | |
| fn node_to_insert_before(mut target_node: SyntaxNode) -> SyntaxNode { | |
| let mut ancestors = target_node.ancestors(); | |
| while let Some(ancestor) = ancestors.next() { | |
| match_ast! { | |
| match ancestor { | |
| ast::Item(item) => { | |
| if item | |
| .syntax() | |
| .parent() | |
| .and_then(|item_list| item_list.parent()) | |
| .and_then(ast::Module::cast) | |
| .is_some() | |
| { | |
| return ancestor; | |
| } | |
| }, | |
| ast::SourceFile(_) => break, | |
| _ => (), | |
| } | |
| } | |
| target_node = ancestor; | |
| } | |
| target_node | |
| } | |
| /// Finds where to put the new enum definition, at the nearest module or at top-level. | |
| fn node_to_insert_before(target_node: SyntaxNode) -> SyntaxNode { | |
| target_node.ancestors().find(|it| matches!(it.kind(), SyntaxKind::Module | SyntaxKind::SourceFile).unwrap_or(target_node) | |
| } | |
Contributor
Author
Sorry, maybe the doc comment isn't too clear but I think the simplified version finds the module or source file itself, while I was aiming to find the furthest ancestor Item that is contained within a module or just at top-level so that I can insert before it. I think something more along the lines of this:
/// Finds where to put the new enum definition.
/// Tries to find the [ast::Item] node at the nearest module or at top-level, otherwise just
/// returns the input node.
fn node_to_insert_before(mut target_node: SyntaxNode) -> SyntaxNode {
let mut ancestors = target_node.ancestors();
while let Some(ancestor) = ancestors.next() {
match_ast! {
match ancestor {
ast::Module(_) => break,
ast::SourceFile(_) => break,
ast::Item(_) => target_node = ancestor,
_ => (),
}
}
}
target_node
}Oh sorry that wes me spacing out, in that case we can do
target_node.ancestors().take_while(|it| !matches!(it.kind(), SyntaxKind::Module | SyntaxKind::SourceFile).last().unwrap_or(target_node)Contributor
Author
No problem! I think this works for the source file case but in the module case we want the second to last node since the last is an ItemList and inserting it before the ItemList would be between the module name and the left brace.
Right, then my snippet only needs a slight adjustment by adding a filter
target_node.ancestors().take_while(|it| !matches!(it.kind(), SyntaxKind::Module | SyntaxKind::SourceFile).filter(|it| ast::Item::can_cast(it.kind())).last().unwrap_or(target_node)Though I feel like there has to be some assist that does something similar here, I'd be surprised if not but I can't think of any of the top oh my head right now.
Contributor
Author
Ah I see okay, that works and is much cleaner, thank you!
I thought that would be the case as well, a few that I looked at were extract_function, which has node_to_insert_after and is what I based this one on but seems to be a bit more complex and generate_function which has next_space_for_fn_in_module and similar functions but is based on text ranges and also seem more complex. Do you think it would be better to try to reuse these?
nah this is simple enough so it should be fine
added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
labels
Member
|
@bors r+ |
Collaborator
|
☀️ Test successful - checks-actions |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
No one assigned
None yet
No milestone
Successfully merging this pull request may close these issues.
None yet
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK