Merge pull request golang#1370 from JackyChiu/master

Manifest Version/Source Validation

context_test.go

@@ -325,7 +325,10 @@ func TestCaseInsentitiveGOPATH(t *testing.T) {
 
 	h.TempDir("src")
 	h.TempDir("src/test1")
-	h.TempFile(filepath.Join("src/test1", ManifestName), `[[constraint]]`)
+	h.TempFile(filepath.Join("src/test1", ManifestName), `
+	[[constraint]]
+		name = "github.com/foo/bar"
+		branch = "master"`)
 
 	// Shuffle letter case
 	rs := []rune(strings.ToLower(h.Path(".")))

manifest.go

@@ -40,6 +40,7 @@ var (
 	errRootPruneContainsName   = errors.Errorf("%q should not include a name", "prune")
 	errInvalidRootPruneValue   = errors.New("root prune options must be omitted instead of being set to false")
 	errInvalidPruneProjectName = errors.Errorf("%q in %q must be a string", "name", "prune.project")
+	errNoName                  = errors.New("no name provided")
 )
 
 // Manifest holds manifest file data and implements gps.RootManifest.
@@ -133,13 +134,17 @@ func validateManifest(s string) ([]error, error) {
 				if valid {
 					// Iterate through each array of tables
 					for _, v := range rawProj {
+						ruleProvided := false
+						props := v.(map[string]interface{})
 						// Check the individual field's key to be valid
-						for key, value := range v.(map[string]interface{}) {
+						for key, value := range props {
 							// Check if the key is valid
 							switch key {
-							case "name", "branch", "version", "source":
-								// valid key
+							case "name":
+							case "branch", "version", "source":
+								ruleProvided = true
 							case "revision":
+								ruleProvided = true
 								if valueStr, ok := value.(string); ok {
 									if abbrevRevHash.MatchString(valueStr) {
 										warns = append(warns, fmt.Errorf("revision %q should not be in abbreviated form", valueStr))
@@ -155,6 +160,11 @@ func validateManifest(s string) ([]error, error) {
 								warns = append(warns, fmt.Errorf("invalid key %q in %q", key, prop))
 							}
 						}
+						if _, ok := props["name"]; !ok {
+							warns = append(warns, errNoName)
+						} else if !ruleProvided && prop == "constraint" {
+							warns = append(warns, fmt.Errorf("branch, version, revision, or source should be provided for %q", props["name"]))
+						}
 					}
 				}
 			} else {

manifest_test.go

@@ -255,6 +255,7 @@ func TestValidateManifest(t *testing.T) {
 			`,
 			wantWarn: []error{
 				errInvalidMetadata,
+				errors.New("branch, version, revision, or source should be provided for \"github.com/foo/bar\""),
 			},
 			wantError: nil,
 		},
@@ -264,15 +265,19 @@ func TestValidateManifest(t *testing.T) {
 			[[constraint]]
 			  name = "github.com/foo/bar"
 			`,
-			wantWarn:  []error{},
+			wantWarn: []error{
+				errors.New("branch, version, revision, or source should be provided for \"github.com/foo/bar\""),
+			},
 			wantError: nil,
 		},
 		{
 			name: "empty constraint",
 			tomlString: `
 			[[constraint]]
 			`,
-			wantWarn:  []error{},
+			wantWarn: []error{
+				errNoName,
+			},
 			wantError: nil,
 		},
 		{
@@ -305,7 +310,9 @@ func TestValidateManifest(t *testing.T) {
 			tomlString: `
 			[[override]]
 			`,
-			wantWarn:  []error{},
+			wantWarn: []error{
+				errNoName,
+			},
 			wantError: nil,
 		},
 		{
@@ -339,8 +346,10 @@ func TestValidateManifest(t *testing.T) {
 			wantWarn: []error{
 				errors.New("invalid key \"location\" in \"constraint\""),
 				errors.New("invalid key \"link\" in \"constraint\""),
-				errors.New("invalid key \"nick\" in \"override\""),
 				errors.New("metadata in \"constraint\" should be a TOML table"),
+				errors.New("branch, version, revision, or source should be provided for \"github.com/foo/bar\""),
+				errors.New("invalid key \"nick\" in \"override\""),
+				errNoName,
 			},
 			wantError: nil,
 		},
@@ -353,7 +362,9 @@ func TestValidateManifest(t *testing.T) {
 			  [constraint.metadata]
 			    color = "blue"
 			`,
-			wantWarn:  []error{},
+			wantWarn: []error{
+				errors.New("branch, version, revision, or source should be provided for \"github.com/foo/bar\""),
+			},
 			wantError: nil,
 		},
 		{