Merge pull request #1116 from darkowlzz/manifest-pr-validate

fix(manifest): validate project roots in manifest

cmd/dep/ensure.go

@@ -160,6 +160,10 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
 	sm.UseDefaultSignalHandling()
 	defer sm.Release()
 
+	if err := dep.ValidateProjectRoots(ctx, p.Manifest, sm); err != nil {
+		return err
+	}
+
 	params := p.MakeParams()
 	if ctx.Verbose {
 		params.TraceLogger = ctx.Err

cmd/dep/status.go

@@ -190,6 +190,10 @@ func (cmd *statusCommand) Run(ctx *dep.Ctx, args []string) error {
 	sm.UseDefaultSignalHandling()
 	defer sm.Release()
 
+	if err := dep.ValidateProjectRoots(ctx, p.Manifest, sm); err != nil {
+		return err
+	}
+
 	var buf bytes.Buffer
 	var out outputter
 	switch {

manifest.go

@@ -11,6 +11,7 @@ import (
 	"reflect"
 	"regexp"
 	"sort"
+	"sync"
 
 	"github.com/golang/dep/internal/gps"
 	"github.com/pelletier/go-toml"
@@ -22,10 +23,11 @@ const ManifestName = "Gopkg.toml"
 
 // Errors
 var (
-	errInvalidConstraint = errors.New("\"constraint\" must be a TOML array of tables")
-	errInvalidOverride   = errors.New("\"override\" must be a TOML array of tables")
-	errInvalidRequired   = errors.New("\"required\" must be a TOML list of strings")
-	errInvalidIgnored    = errors.New("\"ignored\" must be a TOML list of strings")
+	errInvalidConstraint  = errors.New("\"constraint\" must be a TOML array of tables")
+	errInvalidOverride    = errors.New("\"override\" must be a TOML array of tables")
+	errInvalidRequired    = errors.New("\"required\" must be a TOML list of strings")
+	errInvalidIgnored     = errors.New("\"ignored\" must be a TOML list of strings")
+	errInvalidProjectRoot = errors.New("ProjectRoot name validation failed")
 )
 
 // Manifest holds manifest file data and implements gps.RootManifest.
@@ -156,6 +158,48 @@ func validateManifest(s string) ([]error, error) {
 	return warns, nil
 }
 
+// ValidateProjectRoots validates the project roots present in manifest.
+func ValidateProjectRoots(c *Ctx, m *Manifest, sm gps.SourceManager) error {
+	// Channel to receive all the errors
+	errorCh := make(chan error, len(m.Constraints)+len(m.Ovr))
+
+	var wg sync.WaitGroup
+
+	validate := func(pr gps.ProjectRoot) {
+		defer wg.Done()
+		origPR, err := sm.DeduceProjectRoot(string(pr))
+		if err != nil {
+			errorCh <- err
+		} else if origPR != pr {
+			errorCh <- fmt.Errorf("the name for %q should be changed to %q", pr, origPR)
+		}
+	}
+
+	for pr := range m.Constraints {
+		wg.Add(1)
+		go validate(pr)
+	}
+	for pr := range m.Ovr {
+		wg.Add(1)
+		go validate(pr)
+	}
+
+	wg.Wait()
+	close(errorCh)
+
+	var valErr error
+	if len(errorCh) > 0 {
+		valErr = errInvalidProjectRoot
+		c.Err.Printf("The following issues were found in Gopkg.toml:\n\n")
+		for err := range errorCh {
+			c.Err.Println("  ✗", err.Error())
+		}
+		c.Err.Println()
+	}
+
+	return valErr
+}
+
 // readManifest returns a Manifest read from r and a slice of validation warnings.
 func readManifest(r io.Reader) (*Manifest, []error, error) {
 	buf := &bytes.Buffer{}

manifest_test.go

@@ -5,7 +5,10 @@
 package dep
 
 import (
+	"bytes"
 	"errors"
+	"io/ioutil"
+	"log"
 	"reflect"
 	"strings"
 	"testing"
@@ -372,3 +375,110 @@ func TestValidateManifest(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateProjectRoots(t *testing.T) {
+	cases := []struct {
+		name      string
+		manifest  Manifest
+		wantError error
+		wantWarn  []string
+	}{
+		{
+			name:      "empty Manifest",
+			manifest:  Manifest{},
+			wantError: nil,
+			wantWarn:  []string{},
+		},
+		{
+			name: "valid project root",
+			manifest: Manifest{
+				Constraints: map[gps.ProjectRoot]gps.ProjectProperties{
+					gps.ProjectRoot("github.com/golang/dep"): {
+						Constraint: gps.Any(),
+					},
+				},
+			},
+			wantError: nil,
+			wantWarn:  []string{},
+		},
+		{
+			name: "invalid project roots in Constraints and Overrides",
+			manifest: Manifest{
+				Constraints: map[gps.ProjectRoot]gps.ProjectProperties{
+					gps.ProjectRoot("github.com/golang/dep/foo"): {
+						Constraint: gps.Any(),
+					},
+					gps.ProjectRoot("github.com/golang/go/xyz"): {
+						Constraint: gps.Any(),
+					},
+					gps.ProjectRoot("github.com/golang/fmt"): {
+						Constraint: gps.Any(),
+					},
+				},
+				Ovr: map[gps.ProjectRoot]gps.ProjectProperties{
+					gps.ProjectRoot("github.com/golang/mock/bar"): {
+						Constraint: gps.Any(),
+					},
+					gps.ProjectRoot("github.com/golang/mock"): {
+						Constraint: gps.Any(),
+					},
+				},
+			},
+			wantError: errInvalidProjectRoot,
+			wantWarn: []string{
+				"the name for \"github.com/golang/dep/foo\" should be changed to \"github.com/golang/dep\"",
+				"the name for \"github.com/golang/mock/bar\" should be changed to \"github.com/golang/mock\"",
+				"the name for \"github.com/golang/go/xyz\" should be changed to \"github.com/golang/go\"",
+			},
+		},
+		{
+			name: "invalid source path",
+			manifest: Manifest{
+				Constraints: map[gps.ProjectRoot]gps.ProjectProperties{
+					gps.ProjectRoot("github.com/golang"): {
+						Constraint: gps.Any(),
+					},
+				},
+			},
+			wantError: errInvalidProjectRoot,
+			wantWarn:  []string{},
+		},
+	}
+
+	h := test.NewHelper(t)
+	defer h.Cleanup()
+
+	h.TempDir("src")
+	pwd := h.Path(".")
+
+	// Capture the stderr to verify the warnings
+	stderrOutput := &bytes.Buffer{}
+	errLogger := log.New(stderrOutput, "", 0)
+	ctx := &Ctx{
+		GOPATH: pwd,
+		Out:    log.New(ioutil.Discard, "", 0),
+		Err:    errLogger,
+	}
+
+	sm, err := ctx.SourceManager()
+	h.Must(err)
+	defer sm.Release()
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			// Empty the buffer for every case
+			stderrOutput.Reset()
+			err := ValidateProjectRoots(ctx, &c.manifest, sm)
+			if err != c.wantError {
+				t.Fatalf("Unexpected error while validating project roots:\n\t(GOT): %v\n\t(WNT): %v", err, c.wantError)
+			}
+
+			warnings := stderrOutput.String()
+			for _, warn := range c.wantWarn {
+				if !strings.Contains(warnings, warn) {
+					t.Fatalf("Expected ValidateProjectRoot errors to contain: %q", warn)
+				}
+			}
+		})
+	}
+}