Skip to content

Commit 671f168

Browse files
committed
#66: properly merge (not overwrite) slice flags from config and command-line
1 parent 9c07341 commit 671f168

File tree

4 files changed

+183
-10
lines changed

4 files changed

+183
-10
lines changed

Diff for: pkg/commands/run.go

+26-5
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func (e *Executor) initFlagSet(fs *pflag.FlagSet) {
6969
rc := &e.cfg.Run
7070
fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code",
7171
1, wh("Exit code when issues were found"))
72-
fs.StringSliceVar(&rc.BuildTags, "build-tags", []string{}, wh("Build tags (not all linters support them)"))
72+
fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags (not all linters support them)"))
7373
fs.DurationVar(&rc.Deadline, "deadline", time.Minute, wh("Deadline for total work"))
7474
fs.BoolVar(&rc.AnalyzeTests, "tests", true, wh("Analyze tests (*_test.go)"))
7575
fs.BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, wh("Print avg and max memory usage of golangci-lint and total time"))
@@ -127,17 +127,17 @@ func (e *Executor) initFlagSet(fs *pflag.FlagSet) {
127127

128128
// Linters config
129129
lc := &e.cfg.Linters
130-
fs.StringSliceVarP(&lc.Enable, "enable", "E", []string{}, wh("Enable specific linter"))
131-
fs.StringSliceVarP(&lc.Disable, "disable", "D", []string{}, wh("Disable specific linter"))
130+
fs.StringSliceVarP(&lc.Enable, "enable", "E", nil, wh("Enable specific linter"))
131+
fs.StringSliceVarP(&lc.Disable, "disable", "D", nil, wh("Disable specific linter"))
132132
fs.BoolVar(&lc.EnableAll, "enable-all", false, wh("Enable all linters"))
133133
fs.BoolVar(&lc.DisableAll, "disable-all", false, wh("Disable all linters"))
134-
fs.StringSliceVarP(&lc.Presets, "presets", "p", []string{},
134+
fs.StringSliceVarP(&lc.Presets, "presets", "p", nil,
135135
wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint linters' to see them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|"))))
136136
fs.BoolVar(&lc.Fast, "fast", false, wh("Run only fast linters from enabled linters set"))
137137

138138
// Issues config
139139
ic := &e.cfg.Issues
140-
fs.StringSliceVarP(&ic.ExcludePatterns, "exclude", "e", []string{}, wh("Exclude issue by regexp"))
140+
fs.StringSliceVarP(&ic.ExcludePatterns, "exclude", "e", nil, wh("Exclude issue by regexp"))
141141
fs.BoolVar(&ic.UseDefaultExcludes, "exclude-use-default", true, getDefaultExcludeHelp())
142142

143143
fs.IntVar(&ic.MaxIssuesPerLinter, "max-issues-per-linter", 50, wh("Maximum issues count per one linter. Set to 0 to disable"))
@@ -165,6 +165,27 @@ func (e *Executor) initRun() {
165165
e.initFlagSet(fs)
166166

167167
e.parseConfig()
168+
169+
// It's a dirty hack to set flag.Changed to true for every string slice flag.
170+
// It's necessary to merge config and command-line slices: otherwise command-line
171+
// flags will always overwrite ones from the config.
172+
fs.VisitAll(func(f *pflag.Flag) {
173+
if f.Value.Type() != "stringSlice" {
174+
return
175+
}
176+
177+
s, err := fs.GetStringSlice(f.Name)
178+
if err != nil {
179+
return
180+
}
181+
182+
if s == nil { // assume that every string slice flag has nil as the default
183+
return
184+
}
185+
186+
// calling Set sets Changed to true: next Set calls will append, not overwrite
187+
_ = f.Value.Set(strings.Join(s, ","))
188+
})
168189
}
169190

170191
func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan result.Issue, error) {

Diff for: pkg/fsutils/fsutils.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func GetPathsForAnalysis(ctx context.Context, inputPaths []string, includeTests
117117
pr := NewPathResolver(stdExcludeDirs, []string{".go"}, includeTests)
118118
paths, err := pr.Resolve(inputPaths...)
119119
if err != nil {
120-
return nil, fmt.Errorf("can't resolve paths: %s", err)
120+
return nil, fmt.Errorf("can't resolve paths %v: %s", inputPaths, err)
121121
}
122122

123123
return processResolvedPaths(paths)

Diff for: pkg/lint/lintersdb/lintersdb.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package lintersdb
33
import (
44
"fmt"
55
"os"
6+
"sort"
67
"strings"
78
"sync"
89

@@ -192,7 +193,7 @@ func GetAllSupportedLinterConfigs() []linter.Config {
192193
})
193194
}
194195

195-
func getAllEnabledByDefaultLinters() []linter.Config {
196+
func GetAllEnabledByDefaultLinters() []linter.Config {
196197
var ret []linter.Config
197198
for _, lc := range GetAllSupportedLinterConfigs() {
198199
if lc.EnabledByDefault {
@@ -402,7 +403,7 @@ func GetEnabledLinters(cfg *config.Config) ([]linter.Config, error) {
402403
return nil, err
403404
}
404405

405-
resultLintersSet := getEnabledLintersSet(&cfg.Linters, getAllEnabledByDefaultLinters())
406+
resultLintersSet := getEnabledLintersSet(&cfg.Linters, GetAllEnabledByDefaultLinters())
406407

407408
var resultLinters []linter.Config
408409
for _, lc := range resultLintersSet {
@@ -419,6 +420,7 @@ func verbosePrintLintersStatus(cfg *config.Config, lcs []linter.Config) {
419420
for _, lc := range lcs {
420421
linterNames = append(linterNames, lc.Linter.Name())
421422
}
423+
sort.StringSlice(linterNames).Sort()
422424
logrus.Infof("Active linters: %s", linterNames)
423425

424426
if len(cfg.Linters.Presets) != 0 {

Diff for: test/run_test.go

+152-2
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
11
package test
22

33
import (
4+
"fmt"
5+
"io/ioutil"
6+
"log"
7+
"os"
48
"os/exec"
59
"path/filepath"
10+
"sort"
11+
"strings"
612
"sync"
713
"syscall"
814
"testing"
915

16+
"github.com/golangci/golangci-lint/pkg/lint/lintersdb"
17+
1018
"github.com/stretchr/testify/assert"
1119
)
1220

@@ -32,15 +40,17 @@ func TestCongratsMessageIfNoIssues(t *testing.T) {
3240
func TestDeadline(t *testing.T) {
3341
out, exitCode := runGolangciLint(t, "--deadline=1ms", "../...")
3442
assert.Equal(t, 4, exitCode)
35-
assert.Equal(t, "", out) // no 'Congrats! No issues were found.'
43+
assert.Contains(t, out, "deadline exceeded: try increase it by passing --deadline option")
44+
assert.NotContains(t, out, "Congrats! No issues were found.")
3645
}
3746

3847
func runGolangciLint(t *testing.T, args ...string) (string, int) {
3948
installBinary(t)
4049

4150
runArgs := append([]string{"run"}, args...)
51+
log.Print(runArgs)
4252
cmd := exec.Command("golangci-lint", runArgs...)
43-
out, err := cmd.Output()
53+
out, err := cmd.CombinedOutput()
4454
if err != nil {
4555
if exitError, ok := err.(*exec.ExitError); ok {
4656
t.Logf("stderr: %s", exitError.Stderr)
@@ -57,6 +67,30 @@ func runGolangciLint(t *testing.T, args ...string) (string, int) {
5767
return string(out), ws.ExitStatus()
5868
}
5969

70+
func runGolangciLintWithYamlConfig(t *testing.T, cfg string, args ...string) string {
71+
f, err := ioutil.TempFile("", "golangci_lint_test")
72+
assert.NoError(t, err)
73+
f.Close()
74+
75+
cfgPath := f.Name() + ".yml"
76+
err = os.Rename(f.Name(), cfgPath)
77+
assert.NoError(t, err)
78+
79+
defer os.Remove(cfgPath)
80+
81+
cfg = strings.TrimSpace(cfg)
82+
cfg = strings.Replace(cfg, "\t", " ", -1)
83+
84+
err = ioutil.WriteFile(cfgPath, []byte(cfg), os.ModePerm)
85+
assert.NoError(t, err)
86+
87+
pargs := append([]string{"-c", cfgPath}, args...)
88+
out, ec := runGolangciLint(t, pargs...)
89+
assert.Equal(t, 0, ec)
90+
91+
return out
92+
}
93+
6094
func TestTestsAreLintedByDefault(t *testing.T) {
6195
out, exitCode := runGolangciLint(t, "./testdata/withtests")
6296
assert.Equal(t, 0, exitCode, out)
@@ -74,3 +108,119 @@ func TestConfigFileIsDetected(t *testing.T) {
74108
out, exitCode := runGolangciLint(t) // doesn't detect when no args
75109
checkNoIssuesRun(t, out, exitCode)
76110
}
111+
112+
func inSlice(s []string, v string) bool {
113+
for _, sv := range s {
114+
if sv == v {
115+
return true
116+
}
117+
}
118+
119+
return false
120+
}
121+
122+
func getEnabledByDefaultFastLintersExcept(except ...string) []string {
123+
ebdl := lintersdb.GetAllEnabledByDefaultLinters()
124+
ret := []string{}
125+
for _, linter := range ebdl {
126+
if linter.DoesFullImport {
127+
continue
128+
}
129+
130+
if !inSlice(except, linter.Linter.Name()) {
131+
ret = append(ret, linter.Linter.Name())
132+
}
133+
}
134+
135+
return ret
136+
}
137+
138+
func getEnabledByDefaultFastLintersWith(with ...string) []string {
139+
ebdl := lintersdb.GetAllEnabledByDefaultLinters()
140+
ret := append([]string{}, with...)
141+
for _, linter := range ebdl {
142+
if linter.DoesFullImport {
143+
continue
144+
}
145+
146+
ret = append(ret, linter.Linter.Name())
147+
}
148+
149+
return ret
150+
}
151+
152+
func mergeMegacheck(linters []string) []string {
153+
if inSlice(linters, "staticcheck") &&
154+
inSlice(linters, "gosimple") &&
155+
inSlice(linters, "unused") {
156+
ret := []string{"megacheck"}
157+
for _, linter := range linters {
158+
if !inSlice([]string{"staticcheck", "gosimple", "unused"}, linter) {
159+
ret = append(ret, linter)
160+
}
161+
}
162+
163+
return ret
164+
}
165+
166+
return linters
167+
}
168+
169+
func TestEnabledLinters(t *testing.T) {
170+
type tc struct {
171+
name string
172+
cfg string
173+
el []string
174+
args string
175+
}
176+
177+
cases := []tc{
178+
{
179+
name: "disable govet in config",
180+
cfg: `
181+
linters:
182+
disable:
183+
- govet
184+
`,
185+
el: getEnabledByDefaultFastLintersExcept("govet"),
186+
},
187+
{
188+
name: "enable golint in config",
189+
cfg: `
190+
linters:
191+
enable:
192+
- golint
193+
`,
194+
el: getEnabledByDefaultFastLintersWith("golint"),
195+
},
196+
{
197+
name: "disable govet in cmd",
198+
args: "-Dgovet",
199+
el: getEnabledByDefaultFastLintersExcept("govet"),
200+
},
201+
{
202+
name: "enable gofmt in cmd and enable golint in config",
203+
args: "-Egofmt",
204+
cfg: `
205+
linters:
206+
enable:
207+
- golint
208+
`,
209+
el: getEnabledByDefaultFastLintersWith("golint", "gofmt"),
210+
},
211+
}
212+
213+
for _, c := range cases {
214+
t.Run(c.name, func(t *testing.T) {
215+
runArgs := []string{"-v", "--fast"}
216+
if c.args != "" {
217+
runArgs = append(runArgs, strings.Split(c.args, " ")...)
218+
}
219+
out := runGolangciLintWithYamlConfig(t, c.cfg, runArgs...)
220+
el := mergeMegacheck(c.el)
221+
sort.StringSlice(el).Sort()
222+
expectedLine := fmt.Sprintf("Active linters: [%s]", strings.Join(el, " "))
223+
assert.Contains(t, out, expectedLine)
224+
})
225+
}
226+
}

0 commit comments

Comments
 (0)