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.

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

Configure path prefix via processor abstraction #1226

Merged
merged 5 commits into from Jul 10, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .golangci.example.yml
Expand Up @@ -72,6 +72,9 @@ output:
# make issues output unique by line, default is true
uniq-by-line: true

# add a prefix to the output file references; default is no prefix
path-prefix: ""


# all available settings of specific linters
linters-settings:
Expand Down
1 change: 1 addition & 0 deletions pkg/commands/run.go
Expand Up @@ -81,6 +81,7 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is
fs.BoolVar(&oc.PrintLinterName, "print-linter-name", true, wh("Print linter name in issue line"))
fs.BoolVar(&oc.UniqByLine, "uniq-by-line", true, wh("Make issues output unique by line"))
fs.BoolVar(&oc.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message"))
fs.StringVar(&oc.PathPrefix, "path-prefix", "", wh("Path prefix to add to output"))
sayboras marked this conversation as resolved.
Show resolved Hide resolved
hideFlag("print-welcome") // no longer used

// Run config
Expand Down
9 changes: 5 additions & 4 deletions pkg/config/config.go
Expand Up @@ -516,10 +516,11 @@ type Config struct {
Output struct {
Format string
Color string
PrintIssuedLine bool `mapstructure:"print-issued-lines"`
PrintLinterName bool `mapstructure:"print-linter-name"`
UniqByLine bool `mapstructure:"uniq-by-line"`
PrintWelcomeMessage bool `mapstructure:"print-welcome"`
PrintIssuedLine bool `mapstructure:"print-issued-lines"`
PrintLinterName bool `mapstructure:"print-linter-name"`
UniqByLine bool `mapstructure:"uniq-by-line"`
PrintWelcomeMessage bool `mapstructure:"print-welcome"`
PathPrefix string `mapstructure:"path-prefix"`
}

LintersSettings LintersSettings `mapstructure:"linters-settings"`
Expand Down
1 change: 1 addition & 0 deletions pkg/lint/runner.go
Expand Up @@ -79,6 +79,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
processors.NewSourceCode(lineCache, log.Child("source_code")),
processors.NewPathShortener(),
getSeverityRulesProcessor(&cfg.Severity, log, lineCache),
processors.NewPathPrefixer(cfg.Output.PathPrefix),
sayboras marked this conversation as resolved.
Show resolved Hide resolved
},
Log: log,
}, nil
Expand Down
37 changes: 37 additions & 0 deletions pkg/result/processors/path_prefixer.go
@@ -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 != "" {
Copy link
Member

Choose a reason for hiding this comment

The 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 ?

Copy link
Contributor Author

@jwilner jwilner Jul 10, 2020

Choose a reason for hiding this comment

The 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() {}
37 changes: 37 additions & 0 deletions pkg/result/processors/path_prefixer_test.go
@@ -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
})
}
}