Merge pull request #528 from darkowlzz/506-add-metadata-warn-unknown-manifest-fields

Add metadata to manifest & warn on unknown fields

Author: sdboyer

manifest.go

@@ -6,10 +6,13 @@ package dep
 
 import (
 	"bytes"
+	"fmt"
 	"io"
+	"reflect"
 	"sort"
 
 	"github.com/golang/dep/gps"
+	"github.com/golang/dep/internal"
 	"github.com/pelletier/go-toml"
 	"github.com/pkg/errors"
 )
@@ -38,13 +41,74 @@ type rawProject struct {
 	Source   string `toml:"source,omitempty"`
 }
 
+func validateManifest(s string) ([]error, error) {
+	var errs []error
+	// Load the TomlTree from string
+	tree, err := toml.Load(s)
+	if err != nil {
+		return errs, errors.Wrap(err, "Unable to load TomlTree from string")
+	}
+	// Convert tree to a map
+	manifest := tree.ToMap()
+
+	// Look for unknown fields and collect errors
+	for prop, val := range manifest {
+		switch prop {
+		case "metadata":
+			// Check if metadata is of Map type
+			if reflect.TypeOf(val).Kind() != reflect.Map {
+				errs = append(errs, errors.New("metadata should be a TOML table"))
+			}
+		case "dependencies", "overrides":
+			// Invalid if type assertion fails. Not a TOML array of tables.
+			if rawProj, ok := val.([]interface{}); ok {
+				// Iterate through each array of tables
+				for _, v := range rawProj {
+					// Check the individual field's key to be valid
+					for key, value := range v.(map[string]interface{}) {
+						// Check if the key is valid
+						switch key {
+						case "name", "branch", "revision", "version", "source":
+							// valid key
+						case "metadata":
+							// Check if metadata is of Map type
+							if reflect.TypeOf(value).Kind() != reflect.Map {
+								errs = append(errs, fmt.Errorf("metadata in %q should be a TOML table", prop))
+							}
+						default:
+							// unknown/invalid key
+							errs = append(errs, fmt.Errorf("Invalid key %q in %q", key, prop))
+						}
+					}
+				}
+			} else {
+				errs = append(errs, fmt.Errorf("%v should be a TOML array of tables", prop))
+			}
+		case "ignored", "required":
+		default:
+			errs = append(errs, fmt.Errorf("Unknown field in manifest: %v", prop))
+		}
+	}
+
+	return errs, nil
+}
+
 func readManifest(r io.Reader) (*Manifest, error) {
 	buf := &bytes.Buffer{}
 	_, err := buf.ReadFrom(r)
 	if err != nil {
 		return nil, errors.Wrap(err, "Unable to read byte stream")
 	}
 
+	// Validate manifest and log warnings
+	errs, err := validateManifest(buf.String())
+	if err != nil {
+		return nil, errors.Wrap(err, "Manifest validation failed")
+	}
+	for _, e := range errs {
+		internal.Logf("WARNING: %v", e)
+	}
+
 	raw := rawManifest{}
 	err = toml.Unmarshal(buf.Bytes(), &raw)
 	if err != nil {

manifest_test.go

@@ -5,6 +5,7 @@
 package dep
 
 import (
+	"errors"
 	"reflect"
 	"strings"
 	"testing"
@@ -119,3 +120,120 @@ func TestReadManifestErrors(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateManifest(t *testing.T) {
+	cases := []struct {
+		tomlString string
+		want       []error
+	}{
+		{
+			tomlString: `
+			[[dependencies]]
+			  name = "github.com/foo/bar"
+			`,
+			want: []error{},
+		},
+		{
+			tomlString: `
+			[metadata]
+			  authors = "foo"
+			  version = "1.0.0"
+			`,
+			want: []error{},
+		},
+		{
+			tomlString: `
+			foo = "some-value"
+			version = 14
+
+			[[bar]]
+			  author = "xyz"
+
+			[[dependencies]]
+			  name = "github.com/foo/bar"
+			  version = ""
+			`,
+			want: []error{
+				errors.New("Unknown field in manifest: foo"),
+				errors.New("Unknown field in manifest: bar"),
+				errors.New("Unknown field in manifest: version"),
+			},
+		},
+		{
+			tomlString: `
+			metadata = "project-name"
+
+			[[dependencies]]
+			  name = "github.com/foo/bar"
+			`,
+			want: []error{errors.New("metadata should be a TOML table")},
+		},
+		{
+			tomlString: `
+			dependencies = "foo"
+			overrides = "bar"
+			`,
+			want: []error{
+				errors.New("dependencies should be a TOML array of tables"),
+				errors.New("overrides should be a TOML array of tables"),
+			},
+		},
+		{
+			tomlString: `
+			[[dependencies]]
+			  name = "github.com/foo/bar"
+			  location = "some-value"
+			  link = "some-other-value"
+			  metadata = "foo"
+
+			[[overrides]]
+			  nick = "foo"
+			`,
+			want: []error{
+				errors.New("Invalid key \"location\" in \"dependencies\""),
+				errors.New("Invalid key \"link\" in \"dependencies\""),
+				errors.New("Invalid key \"nick\" in \"overrides\""),
+				errors.New("metadata in \"dependencies\" should be a TOML table"),
+			},
+		},
+		{
+			tomlString: `
+			[[dependencies]]
+			  name = "github.com/foo/bar"
+
+			  [dependencies.metadata]
+			    color = "blue"
+			`,
+			want: []error{},
+		},
+	}
+
+	// constains for error
+	contains := func(s []error, e error) bool {
+		for _, a := range s {
+			if a.Error() == e.Error() {
+				return true
+			}
+		}
+		return false
+	}
+
+	for _, c := range cases {
+		errs, err := validateManifest(c.tomlString)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// compare length of error slice
+		if len(errs) != len(c.want) {
+			t.Fatalf("Number of manifest errors are not as expected: \n\t(GOT) %v errors(%v)\n\t(WNT) %v errors(%v).", len(errs), errs, len(c.want), c.want)
+		}
+
+		// check if the expected errors exist in actual errors slice
+		for _, er := range errs {
+			if !contains(c.want, er) {
+				t.Fatalf("Manifest errors are not as expected: \n\t(MISSING) %v\n\t(FROM) %v", er, c.want)
+			}
+		}
+	}
+}