Skip to content

Commit

Permalink
format: import paths similar to the module path aren't std
Browse files Browse the repository at this point in the history
Add the feature, test it, and document it in the README.
It's enough to mention it in the FAQ, as I expect that most users will
never run into this edge case. And those few that may run into it will
hopefully never need to know how it works or why.

Note that this adds a new option to pass the info from go.mod,
much like LangVersion already did.

Fixes #22.
Fixes #117.
Fixes #120.
Fixes #187.
  • Loading branch information
mvdan committed Feb 22, 2022
1 parent acf11b6 commit 487b40b
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 34 deletions.
15 changes: 8 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -561,13 +561,14 @@ drop-in replacement in editors and scripts.
> Why are my module imports being grouped with standard library imports?
Any import paths that don't start with a domain name like `foo.com` are
effectively reserved by the Go toolchain. Otherwise, adding new standard library
packages like `embed` would be a breaking change. See https://github.com/golang/go/issues/32819.

Third party modules should use domain names to avoid conflicts.
If your module is meant for local use only, you can use `foo.local`.
For small example or test modules, `example/...` and `test/...` may be reserved
if a proposal is accepted; see https://github.com/golang/go/issues/37641.
effectively [reserved by the Go toolchain](https://github.com/golang/go/issues/32819).
Third party modules should either start with a domain name,
even a local one like `foo.local`, or use [a reserved path prefix](https://github.com/golang/go/issues/37641).

For backwards compatibility with modules set up before these rules were clear,
`gofumpt` will treat any import path sharing a prefix with the current module
path as third party. For example, if the current module is `mycorp/mod1`, then
all import paths in `mycorp/...` will be considered third party.

> How can I use `gofumpt` if I already use `goimports` to replace `gofmt`?
Expand Down
83 changes: 64 additions & 19 deletions format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,35 @@ import (
"mvdan.cc/gofumpt/internal/version"
)

// Options is the set of formatting options which affect gofumpt.
type Options struct {
// LangVersion corresponds to the Go language version a piece of code is
// written in. The version is used to decide whether to apply formatting
// rules which require new language features. When inside a Go module,
// LangVersion should generally be specified as the result of:
// LangVersion should be:
//
// go list -m -f {{.GoVersion}}
// go mod edit -json | jq -r '.Go'
//
// LangVersion is treated as a semantic version, which might start with
// a "v" prefix. Like Go versions, it might also be incomplete; "1.14"
// is equivalent to "1.14.0". When empty, it is equivalent to "v1", to
// not use language features which could break programs.
// LangVersion is treated as a semantic version, which may start with a "v"
// prefix. Like Go versions, it may also be incomplete; "1.14" is equivalent
// to "1.14.0". When empty, it is equivalent to "v1", to not use language
// features which could break programs.
LangVersion string

// ModulePath corresponds to the Go module path which contains the source
// code being formatted. When inside a Go module, ModulePath should be:
// rules which require new language features. When inside a Go module,
// LangVersion should generally be specified as the result of:
//
// go mod edit -json | jq -r '.Module.Path'
//
// ModulePath is used for formatting decisions like what import paths are
// considered to be not part of the standard library. When empty, the source
// is formatted as if it weren't inside a module.
ModulePath string

// ExtraRules enables extra formatting rules, such as grouping function
// parameters with repeated types together.
ExtraRules bool
}

Expand Down Expand Up @@ -366,6 +381,7 @@ func (f *fumpter) applyPre(c *astutil.Cursor) {
"//gofumpt:diagnose",
version.String(),
"-lang=" + f.LangVersion,
"-modpath=" + f.ModulePath,
}
if f.ExtraRules {
slc = append(slc, "-extra")
Expand Down Expand Up @@ -862,6 +878,23 @@ func (f *fumpter) joinStdImports(d *ast.GenDecl) {
firstGroup := true
lastEnd := d.Pos()
needsSort := false

// If ModulePath is "foo/bar", we assume "foo/..." is not part of std.
// Users shouldn't declare modules that may collide with std this way,
// but historically some private codebases have done so.
// This is a relatively harmless way to make gofumpt compatible with them,
// as it changes nothing for the common external module paths.
var modulePrefix string
if f.ModulePath == "" {
// Nothing to do.
} else if i := strings.IndexByte(f.ModulePath, '/'); i != -1 {
// ModulePath is "foo/bar", so we use "foo" as the prefix.
modulePrefix = f.ModulePath[:i]
} else {
// ModulePath is "foo", so we use "foo" as the prefix.
modulePrefix = f.ModulePath
}

for i, spec := range d.Specs {
spec := spec.(*ast.ImportSpec)
if coms := f.commentsBetween(lastEnd, spec.Pos()); len(coms) > 0 {
Expand All @@ -874,20 +907,32 @@ func (f *fumpter) joinStdImports(d *ast.GenDecl) {
lastEnd = spec.End()
}

path, _ := strconv.Unquote(spec.Path.Value)
path, err := strconv.Unquote(spec.Path.Value)
if err != nil {
panic(err) // should never error
}
periodIndex := strings.IndexByte(path, '.')
slashIndex := strings.IndexByte(path, '/')
switch {
// Imports with a period are definitely third party.
case strings.Contains(path, "."):
fallthrough
// "test" and "example" are reserved as per golang.org/issue/37641.
// "internal" is unreachable.
case strings.HasPrefix(path, "test/") ||
strings.HasPrefix(path, "example/") ||
strings.HasPrefix(path, "internal/"):
fallthrough
// To be conservative, if an import has a name or an inline
// comment, and isn't part of the top group, treat it as non-std.
case !firstGroup && (spec.Name != nil || spec.Comment != nil):

// Imports with a period in the first path element are third party.
// Note that this includes "foo.com" and excludes "foo/bar.com/baz".
case periodIndex > 0 && (slashIndex == -1 || periodIndex < slashIndex),

// "test" and "example" are reserved as per golang.org/issue/37641.
// "internal" is unreachable.
strings.HasPrefix(path, "test/"),
strings.HasPrefix(path, "example/"),
strings.HasPrefix(path, "internal/"),

// See if we match modulePrefix; see its documentation above.
// We match either exactly or with a slash suffix,
// so that the prefix "foo" for "foo/..." does not match "foobar".
path == modulePrefix || strings.HasPrefix(path, modulePrefix+"/"),

// To be conservative, if an import has a name or an inline
// comment, and isn't part of the top group, treat it as non-std.
!firstGroup && (spec.Name != nil || spec.Comment != nil):
other = append(other, spec)
continue
}
Expand Down
26 changes: 21 additions & 5 deletions gofmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ var (

// gofumpt's own flags
langVersion = flag.String("lang", "", "")
modulePath = flag.String("modpath", "", "")
extraRules = flag.Bool("extra", false, "")
showVersion = flag.Bool("version", false, "")

Expand Down Expand Up @@ -92,8 +93,8 @@ func usage() {
-w write result to (source) file instead of stdout
-extra enable extra rules which should be vetted by a human
-cpuprofile str write cpu profile to this file
-lang str target Go version in the form 1.X (default from go.mod)
-lang str target Go version in the form "1.X" (default from go.mod)
-modpath str Go module path containing the source file (default from go.mod)
`)
}

Expand Down Expand Up @@ -285,18 +286,33 @@ func processFile(filename string, info fs.FileInfo, in io.Reader, r *reporter, e

// Apply gofumpt's changes before we print the code in gofumpt's format.

if *langVersion == "" {
// If either -lang or -modpath aren't set, fetch them from go.mod.
if *langVersion == "" || *modulePath == "" {
out, err := exec.Command("go", "mod", "edit", "-json").Output()
if err == nil && len(out) > 0 {
var mod struct{ Go string }
var mod struct {
Go string
Module struct {
Path string
}
}
_ = json.Unmarshal(out, &mod)
*langVersion = mod.Go
if *langVersion == "" {
*langVersion = mod.Go
}
if *modulePath == "" {
*modulePath = mod.Module.Path
}
}
}

// We always apply the gofumpt formatting rules to explicit files, including stdin.
// Otherwise, we don't apply them on generated files.
// We also skip walking vendor directories entirely, but that happens elsewhere.
if explicit || !isGenerated(file) {
gformat.File(fileSet, file, gformat.Options{
LangVersion: *langVersion,
ModulePath: *modulePath,
ExtraRules: *extraRules,
})
}
Expand Down
6 changes: 3 additions & 3 deletions testdata/scripts/diagnose.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ package p
-- foo.go.golden --
package p

//gofumpt:diagnose v0.1.1-0.20210524140936-ba6406b58f36 -lang=v1.16
//gofumpt:diagnose v0.1.1-0.20210524140936-ba6406b58f36 -lang=v1.16 -modpath=test
-- foo.go.golden-extra --
package p

//gofumpt:diagnose v0.1.1-0.20210524140936-ba6406b58f36 -lang=v1.16 -extra
//gofumpt:diagnose v0.1.1-0.20210524140936-ba6406b58f36 -lang=v1.16 -modpath=test -extra
-- foo.go.golden-lang --
package p

//gofumpt:diagnose v0.1.1-0.20210524140936-ba6406b58f36 -lang=v1
//gofumpt:diagnose v0.1.1-0.20210524140936-ba6406b58f36 -lang=v1 -modpath=test
35 changes: 35 additions & 0 deletions testdata/scripts/std-imports.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ cmp foo.go foo.go.golden
gofumpt -d foo.go.golden
! stdout .

-- go.mod --
module nodomainmod/mod1

go 1.16
-- foo.go --
package p

Expand Down Expand Up @@ -42,6 +46,7 @@ import (

// We need to split std vs non-std in this case too.
import (
"foo.local"
"foo.local/three"
math "math"
)
Expand Down Expand Up @@ -70,6 +75,21 @@ import (
"internal/bar"
"test/baz"
)

import (
"io"

"nodomainmod"
"nodomainmod/mod1/pkg1"
"nodomainmod/mod2"
"nodomainmodextra"
)

import (
"io"

"nodomainother/mod.withdot/pkg1"
)
-- foo.go.golden --
package p

Expand Down Expand Up @@ -108,6 +128,7 @@ import (
import (
math "math"

"foo.local"
"foo.local/three"
)

Expand Down Expand Up @@ -136,3 +157,17 @@ import (
"internal/bar"
"test/baz"
)

import (
"io"
"nodomainmodextra"

"nodomainmod"
"nodomainmod/mod1/pkg1"
"nodomainmod/mod2"
)

import (
"io"
"nodomainother/mod.withdot/pkg1"
)

0 comments on commit 487b40b

Please sign in to comment.