From 0db201203909f403b9b6e59c2f02d84a7a646bc6 Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Fri, 11 Mar 2016 11:12:59 +0100 Subject: [PATCH] Refactor assmebling proto files to avoid code duplication --- .../deepcopy-gen/generators/deepcopy.go | 4 +- cmd/libs/go2idl/generator/execute.go | 30 +++++--- cmd/libs/go2idl/generator/generator.go | 2 +- .../go2idl/go-to-protobuf/protobuf/cmd.go | 2 +- .../go-to-protobuf/protobuf/generator.go | 77 +++---------------- .../go2idl/go-to-protobuf/protobuf/namer.go | 3 +- cmd/libs/go2idl/namer/namer.go | 5 ++ 7 files changed, 43 insertions(+), 80 deletions(-) diff --git a/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go b/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go index db8128e290a03..0de799aaace81 100644 --- a/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go +++ b/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go @@ -91,7 +91,7 @@ func Packages(_ *generator.Context, arguments *args.GeneratorArgs) generator.Pac return false } // Also, filter out private types. - if strings.ToLower(t.Name.Name[:1]) == t.Name.Name[:1] { + if namer.IsPrivateGoName(t.Name.Name) { return false } return true @@ -164,7 +164,7 @@ func (g *genDeepCopy) copyableWithinPackage(t *types.Type) bool { // We won't be able to access private fields. // Thus, this type cannot have private fields. for _, member := range t.Members { - if strings.ToLower(member.Name[:1]) == member.Name[:1] { + if namer.IsPrivateGoName(member.Name) { return false } // TODO: This is a temporary hack, to make avoid generating function diff --git a/cmd/libs/go2idl/generator/execute.go b/cmd/libs/go2idl/generator/execute.go index 2e6a868bb1b30..bb9bbb440f2f0 100644 --- a/cmd/libs/go2idl/generator/execute.go +++ b/cmd/libs/go2idl/generator/execute.go @@ -57,9 +57,12 @@ func (c *Context) ExecutePackages(outDir string, packages Packages) error { return nil } -type golangFileType struct{} +type DefaultFileType struct { + Format func([]byte) ([]byte, error) + Assemble func(io.Writer, *File) +} -func (ft golangFileType) AssembleFile(f *File, pathname string) error { +func (ft DefaultFileType) AssembleFile(f *File, pathname string) error { log.Printf("Assembling file %q", pathname) destFile, err := os.Create(pathname) if err != nil { @@ -69,12 +72,12 @@ func (ft golangFileType) AssembleFile(f *File, pathname string) error { b := &bytes.Buffer{} et := NewErrorTracker(b) - ft.assemble(et, f) + ft.Assemble(et, f) if et.Error() != nil { return et.Error() } - if formatted, err := format.Source(b.Bytes()); err != nil { - err = fmt.Errorf("unable to run gofmt on %q (%v).", pathname, err) + if formatted, err := ft.Format(b.Bytes()); err != nil { + err = fmt.Errorf("unable to format file %q (%v).", pathname, err) // Write the file anyway, so they can see what's going wrong and fix the generator. if _, err2 := destFile.Write(b.Bytes()); err2 != nil { return err2 @@ -86,18 +89,18 @@ func (ft golangFileType) AssembleFile(f *File, pathname string) error { } } -func (ft golangFileType) VerifyFile(f *File, pathname string) error { +func (ft DefaultFileType) VerifyFile(f *File, pathname string) error { log.Printf("Verifying file %q", pathname) friendlyName := filepath.Join(f.PackageName, f.Name) b := &bytes.Buffer{} et := NewErrorTracker(b) - ft.assemble(et, f) + ft.Assemble(et, f) if et.Error() != nil { return et.Error() } - formatted, err := format.Source(b.Bytes()) + formatted, err := ft.Format(b.Bytes()) if err != nil { - return fmt.Errorf("unable to gofmt the output for %q: %v", friendlyName, err) + return fmt.Errorf("unable to format the output for %q: %v", friendlyName, err) } existing, err := ioutil.ReadFile(pathname) if err != nil { @@ -121,7 +124,7 @@ func (ft golangFileType) VerifyFile(f *File, pathname string) error { return fmt.Errorf("output for %q differs; first existing/expected diff: \n %q\n %q", friendlyName, string(eDiff), string(fDiff)) } -func (ft golangFileType) assemble(w io.Writer, f *File) { +func assembleGolangFile(w io.Writer, f *File) { w.Write(f.Header) fmt.Fprintf(w, "package %v\n\n", f.PackageName) @@ -155,6 +158,13 @@ func (ft golangFileType) assemble(w io.Writer, f *File) { w.Write(f.Body.Bytes()) } +func NewGolangFile() *DefaultFileType { + return &DefaultFileType{ + Format: format.Source, + Assemble: assembleGolangFile, + } +} + // format should be one line only, and not end with \n. func addIndentHeaderComment(b *bytes.Buffer, format string, args ...interface{}) { if b.Len() > 0 { diff --git a/cmd/libs/go2idl/generator/generator.go b/cmd/libs/go2idl/generator/generator.go index a0b2094295c6e..96c52e7ba985f 100644 --- a/cmd/libs/go2idl/generator/generator.go +++ b/cmd/libs/go2idl/generator/generator.go @@ -177,7 +177,7 @@ func NewContext(b *parser.Builder, nameSystems namer.NameSystems, canonicalOrder Namers: namer.NameSystems{}, Universe: u, FileTypes: map[string]FileType{ - GolangFileType: golangFileType{}, + GolangFileType: NewGolangFile(), }, } diff --git a/cmd/libs/go2idl/go-to-protobuf/protobuf/cmd.go b/cmd/libs/go2idl/go-to-protobuf/protobuf/cmd.go index 20f70fd451316..dee488205661a 100644 --- a/cmd/libs/go2idl/go-to-protobuf/protobuf/cmd.go +++ b/cmd/libs/go2idl/go-to-protobuf/protobuf/cmd.go @@ -170,7 +170,7 @@ func Run(g *Generator) { "public", ) c.Verify = g.Common.VerifyOnly - c.FileTypes["protoidl"] = protoIDLFileType{} + c.FileTypes["protoidl"] = NewProtoFile() if err != nil { log.Fatalf("Failed making a context: %v", err) diff --git a/cmd/libs/go2idl/go-to-protobuf/protobuf/generator.go b/cmd/libs/go2idl/go-to-protobuf/protobuf/generator.go index 561d895393952..61581e6f58e2b 100644 --- a/cmd/libs/go2idl/go-to-protobuf/protobuf/generator.go +++ b/cmd/libs/go2idl/go-to-protobuf/protobuf/generator.go @@ -17,13 +17,9 @@ limitations under the License. package protobuf import ( - "bytes" "fmt" "io" - "io/ioutil" "log" - "os" - "path/filepath" "reflect" "sort" "strconv" @@ -231,7 +227,7 @@ func (b bodyGen) doStruct(sw *generator.SnippetWriter) error { if len(b.t.Name.Name) == 0 { return nil } - if isPrivateGoName(b.t.Name.Name) { + if namer.IsPrivateGoName(b.t.Name.Name) { return nil } @@ -525,7 +521,7 @@ func membersToFields(locator ProtobufLocator, t *types.Type, localPackage types. fields := []protoField{} for _, m := range t.Members { - if isPrivateGoName(m.Name) { + if namer.IsPrivateGoName(m.Name) { // skip private fields continue } @@ -561,7 +557,7 @@ func membersToFields(locator ProtobufLocator, t *types.Type, localPackage types. } } if len(field.Name) == 0 { - field.Name = strings.ToLower(m.Name[:1]) + m.Name[1:] + field.Name = namer.IL(m.Name) } if field.Map && field.Repeated { @@ -573,7 +569,7 @@ func membersToFields(locator ProtobufLocator, t *types.Type, localPackage types. if !field.Nullable { field.Extras["(gogoproto.nullable)"] = "false" } - if (field.Type.Name.Name == "bytes" && field.Type.Name.Package == "") || (field.Repeated && field.Type.Name.Package == "" && isPrivateGoName(field.Type.Name.Name)) { + if (field.Type.Name.Name == "bytes" && field.Type.Name.Package == "") || (field.Repeated && field.Type.Name.Package == "" && namer.IsPrivateGoName(field.Type.Name.Name)) { delete(field.Extras, "(gogoproto.nullable)") } if field.Name != m.Name { @@ -627,61 +623,12 @@ func genComment(out io.Writer, comment, indent string) { } } -type protoIDLFileType struct{} - -func (ft protoIDLFileType) AssembleFile(f *generator.File, pathname string) error { - log.Printf("Assembling IDL file %q", pathname) - destFile, err := os.Create(pathname) - if err != nil { - return err - } - defer destFile.Close() - - b := &bytes.Buffer{} - et := generator.NewErrorTracker(b) - ft.assemble(et, f) - if et.Error() != nil { - return et.Error() - } - - // TODO: is there an IDL formatter? - _, err = destFile.Write(b.Bytes()) - return err +func formatProtoFile(source []byte) ([]byte, error) { + // TODO; Is there any protobuf formatter? + return source, nil } -func (ft protoIDLFileType) VerifyFile(f *generator.File, pathname string) error { - log.Printf("Verifying IDL file %q", pathname) - friendlyName := filepath.Join(f.PackageName, f.Name) - b := &bytes.Buffer{} - et := generator.NewErrorTracker(b) - ft.assemble(et, f) - if et.Error() != nil { - return et.Error() - } - formatted := b.Bytes() - existing, err := ioutil.ReadFile(pathname) - if err != nil { - return fmt.Errorf("unable to read file %q for comparison: %v", friendlyName, err) - } - if bytes.Compare(formatted, existing) == 0 { - return nil - } - // Be nice and find the first place where they differ - i := 0 - for i < len(formatted) && i < len(existing) && formatted[i] == existing[i] { - i++ - } - eDiff, fDiff := existing[i:], formatted[i:] - if len(eDiff) > 100 { - eDiff = eDiff[:100] - } - if len(fDiff) > 100 { - fDiff = fDiff[:100] - } - return fmt.Errorf("output for %q differs; first existing/expected diff: \n %q\n %q", friendlyName, string(eDiff), string(fDiff)) -} - -func (ft protoIDLFileType) assemble(w io.Writer, f *generator.File) { +func assembleProtoFile(w io.Writer, f *generator.File) { w.Write(f.Header) fmt.Fprint(w, "syntax = 'proto2';\n\n") @@ -709,9 +656,9 @@ func (ft protoIDLFileType) assemble(w io.Writer, f *generator.File) { w.Write(f.Body.Bytes()) } -func isPrivateGoName(name string) bool { - if len(name) == 0 { - return true +func NewProtoFile() *generator.DefaultFileType { + return &generator.DefaultFileType{ + Format: formatProtoFile, + Assemble: assembleProtoFile, } - return strings.ToLower(name[:1]) == name[:1] } diff --git a/cmd/libs/go2idl/go-to-protobuf/protobuf/namer.go b/cmd/libs/go2idl/go-to-protobuf/protobuf/namer.go index 82c13288bc90d..7eb1e6181b169 100644 --- a/cmd/libs/go2idl/go-to-protobuf/protobuf/namer.go +++ b/cmd/libs/go2idl/go-to-protobuf/protobuf/namer.go @@ -22,6 +22,7 @@ import ( "strings" "k8s.io/kubernetes/cmd/libs/go2idl/generator" + "k8s.io/kubernetes/cmd/libs/go2idl/namer" "k8s.io/kubernetes/cmd/libs/go2idl/types" ) @@ -124,7 +125,7 @@ func assignGoTypeToProtoPackage(p *protobufPackage, t *types.Type, local, global local[t.Name] = p for _, m := range t.Members { - if isPrivateGoName(m.Name) { + if namer.IsPrivateGoName(m.Name) { continue } field := &protoField{} diff --git a/cmd/libs/go2idl/namer/namer.go b/cmd/libs/go2idl/namer/namer.go index 9b3728089bdc0..de27c1f6d5518 100644 --- a/cmd/libs/go2idl/namer/namer.go +++ b/cmd/libs/go2idl/namer/namer.go @@ -23,6 +23,11 @@ import ( "k8s.io/kubernetes/cmd/libs/go2idl/types" ) +// Returns whether a name is a private Go name. +func IsPrivateGoName(name string) bool { + return len(name) == 0 || strings.ToLower(name[:1]) == name[:1] +} + // NewPublicNamer is a helper function that returns a namer that makes // CamelCase names. See the NameStrategy struct for an explanation of the // arguments to this constructor.