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

Discrepancies of the formatting in different scenarios #238

Closed
adrianlungu opened this issue Jul 27, 2022 · 4 comments
Closed

Discrepancies of the formatting in different scenarios #238

adrianlungu opened this issue Jul 27, 2022 · 4 comments

Comments

@adrianlungu
Copy link

Hello,

I've been diving a bit into gofumpt as a result of this issue.

Let's say we have the following test file:

package p

import (
	"time"
	"adrian/pkg/sql"
	"adrian/pkg/redis"
	"context"
)

var ()

func f() {
	for _ = range v {
	}
}

The output of the existing format_test which tests the format.Source function is as follows:

package p

import (
	"adrian/pkg/sql"
	"adrian/pkg/redis"
	"time"
	"context"
)

func f() {
	for _ = range v {
	}
}

While the out of another test I've written which uses the format.File function, seems to format the file as follows:

package p

import (
	"time"
	"adrian/pkg/sql"
	"adrian/pkg/redis"
	"context"
)

func f() {
	for _ = range v {
	}
}

I've found a bit of a stranger interaction when running the binary on the same .go file but in 2 different locations. I have a big main.go file in a project where the std imports are sorted at the top, before the rest of the imports. I assume the rule that is followed here is std imports must be in a separate group at the top.

However, when I copy the file, or try to replicate the import structure, the std imports no longer get put at the top, and instead, all the internal packages get put at the top, and then right after those the std imports are appended. It's really weird to have the same file get formatted differently based on location 🤔

i.e. I'm getting the following after copy pasting the file in another location

import (
	"adrian/pkg/sql"
	"adrian/pkg/redis"
	"context"
	"time"
)

instead of this which is what I get when the file is in the current existing project

	"time"
	"context"

	"adrian/pkg/sql"
	"adrian/pkg/redis"

It's vaguely similar to #225

It's odd since I see even files in the exact same repo sometimes have the std imports before everything else, and sometimes they do not, it's a bit of an unpredictable scenario but I'm not sure what's causing it or why.

@adrianlungu adrianlungu changed the title Discrepancies between the format package and the binary Discrepancies of the formatting in different scenarios Jul 27, 2022
@mvdan
Copy link
Owner

mvdan commented Jul 27, 2022

Please see https://github.com/mvdan/gofumpt#contributing; use the "diagnose" directive to see exactly how gofumpt is being run in each one of those cases.

@adrianlungu
Copy link
Author

adrianlungu commented Jul 27, 2022

Interesting;

With golangci-lint gofumpt seems to print devel but nonetheless, we have:

//gofumpt:diagnose (devel) -lang=v1.18 -modpath=

which results in

import (
	"adrian/pkg/sql"
	"adrian/pkg/redis"
	"context"
	"time"
)

When running gofumpt via the binary, it automatically detects the modpath which gives us

//gofumpt:diagnose v0.3.1 -lang=v1.18 -modpath=adrian/backend

Alongside the properly formatted code i.e.

import (
	"context"
	"time"

	"adrian/pkg/sql"
	"adrian/pkg/redis"
)

Then, if we get the same modpath and set it for golangci-lint, we get

//gofumpt:diagnose (devel) -lang=v1.18 -modpath=adrian/backend

Which also outputs the expected import order.

Does this mean that the std imports must be in a separate group at the top rule is dependent on modpath existing (and probably being correct) ?

@mvdan
Copy link
Owner

mvdan commented Jul 27, 2022

Correct. See https://pkg.go.dev/mvdan.cc/gofumpt/format#Options. I assume this is a bug in golangci-lint; if it runs inside a module, it should be setting the option just like the regular gofumpt tool. Our changelog also contains this information.

With golangci-lint gofumpt seems to print devel but nonetheless, we have:

Ah, that's our fault - we don't really support printing gofumpt's own version when it is used as a library. I'll look into a fix. You should still be able to figure out manually what golangci-lint version you're using, and from that tell whether its gofumpt version is relatively old, but still.

@adrianlungu
Copy link
Author

Yeah, unfortunately golangci-lint leaves it up to the end user to add the gofumpt options, and by default, uses an empty modpath.

I'll let them know and hopefully they'll add an auto-default instead of empty.

Nonetheless, thanks for the help and the quick replies!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants