This style guide is based on consensus. Our guiding principles and enforcement through PR reviews are the same, so please read the relevant sections of that document, we just repeat here the most important topics. However our formatting rules are slightly different.
We value the following principles in the networking team:
- Optimise for clarity
- Consistency: inconsistent style, especially within a single module, looks sloppy, inspires little confidence in the quality of the code, and distracts. Consistency is also a helpful guiding factor when deciding on style guidelines in the first place; sometimes a choice between formatting something this way or that way seems arbitrary in isolation, but becomes clearer when seen as part of a coherent whole.
We now list the formatting rules we have converged on. As these have grown organically, not all code follows these rules. When touching some existing code, we in general recommend sticking to the existing style, but when it differs from the rules below, it is good practice to update the code's style to match them.
-
Indentation: we indent by 2 spaces.
Why: to avoid wasting horizontal screen space.
Some consequences of this rule:
a. The
where
clause of a function body is indented 2 spaces from the left margin, and the function body is indented 2 spaces from thewhere
:foo x y z = .. where a = ..
The
where
keyword acts as a separator between the body and the bindings. Keeping them at the same indentation level would make it hard to see where the body ends.We stick with this indentation even if the
where
clause is not present, just to avoid unnecessary changes when awhere
clause is added.b. We indent record
data
andnewtype
definitions as follows:data Foo = Foo { fooBar :: Int, fooArgument :: Bool } deriving (Show, Eq) newtype Foo = Foo { unFoo :: Int } deriving (Show, Eq)
The
deriving
is indented from the left margin, and the constructors are indented from thederiving
clause. This provides a consistent style for data types with multiple constructors (see below).Multiple deriving clauses using
DerivingStrategies
are aligned:data Foo = Foo { fooBar :: Int, fooArgument :: Bool } deriving stock (Show, Eq, Generic) deriving anyclass (NoThunks, NFData) newtype Foo = Foo { unFoo :: Int } deriving stock (Show) deriving newtype (Eq) deriving NoThunks via InspectHeapNamed "Foo" Foo
Parentheses around a singleton list of classes are optional.
Records that fit onto a single line can be formatted like this:
data Foo = X {foo :: A, bar :: B} | Y
We aren't very consistent with our data declarations, following the local style is always a good choice.
We prefer this style also when creating terms:
let foo = Foo { fooBar = ..., fooArgument = ... }
but if terms are large (span multiple lines or use
do
notation) it might be better to use hanging,
style:let foo = Foo { fooBar = foo bar baz xi , fooArgument = ... }
c. We indent
data
definitions with multiple constructors as follows:data Foo = Bar Int Int | Baz Int Int (Maybe Bool) [Foo]
or
data Foo = -- | Bar -- Bar Int Int -- | Baz -- | Baz Int Int (Maybe Bool) [Foo]
Note the argument of
Baz
being indented by two spaces.d. Both of the following are fine
let fooBarBaz = fooBar baz let fooBarBaz = fooBar baz
whichever is more natural.
In the rare case that you want a
where
clause on alet
binding, indent by 4 spaces, like described in (a):let fooBarBaz = .. where aux = ..
e.
do
is placed after the=
:foo .. = do bar baz
Function calls can be placed on the same line as the
do
, unless this would make the line too long:foo .. = atomically $ do bar baz -- If the first line too long: foo .. = atomically $ do bar baz
The
where
block can be indented by 2 spaces, as described in (a).In a
case
, use hangingdo
:case foo of X -> do .. Y -> do ..
f. While it is technically possible to add a
where
clause to a pattern match case, use alet
instead, to emphasise that the binding is local:case x of y A x -> A_body B y -> let bl = bl_body y in B_body
Note that we align
B_body
withbl
in thelet
block. At the moment we are not being very consistent with this.Using a
where
clause for acase
can be okay, but tends to make the scope a bit confusing, so we try to avoid it. -
Line length: we limit the number of characters per line to 80.
Why: long lines are less readable (there is a reason why books and newspapers limit their line length). It's also practical: even with (ultra-)wide monitors, most people tend to have many windows side by side.
If you are going beyond 80 characters, wrap the line, introduce local bindings, etc.
Comments and docstrings should also be wrapped at 80 characters.
There are a few exceptions:
-
Sometimes alignment trumps line length. When many lines are aligned and a few of them are too long because of that, the clarity that comes from alignment (emphasising differences and similarities) can outweigh the line length limit.
-
Diagrams, examples, or long URLs in the comments can be wider than 80 characters.
For certain constructs we have concrete recommendations on how to wrap them in case their length exceeds 80 characters:
-
-
Type signatures
a. if a type signature doesn't fit on one line, wrap it like this:
fooBar :: a -> .. -> ..
When there are constraints:
fooBar :: ( Eq a , .. ) => a -> .. -> ..
Using multiple
=>
is also ok, e.g.fooBar :: forall a. Eq a => Ord a => a -> ...
When there is an explicit
forall
:fooBar :: forall a .. z. ( Eq a , .. ) => a -> .. -> ..
Note that the
.
after theforall
stays on the same line and that there is no space before it.If there is a large function argument in the type signature:
fooBar :: a -> ( forall c. Eq c => c -> .. -> .. ) -> ..
Note that the first arrow in the function argument is indented one space relative to the opening parenthesis. The above line wrapping rules apply to the nested function type as well.
b. Function calls: when not all arguments to a function call fit on a single line, either introduce clear local bindings for the arguments or put each argument on a separate line, indented 2 spaces from the function call:
fooBar x (baz + 1) bar (foo (bar x))
Why: multiple lines of multiple arguments are hard to read; for example, in
fooBar x (baz + 1) bar (foo (bar x))
it might look like
bar
is applied to(foo (bar x))
, whereas in fact of course they are both just two arguments tofooBar
. So, either everything on the same line as the function call, or else a line per argument.When writing a function call in the applicative style that does not fit on a single line, indent it as follows:
fooBar <$> x <*> baz + 1 <*> bar <*> foo (bar x)
c. Argument lists: put the formal arguments of a function on a single line when possible:
foo a b (SomeRecord {field = x}) = ..
Bracketing a pattern match on a record is optional, but we feel it aids clarity.
When that does not fit on a single line, move any pattern matches to a
where
block:foo a b c = .. where SomeRecord {field = x} = c
When that is still not enough, then the function has so many arguments that naming them is not only useful for alignment, it also helps to clarify call sites: introduce a record.
foo args = .. where Args { argA = a, argB = b, argC = SomeRecord {field = x} } = args
d. Using
RecordWildCards
for unpacking large records is discuraged.e. Class or instance contexts: when a class or instance declaration doesn't fit onto a single line because of the super-class context, wrap the line before the
=>
and align the class name with the first character in the context:class ( Eq a , .. ) => C a where instance ( Eq a , .. ) => C a where
e. Tuples in type signatures:
foo :: a -> ( .. , .. ) -> ( .. , .. )
f. Datatypes:
data Foo = Bar Arg1 Arg2 .. ArgN | Baz data Foo = Foo { longFieldName :: HasCallStack => Int -> .. }
g. Type synonyms:
type Foo a b = AVeryLongTypeHereAndItKeepsGoing Arg1 (Maybe b) Arg3 type Cts a = ( Eq a , .. , .. )
h. Function composition:
foo = h . g . f
Why: The alignment of the
.
s and the function names makes the structure easy to see at a glance.This generalises to other binary operators, e.g.,
+
,*
, etc. -
Parentheses: avoid redundant parentheses, except when they help with the order of operations. Use your judgement, and aim for clarity. Redundant parentheses sometimes help the reader, but sometimes confuse as they might suggest that they are there to disambiguate something whereas in fact there is nothing to disambiguate.
-- NO foo (Bar x) = (Bar (succ x)) -- YES foo (Bar x) = Bar (succ x) -- NO ((x + y), z) -- YES (x + y, z) -- OKAY (fromIntegral x) * y
-
Spaces: surround binary operators with a space on each side. A comma is always followed by a space.
Why: this is a general convention that is also used in text and math books. Not doing so makes it harder to read and is sloppy.
avg x y = (x + y) / 2 let ((x, y), z) = foo in (y, z)
The only exception is in tuple sections:
(,) <$> foo <*> bar (True,) <$> foo
-
Function composition and the dollar operator:
Choose between using parenthesis,
$
and.
in whichever way you think results in the most readable code. -
Blank lines Keep two empty lines between top level definitions.
This is natural and useful especially when one is using a single empty lines to delimit code sections inside a longer code block.
data Foo = Foo foo :: Foo -> () foo Foo = ()
Use this wisely, sometimes it makes sense to use a single empty line, for example to delimit short instances which all refer to the same data type.
Sometimes it's more logical too keep
Foo
andfoo
close together butBar
more distinct, e.g.data Foo = Foo foo :: Foo -> () foo Foo = () data Bar = Bar bar :: Bar -> () bar Bar = ()
Always end a file with a newline, which is not the same as a blank line.
-- NO .. <EOF> -- NO ..<EOF> -- YES .. <EOF>
Why: see this StackOverflow answer, moreover, GitHub will highlight a missing newline at the end of the file.
-
Sections: we group related definitions in sections that start with a section title. The same grouping can be replicated in the export list.
module AmazingModule ( -- Foo Foo (..) , mkFoo -- Bar , .. ) where -- -- Foo -- data Foo = .. mkFoo :: .. .. -- -- Bar -- -- Bar is bla bla -- type Bar = .. ..
The two lines of the section header are each 80 characters in total. The title is indented by two spaces. The section header can contain more text, which is separated from the first line by one empty comment line. The section header has a single blank line above and below it.
-
Comment style: in general we tend to use
--
instead of{- .. -}
. We sometimes make exceptions for big non-Haddock comments. -
Haddock formatting: we use Haddock formatting in docstrings. We also do this in comments for consistency.
-- | Short title -- -- Longer description .. 'Foo' .. "Data.String" .. @a@ .. /not/ .. -- __never__ .. called \"foo bars\" .. alternative style " foo bars " -- .. @'Foo' a@ -- -- > foo bar baz -- -- more documentation, and an empty comment line before the declaration. -- foo :: ..
Note the space before and after the
|
. We do not align the following lines with the first character of the|
.Haddock treats something between double quotes as a link to a module. So when you try to quote something, either use backslashes or extra spaces as in the example above.
We prefer
-- |
over-- ^
. We only use the latter when documenting the arguments to a constructor or a function:foo :: Word -- ^ Max size -> .. data Foo = -- | Foo -- -- .. Foo Int -- ^ @x@ (Maybe Bool) -- ^ .. long line .. -- | Baar | Baar
Note the indentation of
-- |
, the two spaces before the-- ^
, and the blank line between the constructors.Note that we leave an empty comment line as the last haddock or comment line.
We often document preconditions, invariants, and postcondition using the following style:
-- | Foo -- -- PRECONDITION: x must be greater than y -- > x > y -- -- POSTCONDITION: the result will be positive foo :: .. data Foo = Foo { -- | The bar .. -- fooBar :: Int, -- | The baz .. -- -- INVARIANT: 'fooBaz' is always greater than 7 -- fooBaz :: Int }
-
Alignment: we align things when it helps with readability.
Alignment makes it clear which things are the same and which things are different, compare the following code block
foo (Quux a b c) = bar a b c foo (Bar b c) = bar [] b c foo (FooBar a c) = bar a [] c
with the aligned version:
foo (Quux a b c) = bar a b c foo (Bar b c) = bar [] b c foo (FooBar a c) = bar a [] c
Alignment makes it easier to spot errors. For example, compare the two code blocks, where the
c
argument is forgotten on the second line:foo (Quux a b c) = bar a b c foo (Bar b c) = bar [] b c foo (FooBar a c) = bar a []
foo (Quux a b c) = bar a b c foo (Bar b c) = bar [] b c foo (FooBar a c) = bar a []
It is immediately obvious in the aligned code, but not in the unaligned code.
-
Pattern guard alignment:
This is one area in which we have not yet converged on a single style, and there are two styles in use:
foo x y z | x == y = .. | Just z' <- z , z' == x , let x' = .. = .. x' | otherwise = .. where ..
versus
foo x y z | x == y = .. | otherwise = .. where ..
Similarly for
case
:case mX of Just x | x > 100 -> .. | x > 0 -> .. _otherwise -> ..
versus
case mX of Just x | x > 100 -> .. | x > 0 -> .. _otherwise -> ..
Choose whichever style you prefer. The latter style is more suitable for hanging
do
.In either style, use of
_otherwise
instead of_
, as the latter is easy to miss. -
case vs function with multiple clauses:
The choice between using a
case
and having multiple clauses of the function can help emphasise the structure of the code, and the differences and commonalities between the cases.foo acc visited = \case [] -> .. x:xs -> ..
-
if-then-else:
When using
if-then-else
in combination withdo
, follow the following style:if foo then do bar baz else do quux bar
Why: to avoid wasting horizontal screen space.
-
Import lists: we use
stylish-haskell
to automatically format import lists. See the.stylish-haskell.yaml
file in this repo.We prefer the following order of import groups that has to be maintained manually:
-- Prelude import Prelude hiding (..) -- base + third-party non-Cardano packages import Control.Monad (mplus) import Data.Text (Text) import Data.Text qualified as Text import NoThunks.Class (NoThunks) -- cardano-prelude import Cardano.Prelude (forceElemsToWHNF) -- ouroboros-network and other network packages, -- each namespace in a separate group import Ouroboros.Network.Block (Serialised)
Each group is of course optional and must not be preceded by the comment like in the example above.
The idea behind the ordering is to start with the most general packages/modules and then go more and more specific, ending with the local package. In general, an import group will only depend on packages in import groups above it, not below it. For example, the network layer import group comes before the consensus import group, as the latter depends on the former. The Shelley ledger import group comes before the Shelley ledger consensus integration import group. In case of ties, i.e., when multiple import groups don't depend on each other, we have no real preference. We do put Byron before Shelley.
When importing modules from consensus and in particular modules from the same package, an import list and a qualifier can be omitted. For example, importing
Ouroboros.Consensus.Block
is often done without an import list as it brings many basic definitions that are relied upon in scope.When importing from other packages, we prefer to use either an import list or a qualifier.
-
Export lists: we format export lists in the following way:
module X ( .. , .. ) where
We sometimes use Haddock headings:
module X ( -- * Foo .. -- ** Foo Bar , .. -- * Bar , .. ) where
When exporting something with members, e.g., a datatype with constructors or a class with methods, we format them in the following way (note the space):
module X ( Foo (..) , Bar (MkBar) ) where
Why: this is consistent with how
stylish-haskell
formats it when importing it. (We are not particularly consistent with this at present, however.)When intentionally hiding the constructor of a datatype or newtype, we add a
-- opaque
comment after it in the export list to be explicit about this:module X ( Foo -- opaque ) where
Why: otherwise, people unfamiliar with this type might be tempted to export its constructor without realising they're hidden for a reason. This comment should make them (and the reviewer) think twice.
When re-exporting several modules from one module, use the following pattern:
module Foo (module X) where import Foo.A as X import Foo.B as X import Foo.C as X
Why: one can add extra imports without having to modify the export list.
-
Syntactic extensions: we like to use some syntactic language extensions. Some argue against having to learn additional syntax, but we believe the learning curve is minimal and using them can help improve the clarity of the code.
We like to use
LambdaCase
to avoid giving intermediate results a redundant name:-- OKAY mFoo <- getFoo case mFoo of Nothing -> .. Just foo -> .. -- OKAY getFoo >>= \case Nothing -> .. Just foo -> ..
In the second snippet, there was no need to name the intermediary
mFoo
result. Especially when its name is long or coming up with a reasonable name for it is tricky, we recommend usingLambdaCase
.The use of
MultiWayIf
is also recommended when it improves the readability:if | Set.member pt prevApplied -> Just True | Map.member hash invalid -> Just False | otherwise -> Nothing
In our opinion, this is more readable than alternatives like:
if Set.member pt prevApplied then Just True else if Map.member hash invalid then Just False else Nothing
-
Records:
For records we often use
NamedFieldPuns
to make it convenient to extract fields from the record. We also discourage the useRecordWildCards
when the fields of the record are all of the formdata SomeRecord = SomeRecord { someRecordA :: ..., someRecordB :: .. }
which is a naming convention we use a lot to avoid duplicate record fields (we do not use
DuplicateRecordFields
).To avoid long lines, it is sometimes useful to use record deconstruction in local bindings:
foo someRecord = .. where SomeRecord {someRecordA, someRecordB} = someRecord
We try to avoid partial fields, but replacing partial fields such as
data Foo = FooX {foo :: A, bar :: B} | FooY
with
data Foo = FooX A B | FooY
is not an improvement: replacing record field names with positional arguments is a big loss in clarity. Instead, introduce a record to be used as an argument to the
FooX
constructor.data X = X {foo :: A, bar :: B} data Foo = FooX X | FooY
-
Pointfree: Use your judgement when to use pointfree style and when not to use it; aim for clarity.
-
Warnings: we use the following warnings for each Cabal component:
-Wall -Wcompat -Wincomplete-uni-patterns -Wincomplete-record-updates -Wpartial-fields -Widentities -Wredundant-constraints -Wmissing-export-lists -Wunused-packages -Wno-unticked-promoted-constructors
Why: the warnings produced by the above list of flags signal code smells or enforce good practices. There is seldom a reason to disable one of them. At the time of speaking, we haven't needed any CPP yet to accomplish this.
We also keep the code entirely warning free; doing this consistently and without exception means that important warnings don't get lost. We enforce this by using
-Werror
in CI.We sometimes make exceptions for test code, e.g.,
-Wno-incomplete-uni-patterns
.For consistency, always use
-Wx
and-Wno-x
instead of-fwarn-x
and-fno-warn-x
.Most often (whenever we can), we use
-Wno-unticked-promoted-constructors
. This allows to use unticked promoted data contructors as types. -
HasCallStack: when using
error
in code paths should be impossible and are indicative of bugs, make sure enoughHasCallStack
constraints are in scope so that the error message will result in a useful callstack.Note that
HasCallStack
constraints on record fields will need manual wrappers to work properly:data API m = API { foo_ :: HasCallStack => Maybe a -> m a } foo :: HasCallStack => API m -> Maybe a -> m a foo = foo_
Without the extra wrapper
foo
, the call stack would only start atfoo_
, which is rather useless. -
Ambiguous types: we avoid
AllowAmbiguousTypes
. Instead, we add aProxy
argument for the ambiguous type variable.Why: this makes it explicit which type variable is ambiguous.
When passing the
Proxy
, useProxy @X
whereX
is the concrete type.Why: this is less verbose than
Proxy :: Proxy X
.Generally try to avoid type applications, as they are rather brittle: if the type arguments to the function change order, suddenly a function call might no longer work, often with a hard to understand error message. This gets even worse when a function doesn't have an explicit
forall
, and so the order is not even specified. Prefer to useProxy
, possibly by introducing some auxiliary functions.When the same
Proxy
can be used multiple times, one can define it locally like so:pb :: Proxy blk pb = Proxy
-
Redundant pragmas: remove unused language pragmas when possible.
Why: if a module lists the
CPP
,AllowAmbiguousTypes
,UndecidableInstances
, or any other suspicious extension, it triggers an unnecessary red flag. Even for harmless extensions, it is good practice to remove unused ones. If such extensions are needed, it's good to document why they are need (with an exception ofCPP
).Tip: HLint can warn you about some unused pragmas.
There are more general guidelines on how we write and structure code.
-
Scope: We try to be careful about scope, clarifying where a variable is relevant and where it is not. For example, in
foo x y z = .. where ..
all of
x, y, z
will be in scope in thewhere
clause. If they aren't relevant, limit their scope:foo x = \y z -> .. where ..
this also can help to avoid awkward variable names.
Similarly, choosing
where
overlet
can help to clarify which variables are in scope in those definitions. Writingfoo x = do .. where y = ...
makes it very clear that the definition of
y
does not depend on anything that has happened within thedo
block (it depends only on the formal parameters of the function). The flipside of this is thaty
is then scoped over the entire function body; that typically is less likely to result in confusion (especially for local function definitions), but if it is useful to emphasise thaty
is only used in a small part of the function body, or that this avoids awkward naming, then feel free to uselet
to express that. Use your judgement: use scope wisely. -
Tuples: We generally prefer records over tuples with lots of arguments; positional arguments (in tuples or as arguments to constructors) provide less clues what the arguments are used for.
-
Orphans: Orphans are generally considered bad practice, but unfortunately avoiding orphans at all cost often means being unable to split a module into smaller parts. The reason orphans are considered bad practice is that they might lead to incoherence; we prefer the ability to split modules into smaller parts and accept the loss of the help of the compiler to avoid incoherence as an acceptable compromise.
Orphans in test suites are also acceptable.
-
Assertions: If it helps to explain what a function does, we try to be clear about preconditions, postconditions, and invariants. When possible, it is useful to reinforce such invariants with assertions so that if our reasoning turns out to be invalid, we will notice. The use of
Ouroboros.Consensus.Util.Assert.assertWithMsg
is preferred overassert
, so that if the assertion fails, we get some kind of informative error message rather than just a Prolog-like "no". -
Test packages: In order to make test code from test suite A available in test suite B, we define test suites as a test suite library which is then used by a test suite executable; the test suite library can then be reused by other test suites as well.
When there are multiple test suites within a single package, it is possible to share some code between them by making the same source code directory available to all of them. However, doing so would make it impossible to use that shared code in a test suite defined in another package. To avoid this problem, we avoid sharing source directories in
cabal
files.