Add "extended errors"

[arp242]

The lexer and parser now always return a ParseError; the Error() method
is mostly unchanged:

toml: line 1 (last key "x.key"): newlines not allowed within inline tables

This adds an ErrorWithLocation() method, which will add some context
where the error occurred, similar to e.g. clang or the Rust compiler:

toml: error: newlines not allowed within inline tables

At line 1, column 18:

      1 | x = [{ key = 42 #
			   ^

And the ErrorWithUsage() also adds some usage guidance (not always
present):

toml: error: newlines not allowed within inline tables

At line 1, column 16:

      1 | x = [{ key = 42
			 ^
Error help:

	Inline tables must always be on a single line:

	    table = {key = 42, second = 43}

	It is invalid to split them over multiple lines like so:

	    # INVALID
	    table = {
		key    = 42,
		second = 43
	    }

	Use regular for this:

	    [table]
	    key    = 42
	    second = 43

The line/column information should now also always be correct, and a
number of error message have been tweaked a bit.

Fixes #201
Fixes #217

[cretz]

Note that the removal of Line from the ParseError struct here has broken other tools such as the popular https://staticcheck.io/ because it references it at https://github.com/dominikh/go-tools/blob/ce0ed4dfd45dccf9c3b0b6a923f2c65f73031b45/go/loader/loader.go#L330.

[arp242]

Oops; I think somewhere along the long I forgot that ParseError was already exported. I'll make sure it's fixed before the next release; thanks for the heads-up @cretz.

[dominikh]

Note that the removal of Line from the ParseError struct here has broken other tools such as the popular https://staticcheck.io/ because it references it at https://github.com/dominikh/go-tools/blob/ce0ed4dfd45dccf9c3b0b6a923f2c65f73031b45/go/loader/loader.go#L330.

Note that it doesn't really affect us, because of Go modules. Staticcheck's main module builds with v0.4.1 of this library, and we can update to the new API when you make a new release.

[arp242]

Yeah, but I'd like to make sure the releases remain compatible (technically 0.x don't promise compatibility, but I just treat it as-if it's 1.x, with the current versioning scheme predating modules; maybe I should just release 1.0 as the next version).

Anyway, I added a "deprecated" Line field; it's not a lot of trouble, and I'll remove that in an eventual v2 together with some other deprecated things.

[dominikh]

@arp242 By the way, two tips:

1. The official way of deprecating things is by saying Deprecated: ... (with that specific casing). Tools pick up that pattern. See https://github.com/golang/go/wiki/Deprecated
2. Documentation has to precede the identifier it is documenting, so Field int // Field does X isn't technically correct (and tools won't pick it up as documentation).

[arp242]

Ah, I didn't know about the Deprecated: -convention.

AFAIK it's okay to have struct field comments not start with the identifier name; it's just the package comment, function comments, type comments, etc. that need to follow that format.

[dominikh]

Sorry, my point wasn't that the documentation has to start with the identifier name, but that the documentation has to be written before (above, really) the identifier being documented. That is, you want

type T struct {
  // This is an amazing field
  X int
}

and not

type T struct {
  X int // This is an amazing field
}

The latter will not associate the comment with the field.

[arp242]

Hm, in which cases does it fail? If I remember correctly both styles should be associated with go/ast.Field.Doc when parsing Go code, so all tools should work withthis. I'm fairly sure I checked this a few years ago to be sure, but it could be I'm misremembering this.

As far as I can find all tools work with it anyway; for example this other struct uses a mix of both styles, and it seems to work well in go doc, gopls "show documentation", and on the pkg.go.dev and godocs.io sites:

https://godocs.io/zgo.at/termfo#Terminfo
https://pkg.go.dev/zgo.at/termfo#Terminfo

[arp242]

If I remember correctly both styles should be associated with go/ast.Field.Doc when parsing Go code

Actually, turns out that's not true and I did misremember: it's available as Comment but not Doc: https://play.golang.org/p/690_QeLgLur

Still, I always much preffered this style as it keeps the struct more readable/clearer when you have just a few short comments to add. So unless there's a serious practical problem I'd prefer to keep it this way.

[dominikh]

Heh, I just finished writing https://play.golang.org/p/X7lKgHo93r5.

As far as "serious practical problems" go, I was going to list all the ways in which tools won't be able to pick up the comments as documentation. But seeing how there's precedent for your style of comments in the Go standard library (see e.g. go/ast.File), maybe this is something that should be fixed elsewhere instead (such as golang/go#20744)

[arp242]

I think in practice many tools fall back to Comment if Doc is empty, at least for struct fields. That's what e.g. gopls does.