fix: make bool_to_enum assist create enum at top-level

[rmehri01]

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 :)

[Veykril]

This can be simplified to

Suggested change

	/// 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)
	}

[rmehri01]

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
}

[Veykril]

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)

[rmehri01]

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.

[Veykril]

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.

[rmehri01]

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?

[Veykril]

nah this is simple enough so it should be fine

[Veykril]

@bors r+

[bors]

📌 Commit 1b3e5b2 has been approved by Veykril

It is now in the queue for this repository.

[bors]

⌛ Testing commit 1b3e5b2 with merge 87e2c31...

[bors]

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 87e2c31 to master...