Skip to content

Commit 5ca2dc3

Browse files
jirfagjirfag
committed
fix #513: don't add gofmt "with -s" if not needed
Output File is not `gofmt`-ed insted of File is not `gofmt`-ed with `-s` when gofmt.simplify == false
1 parent 5c86bfc commit 5ca2dc3

File tree

6 files changed

+45
-22
lines changed

6 files changed

+45
-22
lines changed

‎Makefile

+10-11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
.DEFAULT_GOAL = test
22
.PHONY: FORCE
3-
export GO111MODULE = on
43

54
# Build
65

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

21-
build:
22-
go build -o golangci-lint ./cmd/golangci-lint
2320
.PHONY: test
2421

2522
test_race:
@@ -55,18 +52,18 @@ golangci-lint: FORCE pkg/logutils/log_mock.go
5552
go build -o $@ ./cmd/golangci-lint
5653

5754
tools/mockgen: go.mod go.sum
58-
GOBIN=$(CURDIR)/tools go install github.com/golang/mock/mockgen
55+
GOBIN=$(CURDIR)/tools GO111MODULE=on go install github.com/golang/mock/mockgen
5956

6057
tools/goimports: go.mod go.sum
61-
GOBIN=$(CURDIR)/tools go install golang.org/x/tools/cmd/goimports
58+
GOBIN=$(CURDIR)/tools GO111MODULE=on go install golang.org/x/tools/cmd/goimports
6259

6360
tools/go.mod:
6461
@mkdir -p tools
6562
@rm -f $@
66-
cd tools && go mod init local-tools
63+
cd tools && GO111MODULE=on go mod init local-tools
6764

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

7168
tools/svg-term:
7269
@mkdir -p tools
@@ -81,7 +78,8 @@ docs/demo.svg: tools/svg-term tools/Dracula.itermcolors
8178
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
8279

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

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

9391
go.mod: FORCE
94-
go mod verify
95-
go mod tidy
92+
GO111MODULE=on go mod verify
93+
GO111MODULE=on go mod tidy
9694
go.sum: go.mod
9795

96+
.PHONY: vendor
9897
vendor: go.mod go.sum
9998
rm -rf vendor
100-
go mod vendor
99+
GO111MODULE=on go mod vendor

‎pkg/golinters/gofmt.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ func (p *hunkChangesParser) parse(h *diffpkg.Hunk) []Change {
233233
return p.ret
234234
}
235235

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

253253
for _, hunk := range d.Hunks {
254-
text := "File is not `gofmt`-ed with `-s`"
254+
var text string
255255
if g.UseGoimports {
256256
text = "File is not `goimports`-ed"
257+
} else {
258+
text = "File is not `gofmt`-ed"
259+
if lintCtx.Settings().Gofmt.Simplify {
260+
text += " with `-s`"
261+
}
257262
}
258263
p := hunkChangesParser{
259264
log: log,
@@ -301,7 +306,7 @@ func (g Gofmt) Run(ctx context.Context, lintCtx *linter.Context) ([]result.Issue
301306
continue
302307
}
303308

304-
is, err := g.extractIssuesFromPatch(string(diff), lintCtx.Log)
309+
is, err := g.extractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx)
305310
if err != nil {
306311
return nil, fmt.Errorf("can't extract issues from gofmt diff output %q: %s", string(diff), err)
307312
}

‎test/errchk.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
5252
}
5353
matched := false
5454
n := len(out)
55+
var textsToMatch []string
5556
for _, errmsg := range errmsgs {
5657
// Assume errmsg says "file:line: foo".
5758
// Cut leading "file:line: " to avoid accidental matching of file name instead of message.
@@ -63,10 +64,13 @@ func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
6364
matched = true
6465
} else {
6566
out = append(out, errmsg)
67+
textsToMatch = append(textsToMatch, text)
6668
}
6769
}
6870
if !matched {
69-
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")))
71+
err := fmt.Errorf("%s:%d: no match for %#q vs %q in:\n\t%s",
72+
we.file, we.lineNum, we.reStr, textsToMatch, strings.Join(out[n:], "\n\t"))
73+
errs = append(errs, err)
7074
continue
7175
}
7276
}

‎test/testdata/gofmt_no_simplify.go

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//args: -Egofmt
2+
//config: linters-settings.gofmt.simplify=false
3+
package testdata
4+
5+
import "fmt"
6+
7+
func GofmtNotSimplifiedOk() {
8+
var x []string
9+
fmt.Print(x[1:len(x)])
10+
}
11+
12+
func GofmtBadFormat(){ // ERROR "^File is not `gofmt`-ed$"
13+
}

‎test/testshared/testshared.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@ import (
1414
)
1515

1616
type LintRunner struct {
17-
t assert.TestingT
18-
log logutils.Log
19-
env []string
17+
t assert.TestingT
18+
log logutils.Log
19+
env []string
20+
installed bool
2021
}
2122

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

3233
func (r *LintRunner) Install() {
33-
if _, err := os.Stat("../golangci-lint"); err == nil {
34+
if r.installed {
3435
return
3536
}
3637

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

4143
type RunResult struct {
@@ -79,7 +81,7 @@ func (r *LintRunner) Run(args ...string) *RunResult {
7981
r.Install()
8082

8183
runArgs := append([]string{"run"}, args...)
82-
r.log.Infof("golangci-lint %s", strings.Join(runArgs, " "))
84+
r.log.Infof("../golangci-lint %s", strings.Join(runArgs, " "))
8385
cmd := exec.Command("../golangci-lint", runArgs...)
8486
cmd.Env = append(os.Environ(), r.env...)
8587
out, err := cmd.CombinedOutput()

‎vendor/modules.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,12 @@ github.com/spf13/viper
181181
# github.com/stretchr/testify v1.2.2
182182
github.com/stretchr/testify/assert
183183
github.com/stretchr/testify/require
184+
# github.com/timakin/bodyclose v0.0.0-20190407043127-4a873e97b2bb
185+
github.com/timakin/bodyclose/passes/bodyclose
184186
# github.com/valyala/bytebufferpool v1.0.0
185187
github.com/valyala/bytebufferpool
186188
# github.com/valyala/quicktemplate v1.1.1
187189
github.com/valyala/quicktemplate
188-
# github.com/timakin/bodyclose v0.0.0-20190407043127-4a873e97b2bb
189-
github.com/timakin/bodyclose/passes/bodyclose
190190
# golang.org/x/crypto v0.0.0-20190313024323-a1f597ede03a
191191
golang.org/x/crypto/ssh/terminal
192192
# golang.org/x/sys v0.0.0-20190312061237-fead79001313

0 commit comments

Comments
 (0)