Skip to content

Commit

Permalink
Merge pull request #31 from marcuscaisey/absolute-path
Browse files Browse the repository at this point in the history 
Normalise all paths to be relative to plz root when running gofmt
  • Loading branch information
@brian-bice
brian-bice authored Apr 15, 2022
2 parents fb9f8dd + 164be99 commit b19b8e4
Show file tree
Hide file tree
Showing 13 changed files with 304 additions and 47 deletions.
4 changes: 3 additions & 1 deletion adapters/cobra/symlink_list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ func SymlinkListCmd(app ctl.Application) *cobra.Command {
args = []string{filepath.Join(wollemi.GoSrcPath(), "...")}
}

wollemi.SymlinkList(name, broken, prune, exclude, args)
if err := wollemi.SymlinkList(name, broken, prune, exclude, args); err != nil {
return err
}

return nil
},
Expand Down
27 changes: 20 additions & 7 deletions domain/wollemi/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,32 @@ type Service struct {
goFormat *goFormat
}

func (this *Service) validateAbsolutePaths(paths []string) error {
invalidPaths := make([]string, 0, len(paths))
for _, path := range paths {
if filepath.IsAbs(path) && !strings.HasPrefix(path, this.root) {
invalidPaths = append(invalidPaths, path)
}
}
if len(invalidPaths) > 0 {
return fmt.Errorf("absolute paths %q are not under the plz repo root %q", invalidPaths, this.root)
}
return nil
}

func (this *Service) normalizePaths(paths []string) []string {
if len(paths) == 0 {
paths = []string{"..."}
}

var relpath string

if this.wd != this.root && strings.HasPrefix(this.wd, this.root) {
relpath = strings.TrimPrefix(this.wd, this.root+"/")

for i, path := range paths {
paths[i] = filepath.Join(relpath, strings.TrimSuffix(path, "/"))
for i, path := range paths {
if !filepath.IsAbs(path) {
path = filepath.Join(this.wd, path)
}
if strings.HasPrefix(path, this.root) {
path, _ = filepath.Rel(this.root, path)
}
paths[i] = path
}

return paths
Expand Down
4 changes: 4 additions & 0 deletions domain/wollemi/service_format.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,10 @@ func (this *Service) isInternal(path string) bool {
}

func (this *Service) GoFormat(config wollemi.Config, paths []string) error {
if err := this.validateAbsolutePaths(paths); err != nil {
return err
}

this.goFormat = newGoFormat(this.normalizePaths(paths))
defer this.goFormat.resolveLimiter.Close()

Expand Down
177 changes: 176 additions & 1 deletion domain/wollemi/service_format_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ const (
gopkg = "github.com/example"
)

var (
root = filepath.Join(gosrc, gopkg)
wd = root
)

func (t *ServiceSuite) TestService_GoFormat() {
type T = ServiceSuite

Expand Down Expand Up @@ -1542,6 +1547,150 @@ func (t *ServiceSuite) TestService_GoFormat() {
},
},
},
}, { // TEST_CASE -------------------------------------------------------------
Title: "accepts absolute paths when in the root",
Data: &GoFormatTestData{
Gosrc: gosrc,
Gopkg: gopkg,
Root: "/root",
Wd: "/root",
Paths: []string{"/root/app"},
Parse: t.WithThirdPartyGo(nil),
ImportDir: map[string]*golang.Package{
"app": {
Name: "main",
GoFiles: []string{"main.go"},
GoFileImports: map[string][]string{
"main.go": {
"github.com/spf13/cobra",
},
},
},
},
Write: map[string]*please.BuildFile{
"app/BUILD.plz": {
Stmt: []please.Expr{
please.NewCallExpr("go_binary", []please.Expr{
please.NewAssignExpr("=", "name", "app"),
please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")),
please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}),
please.NewAssignExpr("=", "deps", []string{
"//third_party/go/github.com/spf13:cobra",
}),
}),
},
},
},
},
}, { // TEST_CASE -------------------------------------------------------------
Title: "accepts absolute paths when in a child of the root",
Data: &GoFormatTestData{
Gosrc: gosrc,
Gopkg: gopkg,
Root: "/root",
Wd: "/root/wd",
Paths: []string{"/root/wd/app"},
Parse: t.WithThirdPartyGo(nil),
ImportDir: map[string]*golang.Package{
"wd/app": {
Name: "main",
GoFiles: []string{"main.go"},
GoFileImports: map[string][]string{
"main.go": {
"github.com/spf13/cobra",
},
},
},
},
Write: map[string]*please.BuildFile{
"wd/app/BUILD.plz": {
Stmt: []please.Expr{
please.NewCallExpr("go_binary", []please.Expr{
please.NewAssignExpr("=", "name", "app"),
please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")),
please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}),
please.NewAssignExpr("=", "deps", []string{
"//third_party/go/github.com/spf13:cobra",
}),
}),
},
},
},
},
}, { // TEST_CASE -------------------------------------------------------------
Title: "accepts relative paths when in a child of the root",
Data: &GoFormatTestData{
Gosrc: gosrc,
Gopkg: gopkg,
Root: "/root",
Wd: "/root/wd",
Paths: []string{"app"},
Parse: t.WithThirdPartyGo(nil),
ImportDir: map[string]*golang.Package{
"wd/app": {
Name: "main",
GoFiles: []string{"main.go"},
GoFileImports: map[string][]string{
"main.go": {
"github.com/spf13/cobra",
},
},
},
},
Write: map[string]*please.BuildFile{
"wd/app/BUILD.plz": {
Stmt: []please.Expr{
please.NewCallExpr("go_binary", []please.Expr{
please.NewAssignExpr("=", "name", "app"),
please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")),
please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}),
please.NewAssignExpr("=", "deps", []string{
"//third_party/go/github.com/spf13:cobra",
}),
}),
},
},
},
},
}, { // TEST_CASE -------------------------------------------------------------
Title: "reads config from directory when absolute path given",
Config: wollemi.Config{},
Data: &GoFormatTestData{
Gosrc: gosrc,
Gopkg: gopkg,
Root: "/root",
Wd: "/root",
Config: map[string]wollemi.Config{
"parent/app": {KnownDependency: map[string]string{
"github.com/example/dep": "//third_party/go/github.com/example/override"}},
},
Paths: []string{"/root/parent/app"},
ImportDir: map[string]*golang.Package{
"parent/app": {
Name: "main",
GoFiles: []string{"main.go"},
GoFileImports: map[string][]string{
"main.go": {
"github.com/example/dep",
},
},
},
},
Write: map[string]*please.BuildFile{
"parent/app/BUILD.plz": {
Stmt: []please.Expr{
please.NewCallExpr("go_binary", []please.Expr{
please.NewAssignExpr("=", "name", "app"),
please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")),
please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}),
please.NewAssignExpr("=", "deps", []string{
"//third_party/go/github.com/example/override",
}),
}),
},
},
},
},
}} {
focus := ""

Expand All @@ -1554,7 +1703,15 @@ func (t *ServiceSuite) TestService_GoFormat() {

t.MockGoFormat(tt.Data, write)

wollemi := t.New(tt.Data.Gosrc, tt.Data.Gopkg)
root := root
if tt.Data.Root != "" {
root = tt.Data.Root
}
wd := wd
if tt.Data.Wd != "" {
wd = tt.Data.Wd
}
wollemi := t.New(root, wd, tt.Data.Gosrc, tt.Data.Gopkg)

require.NoError(t, wollemi.GoFormat(tt.Config, tt.Data.Paths))
close(write)
Expand All @@ -1572,6 +1729,22 @@ func (t *ServiceSuite) TestService_GoFormat() {
}
})
}

t.It("returns an error if given an absolute path which is not under the repo root", func(t *T) {
w := t.New(root, wd, gosrc, gopkg)

var (
config = wollemi.Config{}
paths = []string{
filepath.Join(root, "subdir"),
"/outside/of/root",
}
)

err := w.GoFormat(config, paths)

assert.Error(t, err)
})
}

func (t *ServiceSuite) MockGoFormat(data *GoFormatTestData, write chan please.File) {
Expand Down Expand Up @@ -1706,6 +1879,8 @@ func (t *ServiceSuite) MockGoFormat(data *GoFormatTestData, write chan please.Fi
type GoFormatTestData struct {
Gosrc string
Gopkg string
Root string
Wd string
Paths []string
Config map[string]wollemi.Config
ImportDir map[string]*golang.Package
Expand Down
4 changes: 4 additions & 0 deletions domain/wollemi/service_rules_unused.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import (
)

func (this *Service) RulesUnused(prune bool, kinds, paths, exclude []string) error {
if err := this.validateAbsolutePaths(paths); err != nil {
return err
}

graph, err := this.please.Graph()
if err != nil {
return err
Expand Down
28 changes: 21 additions & 7 deletions domain/wollemi/service_rules_unused_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package wollemi_test
import (
"bytes"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -18,15 +19,10 @@ func TestService_RulesUnused(t *testing.T) {
func (t *ServiceSuite) TestService_RulesUnused() {
type T = ServiceSuite

const (
gopkg = "github.com/wollemi_test"
gosrc = "/go/src"
)

t.It("can list unused build rules", func(t *T) {
t.MockRulesUnused()

wollemi := t.New(gosrc, gopkg)
wollemi := t.New(root, wd, gosrc, gopkg)

var (
prune bool
Expand Down Expand Up @@ -106,7 +102,7 @@ func (t *ServiceSuite) TestService_RulesUnused() {
expect.Equal(t, want, have)
})

wollemi := t.New(gosrc, gopkg)
wollemi := t.New(root, wd, gosrc, gopkg)

var (
prune bool = true
Expand All @@ -117,6 +113,24 @@ func (t *ServiceSuite) TestService_RulesUnused() {

wollemi.RulesUnused(prune, kinds, paths, excludePaths)
})

t.It("returns an error if given an absolute path which is not under the repo root", func(t *T) {
wollemi := t.New(root, wd, gosrc, gopkg)

var (
prune bool = true
kinds []string
paths = []string{
filepath.Join(root, "subdir"),
"/outside/of/root",
}
excludePaths []string
)

err := wollemi.RulesUnused(prune, kinds, paths, excludePaths)

assert.Error(t, err)
})
}

func (t *ServiceSuite) MockRulesUnused() {
Expand Down
13 changes: 6 additions & 7 deletions domain/wollemi/service_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@ package wollemi_test

import (
"fmt"
"path/filepath"
"strings"
"sync"
"testing"

"github.com/golang/mock/gomock"

"github.com/tcncloud/wollemi/domain/wollemi"
"github.com/tcncloud/wollemi/ports/golang/mock"
"github.com/tcncloud/wollemi/ports/please/mock"
"github.com/tcncloud/wollemi/ports/wollemi/mock"
mock_golang "github.com/tcncloud/wollemi/ports/golang/mock"
mock_please "github.com/tcncloud/wollemi/ports/please/mock"
mock_wollemi "github.com/tcncloud/wollemi/ports/wollemi/mock"
"github.com/tcncloud/wollemi/testdata/mem"
)

Expand Down Expand Up @@ -61,14 +60,14 @@ func (suite *ServiceSuite) Run(name string, yield func(*ServiceSuite)) {
})
}

func (suite *ServiceSuite) New(gosrc, gopkg string) *wollemi.Service {
func (suite *ServiceSuite) New(root, wd, gosrc, gopkg string) *wollemi.Service {
return wollemi.New(
suite.logger,
suite.filesystem,
suite.golang,
suite.please,
filepath.Join(gosrc, gopkg),
filepath.Join(gosrc, gopkg),
root,
wd,
gosrc,
gopkg,
)
Expand Down
4 changes: 4 additions & 0 deletions domain/wollemi/service_symlink_go_path.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import (
)

func (this *Service) SymlinkGoPath(force bool, paths []string) error {
if err := this.validateAbsolutePaths(paths); err != nil {
return err
}

paths = this.normalizePaths(paths)

deps, err := this.please.QueryDeps(paths...)
Expand Down
Loading

0 comments on commit b19b8e4

Please sign in to comment.