Merge pull request #50 from digitalocean/geoff/rename-main-types

Rename the main structure types for clarity

internal/lvgen/generate.go

@@ -83,10 +83,6 @@ var goEquivTypes = map[string]string{
 	// requires us to ditch the typedef that would otherwise be generated.
 	"NonnullString": "string",
 
-	// remote_string gets renamed "String" by the generator, so here we'll
-	// rename that to OptString - these are optional strings, and can be "null".
-	"String": "OptString",
-
 	// TODO: Get rid of these. They're only needed because we lose information
 	// that the parser has (the parser knows it has emitted a go type), and then
 	// we capitalize types to make them public.
@@ -124,6 +120,13 @@ type Decl struct {
 	Name, LVName, Type string
 }
 
+// NewDecl returns a new declaration struct.
+func NewDecl(identifier, itype string) *Decl {
+	goidentifier := identifierTransform(identifier)
+	itype = typeTransform(itype)
+	return &Decl{Name: goidentifier, LVName: identifier, Type: itype}
+}
+
 // Structure records the name and members of a struct definition.
 type Structure struct {
 	Name    string
@@ -287,6 +290,10 @@ func identifierTransform(name string) string {
 	}
 	nn = fixAbbrevs(nn)
 	nn = checkIdentifier(nn)
+	// Many types in libvirt are prefixed with "Nonnull" to distinguish them
+	// from optional values. We add "Opt" to optional values and strip "Nonnull"
+	// because this makes the go code clearer.
+	nn = strings.TrimPrefix(nn, "Nonnull")
 	return nn
 }
 
@@ -525,29 +532,29 @@ func AddCase() {
 // The declaration will be added to any open container (such as a struct, if the
 // parser is working through a struct definition.)
 func AddDeclaration(identifier, itype string) {
-	// fmt.Println("adding", identifier, itype)
-	// If the name is a reserved word, transform it so it isn't.
-	goidentifier := identifierTransform(identifier)
-	itype = typeTransform(itype)
-	decl := Decl{Name: goidentifier, LVName: identifier, Type: itype}
+	addDecl(NewDecl(identifier, itype))
+}
+
+// addDecl adds a declaration to the current container.
+func addDecl(decl *Decl) {
 	if !CurrentStruct.empty() {
 		st := CurrentStruct.peek()
-		st.Members = append(st.Members, decl)
+		st.Members = append(st.Members, *decl)
 	} else if CurrentTypedef != nil {
-		CurrentTypedef.Name = goidentifier
-		CurrentTypedef.LVName = identifier
-		CurrentTypedef.Type = itype
-		if goidentifier != "string" {
+		CurrentTypedef.Name = decl.Name
+		CurrentTypedef.LVName = decl.LVName
+		CurrentTypedef.Type = decl.Type
+		if CurrentTypedef.Name != "string" {
 			// Omit recursive typedefs. These happen because we're massaging
 			// some of the type names.
 			Gen.Typedefs = append(Gen.Typedefs, *CurrentTypedef)
 		}
 		CurrentTypedef = nil
 	} else if CurrentCase != nil {
-		CurrentCase.Name = goidentifier
-		CurrentCase.Type = itype
+		CurrentCase.Name = decl.Name
+		CurrentCase.Type = decl.Type
 	} else if CurrentUnion != nil {
-		CurrentUnion.DiscriminantType = itype
+		CurrentUnion.DiscriminantType = decl.Type
 	}
 }
 
@@ -556,7 +563,7 @@ func AddDeclaration(identifier, itype string) {
 // prefixed.
 func AddFixedArray(identifier, itype, len string) {
 	atype := fmt.Sprintf("[%v]%v", len, itype)
-	AddDeclaration(identifier, atype)
+	addDecl(NewDecl(identifier, atype))
 }
 
 // AddVariableArray is called by the parser to add a variable-length array.
@@ -574,9 +581,26 @@ func AddVariableArray(identifier, itype, len string) {
 	if itype == "string" {
 		atype = itype
 	}
-	AddDeclaration(identifier, atype)
+	addDecl(NewDecl(identifier, atype))
 }
 
+// AddOptValue is called by the parser to add an optional value. These are
+// declared in the protocol definition file using a syntax that looks like a
+// pointer declaration, but are actually represented by a variable-sized array
+// with a maximum size of 1.
+func AddOptValue(identifier, itype string) {
+	atype := "[]" + itype
+	decl := NewDecl(identifier, atype)
+	newType := "Opt" + decl.Name
+	fmt.Printf("Adding mapping %v = %v\n", decl.Name, newType)
+	goEquivTypes[decl.Name] = newType
+	decl.Name = newType
+	addDecl(decl)
+}
+
+// checkIdentifier determines whether an identifier is in our translation list.
+// If so it returns the translated name. This is used to massage the type names
+// we're emitting.
 func checkIdentifier(i string) string {
 	nn, reserved := goEquivTypes[i]
 	if reserved {

internal/lvgen/sunrpc.y

@@ -188,7 +188,7 @@ variable_array_declaration
 // representation to use for these is a variable-length array with a size of 1.
 // See the XDR spec for a more complete explanation of this.
 pointer_declaration
-    : type_specifier '*' variable_ident             { AddVariableArray($3.val, $1.val, "1") }
+    : type_specifier '*' variable_ident             { AddOptValue($3.val, $1.val) }
     ;
 
 struct_definition

internal/lvgen/y.go

@@ -713,7 +713,7 @@ yydefault:
 		yyDollar = yyS[yypt-3 : yypt+1]
 		//line sunrpc.y:191
 		{
-			AddVariableArray(yyDollar[3].val, yyDollar[1].val, "1")
+			AddOptValue(yyDollar[3].val, yyDollar[1].val)
 		}
 	case 49:
 		yyDollar = yyS[yypt-3 : yypt+1]

libvirt.go

@@ -55,7 +55,7 @@ type Libvirt struct {
 // DomainEvent represents a libvirt domain event.
 type DomainEvent struct {
 	CallbackID   uint32
-	Domain       NonnullDomain
+	Domain       Domain
 	Event        string
 	Seconds      uint64
 	Microseconds uint32
@@ -395,7 +395,7 @@ func (l *Libvirt) Disconnect() error {
 }
 
 // Domains returns a list of all domains managed by libvirt.
-func (l *Libvirt) Domains() ([]NonnullDomain, error) {
+func (l *Libvirt) Domains() ([]Domain, error) {
 	// these are the flags as passed by `virsh`, defined in:
 	// src/remote/remote_protocol.x # remote_connect_list_all_domains_args
 	domains, _, err := l.ConnectListAllDomains(1, 3)
@@ -425,7 +425,7 @@ func (l *Libvirt) Events(dom string) (<-chan DomainEvent, error) {
 
 	payload := struct {
 		Padding [4]byte
-		Domain  NonnullDomain
+		Domain  Domain
 		Event   [2]byte
 		Flags   [2]byte
 	}{
@@ -524,7 +524,7 @@ func (l *Libvirt) Run(dom string, cmd []byte) ([]byte, error) {
 	}
 
 	payload := struct {
-		Domain  NonnullDomain
+		Domain  Domain
 		Command []byte
 		Flags   uint32
 	}{
@@ -567,20 +567,20 @@ func (l *Libvirt) Run(dom string, cmd []byte) ([]byte, error) {
 }
 
 // Secrets returns all secrets managed by the libvirt daemon.
-func (l *Libvirt) Secrets() ([]NonnullSecret, error) {
+func (l *Libvirt) Secrets() ([]Secret, error) {
 	secrets, _, err := l.ConnectListAllSecrets(1, 0)
 	return secrets, err
 }
 
 // StoragePool returns the storage pool associated with the provided name.
 // An error is returned if the requested storage pool is not found.
-func (l *Libvirt) StoragePool(name string) (NonnullStoragePool, error) {
+func (l *Libvirt) StoragePool(name string) (StoragePool, error) {
 	return l.StoragePoolLookupByName(name)
 }
 
 // StoragePools returns a list of defined storage pools. Pools are filtered by
 // the provided flags. See StoragePools*.
-func (l *Libvirt) StoragePools(flags StoragePoolsFlags) ([]NonnullStoragePool, error) {
+func (l *Libvirt) StoragePools(flags StoragePoolsFlags) ([]StoragePool, error) {
 	pools, _, err := l.ConnectListAllStoragePools(1, uint32(flags))
 	return pools, err
 }
@@ -775,7 +775,7 @@ func (l *Libvirt) GetBlockIOTune(dom string, disk string) ([]BlockLimit, error)
 }
 
 // lookup returns a domain as seen by libvirt.
-func (l *Libvirt) lookup(name string) (NonnullDomain, error) {
+func (l *Libvirt) lookup(name string) (Domain, error) {
 	return l.DomainLookupByName(name)
 }
 

rpc_test.go

@@ -106,7 +106,7 @@ var (
 		0x00, 0x00, 0x00, 0x02,
 	}
 
-	testDomain = &NonnullDomain{
+	testDomain = &Domain{
 		Name: "test-domain",
 		UUID: testUUID,
 		ID:   1,