Skip to content

Commit

Permalink
Add basic attempt at local module handling
Browse files Browse the repository at this point in the history
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 <matthewhughes934@gmail.com>
  • Loading branch information
matthewhughes934 committed Nov 1, 2023
1 parent 19d84f7 commit 8546008
Show file tree
Hide file tree
Showing 16 changed files with 233 additions and 0 deletions.
17 changes: 17 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var defaultOrder = map[string]int{
section.BlankType: 3,
section.DotType: 4,
section.AliasType: 5,
section.ModuleType: 6,
}

type BoolConfig struct {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
68 changes: 68 additions & 0 deletions pkg/gci/gci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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")
}
23 changes: 23 additions & 0 deletions pkg/gci/testdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
`,
},
}
4 changes: 4 additions & 0 deletions pkg/gci/testdata/module/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
sections:
- Standard
- Default
- Module
3 changes: 3 additions & 0 deletions pkg/gci/testdata/module/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module example.com/simple

go 1.20
1 change: 1 addition & 0 deletions pkg/gci/testdata/module/internal/bar/lib.go
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package bar
6 changes: 6 additions & 0 deletions pkg/gci/testdata/module/internal/foo/lib.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package foo

import (
"example.com/simple/internal/bar"
"log"
)
7 changes: 7 additions & 0 deletions pkg/gci/testdata/module/internal/foo/lib.out.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package foo

import (
"log"

"example.com/simple/internal/bar"
)
1 change: 1 addition & 0 deletions pkg/gci/testdata/module/internal/lib.go
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package internal
9 changes: 9 additions & 0 deletions pkg/gci/testdata/module/main.go
Original file line number Diff line number Diff line change
@@ -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"
)
11 changes: 11 additions & 0 deletions pkg/gci/testdata/module/main.out.go
Original file line number Diff line number Diff line change
@@ -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"
)
64 changes: 64 additions & 0 deletions pkg/section/module.go
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 2 additions & 0 deletions pkg/section/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/section/section.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}}
}

Expand Down
15 changes: 15 additions & 0 deletions pkg/specificity/module.go
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions pkg/specificity/specificity.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const (
StandardClass = 20
MatchClass = 30
NameClass = 40
ModuleClass = 50
)

// MatchSpecificity is used to determine which section matches an import best
Expand Down

0 comments on commit 8546008

Please sign in to comment.