From 8546008fec0dac83983c7c84ae61905fb2d03a9f Mon Sep 17 00:00:00 2001 From: Matthew Hughes Date: Sat, 28 Oct 2023 17:15:42 +0100 Subject: [PATCH] Add basic attempt at local module handling The idea here is to support a new formatting section, `module`, which is the module we're currently running in as a replacement for `prefix(module/we/are/running/in)`. Since this is dependent on the directory structure and where things are run, some tests have been added which run on a real module structure. This approach just focusses on the use-case of running `gci` from somewhere in a module's path. It does not, for example support: * Discovering modules with a nested structure * Discovering modules defined in a `go.work` file Signed-off-by: Matthew Hughes --- pkg/config/config.go | 17 +++++ pkg/gci/gci_test.go | 68 +++++++++++++++++++ pkg/gci/testdata.go | 23 +++++++ pkg/gci/testdata/module/config.yaml | 4 ++ pkg/gci/testdata/module/go.mod | 3 + pkg/gci/testdata/module/internal/bar/lib.go | 1 + pkg/gci/testdata/module/internal/foo/lib.go | 6 ++ .../testdata/module/internal/foo/lib.out.go | 7 ++ pkg/gci/testdata/module/internal/lib.go | 1 + pkg/gci/testdata/module/main.go | 9 +++ pkg/gci/testdata/module/main.out.go | 11 +++ pkg/section/module.go | 64 +++++++++++++++++ pkg/section/parser.go | 2 + pkg/section/section.go | 1 + pkg/specificity/module.go | 15 ++++ pkg/specificity/specificity.go | 1 + 16 files changed, 233 insertions(+) create mode 100644 pkg/gci/testdata/module/config.yaml create mode 100644 pkg/gci/testdata/module/go.mod create mode 100644 pkg/gci/testdata/module/internal/bar/lib.go create mode 100644 pkg/gci/testdata/module/internal/foo/lib.go create mode 100644 pkg/gci/testdata/module/internal/foo/lib.out.go create mode 100644 pkg/gci/testdata/module/internal/lib.go create mode 100644 pkg/gci/testdata/module/main.go create mode 100644 pkg/gci/testdata/module/main.out.go create mode 100644 pkg/section/module.go create mode 100644 pkg/specificity/module.go diff --git a/pkg/config/config.go b/pkg/config/config.go index 51f6ccf..3359124 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -16,6 +16,7 @@ var defaultOrder = map[string]int{ section.BlankType: 3, section.DotType: 4, section.AliasType: 5, + section.ModuleType: 6, } type BoolConfig struct { @@ -49,6 +50,9 @@ func (g YamlConfig) Parse() (*Config, error) { if sections == nil { sections = section.DefaultSections() } + if err := configureSections(sections); err != nil { + return nil, err + } // if default order sorted sections if !g.Cfg.CustomOrder { @@ -88,3 +92,16 @@ func ParseConfig(in string) (*Config, error) { return gciCfg, nil } + +func configureSections(sections section.SectionList) error { + for _, sec := range sections { + switch s := sec.(type) { + case *section.Module: + // XXX: trying out the idea of passing files down here... + if err := s.Populate([]string{}); err != nil { + return err + } + } + } + return nil +} diff --git a/pkg/gci/gci_test.go b/pkg/gci/gci_test.go index 86b9510..b21e0cb 100644 --- a/pkg/gci/gci_test.go +++ b/pkg/gci/gci_test.go @@ -2,11 +2,16 @@ package gci import ( "fmt" + "os" + "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/daixiang0/gci/pkg/config" + "github.com/daixiang0/gci/pkg/io" "github.com/daixiang0/gci/pkg/log" ) @@ -38,3 +43,66 @@ func TestRun(t *testing.T) { }) } } + +func chdir(t *testing.T, dir string) { + oldWd, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(dir)) + + // change back at the end of the test + t.Cleanup(func() { os.Chdir(oldWd) }) +} + +func readConfig(t *testing.T, configPath string) *config.Config { + rawConfig, err := os.ReadFile(configPath) + require.NoError(t, err) + config, err := config.ParseConfig(string(rawConfig)) + require.NoError(t, err) + + return config +} + +func TestRunWithLocalModule(t *testing.T) { + moduleDir := filepath.Join("testdata", "module") + testedFiles := []string{ + "main.go", + filepath.Join("internal", "foo", "lib.go"), + } + + // run subtests for expected module loading behaviour + chdir(t, moduleDir) + + 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}, *readConfig(t, "config.yaml")) + + require.NoError(t, err) + require.Equal(t, string(expected), string(got)) + }) + } +} + +func TestRunWithLocalModuleWithPackageLoadFailure(t *testing.T) { + // just a directory with no Go modules + dir := t.TempDir() + configContent := "sections:\n - Module\n" + + chdir(t, dir) + _, err := config.ParseConfig(configContent) + require.ErrorContains(t, err, "failed to load local modules: ") +} + +func TestRunWithLocalModuleWithModuleLookupError(t *testing.T) { + dir := t.TempDir() + configContent := "sections:\n - Module\n" + goModContent := "module example.com/foo\n" + require.NoError(t, os.WriteFile(filepath.Join(dir, "go.mod"), []byte(goModContent), 0644)) + + chdir(t, dir) + _, err := config.ParseConfig(configContent) + require.ErrorContains(t, err, "error while listing modules") +} diff --git a/pkg/gci/testdata.go b/pkg/gci/testdata.go index 77c06dc..f91522c 100644 --- a/pkg/gci/testdata.go +++ b/pkg/gci/testdata.go @@ -1273,6 +1273,29 @@ import ( testing "github.com/daixiang0/test" g "github.com/golang" ) +`, + }, + { + "basic module", + // implicitly relies on the package name github.com/daixiang0/gci + `sections: + - Standard + - Module +`, + `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/config.yaml b/pkg/gci/testdata/module/config.yaml new file mode 100644 index 0000000..c045c71 --- /dev/null +++ b/pkg/gci/testdata/module/config.yaml @@ -0,0 +1,4 @@ +sections: + - Standard + - Default + - Module diff --git a/pkg/gci/testdata/module/go.mod b/pkg/gci/testdata/module/go.mod new file mode 100644 index 0000000..8531479 --- /dev/null +++ b/pkg/gci/testdata/module/go.mod @@ -0,0 +1,3 @@ +module example.com/simple + +go 1.20 diff --git a/pkg/gci/testdata/module/internal/bar/lib.go b/pkg/gci/testdata/module/internal/bar/lib.go new file mode 100644 index 0000000..ddac0fa --- /dev/null +++ b/pkg/gci/testdata/module/internal/bar/lib.go @@ -0,0 +1 @@ +package bar diff --git a/pkg/gci/testdata/module/internal/foo/lib.go b/pkg/gci/testdata/module/internal/foo/lib.go new file mode 100644 index 0000000..099f27e --- /dev/null +++ b/pkg/gci/testdata/module/internal/foo/lib.go @@ -0,0 +1,6 @@ +package foo + +import ( + "example.com/simple/internal/bar" + "log" +) diff --git a/pkg/gci/testdata/module/internal/foo/lib.out.go b/pkg/gci/testdata/module/internal/foo/lib.out.go new file mode 100644 index 0000000..f132d6b --- /dev/null +++ b/pkg/gci/testdata/module/internal/foo/lib.out.go @@ -0,0 +1,7 @@ +package foo + +import ( + "log" + + "example.com/simple/internal/bar" +) diff --git a/pkg/gci/testdata/module/internal/lib.go b/pkg/gci/testdata/module/internal/lib.go new file mode 100644 index 0000000..5bf0569 --- /dev/null +++ b/pkg/gci/testdata/module/internal/lib.go @@ -0,0 +1 @@ +package internal diff --git a/pkg/gci/testdata/module/main.go b/pkg/gci/testdata/module/main.go new file mode 100644 index 0000000..14409f0 --- /dev/null +++ b/pkg/gci/testdata/module/main.go @@ -0,0 +1,9 @@ +package lib + +import ( + "golang.org/x/net" + "example.com/simple/internal" + "example.com/simple/internal/foo" + "example.com/simple/internal/bar" + "log" +) diff --git a/pkg/gci/testdata/module/main.out.go b/pkg/gci/testdata/module/main.out.go new file mode 100644 index 0000000..ad3a9d0 --- /dev/null +++ b/pkg/gci/testdata/module/main.out.go @@ -0,0 +1,11 @@ +package lib + +import ( + "log" + + "golang.org/x/net" + + "example.com/simple/internal" + "example.com/simple/internal/bar" + "example.com/simple/internal/foo" +) diff --git a/pkg/section/module.go b/pkg/section/module.go new file mode 100644 index 0000000..d02351e --- /dev/null +++ b/pkg/section/module.go @@ -0,0 +1,64 @@ +package section + +import ( + "fmt" + "strings" + + "golang.org/x/tools/go/packages" + + "github.com/daixiang0/gci/pkg/parse" + "github.com/daixiang0/gci/pkg/specificity" +) + +const ModuleType = "module" + +type Module struct { + ModulePaths []string +} + +func (m *Module) MatchSpecificity(spec *parse.GciImports) specificity.MatchSpecificity { + for _, modPath := range m.ModulePaths { + if strings.HasPrefix(spec.Path, modPath) { + return specificity.Module{} + } + } + + return specificity.MisMatch{} +} + +func (m *Module) String() string { + return ModuleType +} + +func (m *Module) Type() string { + return ModuleType +} + +// Populate populates m.ModulePaths by finding modules for files +func (m *Module) Populate(files []string) error { + packages, err := packages.Load( + &packages.Config{Mode: packages.NeedModule}, files..., + ) + if err != nil { + return fmt.Errorf("failed to load local modules: %v", err) + } + + uniqueModules := make(map[string]struct{}) + + for _, pkg := range packages { + if len(pkg.Errors) != 0 { + return fmt.Errorf("error while listing modules: %v", pkg.Errors[0]) + } + if _, ok := uniqueModules[pkg.Module.Path]; !ok { + uniqueModules[pkg.Module.Path] = struct{}{} + } + } + + modPaths := make([]string, 0, len(uniqueModules)) + for mod := range uniqueModules { + modPaths = append(modPaths, mod) + } + + m.ModulePaths = modPaths + return nil +} diff --git a/pkg/section/parser.go b/pkg/section/parser.go index 38435f5..78104d3 100644 --- a/pkg/section/parser.go +++ b/pkg/section/parser.go @@ -35,6 +35,8 @@ func Parse(data []string) (SectionList, error) { list = append(list, Blank{}) } else if s == "alias" { list = append(list, Alias{}) + } else if s == "module" { + list = append(list, &Module{}) } else { errString += fmt.Sprintf(" %s", s) } diff --git a/pkg/section/section.go b/pkg/section/section.go index cc0a43f..6ee7b12 100644 --- a/pkg/section/section.go +++ b/pkg/section/section.go @@ -28,6 +28,7 @@ func (list SectionList) String() []string { } func DefaultSections() SectionList { + // pointer for Module since we need to mutate it return SectionList{Standard{}, Default{}} } diff --git a/pkg/specificity/module.go b/pkg/specificity/module.go new file mode 100644 index 0000000..dac10f0 --- /dev/null +++ b/pkg/specificity/module.go @@ -0,0 +1,15 @@ +package specificity + +type Module struct{} + +func (m Module) IsMoreSpecific(than MatchSpecificity) bool { + return isMoreSpecific(m, than) +} + +func (m Module) Equal(to MatchSpecificity) bool { + return equalSpecificity(m, to) +} + +func (Module) class() specificityClass { + return ModuleClass +} diff --git a/pkg/specificity/specificity.go b/pkg/specificity/specificity.go index 842da18..4e4e1f7 100644 --- a/pkg/specificity/specificity.go +++ b/pkg/specificity/specificity.go @@ -8,6 +8,7 @@ const ( StandardClass = 20 MatchClass = 30 NameClass = 40 + ModuleClass = 50 ) // MatchSpecificity is used to determine which section matches an import best