diff --git a/.golangci.yml b/.golangci.yml index eb7ac5e4..9fe4e8a6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -75,6 +75,7 @@ linters: - unparam - unused - varcheck + # - whitespace - TODO: enable it when golangci.com will support it. # don't enable: # - depguard - until https://github.com/OpenPeeDeeP/depguard/issues/7 gets fixed diff --git a/Makefile b/Makefile index e96ebef7..10a1221a 100644 --- a/Makefile +++ b/Makefile @@ -3,11 +3,13 @@ # Build +fast_build: FORCE + go build -o golangci-lint ./cmd/golangci-lint build: golangci-lint clean: rm -f golangci-lint test/path rm -rf tools -.PHONY: build clean +.PHONY: fast_build build clean # Test diff --git a/README.md b/README.md index d78f4cc4..7ab490b0 100644 --- a/README.md +++ b/README.md @@ -216,7 +216,7 @@ scopelint: Scopelint checks for unpinned variables in go programs [fast: true, a stylecheck: Stylecheck is a replacement for golint [fast: false, auto-fix: false] unconvert: Remove unnecessary type conversions [fast: true, auto-fix: false] unparam: Reports unused function parameters [fast: false, auto-fix: false] -whitespace: Tool for detection of leading and trailing whitespace [fast: true, auto-fix: false] +whitespace: Tool for detection of leading and trailing whitespace [fast: true, auto-fix: true] ``` Pass `-E/--enable` to enable linter and `-D/--disable` to disable: @@ -925,6 +925,7 @@ linters: - unparam - unused - varcheck + # - whitespace - TODO: enable it when golangci.com will support it. # don't enable: # - depguard - until https://github.com/OpenPeeDeeP/depguard/issues/7 gets fixed diff --git a/pkg/commands/config.go b/pkg/commands/config.go index 537b36fb..a1d1bd9b 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -34,7 +34,6 @@ func (e *Executor) initConfig() { } e.initRunConfiguration(pathCmd) // allow --config cmd.AddCommand(pathCmd) - } func (e *Executor) executePathCmd(_ *cobra.Command, args []string) { diff --git a/pkg/golinters/goanalysis/checker/checker.go b/pkg/golinters/goanalysis/checker/checker.go index 56942b80..4d50124a 100644 --- a/pkg/golinters/goanalysis/checker/checker.go +++ b/pkg/golinters/goanalysis/checker/checker.go @@ -348,7 +348,6 @@ func (act *action) execOnce() { // in-memory outputs of prerequisite analyzers // become inputs to this analysis pass. inputs[dep.a] = dep.result - } else if dep.a == act.a { // (always true) // Same analysis, different package (vertical edge): // serialized facts produced by prerequisite analysis diff --git a/pkg/golinters/gocritic.go b/pkg/golinters/gocritic.go index 63b9b154..ccb5721f 100644 --- a/pkg/golinters/gocritic.go +++ b/pkg/golinters/gocritic.go @@ -134,7 +134,6 @@ func (lint Gocritic) Run(ctx context.Context, lintCtx *linter.Context) ([]result func (lint Gocritic) runOnPackage(lintpackCtx *lintpack.Context, checkers []*lintpack.Checker, pkgInfo *loader.PackageInfo, ret chan<- result.Issue) { - for _, f := range pkgInfo.Files { filename := filepath.Base(lintpackCtx.FileSet.Position(f.Pos()).Filename) lintpackCtx.SetFileInfo(filename, f) @@ -145,7 +144,6 @@ func (lint Gocritic) runOnPackage(lintpackCtx *lintpack.Context, checkers []*lin func (lint Gocritic) runOnFile(ctx *lintpack.Context, f *ast.File, checkers []*lintpack.Checker, ret chan<- result.Issue) { - var wg sync.WaitGroup wg.Add(len(checkers)) for _, c := range checkers { diff --git a/pkg/golinters/whitespace.go b/pkg/golinters/whitespace.go index bb6f4aaa..3143232d 100644 --- a/pkg/golinters/whitespace.go +++ b/pkg/golinters/whitespace.go @@ -4,13 +4,16 @@ import ( "context" "go/token" + "github.com/pkg/errors" + "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" "github.com/ultraware/whitespace" ) -type Whitespace struct{} +type Whitespace struct { +} func (Whitespace) Name() string { return "whitespace" @@ -32,14 +35,40 @@ func (w Whitespace) Run(ctx context.Context, lintCtx *linter.Context) ([]result. res := make([]result.Issue, len(issues)) for k, i := range issues { - res[k] = result.Issue{ + issue := result.Issue{ Pos: token.Position{ Filename: i.Pos.Filename, Line: i.Pos.Line, }, - Text: i.Message, - FromLinter: w.Name(), + Text: i.Message, + FromLinter: w.Name(), + Replacement: &result.Replacement{}, } + + // TODO(jirfag): return more information from Whitespace to get rid of string comparisons + if i.Message == "unnecessary leading newline" { + // cover two lines by the issue: opening bracket "{" (issue.Pos.Line) and following empty line + issue.LineRange = &result.Range{From: issue.Pos.Line, To: issue.Pos.Line + 1} + + bracketLine, err := lintCtx.LineCache.GetLine(issue.Pos.Filename, issue.Pos.Line) + if err != nil { + return nil, errors.Wrapf(err, "failed to get line %s:%d", issue.Pos.Filename, issue.Pos.Line) + } + issue.Replacement.NewLines = []string{bracketLine} + } else { + // cover two lines by the issue: closing bracket "}" (issue.Pos.Line) and preceding empty line + issue.LineRange = &result.Range{From: issue.Pos.Line - 1, To: issue.Pos.Line} + + bracketLine, err := lintCtx.LineCache.GetLine(issue.Pos.Filename, issue.Pos.Line) + if err != nil { + return nil, errors.Wrapf(err, "failed to get line %s:%d", issue.Pos.Filename, issue.Pos.Line) + } + issue.Replacement.NewLines = []string{bracketLine} + + issue.Pos.Line-- // set in sync with LineRange.From to not break fixer and other code features + } + + res[k] = issue } return res, nil diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 24644c6b..21bdb01c 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -245,6 +245,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { linter.NewConfig(golinters.Whitespace{}). WithPresets(linter.PresetStyle). WithSpeed(10). + WithAutoFix(). WithURL("https://github.com/ultraware/whitespace"), } diff --git a/pkg/lint/load.go b/pkg/lint/load.go index 9a8d0ccb..b4229ad9 100644 --- a/pkg/lint/load.go +++ b/pkg/lint/load.go @@ -39,7 +39,6 @@ type ContextLoader struct { func NewContextLoader(cfg *config.Config, log logutils.Log, goenv *goutil.Env, lineCache *fsutils.LineCache, fileCache *fsutils.FileCache) *ContextLoader { - return &ContextLoader{ cfg: cfg, log: log, diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 1de78e71..436e7c4d 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -31,7 +31,6 @@ type Runner struct { func NewRunner(astCache *astcache.Cache, cfg *config.Config, log logutils.Log, goenv *goutil.Env, lineCache *fsutils.LineCache, dbManager *lintersdb.Manager) (*Runner, error) { - icfg := cfg.Issues excludePatterns := icfg.ExcludePatterns if icfg.UseDefaultExcludes { @@ -101,7 +100,6 @@ type lintRes struct { func (r *Runner) runLinterSafe(ctx context.Context, lintCtx *linter.Context, lc *linter.Config) (ret []result.Issue, err error) { - defer func() { if panicData := recover(); panicData != nil { err = fmt.Errorf("panic occurred: %s", panicData) @@ -125,7 +123,6 @@ func (r *Runner) runLinterSafe(ctx context.Context, lintCtx *linter.Context, func (r Runner) runWorker(ctx context.Context, lintCtx *linter.Context, tasksCh <-chan *linter.Config, lintResultsCh chan<- lintRes, name string) { - sw := timeutils.NewStopwatch(name, r.Log) defer sw.Print() diff --git a/pkg/result/processors/fixer.go b/pkg/result/processors/fixer.go index a5497b0c..463724c1 100644 --- a/pkg/result/processors/fixer.go +++ b/pkg/result/processors/fixer.go @@ -8,6 +8,8 @@ import ( "sort" "strings" + "github.com/golangci/golangci-lint/pkg/timeutils" + "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/logutils" @@ -22,10 +24,20 @@ type Fixer struct { cfg *config.Config log logutils.Log fileCache *fsutils.FileCache + sw *timeutils.Stopwatch } func NewFixer(cfg *config.Config, log logutils.Log, fileCache *fsutils.FileCache) *Fixer { - return &Fixer{cfg: cfg, log: log, fileCache: fileCache} + return &Fixer{ + cfg: cfg, + log: log, + fileCache: fileCache, + sw: timeutils.NewStopwatch("fixer", log), + } +} + +func (f Fixer) printStat() { + f.sw.PrintStages() } func (f Fixer) Process(issues <-chan result.Issue) <-chan result.Issue { @@ -47,7 +59,11 @@ func (f Fixer) Process(issues <-chan result.Issue) <-chan result.Issue { } for file, issuesToFix := range issuesToFixPerFile { - if err := f.fixIssuesInFile(file, issuesToFix); err != nil { + var err error + f.sw.TrackStage("all", func() { + err = f.fixIssuesInFile(file, issuesToFix) //nolint:scopelint + }) + if err != nil { f.log.Errorf("Failed to fix issues in file %s: %s", file, err) // show issues only if can't fix them @@ -56,6 +72,7 @@ func (f Fixer) Process(issues <-chan result.Issue) <-chan result.Issue { } } } + f.printStat() close(outCh) }() diff --git a/test/fix_test.go b/test/fix_test.go index b480a3ac..3f20c217 100644 --- a/test/fix_test.go +++ b/test/fix_test.go @@ -24,7 +24,9 @@ func TestFix(t *testing.T) { tmpDir := filepath.Join(testdataDir, "fix.tmp") os.RemoveAll(tmpDir) // cleanup after previous runs - if os.Getenv("GL_KEEP_TEMP_FILES") != "1" { + if os.Getenv("GL_KEEP_TEMP_FILES") == "1" { + t.Logf("Temp dir for fix test: %s", tmpDir) + } else { defer os.RemoveAll(tmpDir) } diff --git a/test/testdata/fix/in/whitespace.go b/test/testdata/fix/in/whitespace.go new file mode 100644 index 00000000..915d36e3 --- /dev/null +++ b/test/testdata/fix/in/whitespace.go @@ -0,0 +1,47 @@ +//args: -Ewhitespace +package p + +import "fmt" + +func oneLeadingNewline() { + + fmt.Println("Hello world") +} + +func oneNewlineAtBothEnds() { + + fmt.Println("Hello world") + +} + +func noNewlineFunc() { +} + +func oneNewlineFunc() { + +} + +func twoNewlinesFunc() { + + +} + +func noNewlineWithCommentFunc() { + // some comment +} + +func oneTrailingNewlineWithCommentFunc() { + // some comment + +} + +func oneLeadingNewlineWithCommentFunc() { + + // some comment +} + +func twoLeadingNewlines() { + + + fmt.Println("Hello world") +} diff --git a/test/testdata/fix/out/whitespace.go b/test/testdata/fix/out/whitespace.go new file mode 100644 index 00000000..5c8b997e --- /dev/null +++ b/test/testdata/fix/out/whitespace.go @@ -0,0 +1,43 @@ +//args: -Ewhitespace +package p + +import "fmt" + +func oneLeadingNewline() { + fmt.Println("Hello world") +} + +func oneNewlineAtBothEnds() { + fmt.Println("Hello world") +} + +func noNewlineFunc() { +} + +func oneNewlineFunc() { + +} + +func twoNewlinesFunc() { + + +} + +func noNewlineWithCommentFunc() { + // some comment +} + +func oneTrailingNewlineWithCommentFunc() { + // some comment + +} + +func oneLeadingNewlineWithCommentFunc() { + + // some comment +} + +func twoLeadingNewlines() { + + fmt.Println("Hello world") +} diff --git a/test/testdata/whitespace.go b/test/testdata/whitespace.go deleted file mode 100644 index 5a92cee3..00000000 --- a/test/testdata/whitespace.go +++ /dev/null @@ -1,20 +0,0 @@ -//args: -Ewhitespace -package testdata - -func UselessStart() { // ERROR "unnecessary leading newline" - - a := 1 - _ = a -} - -func UselessEnd() { - a := 1 - _ = a - -} // ERROR "unnecessary trailing newline" - -func CommentsShouldBeIgnored() { - // test - a := 1 - _ = a -}