Skip to content
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

confusing error message "File is not goimports-ed (goimports)" #138

Open
briantkennedy opened this issue Jun 29, 2018 · 29 comments · May be fixed by #5243
Open

confusing error message "File is not goimports-ed (goimports)" #138

briantkennedy opened this issue Jun 29, 2018 · 29 comments · May be fixed by #5243
Labels
enhancement New feature or improvement
Milestone

Comments

@briantkennedy
Copy link

A file not being "goimports-ed" is a little confusing. Can we adjust this error message to something a little clearer like "File has not been formatted (goimports)"

@jirfag jirfag added the enhancement New feature or improvement label Jun 30, 2018
@jirfag
Copy link
Member

jirfag commented Jul 14, 2018

hi!
it's the good idea, but
now it's too late to change it: some users have already added this text to exclude list.

@jirfag jirfag closed this as completed Jul 14, 2018
@pjvds
Copy link

pjvds commented Nov 9, 2018

now it's too late to change it: some users have already added this text to exclude list.

This way we can never improve error message 😇

@jirfag jirfag added the v2 label Nov 10, 2018
@jirfag
Copy link
Member

jirfag commented Nov 10, 2018

You are right, I got an idea to mark it as v2: in the next version (not in the roadmap yet) we can break compatibility and implement id.

@jirfag jirfag reopened this Nov 10, 2018
@qfel
Copy link

qfel commented Dec 21, 2018

Is this using some custom version of goimports? I recently had it complain about one import even though the file was formatted by goimports. Turned out it wanted me to move it to another group of imports (even though regular goimports doesn't move imports between groups).

@daenney
Copy link

daenney commented Dec 23, 2018

This has started to happen to me since 1.24+. Running GolangCI with goimports suggests files aren't properly formatted even though running both gofmt and goimports manually on the files golangci complains about results in no changes and both tools exiting with a status code of 0.

@pierrre
Copy link
Contributor

pierrre commented Dec 26, 2018

Same here (but I'm a new user).
It complains for imports containing the - character, such as:

  • github.com/opentracing/opentracing-go
  • github.com/getsentry/raven-go
  • github.com/michaelklishin/rabbit-hole
  • github.com/oschwald/geoip2-golang

@jirfag
Copy link
Member

jirfag commented Jan 20, 2019

@qfel @daenney @pierrre do you use updated goimports? Did you run go get -u golang.org/x/tools/cmd/goimports?

also, let's continue in #347,
this issue about message text

@bcomnes
Copy link

bcomnes commented Feb 6, 2019

I just ran into this problem too. A better message would have gotten me to a fix quicker.

@detailyang
Copy link

+1; Human readable message is expected and "File is not goimportes-ed" confused me

@vtopc
Copy link

vtopc commented Jun 12, 2019

  1. What message do you want to see? Please, run goimports tool for this file?
  2. Have you tried --fix option?

@bcomnes
Copy link

bcomnes commented Jun 12, 2019

The message should explain what the problem is (e.g. missing an import statement for package x.) It could also maybe propose a solution like, add the import statement or run goimports to add missing import statements. It should not just print what the solution is, because it may not be clear what the problem is for those unfamiliar with the suggested tool.

@vtopc
Copy link

vtopc commented Jun 12, 2019

@bcomnes, import statement != goimports, looks like the tool should be renamed, not a message.

In addition to fixing imports, goimports also formats your code in the same style as gofmt

@matoous matoous removed the v2 label Sep 25, 2019
@matoous matoous added this to the v2.0.0 milestone Sep 25, 2019
@vtolstov
Copy link

vtolstov commented Dec 4, 2019

i have the same error, but run goimports -w on file does not bring any differences.

@Shpota
Copy link

Shpota commented Mar 27, 2020

Is there any update on this? Maybe it makes sense to at least add documentation to this check?

Right now, it reports two lines in my project and I have no idea what it means and how to resolve them (go fmt ./... doesn't change anything and the imports are obviously needed).

@samurnin
Copy link

samurnin commented Mar 27, 2020

@qfel @daenney @pierrre do you use updated goimports? Did you run go get -u golang.org/x/tools/cmd/goimports?

also, let's continue in #347,
this issue about message text

I came across the same issue. It was being fixed by the following steps:

  1. go get -u golang.org/x/tools/cmd/goimports
  2. go mod tidy
  3. go mod vendor
  4. go fmt //==> formatting code
  5. goimports -w ==> clean imports
  6. gofmt -s -w ==> simplify code

@vtopc
Copy link

vtopc commented Mar 30, 2020

@samurnin, the goimports -w is not just managing imports, it's also doing the same formating as regular gofmt -w. P.S. thanks for -s flag.

@zhangguanzhang
Copy link

Built-in and non-built-in packages should be separated and the built-in packages placed first
内置包和非内置包应该要分开,内置包放在前面,自己的外部包放后面,例如

import (
	"fmt"
	"net/http"

	"github.com/containers/podman/v2/libpod/define"
	"github.com/containers/podman/v2/pkg/domain/entities"
	"github.com/pkg/errors"
	log "github.com/sirupsen/logrus"
)

@anubhavcodes
Copy link

+1 for @samurnin for the details steps. goimports -w filename.go fixed it for me. In the end it was what @zhangguanzhang suggested.

Built-in and non-built-in packages should be separated and the built-in packages placed first

@gillesdouaire
Copy link

gillesdouaire commented Sep 11, 2020

"File is not goimports-ed (goimports)" is a really bad message, confusing, wasting time trying to understand what it means.

@rainest
Copy link

rainest commented Sep 12, 2020

Echoing the "what? my vim config runs gofmt automatically, and it doesn't care, so why the hell does golangci?" sentiment. The error

now it's too late to change it: some users have already added this text to exclude list.

@jirfag да блин, кто они? наверно, если эти юзери, ну «перегрузить» бы свои exclude lists, многие другие юзери улыбнулись бы 🤣 ))))))

It's not immediately obvious what to do when this occurs (simply running goimports prints a fixed version to stdout, which neither fixes the issue nor makes it clear where it is). Perhaps:

<file> has improperly-formatted imports. Run `goimports -d <file>` to see the issue or `goimports -w <file>` to fix it in-place.

That offers both a command that shows the issue only, and a command that will just fix it.

@stale
Copy link

stale bot commented Sep 14, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale No recent correspondence or work activity label Sep 14, 2021
@vtopc
Copy link

vtopc commented Sep 14, 2021

@rainest,

has improperly-formatted imports

it's not only about imports - #138 (comment) and #138 (comment)

So it's probably the main confusion with this message when one trying to "fix" imports, but the problem is another place.

@stale stale bot removed the stale No recent correspondence or work activity label Sep 14, 2021
@SVilgelm
Copy link
Member

we already had this discussion and decided that the message will be changed in v2

@konstmonst

This comment has been minimized.

@bombsimon

This comment has been minimized.

@konstmonst

This comment has been minimized.

@konstmonst

This comment has been minimized.

@ldez

This comment has been minimized.

@Kritikazepto

This comment was marked as off-topic.

@ldez ldez linked a pull request Dec 21, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.