Skip to content

Commit

Permalink
Auto merge of rust-lang#12334 - fasterthanlime:rust-langgh-11635, r=V…
Browse files Browse the repository at this point in the history
…eykril

Generate enum variant assist

So, this is kind of a weird PR!

I'm a complete newcomer to the `rust-analyzer` codebase, and so I browsed the "good first issue" tag, and found rust-lang#11635. Then I found two separate folks had taken stabs at it, most recently `@maartenflippo` — and there had been a review 3 days ago, but no activity in a little while, and the PR needed to be rebased since the crates were renamed from `snake_case` to `kebab-case`.

So to get acquainted with the codebase I typed this PR by hand, looking at the diff in rust-lang#11995, and I also added a doc-test (that passes).

I haven't taken into account the comments `@Veykril` left in rust-lang#11995, but I don't want to steal any of `@maartenflippo's` thunder! Closing this PR is perfectly fine. Or Maarten could use it as a "restart point"? Or I could finish it up, whichever feels best to everyone.

I think what remains to be done in this PR, at least, is:

  * [x] Only disable the "generate function" assist if the name is `PascalCase`
  * [x] Only enable the "generate variant" assistant if the name is `PascalCase`
  * [x] Simplify with `adt.source()` as mentioned here: rust-lang/rust-analyzer#11995 (comment)
  * [ ] Add more tests for edge cases? Are there cases where simply adding one more indent level than the enum's indent level is not good enough? Some nested trickery I'm not thinking of right now?

Anyway. This PR can go in any direction. You can tell me "no, tackle your own issue!" And I'll go do that and still be happy I got to take a look at rust-analyzer some by doing this. Or you can tell me "okay, now _you_ finish it", and I guess I'll try and finish it :)

Closes rust-lang#11635
  • Loading branch information
bors committed May 22, 2022
2 parents 65a213c + ae2c0db commit 4b0f9c5
Show file tree
Hide file tree
Showing 4 changed files with 272 additions and 0 deletions.
198 changes: 198 additions & 0 deletions crates/ide-assists/src/handlers/generate_enum_variant.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
use hir::HasSource;
use ide_db::assists::{AssistId, AssistKind};
use syntax::{
ast::{self, edit::IndentLevel},
AstNode, TextSize,
};

use crate::assist_context::{AssistContext, Assists};

// Assist: generate_enum_variant
//
// Adds a variant to an enum.
//
// ```
// enum Countries {
// Ghana,
// }
//
// fn main() {
// let country = Countries::Lesotho$0;
// }
// ```
// ->
// ```
// enum Countries {
// Ghana,
// Lesotho,
// }
//
// fn main() {
// let country = Countries::Lesotho;
// }
// ```
pub(crate) fn generate_enum_variant(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
let path_expr: ast::PathExpr = ctx.find_node_at_offset()?;
let path = path_expr.path()?;

if ctx.sema.resolve_path(&path).is_some() {
// No need to generate anything if the path resolves
return None;
}

let name_ref = path.segment()?.name_ref()?;
if name_ref.text().starts_with(char::is_lowercase) {
// Don't suggest generating variant if the name starts with a lowercase letter
return None;
}

if let Some(hir::PathResolution::Def(hir::ModuleDef::Adt(hir::Adt::Enum(e)))) =
ctx.sema.resolve_path(&path.qualifier()?)
{
let target = path.syntax().text_range();
return add_variant_to_accumulator(acc, ctx, target, e, &name_ref);
}

None
}

fn add_variant_to_accumulator(
acc: &mut Assists,
ctx: &AssistContext,
target: syntax::TextRange,
adt: hir::Enum,
name_ref: &ast::NameRef,
) -> Option<()> {
let adt_ast = adt.source(ctx.db())?.original_ast_node(ctx.db())?.value;
let enum_indent = IndentLevel::from_node(&adt_ast.syntax());

let variant_list = adt_ast.variant_list()?;
let offset = variant_list.syntax().text_range().end() - TextSize::of('}');
let empty_enum = variant_list.variants().next().is_none();

acc.add(
AssistId("generate_enum_variant", AssistKind::Generate),
"Generate variant",
target,
|builder| {
let text = format!(
"{maybe_newline}{indent_1}{name},\n{enum_indent}",
maybe_newline = if empty_enum { "\n" } else { "" },
indent_1 = IndentLevel(1),
name = name_ref,
enum_indent = enum_indent
);
builder.insert(offset, text)
},
)
}

#[cfg(test)]
mod tests {
use crate::tests::{check_assist, check_assist_not_applicable};

use super::*;

#[test]
fn generate_basic_enum_variant_in_empty_enum() {
check_assist(
generate_enum_variant,
r"
enum Foo {}
fn main() {
Foo::Bar$0
}
",
r"
enum Foo {
Bar,
}
fn main() {
Foo::Bar
}
",
)
}

#[test]
fn generate_basic_enum_variant_in_non_empty_enum() {
check_assist(
generate_enum_variant,
r"
enum Foo {
Bar,
}
fn main() {
Foo::Baz$0
}
",
r"
enum Foo {
Bar,
Baz,
}
fn main() {
Foo::Baz
}
",
)
}

#[test]
fn not_applicable_for_existing_variant() {
check_assist_not_applicable(
generate_enum_variant,
r"
enum Foo {
Bar,
}
fn main() {
Foo::Bar$0
}
",
)
}

#[test]
fn not_applicable_for_lowercase() {
check_assist_not_applicable(
generate_enum_variant,
r"
enum Foo {
Bar,
}
fn main() {
Foo::new$0
}
",
)
}

#[test]
fn indentation_level_is_correct() {
check_assist(
generate_enum_variant,
r"
mod m {
enum Foo {
Bar,
}
}
fn main() {
m::Foo::Baz$0
}
",
r"
mod m {
enum Foo {
Bar,
Baz,
}
}
fn main() {
m::Foo::Baz
}
",
)
}
}
46 changes: 46 additions & 0 deletions crates/ide-assists/src/handlers/generate_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ fn gen_fn(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
get_fn_target(ctx, &target_module, call.clone())?
}
Some(hir::PathResolution::Def(hir::ModuleDef::Adt(adt))) => {
if let hir::Adt::Enum(_) = adt {
// Don't suggest generating function if the name starts with an uppercase letter
if name_ref.text().starts_with(char::is_uppercase) {
return None;
}
}

let current_module = ctx.sema.scope(call.syntax())?.module();
let module = adt.module(ctx.sema.db);
target_module = if current_module == module { None } else { Some(module) };
Expand Down Expand Up @@ -1735,6 +1742,45 @@ fn main() {
fn foo(value: usize) ${0:-> _} {
todo!()
}
",
)
}

#[test]
fn not_applicable_for_enum_variant() {
check_assist_not_applicable(
generate_function,
r"
enum Foo {}
fn main() {
Foo::Bar$0(true)
}
",
);
}

#[test]
fn applicable_for_enum_method() {
check_assist(
generate_function,
r"
enum Foo {}
fn main() {
Foo::new$0();
}
",
r"
enum Foo {}
fn main() {
Foo::new();
}
impl Foo {
fn new() ${0:-> _} {
todo!()
}
}
",
)
}
Expand Down
2 changes: 2 additions & 0 deletions crates/ide-assists/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ mod handlers {
mod generate_documentation_template;
mod generate_enum_is_method;
mod generate_enum_projection_method;
mod generate_enum_variant;
mod generate_from_impl_for_enum;
mod generate_function;
mod generate_getter;
Expand Down Expand Up @@ -227,6 +228,7 @@ mod handlers {
generate_enum_is_method::generate_enum_is_method,
generate_enum_projection_method::generate_enum_as_method,
generate_enum_projection_method::generate_enum_try_into_method,
generate_enum_variant::generate_enum_variant,
generate_from_impl_for_enum::generate_from_impl_for_enum,
generate_function::generate_function,
generate_impl::generate_impl,
Expand Down
26 changes: 26 additions & 0 deletions crates/ide-assists/src/tests/generated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1006,6 +1006,32 @@ impl Value {
)
}

#[test]
fn doctest_generate_enum_variant() {
check_doc_test(
"generate_enum_variant",
r#####"
enum Countries {
Ghana,
}
fn main() {
let country = Countries::Lesotho$0;
}
"#####,
r#####"
enum Countries {
Ghana,
Lesotho,
}
fn main() {
let country = Countries::Lesotho;
}
"#####,
)
}

#[test]
fn doctest_generate_from_impl_for_enum() {
check_doc_test(
Expand Down

0 comments on commit 4b0f9c5

Please sign in to comment.