Goimports regression after v1.12.4

[sagikazarmark]

During the latest update of my local golangci-lint binary (from 1.12.3 to 1.12.5) I started to receive an error for package github.com/hashicorp/go-getter: File is not goimports-ed

The package name of go-getter is actually getter which is used in the source file.

Tried with 1.12.4 as well, so I think it's safe to say this is the version where the regression has been introduced.

Version of golangci-lint: golangci-lint has version 1.12.5 built from 609de32 on 2018-12-23T09:40:14Z

Config file

linters:
    enable-all: true
    disable:
        - maligned
linters-settings:

golint:

min-confidence: 0.1

Go environment

go version go1.11.4 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/mark/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/mark/.go:/Users/mark/Projects/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/mark/.goenv/versions/1.11.4"
GOTMPDIR=""
GOTOOLDIR="/Users/mark/.goenv/versions/1.11.4/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/mark/Projects/sagikazarmark/binbrew/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vy/hzm88gt90292ztr9f3qg9zhr0000gn/T/go-build633943090=/tmp/go-build -gno-record-gcc-switches -fno-common"

Verbose output of running

INFO [config_reader] Config search paths: [./ /Users/mark/Projects/sagikazarmark/binbrew /Users/mark/Projects/sagikazarmark /Users/mark/Projects /Users/mark /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO Gocritic enabled checks: [appendAssign assignOp caseOrder dupArg dupBranchBody dupCase flagDeref ifElseChain regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar underef unlambda unslice defaultCaseOrder]
INFO [lintersdb] Active 27 linters: [deadcode depguard dupl errcheck gochecknoglobals gochecknoinits goconst gocritic gocyclo gofmt goimports golint gosec govet ineffassign interfacer lll megacheck misspell nakedret prealloc scopelint structcheck typecheck unconvert unparam varcheck]
INFO [loader] Go packages loading at mode load deps types and syntax took 3.778639462s
INFO [loader] SSA repr building timing: packages building 19.361427ms, total 424.412521ms
INFO [loader] SSA for megacheck repr building timing: packages building 20.150219ms, total 445.594445ms
INFO [runner] worker.5 took 68.265317ms with stages: ineffassign: 64.144787ms, unconvert: 1.638177ms, varcheck: 1.025347ms, gocyclo: 855.962µs, structcheck: 295.084µs, goconst: 135.264µs, gochecknoglobals: 55.05µs, gochecknoinits: 17.83µs, typecheck: 1.443µs
INFO [runner] worker.1 took 69.428503ms with stages: gocritic: 69.378197ms
INFO [runner] worker.8 took 71.239697ms with stages: gosec: 66.114869ms, lll: 2.724629ms, deadcode: 716.852µs, prealloc: 550.502µs, errcheck: 433.873µs, scopelint: 176.408µs, nakedret: 98.911µs
INFO [runner] worker.11 took 72.758935ms with stages: gofmt: 72.711218ms
INFO [runner] worker.7 took 72.882251ms with stages: dupl: 72.870988ms
INFO [runner] worker.3 took 74.314529ms with stages: govet: 74.287511ms
INFO [runner] worker.10 took 102.515028ms with stages: misspell: 102.486605ms
INFO [runner] worker.4 took 148.99719ms with stages: golint: 148.968609ms
INFO [runner] worker.12 took 186.265984ms with stages: interfacer: 186.192702ms, depguard: 14.404µs
INFO [runner] worker.9 took 576.441143ms with stages: megacheck: 576.401018ms
INFO [runner] worker.6 took 670.994728ms with stages: goimports: 670.963529ms
internal/cli/command/install.go:8: File is not `goimports`-ed (goimports)
	"github.com/hashicorp/go-getter"
INFO [runner] worker.2 took 726.117134ms with stages: unparam: 726.062667ms
INFO [runner] Issues before processing: 23, after processing: 1
INFO [runner] processing took 4.418591ms with stages: skip_dirs: 1.333792ms, exclude: 928.893µs, nolint: 732.824µs, path_prettifier: 576.275µs, cgo: 487.021µs, autogenerated_exclude: 209.808µs, source_code: 117.422µs, uniq_by_line: 16.819µs, max_same_issues: 4.233µs, max_per_file_from_linter: 3.539µs, path_shortener: 2.354µs, max_from_linter: 2.172µs, diff: 1.992µs, skip_files: 1.447µs
INFO [runner] Workers idle times: #1: 656.54619ms, #3: 651.749938ms, #4: 577.119644ms, #5: 657.569274ms, #6: 55.002834ms, #7: 653.034839ms, #8: 654.892038ms, #9: 149.68486ms, #10: 623.414629ms, #11: 653.173408ms, #12: 539.717704ms
INFO Memory: 58 samples, avg is 275.6MB, max is 948.1MB
INFO Execution took 5.785180331s

[jirfag]

Hi!
These warnings came from the newest version of goimports (from this commit). Do you use the latest one (go get -u golang.org/x/tools/cmd/goimports)? What behavior did you expect?

[sagikazarmark]

I use prebuilt binaries of golangci, currently downgraded to 1.12.3.

I expect that the linter does not complain when the package path (github.com/hashicorp/go-getter) and the package name (getter) don't match.

[daenney]

I got the same issue in CI where we use the pre-built binaries GolangCI is providing. I wasn't aware I then have to manually update goimports or the other linters it depends on?

[sagikazarmark]

Me neither. I'd find it quite odd.

[jirfag]

You don't need to use external goimports. Golangci-lint doesn't depend on it. You may do it just to compare and ensure that golangci-lint is in sync with upstream goimports.

Also you can fix the issue in all files by running something like:

go get -u golang.org/x/tools/cmd/goimports
find . -name "*.go" | fgrep -v vendor/ | xargs goimports -w

P.S. We are planning to add option like --fix to golangci-lint to fix all goimports (and other linters) issues automatically).

[sagikazarmark]

Okay, so it looks like this is an intended change, not a bug after all. I thought this was some sort of configuration change, but after reading the commit, this looks pretty logical to me.

One thing though: it might make sense not to make these kind of updates in a patch version. Minor version is questionable as well, but that would be acceptable IMHO.

[sagikazarmark]

The main confusion came from Goland removing those aliases.
Looks like this is going to be fixed in 2019.1: https://youtrack.jetbrains.com/issue/GO-6146

[jirfag]

the idea of including this into the patch commit was that it's a bug we don't use the latest version of goimports. A few users reported that we are out of sync with goimports.
But you are right: better was to make a new minor release