mvdan / gofumpt

A stricter gofmt
https://pkg.go.dev/mvdan.cc/gofumpt
BSD 3-Clause "New" or "Revised" License
3.15k stars 110 forks source link

Feature suggestion: Distinguishing between ex-repo, intra-repo, and internal packages #273

Closed adamroyjones closed 11 months ago

adamroyjones commented 1 year ago

I find the following gofumpt rule to be very uesful:

std imports must be in a separate group at the top

I informally take this further in my employer's repository (and my own private work) by grouping imports into four groups:

For example:

import (
    "strings"
    "time"

    "golang.org/x/sync/errgroup"

    "github.com/wonderful-company/fabulous-project/lib/foo"
    "github.com/wonderful-company/fabulous-project/lib/bar"
    "github.com/wonderful-company/fabulous-project/frobinator/types"

    "github.com/wonderful-company/fabulous-project/frobinator/internal/handler"
)

I find that it rarely degenerates into ugly cases like this:

import (
    "strings"

    "golang.org/x/sync/errgroup"

    "github.com/wonderful-company/fabulous-project/lib/foo"

    "github.com/wonderful-company/fabulous-project/frobinator/internal/handler"
)

I find this to be a very helpful extension.

What are your thoughts about the above as a formatting rule? I appreciate that gofumpt shouldn't be a dumping ground for everyone's ideas about what code should look like—as it is, it's currently restrained and judicious—but it might be widely liked.

I don't know how plausible it is for gofumpt to understand the distinction between "packages in the same repository" and "external packages"—the module specification makes it seem that it might be able to if it's able to resolve the repository.

I think it can (fairly trivially) understand which packages are internal.

mvdan commented 11 months ago

I don't know how plausible it is for gofumpt to understand the distinction between "packages in the same repository" and "external packages"—the module specification makes it seem that it might be able to if it's able to resolve the repository.

That's https://github.com/mvdan/gofumpt/issues/38, and certainly planned. It's a fairly well understood and followed rule as well. I just got stuck on some of the minor design details, like where to draw the boundary line.

As for making a fourth group for internal packages - I personally haven't seen this style in any project before, and since we don't want gofumpt to have any options where possible, I'm inclined against it. We could always allow the user to create such a fourth group themselves, and teach gofumpt to leave it alone, but in my opinion that's not useful - the groups are only easy to maintain if a tool does it for you, not if you have to do it by hand.

mvdan commented 11 months ago

I'm going to close this for now in favor of #38, per the above. Happy to reopen if there are good arguments for reconsideration.

adamroyjones commented 11 months ago

I'm sorry for being slow to reply; the wind left my sails.


I'm going to close this for now in favor of #38, per the above.

That's all entirely (and characteristically) reasonable. This is better considered as part of that. It covers the more important distinction between standard, ex-repo, and intra-repo packages.

I personally haven't seen [the four-group] style in any project before, and since we don't want gofumpt to have any options where possible, I'm inclined against it.

There's a context where this style makes sense: a large Go monorepo comprising many services. In my experience, the distinction between internal and intra-repo packages becomes a useful one. (It's important for obvious reasons that services not import each other's innards.)

But that's a narrow context and gofumpt is a broad tool. If you've not seen that style, then it's better that I not impose any further.