haskell / haskell-language-server

Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.
Apache License 2.0
2.65k stars 355 forks source link

Feature Request: Automatic Expansion of uninitialized record fields #462

Open ProofOfKeags opened 3 years ago

ProofOfKeags commented 3 years ago

When a warning for "Fields of ‘SomeRecord’ not initialised: a, b, c, d" fires, I'd like a CodeAction that allows it to initialize all of those rows with typed holes.

Flow would look like this:

  1. let thing = SomeRecord{}
  2. Ctrl+'.' (or other code action trigger)
  3. let thing = SomeRecord{} --> let thing = SomeRecord { a = , b = , c = , d = }

The reason that this is somewhat of a big deal is that in the presence of long record names with the conventional prefix based row names (someRecordA, someRecordB, ...), the typing for this is nontrivial and also entirely mechanical and so the editor is perfectly capable of doing this for me.

ProofOfKeags commented 3 years ago

More sophisticated versions of this could infer what fields are already initialized, and only supply the ones that are missing in this way.

ProofOfKeags commented 3 years ago

Is this something that belongs in ghcide or hls?

googleson78 commented 3 years ago

Even cooler would be, agda supports straight up introducing the entire record. So 0.

let
  thing :: SomeRecordType 
  thing = _
  1. let
    thing :: SomeRecordType 
    thing =
    SomeRecordType
      { field1 = _
      , ...
      }
jneira commented 3 years ago

I would say that i could be done in a plugin in hls, but not sure.

ProofOfKeags commented 3 years ago

@googleson78 yeah that's also great. That said, when the typedef expands (like many records do over time), I'd still like to be able to introduce the missing fields.

antoine-fl commented 3 years ago

Somewhat related, it would be nice to have a shortcut to convert from using the "normal" constructor to using record syntax.

For example if I have the following:

data Person = Person { firstName :: Text, lastName :: Text }

let p = Person "Robert" "Dupont"

Some shortcut could convert the second line to:

let p = Person { firstName = "Robert", lastName = "Dupont" }
jneira commented 2 years ago

Now tactics has a code action: Wingman: Use constructor Person which converts

person :: Person
person = _

in

person :: Person
person = Person {firstName = _wf, lastName = _wg}

So i think the suggestion of @googleson78 is already covered.

It still misses the other two requests:

For the first one there is a dedicated ghc warning usable to add a quick fix:

• Fields of ‘Person’ not initialised: firstName, lastName
• In the expression: Person {}
  In an equation for ‘person’: person = Person {}typecheck(-Wmissing-fields)

So i would say it should not be very much dificult to get

naufik commented 2 years ago

Had a look at the codebase and I think this might be a good start for me to start contributing. Can I attempt this issue?

My outline would be to add a code action for the Fields of '<X>' uninitialized warning. Right now, there is a code action for "suppress errors" so I suppose I can add an alternative quick fix as @jneira mentioned.

For suggestion 2 I'd probably have to have a look on how LHS detects if a token is a record constructor invoked using function syntax.

Hoping to spend the week on this 🤞

jneira commented 2 years ago

@naufik nice! thanks for willing to contribute, feel free to ask anything here or in the chat channels.

salmanjnr commented 2 years ago

Is this still open for contributions?

naufik commented 2 years ago

Hey @salman69e27, unfortunately when I picked this up I couldn't get the build env to work at the time and currently have time constraints to contribute. You can pick up

salmanjnr commented 2 years ago

Thank you @naufik . I will start working on it in a couple of days

pepeiborra commented 2 years ago

Contributions are welcome 🤗 I just want to point out, as it is not mentioned in the thread, that HLS already generates record fields when autocompleting a record constructor.

Is this good enough? Does it just need to be publicised more?

ProofOfKeags commented 2 years ago

The original request is really around having HLS intelligently expand any constructor that had a warning on "uninitialized record fields".

googleson78 commented 2 years ago

This is, of course, an issue with my setup - but actions were trivially set up for me and I use them extensively, while I still haven't gotten around to making the record field autocomplete, so I would be happy with having this as a code action.

dan-blank commented 1 year ago

I'd like to work on this ticket! :)

akshaymankar commented 3 months ago

I'm working on this during Zurihac 2024.

googleson78 commented 3 months ago

Just an update that my usecase is no longer covered either, since wingman is not maintained since 9.0 afaik

akshaymankar commented 3 months ago

Update from me: I have a branch with something that kinda works, just adds some extra new lines (not sure what's going on). I will try to resolve it this weekend, if not I will create a draft for someone more experienced with ghc-exactprint to take a look.