Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure ParseError.Message is always set #411

Merged
merged 1 commit into from
May 26, 2024
Merged

Ensure ParseError.Message is always set #411

merged 1 commit into from
May 26, 2024

Conversation

arp242
Copy link
Collaborator

@arp242 arp242 commented May 26, 2024

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
Ensure ParseError.Message is always set
0482b45 
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 arp242 merged commit 092fca1 into master May 26, 2024
13 checks passed
@arp242 arp242 deleted the msg branch May 26, 2024 19:15
arp242 added a commit to arp242/go-tools that referenced this pull request May 26, 2024
@arp242
lintcmd: print TOML error on invalid config
43dc791 
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
@arp242
lintcmd: print TOML error on invalid config
b795b36 
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 arp242 mentioned this pull request May 26, 2024
Closed
arp242 added a commit to arp242/go-tools that referenced this pull request May 26, 2024
@arp242
lintcmd: print TOML error on invalid config
a3575d2 
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
@arp242
lintcmd: print TOML error on invalid config
c69de30 
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
@arp242 @dominikh
lintcmd: print TOML error on invalid config
a8e4505 
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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@arp242