amnh / PCG

𝙋𝙝𝙮𝙡𝙤𝙜𝙚𝙣𝙚𝙩𝙞𝙘 𝘾𝙤𝙢𝙥𝙤𝙣𝙚𝙣𝙩 𝙂𝙧𝙖𝙥𝙝 ⸺ Haskell program and libraries for general phylogenetic graph search
28 stars 1 forks source link

Update all deriving statements to use DerivingStrategies and DerivingVia style #148

Closed recursion-ninja closed 4 years ago

recursion-ninja commented 5 years ago

The DerivingVia language extension allows for a much more descriptive deriving statement, and in some cases, can remove the need for explicit type-class instance declarations.

The following example shows how by using the DerivingVia style of deriving statement, it is clear how the instance below is derived from the GHC stock deriving mechanism:

data Foo = A | B | C
  deriving stock (Eq, Ord, Show)

The following example shows similarly how we can specify deriving instances by coercing a newtype:

newtype Bar f a = Bar (f a) 
  deriving newtype (Functor, Applicative)

We should replace all the existing deriving statements in our codebase with the new DerivingVia style for improved clarity of the deriving mechanism used.

Additionally, we should inspect existing explicit type-class instance declarations for opportunities to remove tedious wrapping & unwrapping and/or coercion of newtypes and replace the the instance with a deriving via mechanism.

Boarders commented 5 years ago

This is conflating some different issues. The examples of the form:

data Foo = A | B | C
  deriving stock (Eq, Ord, Show)

are from the DerivingStrategies language extension. In newer versions of hlint (after hlint-2-1-25) this can be enabled as a warning and I have done so here. This commit adds the particular deriving strategy we have used for all newtype declarations in our code base and adds a few easy-to-spot instances that can be derived rather than written out explicitly.

DerivingVia goes one step further and allows one to do something like the following:

newtype Index = Index {getIndex :: Int}
  deriving Monoid (via Sum Int)

I can imagine this being quite useful in our code base but it is much less mechanical to figure out where it can be employed.

recursion-ninja commented 5 years ago

Yes, this is what I intended. I thought DerivingVia & DerivingStrategies were a single language pragma.

At some point (it's low priority) we should look for the less mechanical instances that could be replaced with deriving via.

recursion-ninja commented 5 years ago

We can add the new -Wmissing-deriving-strategies flag once our code compiler with GHC 8.8.1.

recursion-ninja commented 4 years ago

This has been completed. See d219f1f2c12cd967497585bc27b5a28778b67c5d.

wardwheeler commented 4 years ago

Nice W