diff --git a/README.md b/README.md index 46c93e9..b76b336 100644 --- a/README.md +++ b/README.md @@ -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`? diff --git a/format/format.go b/format/format.go index 4eec251..6f797ca 100644 --- a/format/format.go +++ b/format/format.go @@ -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 } @@ -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") @@ -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 { @@ -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 } diff --git a/gofmt.go b/gofmt.go index 309a44f..ca60bc3 100644 --- a/gofmt.go +++ b/gofmt.go @@ -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, "") @@ -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) `) } @@ -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, }) } diff --git a/testdata/scripts/diagnose.txt b/testdata/scripts/diagnose.txt index f038a64..52f31c1 100644 --- a/testdata/scripts/diagnose.txt +++ b/testdata/scripts/diagnose.txt @@ -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 diff --git a/testdata/scripts/std-imports.txt b/testdata/scripts/std-imports.txt index 3a68c6a..a1aa02b 100644 --- a/testdata/scripts/std-imports.txt +++ b/testdata/scripts/std-imports.txt @@ -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 @@ -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" ) @@ -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 @@ -108,6 +128,7 @@ import ( import ( math "math" + "foo.local" "foo.local/three" ) @@ -136,3 +157,17 @@ import ( "internal/bar" "test/baz" ) + +import ( + "io" + "nodomainmodextra" + + "nodomainmod" + "nodomainmod/mod1/pkg1" + "nodomainmod/mod2" +) + +import ( + "io" + "nodomainother/mod.withdot/pkg1" +)