diff --git a/manifest.go b/manifest.go index 621edf1a6a..6bd7e518bf 100644 --- a/manifest.go +++ b/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,6 +41,58 @@ 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) @@ -45,6 +100,15 @@ func readManifest(r io.Reader) (*Manifest, error) { 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 { diff --git a/manifest_test.go b/manifest_test.go index 56594f41e6..f7b69dac49 100644 --- a/manifest_test.go +++ b/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) + } + } + } +}