Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Add metadata to manifest & warn on unknown fields #528

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {
Expand Down
118 changes: 118 additions & 0 deletions manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package dep

import (
"errors"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -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)
}
}
}
}