Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic attempt at local module handling #179

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,17 @@ import (
)
```

GCI splits all import blocks into different sections, now support five section type:
GCI splits all import blocks into different sections, now support six section type:

- standard: Go official imports, like "fmt"
- custom: Custom section, use full and the longest match (match full string first, if multiple matches, use the longest one)
- default: All rest import blocks
- blank: Put blank imports together in a separate group
- dot: Put dot imports together in a separate group
- alias: Put alias imports together in a separate group
- localmodule: Put imports from local packages in a separate group

The priority is standard > default > custom > blank > dot > alias, all sections sort alphabetically inside.
The priority is standard > default > custom > blank > dot > alias > localmodule, all sections sort alphabetically inside.
By default, blank , dot and alias sections are not used and the corresponding lines end up in the other groups.

All import blocks use one TAB(`\t`) as Indent.
Expand All @@ -47,6 +48,17 @@ Since v0.9.0, GCI always puts C import block as the first.

`nolint` is hard to handle at section level, GCI will consider it as a single comment.

### LocalModule

Local module detection is done via listing packages from *the directory where
`gci` is invoked* and reading the modules off these. This means:

- This mode works when `gci` is invoked from a module root (i.e. directory
containing `go.mod`)
- This mode doesn't work with a multi-module setup, i.e. when `gci` is invoked
from a directory containing `go.work` (though it would work if invoked from
within one of the modules in the workspace)

## Installation

To download and install the highest available release version -
Expand Down Expand Up @@ -321,6 +333,32 @@ import (
)
```
### with localmodule grouping enabled
Assuming this is run on the root of this repo (i.e. where
`github.com/daixiang0/gci` is a local module)
```go
package main

import (
"os"
"github.com/daixiang0/gci/cmd/gci"
)
```
to
```go
package main

import (
"os"

"github.com/daixiang0/gci/cmd/gci"
)
```
## TODO
- Ensure only one blank between `Name` and `Path` in an import block
Expand Down
28 changes: 22 additions & 6 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import (
)

var defaultOrder = map[string]int{
section.StandardType: 0,
section.DefaultType: 1,
section.CustomType: 2,
section.BlankType: 3,
section.DotType: 4,
section.AliasType: 5,
section.StandardType: 0,
section.DefaultType: 1,
section.CustomType: 2,
section.BlankType: 3,
section.DotType: 4,
section.AliasType: 5,
section.LocalModuleType: 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,15 @@ 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.LocalModule:
if err := s.Configure(); err != nil {
return err
}
}
}
return nil
}
73 changes: 73 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,71 @@ 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")
// files with a corresponding '*.out.go' file containing the expected
// result of formatting
testedFiles := []string{
"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))
})
}
}

func TestRunWithLocalModuleWithPackageLoadFailure(t *testing.T) {
// just a directory with no Go modules
dir := t.TempDir()
daixiang0 marked this conversation as resolved.
Show resolved Hide resolved
configContent := "sections:\n - LocalModule\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()
daixiang0 marked this conversation as resolved.
Show resolved Hide resolved
// 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)
}
23 changes: 23 additions & 0 deletions pkg/gci/testdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -1293,6 +1293,29 @@ 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"
)
`,
},
}
4 changes: 4 additions & 0 deletions pkg/gci/testdata/module/.gitattributes
Original file line number Diff line number Diff line change
@@ -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
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
- LocalModule
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"
)
77 changes: 77 additions & 0 deletions pkg/section/local_module.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package section

import (
"fmt"
"strings"

"golang.org/x/tools/go/packages"

"github.com/daixiang0/gci/pkg/parse"
"github.com/daixiang0/gci/pkg/specificity"
)

const LocalModuleType = "localmodule"

type LocalModule struct {
Paths []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{}
}
}

return specificity.MisMatch{}
}

func (m *LocalModule) String() string {
return LocalModuleType
}

func (m *LocalModule) Type() string {
return LocalModuleType
}

// Configure configures the module section by finding the module
// for the current path
func (m *LocalModule) Configure() error {
modPaths, err := findLocalModules()
if err != nil {
return err
}
m.Paths = modPaths
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},
".",
)
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 modPaths, nil
}
3 changes: 3 additions & 0 deletions pkg/section/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ func Parse(data []string) (SectionList, error) {
list = append(list, Blank{})
} else if s == "alias" {
list = append(list, Alias{})
} else if s == "localmodule" {
// pointer because we need to mutate the section at configuration time
list = append(list, &LocalModule{})
} else {
errString += fmt.Sprintf(" %s", s)
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/specificity/local_module.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package specificity

type LocalModule struct{}

func (m LocalModule) IsMoreSpecific(than MatchSpecificity) bool {
return isMoreSpecific(m, than)
}

func (m LocalModule) Equal(to MatchSpecificity) bool {
return equalSpecificity(m, to)
}

func (LocalModule) class() specificityClass {
return LocalModuleClass
}
Loading
Loading