Skip to content

Commit

Permalink
Checker: more robust WKT wrapper detection (#21)
Browse files Browse the repository at this point in the history
  • Loading branch information
rodaine authored Nov 27, 2017
1 parent 605b154 commit 500ccdb
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 9 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ bazel-*
/tests/harness/cases/go/
/tests/harness/harness.pb.go
/tests/harness/go/go-harness
/tests/harness/cc/cc-harness

/tests/kitchensink/go/
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ lint:
# lints the package for common code smells
which golint || go get -u github.com/golang/lint/golint
test -z "$(gofmt -d -s ./*.go)" || (gofmt -d -s ./*.go && exit 1)
golint -set_exit_status
# golint -set_exit_status
go tool vet -all -shadow -shadowstrict *.go

.PHONY: quick
Expand Down Expand Up @@ -96,4 +96,4 @@ tests/harness/cc/cc-harness: tests/harness/cc/harness.cc
cp bazel-bin/tests/harness/cc/cc-harness $@

.PHONY: ci
ci: build tests kitchensink testcases harness bazel-harness
ci: lint build tests kitchensink testcases harness bazel-harness
39 changes: 36 additions & 3 deletions checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (m Module) CheckRules(msg pgs.Message) {
m.Push(f.Name().String())

var rules validate.FieldRules
_, err := f.Extension(validate.E_Rules, &rules)
_, err = f.Extension(validate.E_Rules, &rules)
m.CheckErr(err, "unable to read validation rules from field")

m.CheckFieldRules(f.Type(), &rules)
Expand Down Expand Up @@ -121,7 +121,7 @@ func (m Module) CheckFieldRules(typ FieldType, rules *validate.FieldRules) {
}

func (m Module) MustType(typ FieldType, pt pgs.ProtoType, wrapper bool) {
if emb := typ.Embed(); wrapper && emb != nil && len(emb.Fields()) == 1 {
if emb := typ.Embed(); wrapper && m.isWKTWrapper(emb) {
m.MustType(emb.Fields()[0].Type(), pt, false)
return
}
Expand Down Expand Up @@ -245,7 +245,7 @@ func (m Module) CheckEnum(ft FieldType, r *validate.EnumRules) {
}

for _, in := range r.In {
if _, ok := vals[in]; !ok {
if _, ok = vals[in]; !ok {
m.Failf("undefined `in` value (%d) conflicts with `defined_only` rule")
}
}
Expand Down Expand Up @@ -447,3 +447,36 @@ func (m Module) checkTS(ts *timestamp.Timestamp) *int64 {
m.CheckErr(err, "could not resolve timestamp")
return proto.Int64(t.UnixNano())
}

func (m Module) isWKTWrapper(emb pgs.Message) bool {
// not an embedded message
if emb == nil {
return false
}

// must be in the correct package
if emb.Package().ProtoName().String() != wktPackage {
return false
}

// lookup message name
if _, ok := wktWrappers[emb.TypeName().String()]; !ok {
return false
}

return true
}

const wktPackage = "google.protobuf"

var wktWrappers = map[string]struct{}{
"DoubleValue": {},
"FloatValue": {},
"Int64Value": {},
"UInt64Value": {},
"Int32Value": {},
"UInt32Value": {},
"BoolValue": {},
"StringValue": {},
"BytesValue": {},
}
4 changes: 2 additions & 2 deletions module.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (m Module) Execute(target pgs.Package, packages map[string]pgs.Package) []p
m.Assert(lang != "", "`lang` parameter must be set")
tpl := templates.Template()[lang]
m.Assert(tpl != nil, "could not find template for `lang`: ", lang)
ext := map[string]string {
ext := map[string]string{
"go": "go",
"cc": "h",
}[lang]
Expand All @@ -36,7 +36,7 @@ func (m Module) Execute(target pgs.Package, packages map[string]pgs.Package) []p
}

m.AddGeneratorTemplateFile(
f.OutputPath().SetExt(".validate." + ext).String(),
f.OutputPath().SetExt(".validate."+ext).String(),
tpl,
f,
)
Expand Down
4 changes: 2 additions & 2 deletions validate/validate.proto
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import "google/protobuf/timestamp.proto";

// Validation rules applied at the message level
extend google.protobuf.MessageOptions {
// Disabled nullifies any validation rules for this message, including any
// Disabled nullifies any validation rules for this message, including any
// message fields associated with it that do support validation.
optional bool disabled = 919191;
}
Expand Down Expand Up @@ -527,7 +527,7 @@ message StringRules {
// by RFC 3986
bool uri = 17;

// UriRef specifies that the field must be a valid URI as defined by RFC
// UriRef specifies that the field must be a valid URI as defined by RFC
// 3986 and may be relative or absolute.
bool uri_ref = 18;
}
Expand Down

0 comments on commit 500ccdb

Please sign in to comment.