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

Unnecessary addition of import logger when symbol exists within a package. #115

Open
TafThorne opened this issue Nov 23, 2023 · 3 comments

Comments

@TafThorne
Copy link

TafThorne commented Nov 23, 2023

As part of a large module (1000+files, 50k+ source lines, 50k+ test lines) I recently tried to use golines but found it frequently adding new sources for a logger symbol. Within the project we import a logging package from a third party module, declare an instance of it in some of our packages and then refer to it from multiple files within the package. Golines seems to not recognise the presence of this private variable from the module and tries to import extra logger instances instead.

I have not seen this happen with any other package. It does not happen with all instances of logger in all packages, only some of them within the module. I cannot spot a consistent pattern in which do or do not show the problem. I have not been able to reproduce it outside of the wider project & I cannot share the source code seeing the issue. I realise that this does not give you much to go on.

I see the project in the large project when doing something like the following:

A file in package instantiating a logger.

package example

import "example.com/logging"

var logger = logging.LoggerType{}

Using the logger in another file within the same package

package example

func Example() {
	logger.Log("An example message")
}

Golines will want import an extra logger symbol in the second file.


As a work around I have found that renaming logger to log stops the issue. var log = logging.LoggerType{} works fine.

@telemachus
Copy link
Contributor

My guess is that this isn't golines per se but goimports (which is the default formatter for golines.)

Does the problem go away if you run golines with the argument --base-formatter=gofmt (or --base-formatter=gofumpt)?

@TafThorne
Copy link
Author

golines -m 120 --base-formatter=gofmt -w . did not show the problem.

When I run goimports -w . directly I do not see the problem.

Running golines -m 120 --base-formatter=goimports -w . recreates the issue.

@telemachus
Copy link
Contributor

That's pretty much what I thought. As I said, the issue is not coming from golines—at least not primarily. I think the easiest solution for you is to explicitly exclude goimports from golines by using the --base-formatter setting.

(As for why goimports directly doesn't show the same problem, I'm not sure, but here's my best guess. golines may be picking up a different versions of golines than your shell because the environment differs in different situations (CI? test servers? etc.). You can see the code where golines picks a base formatter here.)

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

2 participants