Merge pull request #30 from 200sc/hotfix/ascii-opcode-endianness

Hotfix/ascii opcode endianness

bebop.go

@@ -2,7 +2,7 @@
 package bebop
 
 // Version is the library version. Should be used by CLI tools when passed a '--version' flag.
-const Version = "v0.3.1"
+const Version = "v0.3.2"
 
 // A File is a structured representation of a .bop file.
 type File struct {
@@ -40,7 +40,7 @@ type Struct struct {
 	Fields  []Field
 	// If OpCode is defined, wire encodings of the struct can be
 	// preceded by the OpCode.
-	OpCode int32
+	OpCode uint32
 	// Namespace is only provided for imported types, and only
 	// used in code generation.
 	Namespace string
@@ -67,7 +67,7 @@ type Message struct {
 	Name    string
 	Comment string
 	Fields  map[uint8]Field
-	OpCode  int32
+	OpCode  uint32
 	// Namespace is only provided for imported types, and only
 	// used in code generation.
 	Namespace string
@@ -78,7 +78,7 @@ type Union struct {
 	Name    string
 	Comment string
 	Fields  map[uint8]UnionField
-	OpCode  int32
+	OpCode  uint32
 	// Namespace is only provided for imported types, and only
 	// used in code generation.
 	Namespace string

equality_test.go

@@ -102,6 +102,9 @@ func (m Message) equals(m2 Message) error {
 	if m.Comment != m2.Comment {
 		return fmt.Errorf("comment mismatch: %q vs %q", m.Comment, m2.Comment)
 	}
+	if m.OpCode != m2.OpCode {
+		return fmt.Errorf("opcode mismatch: %v vs %v", m.OpCode, m2.OpCode)
+	}
 	if len(m.Fields) != len(m2.Fields) {
 		return fmt.Errorf("field count mismatch: %v vs %v", len(m.Fields), len(m2.Fields))
 	}

gen.go

@@ -69,6 +69,7 @@ func (gs GenerateSettings) Validate() error {
 
 // Validate verifies a File can be successfully generated.
 func (f File) Validate() error {
+	allOpCodes := map[uint32]string{}
 	allConsts := map[string]struct{}{}
 	for _, c := range f.Consts {
 		if _, ok := allConsts[c.Name]; ok {
@@ -124,6 +125,12 @@ func (f File) Validate() error {
 			}
 			stNames[fd.Name] = struct{}{}
 		}
+		if st.OpCode != 0 {
+			if conflict, ok := allOpCodes[st.OpCode]; ok {
+				return fmt.Errorf("struct %s has duplicate opcode %02x (duplicated in %s)", st.Name, st.OpCode, conflict)
+			}
+			allOpCodes[st.OpCode] = st.Name
+		}
 	}
 	for _, msg := range f.Messages {
 		if _, ok := primitiveTypes[msg.Name]; ok {
@@ -145,6 +152,12 @@ func (f File) Validate() error {
 
 			msgNames[fd.Name] = struct{}{}
 		}
+		if msg.OpCode != 0 {
+			if conflict, ok := allOpCodes[msg.OpCode]; ok {
+				return fmt.Errorf("message %s has duplicate opcode %02x (duplicated in %s)", msg.Name, msg.OpCode, conflict)
+			}
+			allOpCodes[msg.OpCode] = msg.Name
+		}
 	}
 	for _, un := range f.Unions {
 		if _, ok := primitiveTypes[un.Name]; ok {
@@ -165,6 +178,12 @@ func (f File) Validate() error {
 			}
 			unionNames[fd.name()] = struct{}{}
 		}
+		if un.OpCode != 0 {
+			if conflict, ok := allOpCodes[un.OpCode]; ok {
+				return fmt.Errorf("union %s has duplicate opcode %02x (duplicated in %s)", un.Name, un.OpCode, conflict)
+			}
+			allOpCodes[un.OpCode] = un.Name
+		}
 	}
 	allTypes := customTypes
 	for typ := range primitiveTypes {
@@ -832,7 +851,7 @@ func writeCloseBlock(w io.Writer) {
 	writeLine(w, "")
 }
 
-func writeOpCode(w io.Writer, name string, opCode int32, settings GenerateSettings) {
+func writeOpCode(w io.Writer, name string, opCode uint32, settings GenerateSettings) {
 	if opCode != 0 {
 		writeLine(w, "const %sOpCode = 0x%x", exposeName(name, settings), opCode)
 		writeLine(w, "")
@@ -848,7 +867,7 @@ func writeGoStructDef(w io.Writer, name string, settings GenerateSettings) {
 	writeLine(w, "type %s struct {", exposeName(name, settings))
 }
 
-func writeRecordTypeDefinition(w io.Writer, name string, opCode int32, comment string, settings GenerateSettings, fields []fieldWithNumber) {
+func writeRecordTypeDefinition(w io.Writer, name string, opCode uint32, comment string, settings GenerateSettings, fields []fieldWithNumber) {
 	writeOpCode(w, name, opCode, settings)
 	writeRecordAssertion(w, name, settings)
 	writeComment(w, 0, comment, settings)

gen_test.go

@@ -96,6 +96,18 @@ func TestValidateError(t *testing.T) {
 	}, {
 		file: "invalid_union_primitive_name",
 		err:  "union shares primitive type name uint8",
+	}, {
+		file: "invalid_op_code_10",
+		err:  "struct InvalidOpCode10B has duplicate opcode 34333231 (duplicated in InvalidOpCode10A)",
+	}, {
+		file: "invalid_op_code_11",
+		err:  "struct InvalidOpCode11B has duplicate opcode 34333231 (duplicated in InvalidOpCode11A)",
+	}, {
+		file: "invalid_op_code_12",
+		err:  "message InvalidOpCode12B has duplicate opcode 37363534 (duplicated in InvalidOpCode12A)",
+	}, {
+		file: "invalid_op_code_13",
+		err:  "union InvalidOpCode13B has duplicate opcode 30313938 (duplicated in InvalidOpCode13A)",
 	}}
 	for _, tc := range tcs {
 		tc := tc

parse.go

@@ -22,7 +22,7 @@ func ReadFile(r io.Reader) (File, []string, error) {
 	}
 	tr := newTokenReader(r)
 	nextCommentLines := []string{}
-	nextRecordOpCode := int32(0)
+	nextRecordOpCode := uint32(0)
 	nextRecordReadOnly := false
 	nextRecordBitFlags := false
 	warnings := []string{}
@@ -733,28 +733,28 @@ func readConst(tr *tokenReader) (Const, []string, error) {
 	return cons, warnings, nil
 }
 
-func readOpCode(tr *tokenReader) (int32, error) {
+func readOpCode(tr *tokenReader) (uint32, error) {
 	if _, err := expectNext(tr, tokenKindOpCode, tokenKindOpenParen); err != nil {
 		return 0, err
 	}
 	if err := expectAnyOfNext(tr, tokenKindIntegerLiteral, tokenKindStringLiteral); err != nil {
 		return 0, err
 	}
-	var opCode int32
+	var opCode uint32
 	tk := tr.Token()
 	if tk.kind == tokenKindIntegerLiteral {
 		content := string(tk.concrete)
-		opc, err := strconv.ParseInt(content, 0, 32)
+		opc, err := strconv.ParseUint(content, 0, 32)
 		if err != nil {
 			return 0, readError(tk, err.Error())
 		}
-		opCode = int32(opc)
+		opCode = uint32(opc)
 	} else if tk.kind == tokenKindStringLiteral {
 		tk.concrete = bytes.Trim(tk.concrete, "\"")
-		if len(tk.concrete) > 4 {
-			return 0, readError(tk, "opcode string %q exceeds 4 ascii characters", string(tk.concrete))
+		if len(tk.concrete) != 4 {
+			return 0, readError(tk, "opcode string %q not 4 ascii characters", string(tk.concrete))
 		}
-		opCode = bytesToOpCode(tk.concrete)
+		opCode = bytesToOpCode(*(*[4]byte)(tk.concrete))
 	}
 	if _, err := expectNext(tr, tokenKindCloseParen, tokenKindCloseSquare); err != nil {
 		return 0, err
@@ -775,12 +775,11 @@ func sanitizeComment(tk token) string {
 	return comment
 }
 
-func bytesToOpCode(data []byte) int32 {
-	opCode := int32(0)
-	for _, b := range data {
-		opCode <<= 8
-		opCode |= int32(b)
-	}
+func bytesToOpCode(data [4]byte) uint32 {
+	opCode := uint32(data[0])
+	opCode |= (uint32(data[1]) << 8)
+	opCode |= (uint32(data[2]) << 16)
+	opCode |= (uint32(data[3]) << 24)
 	return opCode
 }
 

parse_test.go

@@ -13,6 +13,31 @@ func TestReadFile(t *testing.T) {
 		expected File
 	}
 	tcs := []testCase{
+		{
+			file: "opcodes",
+			expected: File{
+				Structs: []Struct{
+					{
+						Name:   "NumericalASCIIOpCode",
+						OpCode: 0x34333231,
+					},
+					{
+						Name:     "NumericalASCIIOpCode2",
+						ReadOnly: true,
+						OpCode:   825373493,
+					},
+					{
+						Name:   "NumericalASCIIOpCode3",
+						OpCode: 842150708,
+					},
+					{
+						Name:     "NumericalASCIIOpCode4",
+						ReadOnly: true,
+						OpCode:   875770418,
+					},
+				},
+			},
+		},
 		{
 			file: "bitflags",
 			expected: File{
@@ -1346,7 +1371,7 @@ func TestReadFile(t *testing.T) {
 				Messages: []Message{
 					{
 						Name:   "RequestCatalog",
-						OpCode: bytesToOpCode([]byte("IKEA")),
+						OpCode: 0x41454B49,
 						Fields: map[uint8]Field{
 							1: {
 								Name: "family",
@@ -1373,7 +1398,7 @@ func TestReadFile(t *testing.T) {
 					{
 						Comment: "*\n * This union is so documented!\n ",
 						Name:    "U",
-						OpCode:  bytesToOpCode([]byte("yeah")),
+						OpCode:  bytesToOpCode([4]byte{'y', 'e', 'a', 'h'}),
 						Fields: map[uint8]UnionField{
 							1: {
 								Message: &Message{
@@ -1507,12 +1532,13 @@ func TestReadFileError(t *testing.T) {
 		{file: "invalid_enum_with_op_code", errMessage: "[1:4] enums may not have attached op codes"},
 		{file: "invalid_op_code_1", errMessage: "[0:2] expected (OpCode, Flags) got Close Square"},
 		{file: "invalid_op_code_2", errMessage: "[0:6] expected (OpCode, Flags) got Ident"},
-		{file: "invalid_op_code_3", errMessage: "[0:15] opcode string \"12345\" exceeds 4 ascii characters"},
+		{file: "invalid_op_code_3", errMessage: "[0:15] opcode string \"12345\" not 4 ascii characters"},
 		{file: "invalid_op_code_4", errMessage: "[0:8] expected (Open Paren) got Open Square"},
-		{file: "invalid_op_code_5", errMessage: "[0:81] strconv.ParseInt: parsing \"1111111111111111111111111111111111111111111111111111111111111111111111111\": value out of range"},
+		{file: "invalid_op_code_5", errMessage: "[0:81] strconv.ParseUint: parsing \"1111111111111111111111111111111111111111111111111111111111111111111111111\": value out of range"},
 		{file: "invalid_op_code_6", errMessage: "[0:15] expected (Close Paren) got Close Square"},
 		{file: "invalid_op_code_7", errMessage: "[0:16] expected (Close Square) got Equals"},
 		{file: "invalid_op_code_8", errMessage: "[0:13] expected (Integer Literal, String Literal) got Ident"},
+		{file: "invalid_op_code_9", errMessage: "[0:13] opcode string \"123\" not 4 ascii characters"},
 		{file: "invalid_enum_bad_deprecated", errMessage: "[1:17] expected (String Literal) got Equals"},
 		{file: "invalid_enum_double_deprecated", errMessage: "[2:5] expected enum option following deprecated annotation"},
 		{file: "invalid_enum_no_close", errMessage: "[2:0] enum definition ended early"},

testdata/base/opcodes.bop

@@ -0,0 +1,15 @@
+[opcode("1234")]
+struct NumericalASCIIOpCode {
+}
+
+[opcode(0x31323335)]
+readonly struct NumericalASCIIOpCode2 {
+}
+
+[opcode("4322")]
+struct NumericalASCIIOpCode3 {
+}
+
+[opcode(0x34333232)]
+readonly struct NumericalASCIIOpCode4 {
+}

testdata/invalid/invalid_op_code_10.bop

@@ -0,0 +1,5 @@
+[opcode("1234")]
+struct InvalidOpCode10A {}
+
+[opcode("1234")]
+struct InvalidOpCode10B {}

testdata/invalid/invalid_op_code_11.bop

@@ -0,0 +1,5 @@
+[opcode("1234")]
+struct InvalidOpCode11A {}
+
+[opcode(0x34333231)]
+struct InvalidOpCode11B {}

testdata/invalid/invalid_op_code_12.bop

@@ -0,0 +1,5 @@
+[opcode("4567")]
+message InvalidOpCode12A {}
+
+[opcode("4567")]
+message InvalidOpCode12B {}

testdata/invalid/invalid_op_code_13.bop

@@ -0,0 +1,5 @@
+[opcode("8910")]
+union InvalidOpCode13A {}
+
+[opcode("8910")]
+union InvalidOpCode13B {}

testdata/invalid/invalid_op_code_9.bop

@@ -0,0 +1 @@
+[opcode("123")]