File layout
The different elements of a sample file have been identified below:
{-# LANGUAGE LambdaCase #-} -- (1) {-# LANGUAGE TypeFamilies #-} -- | -- (2) -- Module : Test.Spec.Rename -- Copyright : (c) 2017 Michael Walker -- License : MIT -- Maintainer : Michael Walker <mike@barrucadu.co.uk> -- -- Functions for projecting expressions into a consistent namespace. module Test.Spec.Rename -- (3) ( -- * Projections projections , rename , allRenamings -- ** The @These@ type , These(..) ) where import Control.Arrow (second) -- (4) import qualified Data.Typeable as T import Test.Spec.Expr (Expr, environment) import Test.Spec.Type (rawTypeRep) -- | The @These@ type is like 'Either', but also has the case for when -- (5) -- we have both values. data These a b = This a | That b | These a b deriving (Eq, Show) ------------------------------------------------------------------------------- -- (6) -- Projections -- | Find all type-correct ways of associating environment variables. projections :: Expr s1 m1 h1 -> Expr s2 m2 h2 -> [[(These String String, TypeRep)]] projections e1 e2 = projectionsFromEnv (env e1) (env e2) where env = map (second rawTypeRep) . environment
- Language pragmas:
- Alphabetical order
- One per line
- Do not align the closing
#-}
s
- Module header:
- Do not use the
stability
orportability
fields
- Do not use the
- Export list:
- One entry per line, with Haddock-style headings (
-- *
,-- **
, and so on) to divide it up - The list may be omitted for small or internal modules
- One entry per line, with Haddock-style headings (
- Imports:
- Split into two groups: modules from outside the project and modules from inside the project
- Add sufficient spacing between the
import
and the module name to include thequalified
keyword - Align import lists and
as
qualifications across both groups (even though there is a blank line) - If only importing instances, use this style:
import Foo.Instances ()
- Put as many import specs on same line as possible, wrapping at 80 characters
- Align import list on lines after the import under the start of the module name
- There is no space between classes/types and the list of its members:
import Data.Foldable (Foldable(fold))
- Definitions:
- One blank line between definitions
- Separators:
- Definitions may be separated into named groups, where a group starts with two blank lines, a sequence of 79 ’-’s, a title, and another single blank line
- Consider multiple modules, if one file has many groups
- Groups do not need to correspond exactly to sections in the export list: the export list is for users of the module, the groups are for its developers
Furthermore, lines should use UNIX-style (lf) line endings and have no trailing whitespace. The file should end with a single newline.
stylish-haskell should be used to format the language pragmas, imports, and whitespace. The bullet points are just an English-language description of the effect of this configuration:
steps: - imports: align: global list_align: after_alias long_list_align: inline empty_list_align: right_after list_padding: module_name separate_lists: false - language_pragmas: style: vertical align: false remove_redundant: true - trailing_whitespace: {} columns: 80 newline: lf
In case of doubt, the tool takes precedence.
Formatting
One day I’d like to use a tool to format all my code for me (like gofmt or rustfmt), but:
- hindent is unusable and I just dislike its output, which is particularly unfortunate as it supports almost no configuration.
- brittany is nice, but still in early stages.
Line length
Soft limit of 80 characters, harder limit of 110 characters, no absolute limit.
Lines longer than 80 characters are acceptable if breaking the line would introduce ugliness in some other way.
Alignment
Similar items on adjacent lines may be surrounded with extra spaces to align them, if doing so would not introduce a large amount of extra spacing. Items on lines which are not adjacent, ignoring lines consisting solely of whitespace or comments, should not be aligned.
Good:
instance NFData DPOR where rnf dpor = rnf ( dporRunnable dpor , dporTodo dpor , dporDone dpor , dporSleep dpor , dporTaken dpor , dporAction dpor )
Bad:
case rest of ((_, runnable, _):_) -> map fst runnable [] -> []
Very bad:
case M.lookup tid' (dporDone dpor) of Just dpor' -> let done = M.insert tid' (grow state' tid' rest dpor') (dporDone dpor) in dpor { dporDone = done } Nothing -> let taken = M.insert tid' a (dporTaken dpor) in dpor { dporTaken = if conservative then dporTaken dpor else taken }
Indentation
Do not use tabs, indent with spaces. Use two spaces for each indentation level.
canInterrupt :: DepState -> ThreadId -> ThreadAction -> Bool canInterrupt depstate tid act | isMaskedInterruptible depstate tid = case act of BlockedPutMVar _ -> True BlockedReadMVar _ -> True BlockedTakeMVar _ -> True BlockedSTM _ -> True BlockedThrowTo _ -> True _ -> False | isMaskedUninterruptible depstate tid = False | otherwise = True autocheckWayIO :: (Eq a, Show a) => Way -> MemType -> ConcIO a -> IO Bool autocheckWayIO way memtype concio = dejafusWayIO way memtype concio autocheckCases
Type signatures
If a type signature is too long to fit on a single line, or you want to add comments to the individual parameters, it can be broken over multiple lines.
Indentation is with two spaces, and includes the ::
or =>
(for the first parameter) and ->
(for subsequent parameters).
incorporateTrace :: MemType -- ^ Memory model -> Bool -- ^ Whether the \"to-do\" point which was used to create this new -- execution was conservative or not. -> Trace -- ^ The execution trace: the decision made, the runnable threads, -- and the action performed. -> DPOR -> DPOR
Typeclass constraints should remain on the same line as the function name:
runSCT :: MonadRef r n => Way -- ^ How to run the concurrent program. -> MemType -- ^ The memory model to use for non-synchronised @CRef@ operations. -> ConcT r n a -- ^ The computation to run many times. -> n [(Either Failure a, Trace)]
Data declarations
If there are multiple constructors, break over lines like so:
data Way where Systematically :: Bounds -> Way Randomly :: RandomGen g => g -> Int -> Way data IdSource = Id { _nextCRId :: Int , _nextMVId :: Int , _nextTVId :: Int , _nextTId :: Int , _usedCRNames :: [String] , _usedMVNames :: [String] , _usedTVNames :: [String] , _usedTNames :: [String] } deriving (Eq, Ord, Show) data ThreadAction = Fork ThreadId | MyThreadId | GetNumCapabilities Int | SetNumCapabilities Int | Yield deriving (Eq, Show)
The deriving
clause comes on a new line with one level of indentation.
Type declarations
If the original type is too long to fit on a single line, break it up just as you would a function type signature:
type Scheduler state = [(Decision, ThreadAction)] -> Maybe (ThreadId, ThreadAction) -> NonEmpty (ThreadId, Lookahead) -> state -> (Maybe ThreadId, state)
List and tuple declarations
Align the elements in the list, with commas and brackets on the left:
failures :: [Failure] failures = [ InternalError , Abort , Deadlock , STMDeadlock , UncaughtException , IllegalSubconcurrency ]
You may optionally avoid the newline, if it looks better in this case:
failures :: [Failure] failures = [ InternalError , Abort , Deadlock , STMDeadlock , UncaughtException , IllegalSubconcurrency ]
Tuples larger than three elements should be avoided. Sometimes large tuples are useful though (for example, in writing NFData
instances), in which case they are formatted like lists.
Case expressions
Case expressions may be indented in either of these ways:
(~=) :: Thread n r -> BlockedOn -> Bool thread ~= theblock = case (_blocking thread, theblock) of (Just (OnMVarFull _), OnMVarFull _) -> True (Just (OnMVarEmpty _), OnMVarEmpty _) -> True (Just (OnTVar _), OnTVar _) -> True (Just (OnMask _), OnMask _) -> True _ -> False stepThrow t ts act e = case propagate (toException e) t ts of Just ts' -> simple ts' act Nothing | t == initialThread -> pure (Left UncaughtException, Single act) | otherwise -> simple (kill t ts) act
Pragmas
Put pragmas between the type signature and the definition of the function they apply to.
concatPartition :: (a -> Bool) -> [[a]] -> ([a], [a]) {-# INLINE concatPartition #-} concatPartition p = foldl (foldr select) ([], []) where select a ~(ts, fs) | p a = (a:ts, fs) | otherwise = (ts, a:fs)
If the function has no type signature (it’s a local definition in a where
clause, for instance), put the pragma immediately before.
Hanging lambdas
Lines after a hanging lambda should be indented:
forkFinally :: MonadConc m => m a -> (Either SomeException a -> m ()) -> m (ThreadId m) forkFinally action and_then = mask $ \restore -> fork $ Ca.try (restore action) >>= and_then
where
clauses
If a function contains a where
clause, the where
should be on the opening line of the function, if short enough:
representative :: Eq a => Predicate a -> Predicate a representative p xs = result { _failures = choose . collect $ _failures result } where result = p xs collect = groupBy' [] ((==) `on` fst) choose = map $ minimumBy (comparing $ \(_, trc) -> (preEmps trc, length trc))
If the function body includes a linebreak, both it and the where
-body should be indented by a further two spaces and the where
put on a new line:
findInstance :: Expr s1 m1 h1 -> Expr s2 m2 h2 -> Maybe [(String, [String])] findInstance eG eS | eS `isInstanceOf` eG = Just nameMap | otherwise = Nothing where env = map fst . environment' nameMap = map (\((s,g):sgs) -> (s, nub (g:map snd sgs))) . groupBy ((==) `on` fst) . sortOn fst $ zip (env eS) (env eG)
Avoid nested where
clauses and consider making multiple top-level definitions instead.
let
expressions
Multiple lines inside the let
and in
should be aligned:
grow state tid trc@((d, _, a):rest) dpor = let tid' = tidOf tid d state' = updateDepState state tid' a in case M.lookup tid' (dporDone dpor) of Just dpor' -> let done = M.insert tid' (grow state' tid' rest dpor') (dporDone dpor) in dpor { dporDone = done } Nothing -> let taken = M.insert tid' a (dporTaken dpor) sleep = dporSleep dpor `M.union` dporTaken dpor done = M.insert tid' (subtree state' tid' sleep trc) (dporDone dpor) in dpor { dporTaken = if conservative then dporTaken dpor else taken , dporTodo = M.delete tid' (dporTodo dpor) , dporDone = done } grow _ _ [] _ = err "incorporateTrace" "trace exhausted without reading a to-do point!"
if-then-else
expressions
Guards and pattern matches should be preferred over if-then-else
expressions generally, use your judgement.
Outside of do
-notation, if the entire expression does not fit on a single line, the then
and else
should begin in the same column as the if
:
($$) :: Ord h => Expr s m h -> Expr s m h -> Maybe (Expr s m h) f $$ e = case funTys (exprTypeRep f) of Just (fArgTy, fResTy) -> if fArgTy == ignoreTypeRep && isJust (unmonad $ exprTypeRep e) then mkfun fResTy else mkfun =<< exprTypeRep f `funResultTy` exprTypeRep e Nothing -> Nothing
In do
-notation the then
and else
should be indented another level:
checkFile :: (MonadError FileError m, MonadIO m) => FileConfig -> FilePath -> Int64 -> m () checkFile fcfg fname bytes = do let maxbytes = maxSizeInBytes fcfg if bytes > maxbytes then throwError (FileTooLarge bytes maxbytes) else do ok <- liftIO (additionalRules fcfg fname) case ok of Right _ -> pure () Left e -> throwError (FileDisallowed e)
Naming
Use camelCase for values and PascalCase for types and constructors.
Don’t capitalise all letters when using an abbreviation: HttpServer
, not HTTPServer
. Two-letter acronyms are an exception to this.
For type variables be consistent across type signatures, and consider a (short) name if there is one more meaningful than just a single letter.
check :: MonadSTM stm => Bool -> stm () throwSTM :: (MonadSTM stm, Exception e) => e -> stm a catchSTM :: (MonadSTM stm, Exception e) => stm a -> (e -> stm a) -> stm a liftedOrElse :: (MonadTransControl t, MonadSTM stm) => (forall x. StT t x -> x) -> t stm a -> t stm a -> t stm a
Primes may be used to indicate that two values are related. The number 0 may be used as a suffix to indicate that a value is “initial” or “default” in some way.
evaluateDyn :: Monad m => Term s m -> [(String, Dynamic s m)] -> Maybe (s -> Dynamic s m) evaluateDyn e0 globals | all check (environment e0) = Just (go [] e0) | otherwise = Nothing where go _ (Lit _ _ dyn) _ = dyn go locals (Var _ var) _ = env locals var go locals (Let ty True _ b e) s = let mx = unwrapMonadicDyn (go locals b s) in unsafeWrapMonadicDyn ty $ mx >>= \x -> unwrapMonadicDyn (go (x:locals) e s) go locals (Let _ False _ b e) s = let x = go locals b s in go (x:locals) e s go locals (Ap _ f e) s = let f' = go locals f s e' = go locals e s in f' `dynApp` (if ignoreArg f' then ignore e' else e') go _ StateVar s = toDyn s
Numbers greater than 0 may be used to indicate two values are similar but unrelated.
dependent :: MemType -> DepState -> (ThreadId, ThreadAction) -> (ThreadId, ThreadAction) -> Bool dependent memtype ds (t1, a1) (t2, a2) = case rewind a2 of Just l2 | isSTM a1 && isSTM a2 -> not . S.null $ tvarsOf a1 `S.intersection` tvarsOf a2 | not (isBlock a1 && isBarrier (simplifyLookahead l2)) -> dependent' memtype ds t1 a1 t2 l2 _ -> dependentActions memtype ds (simplifyAction a1) (simplifyAction a2)
Comments
Write correctly punctuated and spelled English sentences. Haddock comments should have one space between the starting symbol and the text:
-- | This is good. -- |This is bad.
Haddock comments on type parameters and data constructors should use the -- ^
style (not the -- |
style):
incorporateTrace :: MemType -- ^ Memory model -> Bool -- ^ Whether the \"to-do\" point which was used to create this new -- execution was conservative or not. -> Trace -- ^ The execution trace: the decision made, the runnable threads, -- and the action performed. -> DPOR -> DPOR -- | The scheduler state data DPORSchedState = DPORSchedState { schedSleep :: Map ThreadId ThreadAction -- ^ The sleep set: decisions not to make until something dependent -- with them happens. , schedPrefix :: [ThreadId] -- ^ Decisions still to make , schedBPoints :: Seq (NonEmpty (ThreadId, Lookahead), [ThreadId]) -- ^ Which threads are runnable at each step, and the alternative -- decisions still to make. , schedIgnore :: Bool -- ^ Whether to ignore this execution or not: @True@ if the -- execution is aborted due to all possible decisions being in the -- sleep set, as then everything in this execution is covered by -- another. , schedBoundKill :: Bool -- ^ Whether the execution was terminated due to all decisions being -- out of bounds. , schedDepState :: DepState -- ^ State used by the dependency function to determine when to -- remove decisions from the sleep set. } deriving (Eq, Show)
Warnings and linting
Code should compile with -Wall
with no warnings except for unnecessary imports, if you support multiple versions of libraries which have changed their API. Don’t introduce CPP simply to avoid warnings.
Use hlint to lint your code, this .hlint.yaml
configuration file will work for version 2 and later:
# Module export lists should generally be preferred, but may be # omitted if the module is small or internal. - ignore: {name: Use module export list} # Record patterns are just ugly. - ignore: {name: Use record patterns} # Prefer applicative operators over monadic ones. - suggest: {name: Generalise monadic functions, lhs: return, rhs: pure}
All lints should be fixed. However, sometimes hlint is too zealous, and there is a genuine reason why a lint cannot be fixed. Such cases may be added to the configuration file with a comment explaining why:
# GHC treats infix $ specially wrt type checking, so that things like # "runST $ do ..." work even though they're impredicative. # Unfortunately, this means that HLint's "avoid lambda" warning for # this module would lead to code which no longer compiles! - ignore: {name: Avoid lambda, within: Test.DejaFu.Conc}
Remember the comment! You are creating a tooling-approved code smell!