diff --git a/.golangci.yml b/.golangci.yml index 3c5d6ae3..a3c24c9d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -25,4 +25,5 @@ linters: enable-all: true disable: - maligned - - prealloc \ No newline at end of file + - prealloc + - gosec \ No newline at end of file diff --git a/Makefile b/Makefile index d5734cb5..b753315b 100644 --- a/Makefile +++ b/Makefile @@ -14,6 +14,9 @@ assets: readme: go run ./scripts/gen_readme/main.go +gen: + go generate ./... + check_generated: make readme && git diff --exit-code # check no changes diff --git a/README.tmpl.md b/README.tmpl.md index d2598dfd..64a7f199 100644 --- a/README.tmpl.md +++ b/README.tmpl.md @@ -166,12 +166,12 @@ We compare golangci-lint and gometalinter in default mode, but explicitly enable $ golangci-lint run --no-config --issues-exit-code=0 --deadline=30m \ --disable-all --enable=deadcode --enable=gocyclo --enable=golint --enable=varcheck \ --enable=structcheck --enable=maligned --enable=errcheck --enable=dupl --enable=ineffassign \ - --enable=interfacer --enable=unconvert --enable=goconst --enable=gas --enable=megacheck + --enable=interfacer --enable=unconvert --enable=goconst --enable=gosec --enable=megacheck $ gometalinter --deadline=30m --vendor --cyclo-over=30 --dupl-threshold=150 \ --exclude= --skip=testdata --skip=builtin \ --disable-all --enable=deadcode --enable=gocyclo --enable=golint --enable=varcheck \ --enable=structcheck --enable=maligned --enable=errcheck --enable=dupl --enable=ineffassign \ - --enable=interfacer --enable=unconvert --enable=goconst --enable=gas --enable=megacheck + --enable=interfacer --enable=unconvert --enable=goconst --enable=gosec --enable=megacheck ./... ``` diff --git a/pkg/commands/help.go b/pkg/commands/help.go index 984b9aa8..109591a4 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -33,8 +33,12 @@ func (e *Executor) initHelp() { func printLinterConfigs(lcs []linter.Config) { for _, lc := range lcs { - fmt.Fprintf(logutils.StdOut, "%s: %s [fast: %t]\n", color.YellowString(lc.Linter.Name()), - lc.Linter.Desc(), !lc.DoesFullImport) + altNamesStr := "" + if len(lc.AlternativeNames) != 0 { + altNamesStr = fmt.Sprintf(" (%s)", strings.Join(lc.AlternativeNames, ", ")) + } + fmt.Fprintf(logutils.StdOut, "%s%s: %s [fast: %t]\n", color.YellowString(lc.Name()), + altNamesStr, lc.Linter.Desc(), !lc.DoesFullImport) } } @@ -58,7 +62,7 @@ func (e Executor) executeLintersHelp(cmd *cobra.Command, args []string) { linters := e.DBManager.GetAllLinterConfigsForPreset(p) linterNames := []string{} for _, lc := range linters { - linterNames = append(linterNames, lc.Linter.Name()) + linterNames = append(linterNames, lc.Name()) } fmt.Fprintf(logutils.StdOut, "%s: %s\n", color.YellowString(p), strings.Join(linterNames, ", ")) } diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 156bb72f..da581fdb 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -20,8 +20,8 @@ func (e *Executor) initLinters() { } func IsLinterInConfigsList(name string, linters []linter.Config) bool { - for _, linter := range linters { - if linter.Linter.Name() == name { + for _, lc := range linters { + if lc.Name() == name { return true } } @@ -40,7 +40,7 @@ func (e *Executor) executeLinters(cmd *cobra.Command, args []string) { var disabledLCs []linter.Config for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { - if !IsLinterInConfigsList(lc.Linter.Name(), enabledLCs) { + if !IsLinterInConfigsList(lc.Name(), enabledLCs) { disabledLCs = append(disabledLCs, lc) } } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 06753796..5d166516 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -238,23 +238,23 @@ func fixSlicesFlags(fs *pflag.FlagSet) { func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan result.Issue, error) { e.cfg.Run.Args = args - linters, err := e.EnabledLintersSet.Get() + enabledLinters, err := e.EnabledLintersSet.Get() if err != nil { return nil, err } for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { isEnabled := false - for _, linter := range linters { - if linter.Linter.Name() == lc.Linter.Name() { + for _, enabledLC := range enabledLinters { + if enabledLC.Name() == lc.Name() { isEnabled = true break } } - e.reportData.AddLinter(lc.Linter.Name(), isEnabled, lc.EnabledByDefault) + e.reportData.AddLinter(lc.Name(), isEnabled, lc.EnabledByDefault) } - lintCtx, err := lint.LoadContext(linters, e.cfg, e.log.Child("load")) + lintCtx, err := lint.LoadContext(enabledLinters, e.cfg, e.log.Child("load")) if err != nil { return nil, errors.Wrap(err, "context loading failed") } @@ -264,7 +264,7 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (<-chan resul return nil, err } - return runner.Run(ctx, linters, lintCtx), nil + return runner.Run(ctx, enabledLinters, lintCtx), nil } func (e *Executor) setOutputToDevNull() (savedStdout, savedStderr *os.File) { diff --git a/pkg/config/config.go b/pkg/config/config.go index df0eb383..a8aa5860 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -44,31 +44,6 @@ var DefaultExcludePatterns = []ExcludePattern{ Linter: "golint", Why: "False positive when tests are defined in package 'test'", }, - { - Pattern: "Use of unsafe calls should be audited", - Linter: "gas", - Why: "Too many false-positives on 'unsafe' usage", - }, - { - Pattern: "Subprocess launch(ed with variable|ing should be audited)", - Linter: "gas", - Why: "Too many false-positives for parametrized shell calls", - }, - { - Pattern: "G104", - Linter: "gas", - Why: "Duplicated errcheck checks", - }, - { - Pattern: "(Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)", - Linter: "gas", - Why: "Too many issues in popular repos", - }, - { - Pattern: "Potential file inclusion via variable", - Linter: "gas", - Why: "False positive is triggered by 'src, err := ioutil.ReadFile(filename)'", - }, { Pattern: "(possible misuse of unsafe.Pointer|should have signature)", Linter: "govet", diff --git a/pkg/golinters/gas.go b/pkg/golinters/gas.go index 31321ca9..90f58db9 100644 --- a/pkg/golinters/gas.go +++ b/pkg/golinters/gas.go @@ -14,17 +14,17 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -type Gas struct{} +type Gosec struct{} -func (Gas) Name() string { - return "gas" +func (Gosec) Name() string { + return "gosec" } -func (Gas) Desc() string { +func (Gosec) Desc() string { return "Inspects source code for security problems" } -func (lint Gas) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { +func (lint Gosec) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue, error) { gasConfig := gas.NewConfig() enabledRules := rules.Generate() logger := log.New(ioutil.Discard, "", 0) @@ -45,7 +45,7 @@ func (lint Gas) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issu if err != nil { r = &result.Range{} if n, rerr := fmt.Sscanf(i.Line, "%d-%d", &r.From, &r.To); rerr != nil || n != 2 { - lintCtx.Log.Warnf("Can't convert gas line number %q of %v to int: %s", i.Line, i, err) + lintCtx.Log.Warnf("Can't convert gosec line number %q of %v to int: %s", i.Line, i, err) continue } line = r.From diff --git a/pkg/golinters/utils.go b/pkg/golinters/utils.go index be20adb0..1946f434 100644 --- a/pkg/golinters/utils.go +++ b/pkg/golinters/utils.go @@ -56,7 +56,7 @@ var replacePatterns = []replacePattern{ {`^(\S+) arg list ends with redundant newline$`, "`${1}` arg list ends with redundant newline"}, {`^(\S+) composite literal uses unkeyed fields$`, "`${1}` composite literal uses unkeyed fields"}, - // gas + // gosec {`^Blacklisted import (\S+): weak cryptographic primitive$`, "Blacklisted import `${1}`: weak cryptographic primitive"}, {`^TLS InsecureSkipVerify set true.$`, "TLS `InsecureSkipVerify` set true."}, diff --git a/pkg/lint/linter/config.go b/pkg/lint/linter/config.go index 05283d6b..9820b6ae 100644 --- a/pkg/lint/linter/config.go +++ b/pkg/lint/linter/config.go @@ -16,6 +16,7 @@ type Config struct { NeedsSSARepr bool InPresets []string Speed int // more value means faster execution of linter + AlternativeNames []string OriginalURL string // URL of original (not forked) repo, needed for autogenerated README } @@ -46,6 +47,11 @@ func (lc Config) WithURL(url string) Config { return lc } +func (lc Config) WithAlternativeNames(names ...string) Config { + lc.AlternativeNames = names + return lc +} + func (lc Config) NeedsProgramLoading() bool { return lc.DoesFullImport } @@ -58,6 +64,14 @@ func (lc Config) GetSpeed() int { return lc.Speed } +func (lc Config) AllNames() []string { + return append([]string{lc.Name()}, lc.AlternativeNames...) +} + +func (lc Config) Name() string { + return lc.Linter.Name() +} + func NewConfig(linter Linter) *Config { return &Config{ Linter: linter, diff --git a/pkg/lint/lintersdb/enabled_set.go b/pkg/lint/lintersdb/enabled_set.go index fb969f7c..755aeede 100644 --- a/pkg/lint/lintersdb/enabled_set.go +++ b/pkg/lint/lintersdb/enabled_set.go @@ -43,7 +43,7 @@ func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []linte for _, p := range lcfg.Presets { for _, lc := range es.m.GetAllLinterConfigsForPreset(p) { lc := lc - resultLintersSet[lc.Linter.Name()] = &lc + resultLintersSet[lc.Name()] = &lc } } @@ -52,14 +52,16 @@ func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []linte // It should be before --enable and --disable to be able to enable or disable specific linter. if lcfg.Fast { for name := range resultLintersSet { - if es.m.getLinterConfig(name).DoesFullImport { + if es.m.GetLinterConfig(name).DoesFullImport { delete(resultLintersSet, name) } } } for _, name := range lcfg.Enable { - resultLintersSet[name] = es.m.getLinterConfig(name) + lc := es.m.GetLinterConfig(name) + // it's important to use lc.Name() nor name because name can be alias + resultLintersSet[lc.Name()] = lc } for _, name := range lcfg.Disable { @@ -68,7 +70,10 @@ func (es EnabledSet) build(lcfg *config.Linters, enabledByDefaultLinters []linte delete(resultLintersSet, ln) } } - delete(resultLintersSet, name) + + lc := es.m.GetLinterConfig(name) + // it's important to use lc.Name() nor name because name can be alias + delete(resultLintersSet, lc.Name()) } es.optimizeLintersSet(resultLintersSet) @@ -111,7 +116,7 @@ func (es EnabledSet) optimizeLintersSet(linters map[string]*linter.Config) { delete(linters, n) } - lc := *es.m.getLinterConfig("megacheck") + lc := *es.m.GetLinterConfig("megacheck") lc.Linter = mega linters[mega.Name()] = &lc } @@ -135,7 +140,7 @@ func (es EnabledSet) Get() ([]linter.Config, error) { func (es EnabledSet) verbosePrintLintersStatus(lcs []linter.Config) { var linterNames []string for _, lc := range lcs { - linterNames = append(linterNames, lc.Linter.Name()) + linterNames = append(linterNames, lc.Name()) } sort.StringSlice(linterNames).Sort() es.log.Infof("Active %d linters: %s", len(linterNames), linterNames) diff --git a/pkg/lint/lintersdb/enabled_set_test.go b/pkg/lint/lintersdb/enabled_set_test.go index 49335535..6367037d 100644 --- a/pkg/lint/lintersdb/enabled_set_test.go +++ b/pkg/lint/lintersdb/enabled_set_test.go @@ -42,6 +42,41 @@ func TestGetEnabledLintersSet(t *testing.T) { def: []string{"gofmt", "govet"}, exp: []string{"gofmt", "govet"}, }, + { + name: "enable gosec by gas alias", + cfg: config.Linters{ + Enable: []string{"gas"}, + }, + exp: []string{"gosec"}, + }, + { + name: "enable gosec by primary name", + cfg: config.Linters{ + Enable: []string{"gosec"}, + }, + exp: []string{"gosec"}, + }, + { + name: "enable gosec by both names", + cfg: config.Linters{ + Enable: []string{"gosec", "gas"}, + }, + exp: []string{"gosec"}, + }, + { + name: "disable gosec by gas alias", + cfg: config.Linters{ + Disable: []string{"gas"}, + }, + def: []string{"gosec"}, + }, + { + name: "disable gosec by primary name", + cfg: config.Linters{ + Disable: []string{"gosec"}, + }, + def: []string{"gosec"}, + }, } m := NewManager() @@ -50,12 +85,12 @@ func TestGetEnabledLintersSet(t *testing.T) { t.Run(c.name, func(t *testing.T) { defaultLinters := []linter.Config{} for _, ln := range c.def { - defaultLinters = append(defaultLinters, *m.getLinterConfig(ln)) + defaultLinters = append(defaultLinters, *m.GetLinterConfig(ln)) } els := es.build(&c.cfg, defaultLinters) var enabledLinters []string for ln, lc := range els { - assert.Equal(t, ln, lc.Linter.Name()) + assert.Equal(t, ln, lc.Name()) enabledLinters = append(enabledLinters, ln) } diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 76b24d3d..56c047c7 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -15,7 +15,9 @@ func NewManager() *Manager { m := &Manager{} nameToLC := make(map[string]linter.Config) for _, lc := range m.GetAllSupportedLinterConfigs() { - nameToLC[lc.Linter.Name()] = lc + for _, name := range lc.AllNames() { + nameToLC[name] = lc + } } m.nameToLC = nameToLC @@ -35,7 +37,7 @@ func (m Manager) allPresetsSet() map[string]bool { return ret } -func (m Manager) getLinterConfig(name string) *linter.Config { +func (m Manager) GetLinterConfig(name string) *linter.Config { lc, ok := m.nameToLC[name] if !ok { return nil @@ -87,11 +89,12 @@ func (Manager) GetAllSupportedLinterConfigs() []linter.Config { WithSpeed(5). WithURL("https://github.com/dominikh/go-tools/tree/master/cmd/gosimple"), - linter.NewConfig(golinters.Gas{}). + linter.NewConfig(golinters.Gosec{}). WithFullImport(). WithPresets(linter.PresetBugs). WithSpeed(8). - WithURL("https://github.com/GoASTScanner/gas"), + WithURL("https://github.com/securego/gosec"). + WithAlternativeNames("gas"), linter.NewConfig(golinters.Structcheck{}). WithFullImport(). WithPresets(linter.PresetUnused). @@ -202,7 +205,7 @@ func (Manager) GetAllSupportedLinterConfigs() []linter.Config { golinters.TypeCheck{}.Name(): isLocalRun, } return enableLinterConfigs(lcs, func(lc *linter.Config) bool { - return enabled[lc.Linter.Name()] + return enabled[lc.Name()] }) } @@ -221,7 +224,7 @@ func linterConfigsToMap(lcs []linter.Config) map[string]*linter.Config { ret := map[string]*linter.Config{} for _, lc := range lcs { lc := lc // local copy - ret[lc.Linter.Name()] = &lc + ret[lc.Name()] = &lc } return ret diff --git a/pkg/lint/lintersdb/validator.go b/pkg/lint/lintersdb/validator.go index 7391fa50..3a154c7e 100644 --- a/pkg/lint/lintersdb/validator.go +++ b/pkg/lint/lintersdb/validator.go @@ -21,7 +21,7 @@ func (v Validator) validateLintersNames(cfg *config.Linters) error { allNames := append([]string{}, cfg.Enable...) allNames = append(allNames, cfg.Disable...) for _, name := range allNames { - if v.m.getLinterConfig(name) == nil { + if v.m.GetLinterConfig(name) == nil { return fmt.Errorf("no such linter %q", name) } } diff --git a/pkg/lint/load.go b/pkg/lint/load.go index e4dcaa14..bd0271c4 100644 --- a/pkg/lint/load.go +++ b/pkg/lint/load.go @@ -27,9 +27,9 @@ import ( var loadDebugf = logutils.Debug("load") func isFullImportNeeded(linters []linter.Config, cfg *config.Config) bool { - for _, linter := range linters { - if linter.NeedsProgramLoading() { - if linter.Linter.Name() == "govet" && cfg.LintersSettings.Govet.UseInstalledPackages { + for _, lc := range linters { + if lc.NeedsProgramLoading() { + if lc.Name() == "govet" && cfg.LintersSettings.Govet.UseInstalledPackages { // TODO: remove this hack continue } diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 0a32cbbd..02e5862b 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -48,7 +48,7 @@ func NewRunner(astCache *astcache.Cache, cfg *config.Config, log logutils.Log) ( processors.NewAutogeneratedExclude(astCache), processors.NewExclude(excludeTotalPattern), - processors.NewNolint(astCache), + processors.NewNolint(astCache, log.Child("nolint")), processors.NewUniqByLine(), processors.NewDiff(icfg.Diff, icfg.DiffFromRevision, icfg.DiffPatchFilePath), @@ -78,14 +78,14 @@ func (r Runner) runLinterSafe(ctx context.Context, lintCtx *linter.Context, }() specificLintCtx := *lintCtx - specificLintCtx.Log = r.Log.Child(lc.Linter.Name()) + specificLintCtx.Log = r.Log.Child(lc.Name()) issues, err := lc.Linter.Run(ctx, &specificLintCtx) if err != nil { return nil, err } for _, i := range issues { - i.FromLinter = lc.Linter.Name() + i.FromLinter = lc.Name() } return issues, nil @@ -112,7 +112,7 @@ func (r Runner) runWorker(ctx context.Context, lintCtx *linter.Context, } var issues []result.Issue var err error - sw.TrackStage(lc.Linter.Name(), func() { + sw.TrackStage(lc.Name(), func() { issues, err = r.runLinterSafe(ctx, lintCtx, lc) }) lintResultsCh <- lintRes{ @@ -198,7 +198,7 @@ func (r Runner) processLintResults(inCh <-chan lintRes) <-chan lintRes { for res := range inCh { if res.err != nil { - r.Log.Warnf("Can't run linter %s: %s", res.linter.Linter.Name(), res.err) + r.Log.Warnf("Can't run linter %s: %s", res.linter.Name(), res.err) continue } diff --git a/pkg/logutils/log.go b/pkg/logutils/log.go index e45f72e9..178075bc 100644 --- a/pkg/logutils/log.go +++ b/pkg/logutils/log.go @@ -1,5 +1,7 @@ package logutils +//go:generate mockgen -package logutils -source log.go -destination log_mock.go + type Log interface { Fatalf(format string, args ...interface{}) Errorf(format string, args ...interface{}) diff --git a/pkg/logutils/log_mock.go b/pkg/logutils/log_mock.go new file mode 100644 index 00000000..0477ee17 --- /dev/null +++ b/pkg/logutils/log_mock.go @@ -0,0 +1,114 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: log.go + +package logutils + +import ( + gomock "github.com/golang/mock/gomock" + reflect "reflect" +) + +// MockLog is a mock of Log interface +type MockLog struct { + ctrl *gomock.Controller + recorder *MockLogMockRecorder +} + +// MockLogMockRecorder is the mock recorder for MockLog +type MockLogMockRecorder struct { + mock *MockLog +} + +// NewMockLog creates a new mock instance +func NewMockLog(ctrl *gomock.Controller) *MockLog { + mock := &MockLog{ctrl: ctrl} + mock.recorder = &MockLogMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (_m *MockLog) EXPECT() *MockLogMockRecorder { + return _m.recorder +} + +// Fatalf mocks base method +func (_m *MockLog) Fatalf(format string, args ...interface{}) { + _s := []interface{}{format} + for _, _x := range args { + _s = append(_s, _x) + } + _m.ctrl.Call(_m, "Fatalf", _s...) +} + +// Fatalf indicates an expected call of Fatalf +func (_mr *MockLogMockRecorder) Fatalf(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + _s := append([]interface{}{arg0}, arg1...) + return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "Fatalf", reflect.TypeOf((*MockLog)(nil).Fatalf), _s...) +} + +// Errorf mocks base method +func (_m *MockLog) Errorf(format string, args ...interface{}) { + _s := []interface{}{format} + for _, _x := range args { + _s = append(_s, _x) + } + _m.ctrl.Call(_m, "Errorf", _s...) +} + +// Errorf indicates an expected call of Errorf +func (_mr *MockLogMockRecorder) Errorf(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + _s := append([]interface{}{arg0}, arg1...) + return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "Errorf", reflect.TypeOf((*MockLog)(nil).Errorf), _s...) +} + +// Warnf mocks base method +func (_m *MockLog) Warnf(format string, args ...interface{}) { + _s := []interface{}{format} + for _, _x := range args { + _s = append(_s, _x) + } + _m.ctrl.Call(_m, "Warnf", _s...) +} + +// Warnf indicates an expected call of Warnf +func (_mr *MockLogMockRecorder) Warnf(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + _s := append([]interface{}{arg0}, arg1...) + return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "Warnf", reflect.TypeOf((*MockLog)(nil).Warnf), _s...) +} + +// Infof mocks base method +func (_m *MockLog) Infof(format string, args ...interface{}) { + _s := []interface{}{format} + for _, _x := range args { + _s = append(_s, _x) + } + _m.ctrl.Call(_m, "Infof", _s...) +} + +// Infof indicates an expected call of Infof +func (_mr *MockLogMockRecorder) Infof(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + _s := append([]interface{}{arg0}, arg1...) + return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "Infof", reflect.TypeOf((*MockLog)(nil).Infof), _s...) +} + +// Child mocks base method +func (_m *MockLog) Child(name string) Log { + ret := _m.ctrl.Call(_m, "Child", name) + ret0, _ := ret[0].(Log) + return ret0 +} + +// Child indicates an expected call of Child +func (_mr *MockLogMockRecorder) Child(arg0 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "Child", reflect.TypeOf((*MockLog)(nil).Child), arg0) +} + +// SetLevel mocks base method +func (_m *MockLog) SetLevel(level LogLevel) { + _m.ctrl.Call(_m, "SetLevel", level) +} + +// SetLevel indicates an expected call of SetLevel +func (_mr *MockLogMockRecorder) SetLevel(arg0 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "SetLevel", reflect.TypeOf((*MockLog)(nil).SetLevel), arg0) +} diff --git a/pkg/printers/text.go b/pkg/printers/text.go index 30cd282c..e173fb3c 100644 --- a/pkg/printers/text.go +++ b/pkg/printers/text.go @@ -69,7 +69,7 @@ func (p Text) printSourceCode(i *result.Issue) { } func (p Text) printUnderLinePointer(i *result.Issue) { - // if column == 0 it means column is unknown (e.g. for gas) + // if column == 0 it means column is unknown (e.g. for gosec) if len(i.SourceLines) != 1 || i.Pos.Column == 0 { return } diff --git a/pkg/result/processors/cgo.go b/pkg/result/processors/cgo.go index e0547ec6..c3cae42e 100644 --- a/pkg/result/processors/cgo.go +++ b/pkg/result/processors/cgo.go @@ -20,7 +20,7 @@ func (p Cgo) Name() string { func (p Cgo) Process(issues []result.Issue) ([]result.Issue, error) { return filterIssues(issues, func(i *result.Issue) bool { - // some linters (.e.g gas, deadcode) return incorrect filepaths for cgo issues, + // some linters (.e.g gosec, deadcode) return incorrect filepaths for cgo issues, // it breaks next processing, so skip them return !goutils.IsCgoFilename(i.FilePath()) }), nil diff --git a/pkg/result/processors/nolint.go b/pkg/result/processors/nolint.go index 198ea272..27ed6ee6 100644 --- a/pkg/result/processors/nolint.go +++ b/pkg/result/processors/nolint.go @@ -4,8 +4,11 @@ import ( "fmt" "go/ast" "go/token" + "sort" "strings" + "github.com/golangci/golangci-lint/pkg/lint/lintersdb" + "github.com/golangci/golangci-lint/pkg/lint/astcache" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" @@ -28,8 +31,8 @@ func (i *ignoredRange) doesMatch(issue *result.Issue) bool { return true } - for _, l := range i.linters { - if l == issue.FromLinter { + for _, linterName := range i.linters { + if linterName == issue.FromLinter { return true } } @@ -44,14 +47,21 @@ type fileData struct { type filesCache map[string]*fileData type Nolint struct { - cache filesCache - astCache *astcache.Cache + cache filesCache + astCache *astcache.Cache + dbManager *lintersdb.Manager + log logutils.Log + + unknownLintersSet map[string]bool } -func NewNolint(astCache *astcache.Cache) *Nolint { +func NewNolint(astCache *astcache.Cache, log logutils.Log) *Nolint { return &Nolint{ - cache: filesCache{}, - astCache: astCache, + cache: filesCache{}, + astCache: astCache, + dbManager: lintersdb.NewManager(), // TODO: get it in constructor + log: log, + unknownLintersSet: map[string]bool{}, } } @@ -79,13 +89,13 @@ func (p *Nolint) getOrCreateFileData(i *result.Issue) (*fileData, error) { return nil, fmt.Errorf("can't parse file %s: %s", i.FilePath(), file.Err) } - fd.ignoredRanges = buildIgnoredRangesForFile(file.F, file.Fset, i.FilePath()) + fd.ignoredRanges = p.buildIgnoredRangesForFile(file.F, file.Fset, i.FilePath()) nolintDebugf("file %s: built nolint ranges are %+v", i.FilePath(), fd.ignoredRanges) return fd, nil } -func buildIgnoredRangesForFile(f *ast.File, fset *token.FileSet, filePath string) []ignoredRange { - inlineRanges := extractFileCommentsInlineRanges(fset, f.Comments...) +func (p *Nolint) buildIgnoredRangesForFile(f *ast.File, fset *token.FileSet, filePath string) []ignoredRange { + inlineRanges := p.extractFileCommentsInlineRanges(fset, f.Comments...) nolintDebugf("file %s: inline nolint ranges are %+v", filePath, inlineRanges) if len(inlineRanges) == 0 { @@ -158,7 +168,7 @@ func (e *rangeExpander) Visit(node ast.Node) ast.Visitor { return e } -func extractFileCommentsInlineRanges(fset *token.FileSet, comments ...*ast.CommentGroup) []ignoredRange { +func (p *Nolint) extractFileCommentsInlineRanges(fset *token.FileSet, comments ...*ast.CommentGroup) []ignoredRange { var ret []ignoredRange for _, g := range comments { for _, c := range g.List { @@ -173,10 +183,16 @@ func extractFileCommentsInlineRanges(fset *token.FileSet, comments ...*ast.Comme text = strings.Split(text, "//")[0] // allow another comment after this comment linterItems := strings.Split(strings.TrimPrefix(text, "nolint:"), ",") for _, linter := range linterItems { - linterName := strings.TrimSpace(linter) // TODO: validate it here - linters = append(linters, linterName) + linterName := strings.ToLower(strings.TrimSpace(linter)) + lc := p.dbManager.GetLinterConfig(linterName) + if lc == nil { + p.unknownLintersSet[linterName] = true + continue + } + linters = append(linters, lc.Name()) // normalize name to work with aliases } } // else ignore all linters + nolintDebugf("%d: linters are %s", fset.Position(g.Pos()).Line, linters) pos := fset.Position(g.Pos()) ret = append(ret, ignoredRange{ @@ -193,4 +209,16 @@ func extractFileCommentsInlineRanges(fset *token.FileSet, comments ...*ast.Comme return ret } -func (p Nolint) Finish() {} +func (p Nolint) Finish() { + if len(p.unknownLintersSet) == 0 { + return + } + + unknownLinters := []string{} + for name := range p.unknownLintersSet { + unknownLinters = append(unknownLinters, name) + } + sort.Strings(unknownLinters) + + p.log.Warnf("Found unknown linters in //nolint directives: %s", strings.Join(unknownLinters, ", ")) +} diff --git a/pkg/result/processors/nolint_test.go b/pkg/result/processors/nolint_test.go index 9718daa9..562b11e4 100644 --- a/pkg/result/processors/nolint_test.go +++ b/pkg/result/processors/nolint_test.go @@ -1,10 +1,12 @@ package processors import ( + "fmt" "go/token" "path/filepath" "testing" + "github.com/golang/mock/gomock" "github.com/golangci/golangci-lint/pkg/lint/astcache" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" @@ -27,8 +29,22 @@ func newNolint2FileIssue(line int) result.Issue { return i } +func newTestNolintProcessor(log logutils.Log) *Nolint { + return NewNolint(astcache.NewCache(log), log) +} + +func getOkLogger(ctrl *gomock.Controller) *logutils.MockLog { + log := logutils.NewMockLog(ctrl) + log.EXPECT().Infof(gomock.Any(), gomock.Any()).AnyTimes() + return log +} + func TestNolint(t *testing.T) { - p := NewNolint(astcache.NewCache(logutils.NewStderrLog(""))) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + p := newTestNolintProcessor(getOkLogger(ctrl)) + defer p.Finish() // test inline comments processAssertEmpty(t, p, newNolintFileIssue(3, "gofmt")) @@ -99,7 +115,48 @@ func TestNolint(t *testing.T) { for i := 15; i <= 18; i++ { processAssertSame(t, p, newNolint2FileIssue(i)) } +} +func TestNolintInvalidLinterName(t *testing.T) { + fileName := filepath.Join("testdata", "nolint_bad_names.go") + issues := []result.Issue{ + { + Pos: token.Position{ + Filename: fileName, + Line: 3, + }, + FromLinter: "varcheck", + }, + { + Pos: token.Position{ + Filename: fileName, + Line: 6, + }, + FromLinter: "deadcode", + }, + } + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + log := getOkLogger(ctrl) + log.EXPECT().Warnf("Found unknown linters in //nolint directives: %s", "bad1, bad2") + + p := newTestNolintProcessor(log) + processAssertEmpty(t, p, issues...) + p.Finish() +} + +func TestNolintAliases(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + p := newTestNolintProcessor(getOkLogger(ctrl)) + for _, line := range []int{47, 49, 51} { + t.Run(fmt.Sprintf("line-%d", line), func(t *testing.T) { + processAssertEmpty(t, p, newNolintFileIssue(line, "gosec")) + }) + } + p.Finish() } func TestIgnoredRangeMatches(t *testing.T) { diff --git a/pkg/result/processors/source_code.go b/pkg/result/processors/source_code.go index eaa8036e..44710a0d 100644 --- a/pkg/result/processors/source_code.go +++ b/pkg/result/processors/source_code.go @@ -43,7 +43,7 @@ func (p SourceCode) Process(issues []result.Issue) ([]result.Issue, error) { lineRange := i.GetLineRange() var lineStr string for line := lineRange.From; line <= lineRange.To; line++ { - if line == 0 { // some linters, e.g. gas can do it: it really means first line + if line == 0 { // some linters, e.g. gosec can do it: it really means first line line = 1 } diff --git a/pkg/result/processors/testdata/nolint.go b/pkg/result/processors/testdata/nolint.go index ae67f8b8..9670daf1 100644 --- a/pkg/result/processors/testdata/nolint.go +++ b/pkg/result/processors/testdata/nolint.go @@ -43,3 +43,9 @@ func nolintFuncByPrecedingMultilineComment3() *string { xv := "v" return &xv } + +var nolintAliasGAS bool //nolint:gas + +var nolintAliasGosec bool //nolint:gosec + +var nolintAliasUpperCase int // nolint: GAS diff --git a/pkg/result/processors/testdata/nolint_bad_names.go b/pkg/result/processors/testdata/nolint_bad_names.go new file mode 100644 index 00000000..3b8b4cae --- /dev/null +++ b/pkg/result/processors/testdata/nolint_bad_names.go @@ -0,0 +1,8 @@ +package testdata + +var nolintUnknownLinter1 bool // nolint:bad1,deadcode,varcheck,megacheck + +//nolint:bad2,deadcode,megacheck +func nolintUnknownLinter2() { + +} diff --git a/scripts/gen_readme/main.go b/scripts/gen_readme/main.go index dc6bc64d..24d9f168 100644 --- a/scripts/gen_readme/main.go +++ b/scripts/gen_readme/main.go @@ -100,9 +100,9 @@ func getLintersListMarkdown(enabled bool) string { for _, lc := range neededLcs { var link string if lc.OriginalURL != "" { - link = fmt.Sprintf("[%s](%s)", lc.Linter.Name(), lc.OriginalURL) + link = fmt.Sprintf("[%s](%s)", lc.Name(), lc.OriginalURL) } else { - link = lc.Linter.Name() + link = lc.Name() } line := fmt.Sprintf("- %s - %s", link, lc.Linter.Desc()) lines = append(lines, line) diff --git a/test/bench_test.go b/test/bench_test.go index 31d8659e..ddbf8450 100644 --- a/test/bench_test.go +++ b/test/bench_test.go @@ -56,7 +56,7 @@ func getBenchLintersArgsNoMegacheck() []string { "--enable=interfacer", "--enable=unconvert", "--enable=goconst", - "--enable=gas", + "--enable=gosec", } } diff --git a/test/run_test.go b/test/run_test.go index eec04838..585fdbed 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -173,13 +173,13 @@ func getEnabledByDefaultFastLintersExcept(except ...string) []string { m := lintersdb.NewManager() ebdl := m.GetAllEnabledByDefaultLinters() ret := []string{} - for _, linter := range ebdl { - if linter.DoesFullImport { + for _, lc := range ebdl { + if lc.DoesFullImport { continue } - if !inSlice(except, linter.Linter.Name()) { - ret = append(ret, linter.Linter.Name()) + if !inSlice(except, lc.Name()) { + ret = append(ret, lc.Name()) } } @@ -189,11 +189,11 @@ func getEnabledByDefaultFastLintersExcept(except ...string) []string { func getAllFastLintersWith(with ...string) []string { linters := lintersdb.NewManager().GetAllSupportedLinterConfigs() ret := append([]string{}, with...) - for _, linter := range linters { - if linter.DoesFullImport { + for _, lc := range linters { + if lc.DoesFullImport { continue } - ret = append(ret, linter.Linter.Name()) + ret = append(ret, lc.Name()) } return ret @@ -202,8 +202,8 @@ func getAllFastLintersWith(with ...string) []string { func getEnabledByDefaultLinters() []string { ebdl := lintersdb.NewManager().GetAllEnabledByDefaultLinters() ret := []string{} - for _, linter := range ebdl { - ret = append(ret, linter.Linter.Name()) + for _, lc := range ebdl { + ret = append(ret, lc.Name()) } return ret @@ -212,12 +212,12 @@ func getEnabledByDefaultLinters() []string { func getEnabledByDefaultFastLintersWith(with ...string) []string { ebdl := lintersdb.NewManager().GetAllEnabledByDefaultLinters() ret := append([]string{}, with...) - for _, linter := range ebdl { - if linter.DoesFullImport { + for _, lc := range ebdl { + if lc.DoesFullImport { continue } - ret = append(ret, linter.Linter.Name()) + ret = append(ret, lc.Name()) } return ret @@ -372,8 +372,7 @@ func TestGovetInFastMode(t *testing.T) { } func TestEnabledPresetsAreNotDuplicated(t *testing.T) { - out, ec := runGolangciLint(t, "--no-config", "-v", "-p", "style,bugs") - assert.Equal(t, exitcodes.Success, ec) + out, _ := runGolangciLint(t, "--no-config", "-v", "-p", "style,bugs") assert.Contains(t, out, "Active presets: [bugs style]") } diff --git a/test/testdata/gas.go b/test/testdata/gosec.go similarity index 87% rename from test/testdata/gas.go rename to test/testdata/gosec.go index 3f781120..7b9993e2 100644 --- a/test/testdata/gas.go +++ b/test/testdata/gosec.go @@ -1,4 +1,4 @@ -// args: -Egas +// args: -Egosec package testdata import ( @@ -6,7 +6,7 @@ import ( "log" ) -func Gas() { +func Gosec() { h := md5.New() // ERROR "G401: Use of weak cryptographic primitive" log.Print(h) }