Skip to content

Goimports "local" mode #209

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

Closed
AlekSi opened this issue Aug 31, 2018 · 6 comments
Closed

Goimports "local" mode #209

AlekSi opened this issue Aug 31, 2018 · 6 comments

Comments

@AlekSi
Copy link
Contributor

AlekSi commented Aug 31, 2018

Specifically, -local flag:

-local string
put imports beginning with this string after 3rd-party packages

That is very useful for projects that follow this grouping style:

import (
  // stdlib

  // third-party

  // other packages of that project
)
@alecthomas
Copy link
Contributor

goimports shouldn't complain if the file is already formatted with -local - it's still correctly imported as goimports won't merge/sort newline-separated blocks of imports.

@AlekSi
Copy link
Contributor Author

AlekSi commented Sep 14, 2018

Yes, but I want it to complain if imports are not formatted with -local flag, but local setting is present in the configuration.

@jirfag jirfag closed this as completed in 8fceb7f Oct 28, 2018
@jirfag
Copy link
Contributor

jirfag commented Oct 28, 2018

Hi!
Thank you for the idea,
I supported it in the master branch. I will include it into a next release

@thepwagner
Copy link

thepwagner commented Nov 7, 2018

@jirfag I don't think this is working as intended yet.

We're setting https://github.com/golangci/golangci-lint/blob/master/pkg/golinters/gofmt.go#L115 , which resolves to golang.org/x/tools/imports.LocalPrefix ( https://github.com/golangci/golangci-lint/blob/master/pkg/golinters/gofmt.go#L9 )

Then we invoke https://github.com/golangci/golangci-lint/blob/master/vendor/github.com/golangci/gofmt/goimports/golangci.go#L11 , which in turn uses https://github.com/golangci/golangci-lint/blob/master/vendor/github.com/golangci/gofmt/goimports/golangci.go#L8 - golang.org/golangci/tools/imports.LocalPrefix

So when GoImports is invoked, it doesn't have the LocalPrefix that we attempted to set - we set another LocalPrefix that isn't used.

I won't PR this as it's pretty deep into dep hell - swapping https://github.com/golangci/golangci-lint/blob/master/pkg/golinters/gofmt.go#L9 to github.com/golangci/tools/imports works for me - but there may be a better solution (or more cleanup: like picking one of the vendored goimports and using it).

@jirfag jirfag reopened this Nov 7, 2018
@jirfag
Copy link
Contributor

jirfag commented Nov 7, 2018

thank you

@jirfag jirfag changed the title Please allow goimports configuration Goimports "local" mode Nov 7, 2018
@jirfag
Copy link
Contributor

jirfag commented Nov 7, 2018

@thepwagner you are right, the bug was introduced in commit c02a6da.
I need to make a test for it, the fix will be to use golang.org/x/tools (I've switched to the fork because of the bug in x/tools, now it should be fixed)

jirfag added a commit that referenced this issue Nov 10, 2018
ed64e33c8c8bc9a919e2b85a1a08225b5ae59d70.

Also add tests for local mode of goimports and do refactoring of tests.
jirfag added a commit that referenced this issue Nov 10, 2018
ed64e33c8c8bc9a919e2b85a1a08225b5ae59d70.

Also add tests for local mode of goimports and do refactoring of tests.
jirfag added a commit that referenced this issue Nov 10, 2018
ed64e33c8c8bc9a919e2b85a1a08225b5ae59d70.

Also add tests for local mode of goimports and do refactoring of tests.
jirfag added a commit that referenced this issue Nov 10, 2018
ed64e33c8c8bc9a919e2b85a1a08225b5ae59d70.

Also add tests for local mode of goimports and do refactoring of tests.
jirfag added a commit that referenced this issue Nov 10, 2018
ed64e33c8c8bc9a919e2b85a1a08225b5ae59d70.

Also add tests for local mode of goimports and do refactoring of tests.
jirfag added a commit that referenced this issue Nov 10, 2018
ed64e33c8c8bc9a919e2b85a1a08225b5ae59d70.

Also add tests for local mode of goimports and do refactoring of tests.
jirfag added a commit that referenced this issue Nov 10, 2018
ed64e33c8c8bc9a919e2b85a1a08225b5ae59d70.

Also add tests for local mode of goimports and do refactoring of tests.
jirfag added a commit that referenced this issue Nov 10, 2018
ed64e33c8c8bc9a919e2b85a1a08225b5ae59d70. Also add tests
for local mode of goimports and do refactoring of tests.
@jirfag jirfag closed this as completed in ac77eaa Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants