De-unwrap all assists

[lnicola]

Historically, the assists have been one of our major source of crashes, usually because they didn't handle invalid or incomplete code. Let's try to clean these up a bit.

Here are the ones I've found, though some of the usages there might actually be correct.

• generate_derive internal: Deunwrap generate_derive #15434
• generate_delegate_trait fix: Remove unwraps from "Generate delegate trait" #15397
• remove_unused_imports minor : Deunwrap remove_unused_imports #15433
• inline_call minor : Deunwrap inline call #15432
• extract_function minor : Deunwrap extract_function #15431
• add_missing_match_arms Deunwrap add_missing_match_arms #15594
• wrap_return_type_in_result internal: Deunwrap wrap_return_type_in_result #15430
• generate_delegate_methods minor : Deunwrap generate_delegate_methods #15429
• desugar_doc_comment minor : Deunwrap convert_comment_block and desugar_doc_comment #15425
• remove_dbg minor: Remove unwrap from Remove dbg! #15424
• generate_default_from_new Don't provide generate_default_from_new when impl self ty is missing #15406
• convert_to_guarded_return minor : Deunwrap convert_to_guarded_return #15426
• convert_comment_block minor : Deunwrap convert_comment_block and desugar_doc_comment #15425
• convert_named_struct_to_tuple_struct internal : Deunwrap convert_named_struct_to_tuple_struct #15423
• apply_demorgan fix: rewrite DeMorgan assist #15410

[lowr]

The unwrap() in remove_dbg is fine because replacements is checked to be non-empty right before it.

[lnicola]

That's right, but we could still extract that into a local variable and check it properly. It's not worse, and we won't spend time figuring it out again the next 10 times we go through the assists.

EDIT: even a single ? might do the job.

[lowr]

A comment would suffice I'd say. It's definitely bad to unwrap() without any explanation but for those as simple as this one I'd rather not write unnecessary control flow. I'll file a PR that adds a comment if that sounds reasonable.

Meanwhile, I've found that unwrap() in generate_default_from_new is problematic, sigh.

[lnicola]

I mean, a ? (or even my original suggestion, but not I'm not at the computer) will let us remove the check above, so it's not more control flow. It's much easier to grep and not have the unwrap there than to read the comment and think 😅.

[lnicola]

By the way, my idea here is to clean up as much of these as we can, then add a "ratchet"-style test to make sure we're not adding more by mistake.

[Milo123459]

can this be closed because all the tasks have been completed?

[lnicola]

Almost. I think there are a couple more in generate_function which we can turn into expect.

[Milo123459]

can i potentially look into doing those?

[lnicola]

Sure. Check out lines 407 and 686 in generate_function.rs.

The other thing I wanted to have is a tidy test to count how many unwraps there are in this directory.

[Milo123459]

is there a place i can ask for guidance? some of these aren't that clear - for example line 406 suggests that the unwrap is fine? or am i misinterpreting it.

[lnicola]

Yeah, I meant replacing .unwrap() with .expect("generated function should have a body") and so on.

[Milo123459]

should be all good, not sure about the tail_expr expect.