diff --git a/pkg/fsutils/fsutils.go b/pkg/fsutils/fsutils.go index 91d12e5cd341..33614a712194 100644 --- a/pkg/fsutils/fsutils.go +++ b/pkg/fsutils/fsutils.go @@ -34,13 +34,13 @@ func Getwd() (string, error) { return } - evaledWd, err := EvalSymlinks(cachedWd) + evaluatedWd, err := EvalSymlinks(cachedWd) if err != nil { cachedWd, cachedWdError = "", fmt.Errorf("can't eval symlinks on wd %s: %w", cachedWd, err) return } - cachedWd = evaledWd + cachedWd = evaluatedWd }) return cachedWd, cachedWdError @@ -83,8 +83,8 @@ func ShortestRelPath(path, wd string) (string, error) { path = evaledPath // make path absolute and then relative to be able to fix this case: - // we are in /test dir, we want to normalize ../test, and have file file.go in this dir; - // it must have normalized path file.go, not ../test/file.go, + // we are in `/test` dir, we want to normalize `../test`, and have file `file.go` in this dir; + // it must have normalized path `file.go`, not `../test/file.go`, var absPath string if filepath.IsAbs(path) { absPath = path diff --git a/pkg/fsutils/fsutils_test.go b/pkg/fsutils/fsutils_test.go new file mode 100644 index 000000000000..6c6519cec4a1 --- /dev/null +++ b/pkg/fsutils/fsutils_test.go @@ -0,0 +1,42 @@ +package fsutils + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestShortestRelPath(t *testing.T) { + testCases := []struct { + desc string + path string + wd string + expected string + }{ + { + desc: "based on parent path", + path: "fsutils_test.go", + wd: filepath.Join("..", "fsutils"), + expected: "fsutils_test.go", + }, + { + desc: "based on current working directory", + path: "fsutils_test.go", + wd: "", + expected: "fsutils_test.go", + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + rel, err := ShortestRelPath("fsutils_test.go", filepath.Join("..", "fsutils")) + require.NoError(t, err) + + assert.Equal(t, test.expected, rel) + }) + } +} diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index c1f47e58fe17..4900986a9aa8 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -79,10 +79,12 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti // Must be after FilenameUnadjuster. processors.NewInvalidIssue(log.Child(logutils.DebugKeyInvalidIssue)), - // Must be before diff, nolint and exclude autogenerated processor at least. + // Must be before Diff, SkipFiles, SkipDirs, ExcludeRules processors at least. processors.NewPathPrettifier(log), + + // must be after PathPrettifier. skipFilesProcessor, - skipDirsProcessor, // must be after path prettifier + skipDirsProcessor, processors.NewAutogeneratedExclude(cfg.Issues.ExcludeGenerated), @@ -94,20 +96,21 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti processors.NewNolint(log.Child(logutils.DebugKeyNolint), dbManager, enabledLinters), - processors.NewUniqByLine(cfg), processors.NewDiff(&cfg.Issues), + + // The fixer still needs to see paths for the issues that are relative to the current directory. + processors.NewFixer(cfg, log, fileCache, metaFormatter), + + // Must be after the Fixer. + processors.NewUniqByLine(cfg), processors.NewMaxPerFileFromLinter(cfg), processors.NewMaxSameIssues(cfg.Issues.MaxSameIssues, log.Child(logutils.DebugKeyMaxSameIssues), cfg), processors.NewMaxFromLinter(cfg.Issues.MaxIssuesPerLinter, log.Child(logutils.DebugKeyMaxFromLinter), cfg), + // Now we can modify the issues for output. processors.NewSourceCode(lineCache, log.Child(logutils.DebugKeySourceCode)), processors.NewPathShortener(), processors.NewSeverity(log.Child(logutils.DebugKeySeverityRules), files, &cfg.Severity), - - // The fixer still needs to see paths for the issues that are relative to the current directory. - processors.NewFixer(cfg, log, fileCache, metaFormatter), - - // Now we can modify the issues for output. processors.NewPathPrefixer(cfg.Output.PathPrefix), processors.NewSortResults(cfg), }, diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index 82316f6a0a6a..abf73d9c172d 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -39,6 +39,10 @@ type fileSummary struct { generated bool } +// AutogeneratedExclude filters generated files. +// - mode "lax": see `isGeneratedFileLax` documentation. +// - mode "strict": see `isGeneratedFileStrict` documentation. +// - mode "disable": skips this processor. type AutogeneratedExclude struct { debugf logutils.DebugFunc diff --git a/pkg/result/processors/cgo.go b/pkg/result/processors/cgo.go index 2fc907b401c7..34dc8a7cfb92 100644 --- a/pkg/result/processors/cgo.go +++ b/pkg/result/processors/cgo.go @@ -10,10 +10,11 @@ import ( var _ Processor = (*Cgo)(nil) -// Cgo some linters (e.g. gosec, deadcode) return incorrect filepaths for cgo issues, -// also cgo files have strange issues looking like false positives. +// Cgo filters cgo artifacts. // -// Require absolute filepath. +// Some linters (e.g. gosec, etc.) return incorrect file paths for cgo files. +// +// Require absolute file path. type Cgo struct { goCacheDir string } diff --git a/pkg/result/processors/diff.go b/pkg/result/processors/diff.go index b88d4dd763bd..5b07ffd0f0c1 100644 --- a/pkg/result/processors/diff.go +++ b/pkg/result/processors/diff.go @@ -18,6 +18,12 @@ const envGolangciDiffProcessorPatch = "GOLANGCI_DIFF_PROCESSOR_PATCH" var _ Processor = (*Diff)(nil) +// Diff filters issues based on options `new`, `new-from-rev`, etc. +// +// Uses `git`. +// The paths inside the patch are relative to the path where git is run (the same location where golangci-lint is run). +// +// Warning: it doesn't use `path-prefix` option. type Diff struct { onlyNew bool fromRev string diff --git a/pkg/result/processors/exclude.go b/pkg/result/processors/exclude.go index 5431204502a7..c9bee7f9f4ff 100644 --- a/pkg/result/processors/exclude.go +++ b/pkg/result/processors/exclude.go @@ -11,6 +11,7 @@ import ( var _ Processor = (*Exclude)(nil) +// Exclude filters reports only based on regular expressions applied to the report text. type Exclude struct { name string diff --git a/pkg/result/processors/exclude_rules.go b/pkg/result/processors/exclude_rules.go index bf255ae82f50..dfd7a35eaa6d 100644 --- a/pkg/result/processors/exclude_rules.go +++ b/pkg/result/processors/exclude_rules.go @@ -15,6 +15,13 @@ type excludeRule struct { baseRule } +// ExcludeRules filters reports based on multiple criteria: +// - linter names (string) +// - file path (regular expressions) +// - text (regular expressions) +// - code source (regular expressions) +// +// It uses the shortest relative paths and `path-prefix` option. type ExcludeRules struct { name string diff --git a/pkg/result/processors/filename_unadjuster.go b/pkg/result/processors/filename_unadjuster.go index dbbc536dbef0..5f39e064b9a3 100644 --- a/pkg/result/processors/filename_unadjuster.go +++ b/pkg/result/processors/filename_unadjuster.go @@ -22,11 +22,13 @@ type adjustMap struct { m map[string]posMapper } -// FilenameUnadjuster is needed because a lot of linters use `fset.Position(f.Pos())` to get filename. -// And they return adjusted filename (e.g.` *.qtpl`) for an issue. +// FilenameUnadjuster fixes filename based on adjusted and unadjusted position (related to line directives and cgo). +// +// A lot of linters use `fset.Position(f.Pos())` to get filename, +// and they return adjusted filename (e.g.` *.qtpl`) for an issue. // We need restore real `.go` filename to properly output it, parse it, etc. // -// Require absolute filepath. +// Require absolute file path. type FilenameUnadjuster struct { m map[string]posMapper // map from adjusted filename to position mapper: adjusted -> unadjusted position log logutils.Log diff --git a/pkg/result/processors/fixer.go b/pkg/result/processors/fixer.go index b82c7f207192..c49630da8368 100644 --- a/pkg/result/processors/fixer.go +++ b/pkg/result/processors/fixer.go @@ -31,6 +31,8 @@ var _ Processor = (*Fixer)(nil) const filePerm = 0644 +// Fixer fixes reports if possible. +// The reports that are not fixed are passed to the next processor. type Fixer struct { cfg *config.Config log logutils.Log diff --git a/pkg/result/processors/identifier_marker.go b/pkg/result/processors/identifier_marker.go index a1590dfacfca..9f332705e1f1 100644 --- a/pkg/result/processors/identifier_marker.go +++ b/pkg/result/processors/identifier_marker.go @@ -13,6 +13,9 @@ type replacePattern struct { repl string } +// IdentifierMarker modifies report text. +// It must be before [Exclude] and [ExcludeRules]: +// users configure exclusions based on the modified text. type IdentifierMarker struct { patterns map[string][]replacePattern } diff --git a/pkg/result/processors/invalid_issue.go b/pkg/result/processors/invalid_issue.go index 3f6cfc540b42..042675b59e1a 100644 --- a/pkg/result/processors/invalid_issue.go +++ b/pkg/result/processors/invalid_issue.go @@ -9,6 +9,9 @@ import ( var _ Processor = (*InvalidIssue)(nil) +// InvalidIssue filters invalid reports. +// - non-go files (except `go.mod`) +// - reports without file path type InvalidIssue struct { log logutils.Log } diff --git a/pkg/result/processors/max_from_linter.go b/pkg/result/processors/max_from_linter.go index 690cdf3f8ab8..ced200af722e 100644 --- a/pkg/result/processors/max_from_linter.go +++ b/pkg/result/processors/max_from_linter.go @@ -8,6 +8,7 @@ import ( var _ Processor = (*MaxFromLinter)(nil) +// MaxFromLinter limits the number of reports from the same linter. type MaxFromLinter struct { linterCounter map[string]int limit int @@ -34,11 +35,6 @@ func (p *MaxFromLinter) Process(issues []result.Issue) ([]result.Issue, error) { } return filterIssuesUnsafe(issues, func(issue *result.Issue) bool { - if issue.SuggestedFixes != nil && p.cfg.Issues.NeedFix { - // we need to fix all issues at once => we need to return all of them - return true - } - p.linterCounter[issue.FromLinter]++ // always inc for stat return p.linterCounter[issue.FromLinter] <= p.limit diff --git a/pkg/result/processors/max_per_file_from_linter.go b/pkg/result/processors/max_per_file_from_linter.go index 3ec49f3acfad..7c59b5dd60e4 100644 --- a/pkg/result/processors/max_per_file_from_linter.go +++ b/pkg/result/processors/max_per_file_from_linter.go @@ -7,6 +7,7 @@ import ( var _ Processor = (*MaxPerFileFromLinter)(nil) +// MaxPerFileFromLinter limits the number of reports by file and by linter. type MaxPerFileFromLinter struct { fileLinterCounter fileLinterCounter maxPerFileFromLinterConfig map[string]int diff --git a/pkg/result/processors/max_same_issues.go b/pkg/result/processors/max_same_issues.go index 0d1c28628251..349f6a9afacf 100644 --- a/pkg/result/processors/max_same_issues.go +++ b/pkg/result/processors/max_same_issues.go @@ -10,6 +10,7 @@ import ( var _ Processor = (*MaxSameIssues)(nil) +// MaxSameIssues limits the number of reports with the same text. type MaxSameIssues struct { textCounter map[string]int limit int @@ -36,12 +37,8 @@ func (p *MaxSameIssues) Process(issues []result.Issue) ([]result.Issue, error) { } return filterIssuesUnsafe(issues, func(issue *result.Issue) bool { - if issue.SuggestedFixes != nil && p.cfg.Issues.NeedFix { - // we need to fix all issues at once => we need to return all of them - return true - } - p.textCounter[issue.Text]++ // always inc for stat + return p.textCounter[issue.Text] <= p.limit }), nil } diff --git a/pkg/result/processors/nolint.go b/pkg/result/processors/nolint.go index 7794bd3ecb6e..6c051229201b 100644 --- a/pkg/result/processors/nolint.go +++ b/pkg/result/processors/nolint.go @@ -64,6 +64,7 @@ type fileData struct { ignoredRanges []ignoredRange } +// Nolint filters and sorts reports related to `nolint` directives. type Nolint struct { fileCache map[string]*fileData dbManager *lintersdb.Manager diff --git a/pkg/result/processors/path_prefixer.go b/pkg/result/processors/path_prefixer.go index 8036e3fd6d7f..09f290a2b695 100644 --- a/pkg/result/processors/path_prefixer.go +++ b/pkg/result/processors/path_prefixer.go @@ -7,7 +7,8 @@ import ( var _ Processor = (*PathPrefixer)(nil) -// PathPrefixer adds a customizable prefix to every output path +// PathPrefixer adds a customizable path prefix to report file paths for user facing. +// It uses the shortest relative paths and `path-prefix` option. type PathPrefixer struct { prefix string } @@ -24,11 +25,14 @@ func (*PathPrefixer) Name() string { // Process adds the prefix to each path func (p *PathPrefixer) Process(issues []result.Issue) ([]result.Issue, error) { - if p.prefix != "" { - for i := range issues { - issues[i].Pos.Filename = fsutils.WithPathPrefix(p.prefix, issues[i].Pos.Filename) - } + if p.prefix == "" { + return issues, nil } + + for i := range issues { + issues[i].Pos.Filename = fsutils.WithPathPrefix(p.prefix, issues[i].Pos.Filename) + } + return issues, nil } diff --git a/pkg/result/processors/path_prettifier.go b/pkg/result/processors/path_prettifier.go index 4135265a6f18..7ccdb5ea98e5 100644 --- a/pkg/result/processors/path_prettifier.go +++ b/pkg/result/processors/path_prettifier.go @@ -10,6 +10,7 @@ import ( var _ Processor = (*PathPrettifier)(nil) +// PathPrettifier modifies report file path with the shortest relative path. type PathPrettifier struct { log logutils.Log } diff --git a/pkg/result/processors/path_shortener.go b/pkg/result/processors/path_shortener.go index b161e86c2f46..0c0288269e1a 100644 --- a/pkg/result/processors/path_shortener.go +++ b/pkg/result/processors/path_shortener.go @@ -10,6 +10,8 @@ import ( var _ Processor = (*PathShortener)(nil) +// PathShortener modifies text of the reports to reduce file path inside the text. +// It uses the rooted path name corresponding to the current directory (`wd`). type PathShortener struct { wd string } diff --git a/pkg/result/processors/severity.go b/pkg/result/processors/severity.go index fcd65326f971..fa853527a759 100644 --- a/pkg/result/processors/severity.go +++ b/pkg/result/processors/severity.go @@ -19,6 +19,10 @@ type severityRule struct { severity string } +// Severity modifies report severity. +// It uses the same `baseRule` structure as [ExcludeRules] processor. +// +// Warning: it doesn't use `path-prefix` option. type Severity struct { name string diff --git a/pkg/result/processors/skip_dirs.go b/pkg/result/processors/skip_dirs.go index 39dbfd1d388c..6f50ece18480 100644 --- a/pkg/result/processors/skip_dirs.go +++ b/pkg/result/processors/skip_dirs.go @@ -21,6 +21,8 @@ var StdExcludeDirRegexps = []string{ normalizePathRegex("builtin"), } +// SkipDirs filters reports based on directory names. +// It uses the shortest relative paths and `path-prefix` option. type skipStat struct { pattern string count int diff --git a/pkg/result/processors/skip_files.go b/pkg/result/processors/skip_files.go index 3b17a9f327cf..fea516f62aa3 100644 --- a/pkg/result/processors/skip_files.go +++ b/pkg/result/processors/skip_files.go @@ -10,6 +10,9 @@ import ( var _ Processor = (*SkipFiles)(nil) +// SkipFiles filters reports based on filename. +// +// It uses the shortest relative paths and `path-prefix` option. type SkipFiles struct { patterns []*regexp.Regexp pathPrefix string diff --git a/pkg/result/processors/sort_results.go b/pkg/result/processors/sort_results.go index 7eebea6315ef..0bb9c51b975e 100644 --- a/pkg/result/processors/sort_results.go +++ b/pkg/result/processors/sort_results.go @@ -10,11 +10,6 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) -// Base propose of this functionality to sort results (issues) -// produced by various linters by analyzing code. We're achieving this -// by sorting results.Issues using processor step, and chain based -// rules that can compare different properties of the Issues struct. - const ( orderNameFile = "file" orderNameLinter = "linter" @@ -31,6 +26,10 @@ var _ Processor = (*SortResults)(nil) type issueComparator func(a, b *result.Issue) int +// SortResults sorts reports based on criteria: +// - file names, line numbers, positions +// - linter names +// - severity names type SortResults struct { cmps map[string][]issueComparator diff --git a/pkg/result/processors/source_code.go b/pkg/result/processors/source_code.go index 4a89fc73ed7e..3f20b2f5600c 100644 --- a/pkg/result/processors/source_code.go +++ b/pkg/result/processors/source_code.go @@ -8,6 +8,13 @@ import ( var _ Processor = (*SourceCode)(nil) +// SourceCode modifies displayed information based on [result.Issue.GetLineRange()]. +// +// This is used: +// - to display the "UnderLinePointer". +// - in some rare cases to display multiple lines instead of one (ex: `dupl`) +// +// It requires to use [fsutils.LineCache] ([fsutils.FileCache]) to get the file information before the fixes. type SourceCode struct { lineCache *fsutils.LineCache log logutils.Log diff --git a/pkg/result/processors/uniq_by_line.go b/pkg/result/processors/uniq_by_line.go index ec134f25f6ae..3f84f705e7c5 100644 --- a/pkg/result/processors/uniq_by_line.go +++ b/pkg/result/processors/uniq_by_line.go @@ -9,6 +9,7 @@ const uniqByLineLimit = 1 var _ Processor = (*UniqByLine)(nil) +// UniqByLine filters reports to keep only one report by line of code. type UniqByLine struct { fileLineCounter fileLineCounter cfg *config.Config @@ -36,12 +37,6 @@ func (p *UniqByLine) Process(issues []result.Issue) ([]result.Issue, error) { func (*UniqByLine) Finish() {} func (p *UniqByLine) shouldPassIssue(issue *result.Issue) bool { - if issue.SuggestedFixes != nil && p.cfg.Issues.NeedFix { - // if issue will be auto-fixed we shouldn't collapse issues: - // e.g. one line can contain 2 misspellings, they will be in 2 issues and misspell should fix both of them. - return true - } - if p.fileLineCounter.GetCount(issue) == uniqByLineLimit { return false }