Add "extended errors" #299
Conversation
Refactor things a wee bit to add a decoder struct. This doesn't really *do* anything right now, but will be needed to add options, such as "strict mode" in #197 or "extended errors" in #299. Also add DecodeFS() as a companion to DecodeFile() to read from a fs.FS, and since using a reader is the default with NewDecoder() (and was already easy to do before with strings.NewReader()) I marked DecodeReader() as deprecated.
arp242
force-pushed
the
exterr
branch
2 times, most recently
from
d45efc1 to
5a19592
Compare
arp242
force-pushed
the
exterr
branch
2 times, most recently
from
154bb76 to
075c209
Compare
arp242
force-pushed
the
exterr
branch
4 times, most recently
from
d6f7ab6 to
dd29c79
Compare
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
|
Note that the removal of |
|
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. |
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. |
For compatibility; mark it as deprecated, but ensure it's still set to the correct value. See comments on #299; Will remove in eventual v2 release.
|
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" |
|
@arp242 By the way, two tips:
|
|
Ah, I didn't know about the 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. |
|
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 and not The latter will not associate the comment with the field. |
|
Hm, in which cases does it fail? If I remember correctly both styles should be associated with 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 https://godocs.io/zgo.at/termfo#Terminfo |
Actually, turns out that's not true and I did misremember: it's available as 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. |
|
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) |
|
I think in practice many tools fall back to |
The lexer and parser now always return a ParseError; the Error() method
is mostly unchanged:
This adds an ErrorWithLocation() method, which will add some context
where the error occurred, similar to e.g. clang or the Rust compiler:
And the ErrorWithUsage() also adds some usage guidance (not always
present):
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