Refactor error position parsing to support path with colon.#1673
Refactor error position parsing to support path with colon.#1673jim-liu wants to merge 2 commits into
Conversation
ccojocar
left a comment
ccojocar
left a comment
There was a problem hiding this comment.
Please can you add some unit tests for this change? Thanks
Thanks for the heads-up - added a unit test and updated the regex to emit error messages with invalid line/column. |
| pkg := &packages.Package{ | ||
| Errors: []packages.Error{ | ||
| { | ||
| Pos: "C:\\file:1:2", |
There was a problem hiding this comment.
Can you also add an Linux example of path?
For instance:
touch "my:file:with:colons.txt"
It is a valid file name in Linux/Unix.
Can you also add a tests for a path without build errors.
| return nil, nil | ||
| } | ||
| errs := make(map[string][]Error) | ||
| var posRegexp = regexp.MustCompile(`^(.*?)(?::(\w+))?(?::(\w+))?$`) |
There was a problem hiding this comment.
What about the Linux/Unix file systems? Does this regex cover the case of this file paths:
touch "my:file:with:colons.txt"
This is a valid path in LInux/Unix.
There was a problem hiding this comment.
The original regex used \d, but it will not throw errors for positions with invalid path like path.go:one:two and will instead treat the position as path.
I updated to regex to use \w in order to throw errors when parsing positions with invalid line/column. However it will fail path with colons but without extension to fail if the position has no line nor column.
So the updated regex actually worked for your particular case (because of the .txt) but won't work if it's my:file:with:colons.
2 participants
Refactor error position parsing with regex to support path with colon.
This is my attempt to solve #327. To reproduce the issue, run gosec with a Go file with invalid syntax either on Windows, or on Linux but in a path with colons (such as
~/home/test:code)