Skip to content

Refactor error position parsing to support path with colon. #1673

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

Open
jim-liu wants to merge 2 commits into
base: master
Choose a base branch
from
+46 −12
Open

Refactor error position parsing to support path with colon. #1673

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6938f70
Refactor error position parsing to support path with colons
jim-liu May 15, 2026
9a8ae28
Add additional unit test for paths with colons
jim-liu May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions analyzer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"path"
"path/filepath"
"reflect"
"regexp"
"runtime/debug"
"strconv"
"strings"
Expand Down Expand Up @@ -761,20 +762,24 @@
return nil, nil
}
errs := make(map[string][]Error)
var posRegexp = regexp.MustCompile(`^(.*?)(?::(\w+))?(?::(\w+))?$`)

Check failure on line 765 in analyzer.go

View workflow job for this annotation

GitHub Actions / test (1.25.10, latest) 

File is not properly formatted (gofumpt)
Copy link
Copy Markdown
Member

@ccojocar ccojocar May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Author

@jim-liu jim-liu May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

for _, pkgErr := range pkg.Errors {
parts := strings.Split(pkgErr.Pos, ":")
file := parts[0]
matches := posRegexp.FindStringSubmatch(pkgErr.Pos)
file := pkgErr.Pos
var err error
var line int
if len(parts) > 1 {
if line, err = strconv.Atoi(parts[1]); err != nil {
return nil, fmt.Errorf("parsing line: %w", err)
var line, column int
if len(matches) > 0 {
file = matches[1]
file = strings.TrimSuffix(file, ":")
if matches[2] != "" {
Copy link
Copy Markdown
Member

@ccojocar ccojocar May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still need to check if matches is greater than 2 here otherwise it will panic if there are only 2 matches.

If len(matches) > 2 && matches[2] != ""

if line, err = strconv.Atoi(matches[2]); err != nil {
return nil, fmt.Errorf("parsing line: %w", err)
}
}
}
var column int
if len(parts) > 2 {
if column, err = strconv.Atoi(parts[2]); err != nil {
return nil, fmt.Errorf("parsing column: %w", err)
if matches[3] != "" {
Copy link
Copy Markdown
Member

@ccojocar ccojocar May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same here len(matches) > 3 otherwise, it will panic.

if column, err = strconv.Atoi(matches[3]); err != nil {
return nil, fmt.Errorf("parsing column: %w", err)
}
}
}
msg := strings.TrimSpace(pkgErr.Msg)
Expand Down
20 changes: 20 additions & 0 deletions analyzer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1803,6 +1803,26 @@ func main() {
Expect(err).Should(HaveOccurred())
})

It("should properly parse the errors with colons in path", func() {
pkg := &packages.Package{
Errors: []packages.Error{
{
Pos: "C:\\file:1:2",
Copy link
Copy Markdown
Member

@ccojocar ccojocar May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Msg: "build error",
},
},
}
errors, err := gosec.ParseErrors(pkg)
Expect(err).ShouldNot(HaveOccurred())
Expect(errors).To(HaveLen(1))
for _, ferr := range errors {
Expect(ferr).To(HaveLen(1))
Expect(ferr[0].Line).To(Equal(1))
Expect(ferr[0].Column).To(Equal(2))
Expect(ferr[0].Err).Should(MatchRegexp(`build error`))
}
})

It("should append error to the same file", func() {
pkg := &packages.Package{
Errors: []packages.Error{
Expand Down
Loading