Case split plugin - #5014
Conversation
|
Everybody is eagerly anticipating this change, thank you! Given the many WIP commits, I just wanted to ask if you could sqaush or reword them. I guess you had this in mind anyways. |
Glad to hear that!
Yep, see 3rd paragraph at the top :P |
|
Ah, the second sub-clause :-P my attention had already shifted before that one, I apologize :-)! Thanks! |
5456d10 to
038348b
Compare
MangoIV
left a comment
There was a problem hiding this comment.
First pass. I think after you clean up the main logic a bit more and add some documentation there, I can take another look. :)
Very good work, looking forward to having this in HLS!
82aad14 to
c7f9b61
Compare
There was a problem hiding this comment.
Very nice, this looks like it is making great progress!
I reviewed the plugin, I have mostly comments regarding readability and suggestions for improvements that hopefully improve the maintainability in the long run.
Some things are nitpicks, and you should feel free to ignore them.
I think we need one Note that outlines what the implementation strategy of the case split plugin is (e.g., reads the structured error message, etc...) so that it is easier to tell what the individual steps for case splitting are.
1fe48da to
38c773c
Compare
| typeOf, typeRep) | ||
|
|
||
|
|
||
| {- Note [Implementation strategy] |
There was a problem hiding this comment.
Put | and move at the top. Or use $, there's some examples around.
There was a problem hiding this comment.
There was a problem hiding this comment.
Notes are not haddock comments but a somewhat organically grown convention used by GHC that we adopted. They don't need a haddock | and probably shouldn't be part of a haddock comment.
There was a problem hiding this comment.
You mean I should do this?
--- {- | __Implementation strategy__
+++ {- __Implementation strategy__
The present plugin achieves its target of appending the missing patterns to a
non-exhaustive @case@ (or @\\case@) expression via the following strategy:fc57b6f to
1078e02
Compare
45d469c to
ca77d9d
Compare
aa765ce to
418a075
Compare
MangoIV
left a comment
There was a problem hiding this comment.
Looks very good!
If you could do another pass to fix up at least the comments that are not marked as nits and fix the test, I think this is read for a merge!
Also never mind the two "outdated" comments that were stale but I couldn't remove from this review somehow.
This commit implements the case-split plugin (see the description of the
owning PR and the included tests for examples of what it does).
We achieve the target of appending the missing patterns to a
non-exhaustive `case` (or `\case`) expression via the following
strategy:
1. retrieve the diagnostics under the client-provided range
(representing the cursor position or a text selection),
2. extract the innermost among the "non-exhaustive patterns"
diagnostics (because multiple of them can be nested),
3. extract the list of missing patterns from such a diagnostic,
4. traverse the AST, to
- pinpoint the one node representing the innermost `case` (or
`\case`) expression encompassing the aforementioned
client-provided range,
- turn the missing patterns (obtained from the diagnostic in
step 2 above) into matches to be inserted in the AST,
- appending these new matches to the existing ones, honoring the
existing layout, which means:
- choose `->` or `→` according to whether the
`UnicodeSyntax` extension is in use,
- put `;` if needed,
- use some heuristic to decide how many patterns to lay per
line
This PR is for (re-¹)introducing the so called case-split plugin, as requested in #5013, and it is part of my project for GSoC 2026.
To see the full summary of the project, click here. The summary also contains a list of stretch goals, some of which have been addressed already.
The work I've done and that I plan to submit via the present pull request, has consisted mainly of:
ghc-exactprintfor correctly layout out the patterns to be inserted,The result of such work is a plugin providing a code action for adding missing patterns to a
case/\caseexpression that has a non-exhaustive list of patterns.For instance, the plugin turns this
into this
¹ Earlier attempts
Indeed, there were some earlier attempts. See the summary for some pointers.