Illegal .. notation for constructor LogOutView. The constructor has no labeled field

[mpscholten]

Given this code:

data LogOutView = LogOutView

instance View LogOutView where
    html LogOutView { .. } = [hsx||]

GHC errors with:

Illegal .. notation for constructor LogOutView. The constructor has no labeled field

A better error message would be:

The 'data LogOutView' has no fields, so you cannot use 'LogOutView { .. }' here. Perhaps remove the '{ .. }' here, or add some fields to 'data LogOutView'? 

(Taken from https://github.com/digitallyinduced/haskell-ux)

[goldfirere]

Good point. Can I suggest a different phrasing of the message?

The data constructor 'LogOutView' does not have named record fields,
so a pattern match 'LogOutView { .. }' is incorrect.
Possible fixes:
 * Replace the pattern 'LogOutView { .. }' with 'LogOutView'
 * Replace the pattern 'LogOutView { .. }' with 'LogOutView {}'. This version works even if
   you add/remove fields to 'LogOutView' later.
 * Add named record fields to the declaration of constructor 'LogOutView' at LogOutView.hs:3.

Note that this error message can arise even if the constructor has fields -- as long as they aren't record fields. In the first "possible fix", the suggested replacement would have dummy variables (or maybe blanks?) for each field of the constructor.

What do we think?

[Ericson2314]

Perhaps this should just be legal? An empty data type has not "committed" to having fields that are only positional. FWIW this is how rust does it, with:

struct Foo;
struct Foo();
struct Foo {}

all meaning the same thing.

[goldfirere]

I don't think #15 (comment) is quite relevant for this ticket -- this ticket is about a pattern-match, while that comment looks like it is about structure declarations. Regardless, this repo is all (and only!) about error messages, not about changing the language, which is discussed elsewhere.

[Ericson2314]

Well my point is that as a new user,

data Foo = Foo { a :: Int }

f (Foo {..}) = 1

being allowed, but

data Foo = Foo { }

f (Foo {..}) = 1

not being allowed, feels arbitrary and annoying.

A big message from functional programming is that "base cases are not edge cases", e.g. when teaching structural recursion. GHC special casing for the "empty record" for no good reason feels like a betrayal of the principles Haskell is supposed to espouse.

I am happy to open an issue elsewhere, but I do think it is perfectly natural for auditing error messages to lead us to question language decisions. If there isn't in fact a problem, the best error message is none at all!

[Ericson2314]

Separately, while testing this out, I realized the "no fields" error message supersedes the "RecordWildCards is disabled" error message for this. That's a bug!

[Ericson2314]

I opened language-change issue in ghc-proposals/ghc-proposals#484

[nomeata]

If the proposal doesn't go through, the error message should at least point out Foo{} as the recommended way to match on a constructer in a way that explicitly says “i don't care about the arguments and want this code to work even if that changes”

[goldfirere]

OK. To my own surprise, I'm actually convinced here that the GHC proposal is a better fix than any rewording for the empty-constructor case. Thanks for opening that!

But the error message phrasing still could be improved for the non-empty case:

data Foo = MkFoo Int Bool

blah :: Foo -> Int
blah (MkFoo {}) = 5

This is accepted. But

blah (MkFoo { .. }) = 5

is rejected with the error message at the top of this ticket:

    Illegal `..' notation for constructor ‘MkFoo’
      The constructor has no labelled fields

And I still think that a message such as #15 (comment) would be a nice improvement.

[Ericson2314]

:) Glad it was persuasive! I will try to make it into an actual proposal soon.

I agree it still needs a reword.

The data constructor 'LogOutView' does not have named record fields

If we are to accept the proposal, I might further tweak your suggestion as

The fields of the data constructor 'LogOutView' are not name named

or

The data constructor 'LogOutView' is not a record constructor

since "have named record fields" to me implies there must be at least one (named) field.

[goldfirere]

Fair point. Except that I don't want to say "record constructor" because data T = MkT does not look like a record constructor, and yet the proposal suggests that f (MkT { .. }) should be acceptable.

Here is the full message for consideration:

The data constructor 'LogOutView' has unnamed fields,
so a pattern match 'LogOutView { .. }' is incorrect.
Possible fixes:
 * Replace the pattern 'LogOutView { .. }' with 'LogOutView _ _'
 * Replace the pattern 'LogOutView { .. }' with 'LogOutView {}'. This version works even if
   you add/remove fields to 'LogOutView' later.
 * Add named record fields to the declaration of constructor 'LogOutView' at LogOutView.hs:3.

where the number of _s after LogOutView in the first possible fix is the number of fields in the constructor.

[Ericson2314]

Perfect!

Yes "record structure" has those problem, but my first one felt a bit too verbose. "has unnamed fields" avoids both pitfalls!

[mpscholten]

This has been fixed via ghc-proposals/ghc-proposals#496

[noughtmare]

@mpscholten that proposal doesn't seem to be implemented yet. I think we should keep issues here open until the improvements have actually been incorporated in a released version of GHC.

[googleson78]

Hey all! I'm the one implementing ghc-proposals/ghc-proposals#496. Since you are somewhat invested in error messages and in this one in particular already, I would appreciate it if you gave your opinion(s) regarding what the error message should be in the case that {..} is used with a constructor that has positional args.

Here's the relevant part of the patch.