-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Configure path prefix via processor abstraction #1226
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package processors | ||
|
||
import ( | ||
"path" | ||
|
||
"github.com/golangci/golangci-lint/pkg/result" | ||
) | ||
|
||
// PathPrefixer adds a customizable prefix to every output path | ||
type PathPrefixer struct { | ||
prefix string | ||
} | ||
|
||
var _ Processor = new(PathPrefixer) | ||
|
||
// NewPathPrefixer returns a new path prefixer for the provided string | ||
func NewPathPrefixer(prefix string) *PathPrefixer { | ||
return &PathPrefixer{prefix: prefix} | ||
} | ||
|
||
// Name returns the name of this processor | ||
func (*PathPrefixer) Name() string { | ||
return "path_prefixer" | ||
} | ||
|
||
// Process adds the prefix to each path | ||
func (p *PathPrefixer) Process(issues []result.Issue) ([]result.Issue, error) { | ||
sayboras marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if p.prefix != "" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current logic seems fine to me. Just a note on immutability, I just take a look at existing implementation of processors. Seems like https://github.com/golangci/golangci-lint/blob/master/pkg/result/processors/utils.go#L40 is widely used (this function create a copy of slides). What do you think ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, it's safe for this to be mutating the passed in slice because, by definition, the processors happen serially, so there's no risk of a race on the mutated memory. I would guess the transformSlices function exists because it's easy to mess up working with a slice of values -- every value in a range loop is a copy, etc, etc. As it is, I figured this is a memory-sensitive application and the Issue struct is non-trivial in its size, so I'd avoid the unnecessary copies and alloc. Without running any sort of measurements, I'm guessing there's a lot of fat around this processor chain's memory usage and shifting to a pattern like that I've used here would cut it down a bunch. |
||
for i := range issues { | ||
issues[i].Pos.Filename = path.Join(p.prefix, issues[i].Pos.Filename) | ||
} | ||
} | ||
return issues, nil | ||
} | ||
|
||
// Finish is implemented to satisfy the Processor interface | ||
func (*PathPrefixer) Finish() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package processors | ||
|
||
import ( | ||
"go/token" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/golangci/golangci-lint/pkg/result" | ||
) | ||
|
||
func TestPathPrefixer_Process(t *testing.T) { | ||
paths := func(ps ...string) (issues []result.Issue) { | ||
for _, p := range ps { | ||
issues = append(issues, result.Issue{Pos: token.Position{Filename: p}}) | ||
} | ||
return | ||
} | ||
for _, tt := range []struct { | ||
name, prefix string | ||
issues, want []result.Issue | ||
}{ | ||
{"empty prefix", "", paths("some/path", "cool"), paths("some/path", "cool")}, | ||
{"prefix", "ok", paths("some/path", "cool"), paths("ok/some/path", "ok/cool")}, | ||
{"prefix slashed", "ok/", paths("some/path", "cool"), paths("ok/some/path", "ok/cool")}, | ||
} { | ||
t.Run(tt.name, func(t *testing.T) { | ||
r := require.New(t) | ||
|
||
p := NewPathPrefixer(tt.prefix) //nolint:scopelint | ||
sayboras marked this conversation as resolved.
Show resolved
Hide resolved
|
||
got, err := p.Process(tt.issues) //nolint:scopelint | ||
r.NoError(err, "prefixer should never error") | ||
|
||
r.Equal(got, tt.want) //nolint:scopelint | ||
}) | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.