Discourage use of RecordWildCards

[chris-martin]

This came up in Slack today, bringing it here for codification or further discussion.

The case for RecordWildCards

The RecordWildCards extension allows using .. in record patterns to bring all of a record's fields into scope, and in record-constructing expressions to implicitly pull appropriately-named values from anywhere in scope into a record value.

Record wildcards are often nice, especially in conjunction with ApplicativeDo, for codecs.

data StudentStats = StudentStats
  { assignmentType :: MathAssignmentType
  , completedAt    :: Maybe UTCTime
  , timeSpent      :: Int }

instance HasObjectCodec StudentStats where
  objectCodec = do
    assignmentType <- requiredField       "assignmentType" "" .= (.assignmentType)
    completedAt    <- optionalFieldOrNull "completedAt"    "" .= (.completedAt)
    timeSpent      <- requiredField       "timeSpent"      "" .= (.timeSpent)
    pure StudentStats {..}

They're also convenient when converting between types that share a lot of fields of the same name.

data Assignment = Assignment
  { id        :: AssignmentId
  , skill     :: RlSkill
  , createdAt :: UTCTime }

data ApiAssignment = ApiAssignment
  { id        :: AssignmentId
  , skill     :: RlSkill
  , createdAt :: UTCTime
  , title     :: Text }

toApiAssignment :: Something m => Assignment -> m ApiAssignment
toApiAssignment x@Assignment{..} = do
  title <- getTitle x
  pure ApiAssignment{..}

The case against RecordWildCards

The biggest complaint I hear (and feel) is that the convenience of what RecordWildCards allows you to make implicit when writing code comes at the expense of what you wish were explicit when you're reading or searching the code.

• When a record is destructured using .., within the pattern's scope is can be unclear which identifiers came from the record and which came from imports.
• When a value is copied from one record to another using .. at both the use and binding site, it passes through without being explicitly named at all, making it difficult to search for a field's use sites.
• It can make it difficult to trace and to gain a general understand what's going on by reading the code.

Some of the same convenience without the drawbacks is afforded by NamedFieldPuns, which we have enabled everywhere. Revisiting the above examples, without wildcards we can write:

instance HasObjectCodec StudentStats where
  objectCodec = do
    assignmentType <- requiredField       "assignmentType" "" .= (.assignmentType)
    completedAt    <- optionalFieldOrNull "completedAt"    "" .= (.completedAt)
    timeSpent      <- requiredField       "timeSpent"      "" .= (.timeSpent)
    pure StudentStats {assignmentType, completedAt, timeSpent}

toApiAssignment x@Assignment{id, skill, createdAt} = do
  title <- getTitle x
  pure ApiAssignment{id, skill, createdAt, title}

A separate related issue I hear (and feel) complaints about is whether we should encourage the use of more explicit import lists rather than relying on whole-module imports. It is essentially the same question - How much do you want in scope that has not been explicitly enumerated - and a very similar convenience-when-writing vs searchability-when-reading tradeoff.

The middle ground

It is an open question, I think, whether the guide should say "never use RecordWildCards" or whether we should suggest using it sparingly. In some small, focused modules I think it is mostly harmless, and would not be entirely opposed to enabling it with a LANGUAGE pragma where appropriate. If we go this route, there's then a question of whether we want to discourage RecordWildCards by turning off the extension by default, or to leave it on and simply discourage its use by verbal suggestion.

[pbrisbin]

So it sounds like we have the following potential Guides:

1. Don't use RecordWildCards
2. Avoid RecordWildCards
3. Use RecordWildCards freely, but enable it by pragma where you do it (this PR)
4. Use RecordWildCards freely, and it's on by default (status quo)

I'm cool with any of these, actually. So I'll leave it to others to decide.

[chris-martin]

Seems like consensus on turning it off by default, not quite consensus on how strongly to discourage, so I'm merging just the small change.