Prepare for #164: rename GAS to gosec

1. Rename in a backward compatible way
2. Remove gosec default exclude list because gosec is already disabled
by default.
3. Warn about unmatched linter names in //nolint directives
4. Process linter names in //nolint directives in upper case
5. Disable gosec for golangci-lint in .golangci.yml
This commit is contained in:
Denis Isaev 2018-09-01 14:16:30 +03:00 committed by Isaev Denis
parent 47440bc2cc
commit 8a478c47ac
29 changed files with 361 additions and 107 deletions

View File

@ -26,3 +26,4 @@ linters:
disable:
- maligned
- prealloc
- gosec

View File

@ -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

View File

@ -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=<defaul golangci-lint excludes> --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
./...
```

View File

@ -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, ", "))
}

View File

@ -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)
}
}

View File

@ -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) {

View File

@ -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",

View File

@ -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

View File

@ -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."},

View File

@ -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,

View File

@ -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)

View File

@ -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)
}

View File

@ -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

View File

@ -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)
}
}

View File

@ -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
}

View File

@ -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
}

View File

@ -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{})

114
pkg/logutils/log_mock.go Normal file
View File

@ -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)
}

View File

@ -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
}

View File

@ -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

View File

@ -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
}
}
@ -46,12 +49,19 @@ type filesCache map[string]*fileData
type Nolint struct {
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,
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, ", "))
}

View File

@ -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) {

View File

@ -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
}

View File

@ -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

View File

@ -0,0 +1,8 @@
package testdata
var nolintUnknownLinter1 bool // nolint:bad1,deadcode,varcheck,megacheck
//nolint:bad2,deadcode,megacheck
func nolintUnknownLinter2() {
}

View File

@ -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)

View File

@ -56,7 +56,7 @@ func getBenchLintersArgsNoMegacheck() []string {
"--enable=interfacer",
"--enable=unconvert",
"--enable=goconst",
"--enable=gas",
"--enable=gosec",
}
}

View File

@ -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]")
}

View File

@ -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)
}