Skip to content

Commit

Permalink
Auto merge of rust-lang#17571 - winstxnhdw:bool-to-enum-no-dupe, r=Ve…
Browse files Browse the repository at this point in the history
…ykril

feat: do not add new enum if it already exists

## Summary

This PR introduces a check for the existence of another enum within the current scope, and if it exist, we skip `add_enum_def`.

## Why?

Currently, when using the `bool_to_enum` assist more than once, it is possible to add multiple enum definitions. For example, the following snippet,

```rs
#[derive(PartialEq, Eq)]
enum Bool {
    True,
    False,
}

fn main() {
    let a = Bool::True;
    let b = true;
    println!("Hello, world!");
}
```

will be transformed into,

```rs
#[derive(PartialEq, Eq)]
enum Bool {
    True,
    False,
}

#[derive(PartialEq, Eq)]
enum Bool {
    True,
    False,
}

fn main() {
    let a = Bool::True;
    let b = Bool::True;
    println!("Hello, world!");
}
```

This can be annoying for users to clean up.
  • Loading branch information
bors committed Jul 11, 2024
2 parents 13ac073 + c4bcec2 commit 5577e4e
Showing 1 changed file with 44 additions and 3 deletions.
47 changes: 44 additions & 3 deletions crates/ide-assists/src/handlers/bool_to_enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ struct BoolNodeData {

/// Attempts to find an appropriate node to apply the action to.
fn find_bool_node(ctx: &AssistContext<'_>) -> Option<BoolNodeData> {
let name: ast::Name = ctx.find_node_at_offset()?;
let name = ctx.find_node_at_offset::<ast::Name>()?;

if let Some(ident_pat) = name.syntax().parent().and_then(ast::IdentPat::cast) {
let def = ctx.sema.to_def(&ident_pat)?;
Expand Down Expand Up @@ -461,7 +461,20 @@ fn add_enum_def(
usages: &UsageSearchResult,
target_node: SyntaxNode,
target_module: &hir::Module,
) {
) -> Option<()> {
let insert_before = node_to_insert_before(target_node);

if ctx
.sema
.scope(&insert_before)?
.module()
.scope(ctx.db(), Some(*target_module))
.iter()
.any(|(name, _)| name.as_str() == Some("Bool"))
{
return None;
}

let make_enum_pub = usages
.iter()
.flat_map(|(_, refs)| refs)
Expand All @@ -472,14 +485,15 @@ fn add_enum_def(
.any(|module| module.nearest_non_block_module(ctx.db()) != *target_module);
let enum_def = make_bool_enum(make_enum_pub);

let insert_before = node_to_insert_before(target_node);
let indent = IndentLevel::from_node(&insert_before);
enum_def.reindent_to(indent);

edit.insert(
insert_before.text_range().start(),
format!("{}\n\n{indent}", enum_def.syntax().text()),
);

Some(())
}

/// Finds where to put the new enum definition.
Expand Down Expand Up @@ -553,6 +567,33 @@ fn function(foo: Bool, bar: bool) {
)
}

#[test]
fn no_duplicate_enums() {
check_assist(
bool_to_enum,
r#"
#[derive(PartialEq, Eq)]
enum Bool { True, False }
fn function(foo: bool, $0bar: bool) {
if bar {
println!("bar");
}
}
"#,
r#"
#[derive(PartialEq, Eq)]
enum Bool { True, False }
fn function(foo: bool, bar: Bool) {
if bar == Bool::True {
println!("bar");
}
}
"#,
)
}

#[test]
fn parameter_with_last_param_usage() {
check_assist(
Expand Down

0 comments on commit 5577e4e

Please sign in to comment.