From a64cee5aaba13277c4b42240f57be58eed7dff44 Mon Sep 17 00:00:00 2001 From: Chris Masone Date: Mon, 2 Nov 2015 12:17:42 -0800 Subject: [PATCH] Clean up codegen -out-dir flag handling Get rid of -out and ensure that Go package names generated from -out-dir are actually legal Go identifiers. Fixes #476 --- nomdl/codegen/codegen.go | 50 +++--- nomdl/codegen/codegen_test.go | 11 ++ .../test/gen/struct_with_dup_list.noms.go | 142 ++++++++++++++++++ .../codegen/test/gen/struct_with_list.noms.go | 142 ------------------ nomdl/codegen/test/rungen.go | 2 +- 5 files changed, 184 insertions(+), 163 deletions(-) diff --git a/nomdl/codegen/codegen.go b/nomdl/codegen/codegen.go index 288ee97aec..7c7998d9f4 100644 --- a/nomdl/codegen/codegen.go +++ b/nomdl/codegen/codegen.go @@ -11,9 +11,12 @@ import ( "os" "path" "path/filepath" + "regexp" "runtime" "strings" "text/template" + "unicode" + "unicode/utf8" "github.com/attic-labs/noms/Godeps/_workspace/src/golang.org/x/tools/imports" "github.com/attic-labs/noms/chunks" @@ -27,11 +30,13 @@ import ( ) var ( - outDirFlag = flag.String("out-dir", "", "Directory where generated code will be written") + outDirFlag = flag.String("out-dir", ".", "Directory where generated code will be written") inFlag = flag.String("in", "", "The name of the noms file to read") - outFlag = flag.String("out", "", "The name of the go file to write") pkgDSFlag = flag.String("package-ds", "", "The dataset to read/write packages from/to.") packageFlag = flag.String("package", "", "The name of the go package to write") + + idRegexp = regexp.MustCompile(`[_\pL][_\pL\pN]*`) + illegalRune = regexp.MustCompile(`[^_\pL\pN]`) ) const ext = ".noms" @@ -67,23 +72,21 @@ func main() { } localPkgs := refSet{} + outDir, err := filepath.Abs(*outDirFlag) + d.Chk.NoError(err, "Could not canonicalize -out-dir: %v", err) + packageName := getGoPackageName(outDir) + if *inFlag != "" { - out := *outFlag - if out == "" { - out = getOutFileName(*inFlag) - } - p := parsePackageFile(getGoPackageName(), *inFlag, pkgDS) + out := getOutFileName(*inFlag) + p := parsePackageFile(packageName, *inFlag, pkgDS) localPkgs[p.Ref()] = true - generate(getGoPackageName(), *inFlag, out, filepath.Dir(out), map[string]bool{}, p, localPkgs, pkgDS) + generate(packageName, *inFlag, filepath.Join(outDir, out), outDir, map[string]bool{}, p, localPkgs, pkgDS) return } // Generate code from all .noms file in the current directory nomsFiles, err := filepath.Glob("*" + ext) - outDir, err := filepath.Abs(*outDirFlag) - d.Chk.NoError(err, "Could not canonicalize -out-dir: %v", err) - packageName := getGoPackageName() written := map[string]bool{} packages := map[string]pkg.Parsed{} for _, inFile := range nomsFiles { @@ -179,19 +182,21 @@ func getBareFileName(in string) string { return base[:len(base)-len(filepath.Ext(base))] } -func getGoPackageName() string { +func getGoPackageName(outDir string) string { if *packageFlag != "" { + d.Exp.True(idRegexp.MatchString(*packageFlag), "%s is not a legal Go identifier.", *packageFlag) return *packageFlag } - if *outDirFlag != "" { - return filepath.Base(*outDirFlag) - } // It is illegal to have multiple go files in the same directory with different package names. // We can therefore just pick the first one and get the package name from there. - goFiles, err := filepath.Glob("*.go") + goFiles, err := filepath.Glob(filepath.Join(outDir, "*.go")) d.Chk.NoError(err) - d.Chk.True(len(goFiles) > 0) + if len(goFiles) == 0 { + d.Exp.NotEmpty(outDir, "Cannot convert empty path into a Go package name.") + return makeGoIdentifier(filepath.Base(outDir)) + } + d.Chk.True(len(goFiles) > 0, "No Go files in current directory; cannot infer pacakge name.") fset := token.NewFileSet() f, err := parser.ParseFile(fset, goFiles[0], nil, parser.PackageClauseOnly) @@ -199,6 +204,14 @@ func getGoPackageName() string { return f.Name.String() } +func makeGoIdentifier(in string) string { + d.Chk.NotEmpty(in, "Cannot convert empty string to legal Go identifier.") + if r, _ := utf8.DecodeRuneInString(in); unicode.IsNumber(r) { + in = "_" + in + } + return illegalRune.ReplaceAllLiteralString(in, "_") +} + type codeGen struct { w io.Writer pkg pkg.Parsed @@ -210,9 +223,6 @@ type codeGen struct { sharedData sharedData } -type resolver struct { -} - func newCodeGen(w io.Writer, fileID string, written map[string]bool, deps depsMap, pkg pkg.Parsed) *codeGen { typesPackage := "types." if pkg.Name == "types" { diff --git a/nomdl/codegen/codegen_test.go b/nomdl/codegen/codegen_test.go index 6d76a9eee7..0b94daef0d 100644 --- a/nomdl/codegen/codegen_test.go +++ b/nomdl/codegen/codegen_test.go @@ -233,3 +233,14 @@ func TestCommitNewPackages(t *testing.T) { tr := s.Any().TargetValue(ds).Types()[0] assert.EqualValues(types.StructKind, tr.Kind()) } + +func TestMakeGoIdentifier(t *testing.T) { + assert := assert.New(t) + assert.Equal("hello", makeGoIdentifier("hello")) + assert.Equal("hello88", makeGoIdentifier("hello88")) + assert.Equal("_88hello", makeGoIdentifier("88hello")) + assert.Equal("h_e_l_l_0", makeGoIdentifier("h-e-l-l-0")) + assert.Equal("_hello", makeGoIdentifier("\u2318hello")) + assert.Equal("he_llo", makeGoIdentifier("he\u2318llo")) + +} diff --git a/nomdl/codegen/test/gen/struct_with_dup_list.noms.go b/nomdl/codegen/test/gen/struct_with_dup_list.noms.go index 06c4c727a1..f2658d842b 100644 --- a/nomdl/codegen/test/gen/struct_with_dup_list.noms.go +++ b/nomdl/codegen/test/gen/struct_with_dup_list.noms.go @@ -100,3 +100,145 @@ func (s StructWithDupList) L() ListOfUInt8 { func (s StructWithDupList) SetL(val ListOfUInt8) StructWithDupList { return StructWithDupList{s.m.Set(types.NewString("l"), val), &ref.Ref{}} } + +// ListOfUInt8 + +type ListOfUInt8 struct { + l types.List + ref *ref.Ref +} + +func NewListOfUInt8() ListOfUInt8 { + return ListOfUInt8{types.NewList(), &ref.Ref{}} +} + +type ListOfUInt8Def []uint8 + +func (def ListOfUInt8Def) New() ListOfUInt8 { + l := make([]types.Value, len(def)) + for i, d := range def { + l[i] = types.UInt8(d) + } + return ListOfUInt8{types.NewList(l...), &ref.Ref{}} +} + +func (l ListOfUInt8) Def() ListOfUInt8Def { + d := make([]uint8, l.Len()) + for i := uint64(0); i < l.Len(); i++ { + d[i] = uint8(l.l.Get(i).(types.UInt8)) + } + return d +} + +func ListOfUInt8FromVal(val types.Value) ListOfUInt8 { + // TODO: Do we still need FromVal? + if val, ok := val.(ListOfUInt8); ok { + return val + } + // TODO: Validate here + return ListOfUInt8{val.(types.List), &ref.Ref{}} +} + +func (l ListOfUInt8) InternalImplementation() types.List { + return l.l +} + +func (l ListOfUInt8) Equals(other types.Value) bool { + return other != nil && __typeRefForListOfUInt8.Equals(other.TypeRef()) && l.Ref() == other.Ref() +} + +func (l ListOfUInt8) Ref() ref.Ref { + return types.EnsureRef(l.ref, l) +} + +func (l ListOfUInt8) Chunks() (chunks []ref.Ref) { + chunks = append(chunks, l.TypeRef().Chunks()...) + chunks = append(chunks, l.l.Chunks()...) + return +} + +// A Noms Value that describes ListOfUInt8. +var __typeRefForListOfUInt8 types.TypeRef + +func (m ListOfUInt8) TypeRef() types.TypeRef { + return __typeRefForListOfUInt8 +} + +func init() { + __typeRefForListOfUInt8 = types.MakeCompoundTypeRef(types.ListKind, types.MakePrimitiveTypeRef(types.UInt8Kind)) + types.RegisterFromValFunction(__typeRefForListOfUInt8, func(v types.Value) types.Value { + return ListOfUInt8FromVal(v) + }) +} + +func (l ListOfUInt8) Len() uint64 { + return l.l.Len() +} + +func (l ListOfUInt8) Empty() bool { + return l.Len() == uint64(0) +} + +func (l ListOfUInt8) Get(i uint64) uint8 { + return uint8(l.l.Get(i).(types.UInt8)) +} + +func (l ListOfUInt8) Slice(idx uint64, end uint64) ListOfUInt8 { + return ListOfUInt8{l.l.Slice(idx, end), &ref.Ref{}} +} + +func (l ListOfUInt8) Set(i uint64, val uint8) ListOfUInt8 { + return ListOfUInt8{l.l.Set(i, types.UInt8(val)), &ref.Ref{}} +} + +func (l ListOfUInt8) Append(v ...uint8) ListOfUInt8 { + return ListOfUInt8{l.l.Append(l.fromElemSlice(v)...), &ref.Ref{}} +} + +func (l ListOfUInt8) Insert(idx uint64, v ...uint8) ListOfUInt8 { + return ListOfUInt8{l.l.Insert(idx, l.fromElemSlice(v)...), &ref.Ref{}} +} + +func (l ListOfUInt8) Remove(idx uint64, end uint64) ListOfUInt8 { + return ListOfUInt8{l.l.Remove(idx, end), &ref.Ref{}} +} + +func (l ListOfUInt8) RemoveAt(idx uint64) ListOfUInt8 { + return ListOfUInt8{(l.l.RemoveAt(idx)), &ref.Ref{}} +} + +func (l ListOfUInt8) fromElemSlice(p []uint8) []types.Value { + r := make([]types.Value, len(p)) + for i, v := range p { + r[i] = types.UInt8(v) + } + return r +} + +type ListOfUInt8IterCallback func(v uint8, i uint64) (stop bool) + +func (l ListOfUInt8) Iter(cb ListOfUInt8IterCallback) { + l.l.Iter(func(v types.Value, i uint64) bool { + return cb(uint8(v.(types.UInt8)), i) + }) +} + +type ListOfUInt8IterAllCallback func(v uint8, i uint64) + +func (l ListOfUInt8) IterAll(cb ListOfUInt8IterAllCallback) { + l.l.IterAll(func(v types.Value, i uint64) { + cb(uint8(v.(types.UInt8)), i) + }) +} + +type ListOfUInt8FilterCallback func(v uint8, i uint64) (keep bool) + +func (l ListOfUInt8) Filter(cb ListOfUInt8FilterCallback) ListOfUInt8 { + nl := NewListOfUInt8() + l.IterAll(func(v uint8, i uint64) { + if cb(v, i) { + nl = nl.Append(v) + } + }) + return nl +} diff --git a/nomdl/codegen/test/gen/struct_with_list.noms.go b/nomdl/codegen/test/gen/struct_with_list.noms.go index 7e25c70a53..8cdfe4e223 100644 --- a/nomdl/codegen/test/gen/struct_with_list.noms.go +++ b/nomdl/codegen/test/gen/struct_with_list.noms.go @@ -139,145 +139,3 @@ func (s StructWithList) I() int64 { func (s StructWithList) SetI(val int64) StructWithList { return StructWithList{s.m.Set(types.NewString("i"), types.Int64(val)), &ref.Ref{}} } - -// ListOfUInt8 - -type ListOfUInt8 struct { - l types.List - ref *ref.Ref -} - -func NewListOfUInt8() ListOfUInt8 { - return ListOfUInt8{types.NewList(), &ref.Ref{}} -} - -type ListOfUInt8Def []uint8 - -func (def ListOfUInt8Def) New() ListOfUInt8 { - l := make([]types.Value, len(def)) - for i, d := range def { - l[i] = types.UInt8(d) - } - return ListOfUInt8{types.NewList(l...), &ref.Ref{}} -} - -func (l ListOfUInt8) Def() ListOfUInt8Def { - d := make([]uint8, l.Len()) - for i := uint64(0); i < l.Len(); i++ { - d[i] = uint8(l.l.Get(i).(types.UInt8)) - } - return d -} - -func ListOfUInt8FromVal(val types.Value) ListOfUInt8 { - // TODO: Do we still need FromVal? - if val, ok := val.(ListOfUInt8); ok { - return val - } - // TODO: Validate here - return ListOfUInt8{val.(types.List), &ref.Ref{}} -} - -func (l ListOfUInt8) InternalImplementation() types.List { - return l.l -} - -func (l ListOfUInt8) Equals(other types.Value) bool { - return other != nil && __typeRefForListOfUInt8.Equals(other.TypeRef()) && l.Ref() == other.Ref() -} - -func (l ListOfUInt8) Ref() ref.Ref { - return types.EnsureRef(l.ref, l) -} - -func (l ListOfUInt8) Chunks() (chunks []ref.Ref) { - chunks = append(chunks, l.TypeRef().Chunks()...) - chunks = append(chunks, l.l.Chunks()...) - return -} - -// A Noms Value that describes ListOfUInt8. -var __typeRefForListOfUInt8 types.TypeRef - -func (m ListOfUInt8) TypeRef() types.TypeRef { - return __typeRefForListOfUInt8 -} - -func init() { - __typeRefForListOfUInt8 = types.MakeCompoundTypeRef(types.ListKind, types.MakePrimitiveTypeRef(types.UInt8Kind)) - types.RegisterFromValFunction(__typeRefForListOfUInt8, func(v types.Value) types.Value { - return ListOfUInt8FromVal(v) - }) -} - -func (l ListOfUInt8) Len() uint64 { - return l.l.Len() -} - -func (l ListOfUInt8) Empty() bool { - return l.Len() == uint64(0) -} - -func (l ListOfUInt8) Get(i uint64) uint8 { - return uint8(l.l.Get(i).(types.UInt8)) -} - -func (l ListOfUInt8) Slice(idx uint64, end uint64) ListOfUInt8 { - return ListOfUInt8{l.l.Slice(idx, end), &ref.Ref{}} -} - -func (l ListOfUInt8) Set(i uint64, val uint8) ListOfUInt8 { - return ListOfUInt8{l.l.Set(i, types.UInt8(val)), &ref.Ref{}} -} - -func (l ListOfUInt8) Append(v ...uint8) ListOfUInt8 { - return ListOfUInt8{l.l.Append(l.fromElemSlice(v)...), &ref.Ref{}} -} - -func (l ListOfUInt8) Insert(idx uint64, v ...uint8) ListOfUInt8 { - return ListOfUInt8{l.l.Insert(idx, l.fromElemSlice(v)...), &ref.Ref{}} -} - -func (l ListOfUInt8) Remove(idx uint64, end uint64) ListOfUInt8 { - return ListOfUInt8{l.l.Remove(idx, end), &ref.Ref{}} -} - -func (l ListOfUInt8) RemoveAt(idx uint64) ListOfUInt8 { - return ListOfUInt8{(l.l.RemoveAt(idx)), &ref.Ref{}} -} - -func (l ListOfUInt8) fromElemSlice(p []uint8) []types.Value { - r := make([]types.Value, len(p)) - for i, v := range p { - r[i] = types.UInt8(v) - } - return r -} - -type ListOfUInt8IterCallback func(v uint8, i uint64) (stop bool) - -func (l ListOfUInt8) Iter(cb ListOfUInt8IterCallback) { - l.l.Iter(func(v types.Value, i uint64) bool { - return cb(uint8(v.(types.UInt8)), i) - }) -} - -type ListOfUInt8IterAllCallback func(v uint8, i uint64) - -func (l ListOfUInt8) IterAll(cb ListOfUInt8IterAllCallback) { - l.l.IterAll(func(v types.Value, i uint64) { - cb(uint8(v.(types.UInt8)), i) - }) -} - -type ListOfUInt8FilterCallback func(v uint8, i uint64) (keep bool) - -func (l ListOfUInt8) Filter(cb ListOfUInt8FilterCallback) ListOfUInt8 { - nl := NewListOfUInt8() - l.IterAll(func(v uint8, i uint64) { - if cb(v, i) { - nl = nl.Append(v) - } - }) - return nl -} diff --git a/nomdl/codegen/test/rungen.go b/nomdl/codegen/test/rungen.go index d4616dc622..80a22d5849 100644 --- a/nomdl/codegen/test/rungen.go +++ b/nomdl/codegen/test/rungen.go @@ -2,7 +2,7 @@ package test //go:generate rm -rf /tmp/depGenTest -//go:generate go run ../codegen.go -ldb=/tmp/depGenTest -package-ds=testDeps -in=../testDeps/leafDep/leafDep.noms -out=../testDeps/leafDep/leafDep.go +//go:generate go run ../codegen.go -ldb=/tmp/depGenTest -package-ds=testDeps -in=../testDeps/leafDep/leafDep.noms -out-dir=../testDeps/leafDep //go:generate go run ../codegen.go -out-dir=gen -ldb=/tmp/depGenTest -package-ds=testDeps