Simplify single-case pattern match using let destructuring

[vkobinski]

Fixes #3562

[vkobinski]

@lpil What should I do with the tests that use a single case pattern match? Change them?

[GearsDatapacks]

Probably, yes. Also, you'll want to add tests for this new warning.
And in the issue I mentioned adding a code action to fix this warning. Do you think you could implement that?

[vkobinski]

Probably, yes. Also, you'll want to add tests for this new warning. And in the issue I mentioned adding a code action to fix this warning. Do you think you could implement that?

The code action should be implemented in the lsp area of the repository? I think i could implement it.

[GearsDatapacks]

The code action should be implemented in the lsp area of the repository?

Correct. It should be in the language_server/code_action.rs file. You can look at the code_action_unused_imports function (which still might be in engine.rs), and see how it finds all the matching warnings, and iterates through them. Instead of replacing the text with an empty string, you can just replace it with whatever the hint is for that warning

[lpil]

Does this emit a warning if the case is inexhaustive? It shouldn't.

We also want to emit a warning for non-constructor single clause case expressions.

[lpil]

Thank you!

[GearsDatapacks]

I don't think it's a good idea to reformat the pattern here. Some information, such as whether the constructor is qualified, will be lost. And, as louis pointed out, this only works for constructor patterns at the moment. What I did for the "let assert to case" action is to get the location of the pattern and copy the exact string value from the source. I'm not sure if you have access to the source here though

Also, you can use eco_format! instead of format! and EcoString::from

[vkobinski]

I am implementing the changes you both suggested!

[vkobinski]

@GearsDatapacks can you suggest any example of a single case simplification where the pattern is not a constructor?

[vkobinski]

I was able to implement the code action for the constructor case! I'm going to make the code action work for other cases too.

[GearsDatapacks]

@GearsDatapacks can you suggest any example of a single case simplification where the pattern is not a constructor?

For example,

case my_tuple {
  #(a, b) -> do_stuff(a, b)
}

Can be rewritten as

let #(a, b) = my_tuple
do_stuff(a, b)

[GearsDatapacks]

I've just realised something which might make this code action a little more complex.

What do you expect to happen if you run the code action on:

let v = case a {
  #(_, value, _) -> value
}

We still need to make sure we bind v in this case, and that we don't generate let v = let #(_, value, _) = a, which is invalid.

Maybe for now we should only offer the code action if the case is in statement position, since that's the only place where let is allowed. It's possible to do this rewrite anywhere, but it requires a lot more calculations for the non-trivial case.

[lpil]

That's a good observation. Is this code action very useful given that limitation?

[GearsDatapacks]

Perhaps not. It could still me useful in some cases, but I imagine the majority will be using let assignment to extract values. Maybe this should just be a warning for now

[giacomocavalieri]

Maybe this should just be a warning for now

But what if someone is using a case because they know they'll add more variants to the type to make it easier to change the code later?