Ensure ParseError.Message is always set #411
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ParseError.Error() did:
msg := pe.Message
if msg == "" {
msg = pe.err.Error()
}
That was fine, but not very useful for people wanting to access the
Message field themselves, especially since err isn't exported.
For example staticcheck does this to add in the filename, and the error
is lost, because Message is often blank:
% staticcheck
staticcheck.conf:5:0: (last key parsed: "dot_import_whitelist") (compile)
We now only use the err field to determine which error usage to display
in ErrorWithUsage()
arp242
added a commit
to arp242/go-tools
that referenced
this pull request
May 26, 2024
Previously the error message would get lost:
% staticcheck
staticcheck.conf:5:0: (last key parsed: "dot_import_whitelist") (compile)
This is actually my bug; the ParseError.Message wasn't always set; fixed
with: BurntSushi/toml#411
Also print a more verbose errors, which is nicer IMHO:
% staticcheck
staticcheck.conf:5:60: toml: error: expected a comma (',') or array terminator (']'), but got '"'
At line 5, column 60:
3 |
4 | checks = ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023"]
5 | dot_import_whitelist = ["github.com/mmcloughlin/avo/build" "github.com/mmcloughlin/avo/operand", "github.com/mmcloughlin/avo/reg"]
^
(config)
% staticcheck -f json
/home/martin/src/go-tools/staticcheck.conf
(5, 60) config toml: error: expected a comma (',') or array terminator (']'), but got '"'
At line 5, column 60:
3 |
4 | checks = ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023"]
5 | dot_import_whitelist = ["github.com/mmcloughlin/avo/build" "github.com/mmcloughlin/avo/operand", "github.com/mmcloughlin/avo/reg"]
^
✖ 1 problems (0 errors, 1 warnings, 0 ignored)
% staticcheck -f json | jfmt
{
"code": "config",
"end": {"column": 0, "file": "", "line": 0},
"location": {"column": 60, "file": "/home/martin/src/go-tools/staticcheck.conf", "line": 5},
"message": "toml: error: expected a comma (',') or array terminator (']'), but got '\"'\n\nAt line 5, column 60:\n\n 3 | \n 4 | checks = [\"all\", \"-ST1000\", \"-ST1003\", \"-ST1016\", \"-ST1020\", \"-ST1021\", \"-ST1022\", \"-ST1023\"]\n 5 | dot_import_whitelist = [\"github.com/mmcloughlin/avo/build\" \"github.com/mmcloughlin/avo/operand\", \"github.com/mmcloughlin/avo/reg\"]\n
^\n",
"severity": "warning"
}
arp242
added a commit
to arp242/go-tools
that referenced
this pull request
May 26, 2024
Previously the error message would get lost:
% staticcheck
staticcheck.conf:5:0: (last key parsed: "dot_import_whitelist") (compile)
This is actually my bug; the ParseError.Message wasn't always set; fixed
with: BurntSushi/toml#411
Also print a more verbose errors, which is nicer IMHO:
% staticcheck
staticcheck.conf:5:60: toml: error: expected a comma (',') or array terminator (']'), but got '"'
At line 5, column 60:
3 |
4 | checks = ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023"]
5 | dot_import_whitelist = ["github.com/mmcloughlin/avo/build" "github.com/mmcloughlin/avo/operand", "github.com/mmcloughlin/avo/reg"]
^
(config)
% staticcheck -f stylish
/home/martin/src/go-tools/staticcheck.conf
(5, 60) config toml: error: expected a comma (',') or array terminator (']'), but got '"'
At line 5, column 60:
3 |
4 | checks = ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023"]
5 | dot_import_whitelist = ["github.com/mmcloughlin/avo/build" "github.com/mmcloughlin/avo/operand", "github.com/mmcloughlin/avo/reg"]
^
✖ 1 problems (0 errors, 1 warnings, 0 ignored)
% staticcheck -f json | jfmt
{
"code": "config",
"end": {"column": 0, "file": "", "line": 0},
"location": {"column": 60, "file": "/home/martin/src/go-tools/staticcheck.conf", "line": 5},
"message": "toml: error: expected a comma (',') or array terminator (']'), but got '\"'\n\nAt line 5, column 60:\n\n 3 | \n 4 | checks = [\"all\", \"-ST1000\", \"-ST1003\", \"-ST1016\", \"-ST1020\", \"-ST1021\", \"-ST1022\", \"-ST1023\"]\n 5 | dot_import_whitelist = [\"github.com/mmcloughlin/avo/build\" \"github.com/mmcloughlin/avo/operand\", \"github.com/mmcloughlin/avo/reg\"]\n
^\n",
"severity": "warning"
}
arp242
added a commit
to arp242/go-tools
that referenced
this pull request
May 26, 2024
Previously the error message would get lost:
% staticcheck
staticcheck.conf:5:0: (last key parsed: "dot_import_whitelist") (compile)
This is actually my bug; the ParseError.Message wasn't always set; fixed
with: BurntSushi/toml#411
Now it prints the expected:
% staticcheck
staticcheck.conf:2:32: expected a comma (',') or array terminator (']'), but got end of file (last key parsed: "checks") (config)
arp242
added a commit
to arp242/go-tools
that referenced
this pull request
Jun 1, 2024
Previously the error message would get lost:
% staticcheck
staticcheck.conf:5:0: (last key parsed: "dot_import_whitelist") (compile)
This is actually my bug; the ParseError.Message wasn't always set; fixed
with: BurntSushi/toml#411
Now it prints the expected:
% staticcheck
staticcheck.conf:2:32: expected a comma (',') or array terminator (']'), but got end of file (last key parsed: "checks") (config)
dominikh
pushed a commit
to dominikh/go-tools
that referenced
this pull request
Jun 16, 2024
Previously the error message would get lost:
% staticcheck
staticcheck.conf:5:0: (last key parsed: "dot_import_whitelist") (compile)
This is actually my bug; the ParseError.Message wasn't always set; fixed
with: BurntSushi/toml#411
Now it prints the expected:
% staticcheck
staticcheck.conf:2:32: expected a comma (',') or array terminator (']'), but got end of file (last key parsed: "checks") (config)
Closes: gh-1547 [via git-merge-pr]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ParseError.Error() did:
That was fine, but not very useful for people wanting to access the Message field themselves, especially since err isn't exported.
For example staticcheck does this to add in the filename, and the error is lost, because Message is often blank:
We now only use the err field to determine which error usage to display in ErrorWithUsage()