From 0ae02bf52ffe17ff5b66e79d3ebd582bf867344c Mon Sep 17 00:00:00 2001 From: Petr Korobeinikov Date: Tue, 12 Mar 2024 06:56:19 +0300 Subject: [PATCH] Enhance LocalModule when no go sources found in project root Signed-off-by: Petr Korobeinikov --- README.md | 4 +- go.mod | 2 +- pkg/gci/gci_test.go | 95 +++++++++++-------- pkg/gci/testdata.go | 23 ----- .../testdata/module_canonical/.gitattributes | 4 + .../module_canonical/cmd/client/main.go | 9 ++ .../module_canonical/cmd/client/main.out.go | 11 +++ .../module_canonical/cmd/server/main.go | 9 ++ .../module_canonical/cmd/server/main.out.go | 11 +++ pkg/gci/testdata/module_canonical/config.yaml | 4 + pkg/gci/testdata/module_canonical/go.mod | 3 + .../module_canonical/internal/bar/lib.go | 1 + .../module_canonical/internal/foo/lib.go | 6 ++ .../module_canonical/internal/foo/lib.out.go | 7 ++ .../testdata/module_canonical/internal/lib.go | 1 + .../module_noncanonical/.gitattributes | 4 + .../module_noncanonical/cmd/client/main.go | 9 ++ .../cmd/client/main.out.go | 11 +++ .../module_noncanonical/cmd/server/main.go | 9 ++ .../cmd/server/main.out.go | 11 +++ .../testdata/module_noncanonical/config.yaml | 4 + pkg/gci/testdata/module_noncanonical/go.mod | 3 + .../module_noncanonical/internal/bar/lib.go | 1 + .../module_noncanonical/internal/foo/lib.go | 6 ++ .../internal/foo/lib.out.go | 7 ++ .../module_noncanonical/internal/lib.go | 1 + pkg/section/local_module.go | 49 +++------- 27 files changed, 203 insertions(+), 102 deletions(-) create mode 100644 pkg/gci/testdata/module_canonical/.gitattributes create mode 100644 pkg/gci/testdata/module_canonical/cmd/client/main.go create mode 100644 pkg/gci/testdata/module_canonical/cmd/client/main.out.go create mode 100644 pkg/gci/testdata/module_canonical/cmd/server/main.go create mode 100644 pkg/gci/testdata/module_canonical/cmd/server/main.out.go create mode 100644 pkg/gci/testdata/module_canonical/config.yaml create mode 100644 pkg/gci/testdata/module_canonical/go.mod create mode 100644 pkg/gci/testdata/module_canonical/internal/bar/lib.go create mode 100644 pkg/gci/testdata/module_canonical/internal/foo/lib.go create mode 100644 pkg/gci/testdata/module_canonical/internal/foo/lib.out.go create mode 100644 pkg/gci/testdata/module_canonical/internal/lib.go create mode 100644 pkg/gci/testdata/module_noncanonical/.gitattributes create mode 100644 pkg/gci/testdata/module_noncanonical/cmd/client/main.go create mode 100644 pkg/gci/testdata/module_noncanonical/cmd/client/main.out.go create mode 100644 pkg/gci/testdata/module_noncanonical/cmd/server/main.go create mode 100644 pkg/gci/testdata/module_noncanonical/cmd/server/main.out.go create mode 100644 pkg/gci/testdata/module_noncanonical/config.yaml create mode 100644 pkg/gci/testdata/module_noncanonical/go.mod create mode 100644 pkg/gci/testdata/module_noncanonical/internal/bar/lib.go create mode 100644 pkg/gci/testdata/module_noncanonical/internal/foo/lib.go create mode 100644 pkg/gci/testdata/module_noncanonical/internal/foo/lib.out.go create mode 100644 pkg/gci/testdata/module_noncanonical/internal/lib.go diff --git a/README.md b/README.md index 0f37fa5..b68248d 100644 --- a/README.md +++ b/README.md @@ -50,8 +50,8 @@ Since v0.9.0, GCI always puts C import block as the first. ### LocalModule -Local module detection is done via listing packages from *the directory where -`gci` is invoked* and reading the modules off these. This means: +Local module detection is done via reading the module name from the `go.mod` +file in *the directory where `gci` is invoked*. This means: - This mode works when `gci` is invoked from a module root (i.e. directory containing `go.mod`) diff --git a/go.mod b/go.mod index 68161f1..ea8cc8f 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/spf13/cobra v1.6.1 github.com/stretchr/testify v1.8.1 go.uber.org/zap v1.24.0 + golang.org/x/mod v0.8.0 golang.org/x/sync v0.1.0 golang.org/x/tools v0.6.0 gopkg.in/yaml.v3 v3.0.1 @@ -19,6 +20,5 @@ require ( github.com/spf13/pflag v1.0.5 // indirect go.uber.org/atomic v1.7.0 // indirect go.uber.org/multierr v1.6.0 // indirect - golang.org/x/mod v0.8.0 // indirect golang.org/x/sys v0.5.0 // indirect ) diff --git a/pkg/gci/gci_test.go b/pkg/gci/gci_test.go index 1ba9d30..f6db3e0 100644 --- a/pkg/gci/gci_test.go +++ b/pkg/gci/gci_test.go @@ -21,10 +21,6 @@ func init() { } func TestRun(t *testing.T) { - // if runtime.GOOS == "windows" { - // t.Skip("Skipping test on Windows") - // } - for i := range testCases { t.Run(fmt.Sprintf("run case: %s", testCases[i].name), func(t *testing.T) { config, err := config.ParseConfig(testCases[i].config) @@ -56,35 +52,65 @@ func chdir(t *testing.T, dir string) { func readConfig(t *testing.T, configPath string) *config.Config { rawConfig, err := os.ReadFile(configPath) require.NoError(t, err) - config, err := config.ParseConfig(string(rawConfig)) + cfg, err := config.ParseConfig(string(rawConfig)) require.NoError(t, err) - return config + return cfg } func TestRunWithLocalModule(t *testing.T) { - moduleDir := filepath.Join("testdata", "module") - // files with a corresponding '*.out.go' file containing the expected - // result of formatting - testedFiles := []string{ - "main.go", - filepath.Join("internal", "foo", "lib.go"), + tests := []struct { + name string + moduleDir string + // files with a corresponding '*.out.go' file containing the expected + // result of formatting + testedFiles []string + }{ + { + name: `default module test case`, + moduleDir: filepath.Join("testdata", "module"), + testedFiles: []string{ + "main.go", + filepath.Join("internal", "foo", "lib.go"), + }, + }, + { + name: `canonical module without go sources in root dir`, + moduleDir: filepath.Join("testdata", "module_canonical"), + testedFiles: []string{ + filepath.Join("cmd", "client", "main.go"), + filepath.Join("cmd", "server", "main.go"), + filepath.Join("internal", "foo", "lib.go"), + }, + }, + { + name: `non-canonical module without go sources in root dir`, + moduleDir: filepath.Join("testdata", "module_noncanonical"), + testedFiles: []string{ + filepath.Join("cmd", "client", "main.go"), + filepath.Join("cmd", "server", "main.go"), + filepath.Join("internal", "foo", "lib.go"), + }, + }, } - - // run subtests for expected module loading behaviour - chdir(t, moduleDir) - cfg := readConfig(t, "config.yaml") - - for _, path := range testedFiles { - t.Run(path, func(t *testing.T) { - // *.go -> *.out.go - expected, err := os.ReadFile(strings.TrimSuffix(path, ".go") + ".out.go") - require.NoError(t, err) - - _, got, err := LoadFormatGoFile(io.File{path}, *cfg) - - require.NoError(t, err) - require.Equal(t, string(expected), string(got)) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // run subtests for expected module loading behaviour + chdir(t, tt.moduleDir) + cfg := readConfig(t, "config.yaml") + + for _, path := range tt.testedFiles { + t.Run(path, func(t *testing.T) { + // *.go -> *.out.go + expected, err := os.ReadFile(strings.TrimSuffix(path, ".go") + ".out.go") + require.NoError(t, err) + + _, got, err := LoadFormatGoFile(io.File{path}, *cfg) + + require.NoError(t, err) + require.Equal(t, string(expected), string(got)) + }) + } }) } } @@ -96,18 +122,5 @@ func TestRunWithLocalModuleWithPackageLoadFailure(t *testing.T) { chdir(t, dir) _, err := config.ParseConfig(configContent) - require.ErrorContains(t, err, "failed to load local modules: ") -} - -func TestRunWithLocalModuleWithModuleLookupError(t *testing.T) { - dir := t.TempDir() - // error from trying to list packages under module with no go files - configContent := "sections:\n - LocalModule\n" - goModContent := "module example.com/foo\n" - require.NoError(t, os.WriteFile(filepath.Join(dir, "go.mod"), []byte(goModContent), 0o644)) - - chdir(t, dir) - _, err := config.ParseConfig(configContent) - require.ErrorContains(t, err, "error reading local packages: ") - require.ErrorContains(t, err, dir) + require.ErrorContains(t, err, "go.mod: open go.mod:") } diff --git a/pkg/gci/testdata.go b/pkg/gci/testdata.go index 9723981..4f60e88 100644 --- a/pkg/gci/testdata.go +++ b/pkg/gci/testdata.go @@ -1293,29 +1293,6 @@ import ( "fmt" "net" ) -`, - }, - { - "basic module", - // implicitly relies on the local module name being: github.com/daixiang0/gci - `sections: - - Standard - - LocalModule -`, - `package main - -import ( - "os" - "github.com/daixiang0/gci/cmd/gci" -) -`, - `package main - -import ( - "os" - - "github.com/daixiang0/gci/cmd/gci" -) `, }, } diff --git a/pkg/gci/testdata/module_canonical/.gitattributes b/pkg/gci/testdata/module_canonical/.gitattributes new file mode 100644 index 0000000..b63e502 --- /dev/null +++ b/pkg/gci/testdata/module_canonical/.gitattributes @@ -0,0 +1,4 @@ +# try and stop git running on Windows from converting line endings from +# in all Go files under this directory. This is to avoid inconsistent test +# results when `gci` strips "\r" characters +**/*.go eol=lf diff --git a/pkg/gci/testdata/module_canonical/cmd/client/main.go b/pkg/gci/testdata/module_canonical/cmd/client/main.go new file mode 100644 index 0000000..10c9826 --- /dev/null +++ b/pkg/gci/testdata/module_canonical/cmd/client/main.go @@ -0,0 +1,9 @@ +package main + +import ( + "example.com/service/internal" + "example.com/service/internal/bar" + "example.com/service/internal/foo" + "golang.org/x/net" + "log" +) diff --git a/pkg/gci/testdata/module_canonical/cmd/client/main.out.go b/pkg/gci/testdata/module_canonical/cmd/client/main.out.go new file mode 100644 index 0000000..52c3cef --- /dev/null +++ b/pkg/gci/testdata/module_canonical/cmd/client/main.out.go @@ -0,0 +1,11 @@ +package main + +import ( + "log" + + "golang.org/x/net" + + "example.com/service/internal" + "example.com/service/internal/bar" + "example.com/service/internal/foo" +) diff --git a/pkg/gci/testdata/module_canonical/cmd/server/main.go b/pkg/gci/testdata/module_canonical/cmd/server/main.go new file mode 100644 index 0000000..10c9826 --- /dev/null +++ b/pkg/gci/testdata/module_canonical/cmd/server/main.go @@ -0,0 +1,9 @@ +package main + +import ( + "example.com/service/internal" + "example.com/service/internal/bar" + "example.com/service/internal/foo" + "golang.org/x/net" + "log" +) diff --git a/pkg/gci/testdata/module_canonical/cmd/server/main.out.go b/pkg/gci/testdata/module_canonical/cmd/server/main.out.go new file mode 100644 index 0000000..52c3cef --- /dev/null +++ b/pkg/gci/testdata/module_canonical/cmd/server/main.out.go @@ -0,0 +1,11 @@ +package main + +import ( + "log" + + "golang.org/x/net" + + "example.com/service/internal" + "example.com/service/internal/bar" + "example.com/service/internal/foo" +) diff --git a/pkg/gci/testdata/module_canonical/config.yaml b/pkg/gci/testdata/module_canonical/config.yaml new file mode 100644 index 0000000..5ae453b --- /dev/null +++ b/pkg/gci/testdata/module_canonical/config.yaml @@ -0,0 +1,4 @@ +sections: + - Standard + - Default + - LocalModule diff --git a/pkg/gci/testdata/module_canonical/go.mod b/pkg/gci/testdata/module_canonical/go.mod new file mode 100644 index 0000000..8299e9f --- /dev/null +++ b/pkg/gci/testdata/module_canonical/go.mod @@ -0,0 +1,3 @@ +module example.com/service + +go 1.20 diff --git a/pkg/gci/testdata/module_canonical/internal/bar/lib.go b/pkg/gci/testdata/module_canonical/internal/bar/lib.go new file mode 100644 index 0000000..ddac0fa --- /dev/null +++ b/pkg/gci/testdata/module_canonical/internal/bar/lib.go @@ -0,0 +1 @@ +package bar diff --git a/pkg/gci/testdata/module_canonical/internal/foo/lib.go b/pkg/gci/testdata/module_canonical/internal/foo/lib.go new file mode 100644 index 0000000..07a3ead --- /dev/null +++ b/pkg/gci/testdata/module_canonical/internal/foo/lib.go @@ -0,0 +1,6 @@ +package foo + +import ( + "example.com/service/internal/bar" + "log" +) diff --git a/pkg/gci/testdata/module_canonical/internal/foo/lib.out.go b/pkg/gci/testdata/module_canonical/internal/foo/lib.out.go new file mode 100644 index 0000000..de5b6f5 --- /dev/null +++ b/pkg/gci/testdata/module_canonical/internal/foo/lib.out.go @@ -0,0 +1,7 @@ +package foo + +import ( + "log" + + "example.com/service/internal/bar" +) diff --git a/pkg/gci/testdata/module_canonical/internal/lib.go b/pkg/gci/testdata/module_canonical/internal/lib.go new file mode 100644 index 0000000..5bf0569 --- /dev/null +++ b/pkg/gci/testdata/module_canonical/internal/lib.go @@ -0,0 +1 @@ +package internal diff --git a/pkg/gci/testdata/module_noncanonical/.gitattributes b/pkg/gci/testdata/module_noncanonical/.gitattributes new file mode 100644 index 0000000..b63e502 --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/.gitattributes @@ -0,0 +1,4 @@ +# try and stop git running on Windows from converting line endings from +# in all Go files under this directory. This is to avoid inconsistent test +# results when `gci` strips "\r" characters +**/*.go eol=lf diff --git a/pkg/gci/testdata/module_noncanonical/cmd/client/main.go b/pkg/gci/testdata/module_noncanonical/cmd/client/main.go new file mode 100644 index 0000000..aa41ef3 --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/cmd/client/main.go @@ -0,0 +1,9 @@ +package main + +import ( + "golang.org/x/net" + "log" + "service/internal" + "service/internal/bar" + "service/internal/foo" +) diff --git a/pkg/gci/testdata/module_noncanonical/cmd/client/main.out.go b/pkg/gci/testdata/module_noncanonical/cmd/client/main.out.go new file mode 100644 index 0000000..96d4449 --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/cmd/client/main.out.go @@ -0,0 +1,11 @@ +package main + +import ( + "log" + + "golang.org/x/net" + + "service/internal" + "service/internal/bar" + "service/internal/foo" +) diff --git a/pkg/gci/testdata/module_noncanonical/cmd/server/main.go b/pkg/gci/testdata/module_noncanonical/cmd/server/main.go new file mode 100644 index 0000000..aa41ef3 --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/cmd/server/main.go @@ -0,0 +1,9 @@ +package main + +import ( + "golang.org/x/net" + "log" + "service/internal" + "service/internal/bar" + "service/internal/foo" +) diff --git a/pkg/gci/testdata/module_noncanonical/cmd/server/main.out.go b/pkg/gci/testdata/module_noncanonical/cmd/server/main.out.go new file mode 100644 index 0000000..96d4449 --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/cmd/server/main.out.go @@ -0,0 +1,11 @@ +package main + +import ( + "log" + + "golang.org/x/net" + + "service/internal" + "service/internal/bar" + "service/internal/foo" +) diff --git a/pkg/gci/testdata/module_noncanonical/config.yaml b/pkg/gci/testdata/module_noncanonical/config.yaml new file mode 100644 index 0000000..5ae453b --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/config.yaml @@ -0,0 +1,4 @@ +sections: + - Standard + - Default + - LocalModule diff --git a/pkg/gci/testdata/module_noncanonical/go.mod b/pkg/gci/testdata/module_noncanonical/go.mod new file mode 100644 index 0000000..7deade2 --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/go.mod @@ -0,0 +1,3 @@ +module service + +go 1.20 diff --git a/pkg/gci/testdata/module_noncanonical/internal/bar/lib.go b/pkg/gci/testdata/module_noncanonical/internal/bar/lib.go new file mode 100644 index 0000000..ddac0fa --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/internal/bar/lib.go @@ -0,0 +1 @@ +package bar diff --git a/pkg/gci/testdata/module_noncanonical/internal/foo/lib.go b/pkg/gci/testdata/module_noncanonical/internal/foo/lib.go new file mode 100644 index 0000000..a56f1c2 --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/internal/foo/lib.go @@ -0,0 +1,6 @@ +package foo + +import ( + "log" + "service/internal/bar" +) diff --git a/pkg/gci/testdata/module_noncanonical/internal/foo/lib.out.go b/pkg/gci/testdata/module_noncanonical/internal/foo/lib.out.go new file mode 100644 index 0000000..e4282eb --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/internal/foo/lib.out.go @@ -0,0 +1,7 @@ +package foo + +import ( + "log" + + "service/internal/bar" +) diff --git a/pkg/gci/testdata/module_noncanonical/internal/lib.go b/pkg/gci/testdata/module_noncanonical/internal/lib.go new file mode 100644 index 0000000..5bf0569 --- /dev/null +++ b/pkg/gci/testdata/module_noncanonical/internal/lib.go @@ -0,0 +1 @@ +package internal diff --git a/pkg/section/local_module.go b/pkg/section/local_module.go index 4e1edcb..07821fe 100644 --- a/pkg/section/local_module.go +++ b/pkg/section/local_module.go @@ -2,9 +2,10 @@ package section import ( "fmt" + "os" "strings" - "golang.org/x/tools/go/packages" + "golang.org/x/mod/modfile" "github.com/daixiang0/gci/pkg/parse" "github.com/daixiang0/gci/pkg/specificity" @@ -13,15 +14,12 @@ import ( const LocalModuleType = "localmodule" type LocalModule struct { - Paths []string + Path string } func (m *LocalModule) MatchSpecificity(spec *parse.GciImports) specificity.MatchSpecificity { - for _, modPath := range m.Paths { - // also check path etc. - if strings.HasPrefix(spec.Path, modPath) { - return specificity.LocalModule{} - } + if strings.HasPrefix(spec.Path, m.Path) { + return specificity.LocalModule{} } return specificity.MisMatch{} @@ -38,40 +36,21 @@ func (m *LocalModule) Type() string { // Configure configures the module section by finding the module // for the current path func (m *LocalModule) Configure() error { - modPaths, err := findLocalModules() + modPath, err := findLocalModule() if err != nil { - return err + return fmt.Errorf("finding local modules for `localModule` configuration: %w", err) } - m.Paths = modPaths + + m.Path = modPath + return nil } -func findLocalModules() ([]string, error) { - packages, err := packages.Load( - // find the package in the current dir and load its module - // NeedFiles so there is some more info in package errors - &packages.Config{Mode: packages.NeedModule | packages.NeedFiles}, - ".", - ) +func findLocalModule() (string, error) { + b, err := os.ReadFile("go.mod") if err != nil { - return nil, fmt.Errorf("failed to load local modules: %v", err) - } - - uniqueModules := make(map[string]struct{}) - - for _, pkg := range packages { - if len(pkg.Errors) != 0 { - return nil, fmt.Errorf("error reading local packages: %v", pkg.Errors) - } - if pkg.Module != nil { - uniqueModules[pkg.Module.Path] = struct{}{} - } - } - - modPaths := make([]string, 0, len(uniqueModules)) - for mod := range uniqueModules { - modPaths = append(modPaths, mod) + return "", fmt.Errorf("reading go.mod: %w", err) } - return modPaths, nil + return modfile.ModulePath(b), nil }