Skip to content

Feature/enable autofix on whitespace #674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion pkg/commands/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func (e *Executor) initConfig() {
}
e.initRunConfiguration(pathCmd) // allow --config
cmd.AddCommand(pathCmd)

}

func (e *Executor) executePathCmd(_ *cobra.Command, args []string) {
Expand Down
1 change: 0 additions & 1 deletion pkg/golinters/goanalysis/checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions pkg/golinters/gocritic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
37 changes: 33 additions & 4 deletions pkg/golinters/whitespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/lint/lintersdb/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}

Expand Down
1 change: 0 additions & 1 deletion pkg/lint/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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()

Expand Down
21 changes: 19 additions & 2 deletions pkg/result/processors/fixer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -56,6 +72,7 @@ func (f Fixer) Process(issues <-chan result.Issue) <-chan result.Issue {
}
}
}
f.printStat()
close(outCh)
}()

Expand Down
4 changes: 3 additions & 1 deletion test/fix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
47 changes: 47 additions & 0 deletions test/testdata/fix/in/whitespace.go
Original file line number Diff line number Diff line change
@@ -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")
}
43 changes: 43 additions & 0 deletions test/testdata/fix/out/whitespace.go
Original file line number Diff line number Diff line change
@@ -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")
}
20 changes: 0 additions & 20 deletions test/testdata/whitespace.go

This file was deleted.