Skip to content

Commit

Permalink
fix #513: don't add gofmt "with -s" if not needed
Browse files Browse the repository at this point in the history
Output
  File is not `gofmt`-ed
insted of
  File is not `gofmt`-ed  with `-s`
when gofmt.simplify == false
  • Loading branch information
jirfag committed Jun 9, 2019
1 parent 5c86bfc commit 6508d16
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 22 deletions.
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ go:
- 1.11.x
- 1.12.x

before_script:
- go get github.com/valyala/quicktemplate

script: make check_generated test

after_success:
Expand Down
21 changes: 10 additions & 11 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
.DEFAULT_GOAL = test
.PHONY: FORCE
export GO111MODULE = on

# Build

Expand All @@ -18,8 +17,6 @@ test: build
GL_TEST_RUN=1 ./golangci-lint run --no-config -v --skip-dirs 'test/testdata_etc,pkg/golinters/goanalysis/(checker|passes)'
GL_TEST_RUN=1 go test -v ./...

build:
go build -o golangci-lint ./cmd/golangci-lint
.PHONY: test

test_race:
Expand Down Expand Up @@ -55,18 +52,18 @@ golangci-lint: FORCE pkg/logutils/log_mock.go
go build -o $@ ./cmd/golangci-lint

tools/mockgen: go.mod go.sum
GOBIN=$(CURDIR)/tools go install github.com/golang/mock/mockgen
GOBIN=$(CURDIR)/tools GO111MODULE=on go install github.com/golang/mock/mockgen

tools/goimports: go.mod go.sum
GOBIN=$(CURDIR)/tools go install golang.org/x/tools/cmd/goimports
GOBIN=$(CURDIR)/tools GO111MODULE=on go install golang.org/x/tools/cmd/goimports

tools/go.mod:
@mkdir -p tools
@rm -f $@
cd tools && go mod init local-tools
cd tools && GO111MODULE=on go mod init local-tools

tools/godownloader: Makefile tools/go.mod
cd tools && GOBIN=$(CURDIR)/tools go get github.com/goreleaser/godownloader@3b90d248ba30307915288f08ab3f2fc2d9f6710c
cd tools && GOBIN=$(CURDIR)/tools GO111MODULE=on go get github.com/goreleaser/godownloader@3b90d248ba30307915288f08ab3f2fc2d9f6710c

tools/svg-term:
@mkdir -p tools
Expand All @@ -81,7 +78,8 @@ docs/demo.svg: tools/svg-term tools/Dracula.itermcolors
PATH=$(CURDIR)/tools:$${PATH} svg-term --cast=183662 --out docs/demo.svg --window --width 110 --height 30 --from 2000 --to 20000 --profile ./tools/Dracula.itermcolors --term iterm2

install.sh: tools/godownloader .goreleaser.yml
PATH=$(CURDIR)/tools:$${PATH} tools/godownloader .goreleaser.yml | sed '/DO NOT EDIT/s/ on [0-9TZ:-]*//' > $@
# TODO: use when Windows installation will be fixed in the upstream
#PATH=$(CURDIR)/tools:$${PATH} tools/godownloader .goreleaser.yml | sed '/DO NOT EDIT/s/ on [0-9TZ:-]*//' > $@

README.md: FORCE golangci-lint
go run ./scripts/gen_readme/main.go
Expand All @@ -91,10 +89,11 @@ pkg/logutils/log_mock.go: tools/mockgen tools/goimports pkg/logutils/log.go
PATH=$(CURDIR)/tools:$${PATH} go generate ./...

go.mod: FORCE
go mod verify
go mod tidy
GO111MODULE=on go mod verify
GO111MODULE=on go mod tidy
go.sum: go.mod

.PHONY: vendor
vendor: go.mod go.sum
rm -rf vendor
go mod vendor
GO111MODULE=on go mod vendor
11 changes: 8 additions & 3 deletions pkg/golinters/gofmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (p *hunkChangesParser) parse(h *diffpkg.Hunk) []Change {
return p.ret
}

func (g Gofmt) extractIssuesFromPatch(patch string, log logutils.Log) ([]result.Issue, error) {
func (g Gofmt) extractIssuesFromPatch(patch string, log logutils.Log, lintCtx *linter.Context) ([]result.Issue, error) {
diffs, err := diffpkg.ParseMultiFileDiff([]byte(patch))
if err != nil {
return nil, errors.Wrap(err, "can't parse patch")
Expand All @@ -251,9 +251,14 @@ func (g Gofmt) extractIssuesFromPatch(patch string, log logutils.Log) ([]result.
}

for _, hunk := range d.Hunks {
text := "File is not `gofmt`-ed with `-s`"
var text string
if g.UseGoimports {
text = "File is not `goimports`-ed"
} else {
text = "File is not `gofmt`-ed"
if lintCtx.Settings().Gofmt.Simplify {
text += " with `-s`"
}
}
p := hunkChangesParser{
log: log,
Expand Down Expand Up @@ -301,7 +306,7 @@ func (g Gofmt) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue
continue
}

is, err := g.extractIssuesFromPatch(string(diff), lintCtx.Log)
is, err := g.extractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx)
if err != nil {
return nil, fmt.Errorf("can't extract issues from gofmt diff output %q: %s", string(diff), err)
}
Expand Down
6 changes: 5 additions & 1 deletion test/errchk.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
}
matched := false
n := len(out)
var textsToMatch []string
for _, errmsg := range errmsgs {
// Assume errmsg says "file:line: foo".
// Cut leading "file:line: " to avoid accidental matching of file name instead of message.
Expand All @@ -63,10 +64,13 @@ func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
matched = true
} else {
out = append(out, errmsg)
textsToMatch = append(textsToMatch, text)
}
}
if !matched {
errs = append(errs, fmt.Errorf("%s:%d: no match for %#q in:\n\t%s", we.file, we.lineNum, we.reStr, strings.Join(out[n:], "\n\t")))
err := fmt.Errorf("%s:%d: no match for %#q vs %q in:\n\t%s",
we.file, we.lineNum, we.reStr, textsToMatch, strings.Join(out[n:], "\n\t"))
errs = append(errs, err)
continue
}
}
Expand Down
13 changes: 13 additions & 0 deletions test/testdata/gofmt_no_simplify.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//args: -Egofmt
//config: linters-settings.gofmt.simplify=false
package testdata

import "fmt"

func GofmtNotSimplifiedOk() {
var x []string
fmt.Print(x[1:len(x)])
}

func GofmtBadFormat(){ // ERROR "^File is not `gofmt`-ed$"
}
12 changes: 7 additions & 5 deletions test/testshared/testshared.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ import (
)

type LintRunner struct {
t assert.TestingT
log logutils.Log
env []string
t assert.TestingT
log logutils.Log
env []string
installed bool
}

func NewLintRunner(t assert.TestingT, environ ...string) *LintRunner {
Expand All @@ -30,12 +31,13 @@ func NewLintRunner(t assert.TestingT, environ ...string) *LintRunner {
}

func (r *LintRunner) Install() {
if _, err := os.Stat("../golangci-lint"); err == nil {
if r.installed {
return
}

cmd := exec.Command("make", "-C", "..", "build")
assert.NoError(r.t, cmd.Run(), "Can't go install golangci-lint")
r.installed = true
}

type RunResult struct {
Expand Down Expand Up @@ -79,7 +81,7 @@ func (r *LintRunner) Run(args ...string) *RunResult {
r.Install()

runArgs := append([]string{"run"}, args...)
r.log.Infof("golangci-lint %s", strings.Join(runArgs, " "))
r.log.Infof("../golangci-lint %s", strings.Join(runArgs, " "))
cmd := exec.Command("../golangci-lint", runArgs...)
cmd.Env = append(os.Environ(), r.env...)
out, err := cmd.CombinedOutput()
Expand Down
4 changes: 2 additions & 2 deletions vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,12 @@ github.com/spf13/viper
# github.com/stretchr/testify v1.2.2
github.com/stretchr/testify/assert
github.com/stretchr/testify/require
# github.com/timakin/bodyclose v0.0.0-20190407043127-4a873e97b2bb
github.com/timakin/bodyclose/passes/bodyclose
# github.com/valyala/bytebufferpool v1.0.0
github.com/valyala/bytebufferpool
# github.com/valyala/quicktemplate v1.1.1
github.com/valyala/quicktemplate
# github.com/timakin/bodyclose v0.0.0-20190407043127-4a873e97b2bb
github.com/timakin/bodyclose/passes/bodyclose
# golang.org/x/crypto v0.0.0-20190313024323-a1f597ede03a
golang.org/x/crypto/ssh/terminal
# golang.org/x/sys v0.0.0-20190312061237-fead79001313
Expand Down

0 comments on commit 6508d16

Please sign in to comment.