Clean up parser

subpop · May 8, 2020 · 14a5e4c · 14a5e4c
1 parent b689803
commit 14a5e4c
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 103 deletions.
diff --git a/parse.go b/parse.go
@@ -1,25 +1,13 @@
 package ini
 
-import (
-	"errors"
-	"fmt"
-)
-
+// unexpectedTokenErr describes a token that was not expected by the parser in
+// the lexer's current state.
 type unexpectedTokenErr struct {
-	got  token
-	want token
-}
-
-func (e *unexpectedTokenErr) Error() string {
-	return fmt.Sprintf("unexpected token: %v, want %v", e.got, e.want)
+	got token
 }
 
-type invalidPropertyErr struct {
-	p property
-}
-
-func (e *invalidPropertyErr) Error() string {
-	return "invalid property: " + e.p.key
+func (e unexpectedTokenErr) Error() string {
+	return "unexpected token: " + e.got.val
 }
 
 type parser struct {
@@ -50,6 +38,8 @@ func (p *parser) backup() {
 	p.prev = &p.tok
 }
 
+// parse advances the token scanner repeatedly, constructing a parseTree on
+// each step through the token stream until an EOF token is encountered.
 func (p *parser) parse() error {
 	for {
 		if p.tok.typ == tokenEOF {
@@ -61,7 +51,7 @@ func (p *parser) parse() error {
 		case tokenEOF:
 			return nil
 		case tokenError:
-			return errors.New(p.tok.val)
+			return &unexpectedTokenErr{p.tok}
 		case tokenSection:
 			sec := newSection(p.tok.val)
 			if err := p.parseSection(&sec); err != nil {
@@ -85,6 +75,8 @@ func (p *parser) parse() error {
 	}
 }
 
+// parseSection repeatedly advances the token scanner, constructing a section
+// parseTree element from the scanned values.
 func (p *parser) parseSection(out *section) error {
 	name := p.tok.val
 	out.name = name
@@ -93,7 +85,7 @@ func (p *parser) parseSection(out *section) error {
 		p.nextToken()
 		switch p.tok.typ {
 		case tokenError:
-			return errors.New(p.tok.val)
+			return &unexpectedTokenErr{got: p.tok}
 		case tokenPropKey:
 			prop, err := out.get(p.tok.val)
 			if err != nil {
@@ -115,6 +107,8 @@ func (p *parser) parseSection(out *section) error {
 	}
 }
 
+// parseProperty repeatedly advances the token scanner, constructing a property
+// parseTree element from the scanned values.
 func (p *parser) parseProperty(out *property) error {
 	key := p.tok.val
 	subkey := ""
@@ -125,23 +119,10 @@ func (p *parser) parseProperty(out *property) error {
 		p.nextToken()
 	}
 
-	if p.tok.typ != tokenAssignment {
-		return &unexpectedTokenErr{
-			got: p.tok,
-			want: token{
-				typ: tokenAssignment,
-				val: "=",
-			},
-		}
-	}
-
 	p.nextToken()
 	if p.tok.typ != tokenPropValue {
 		return &unexpectedTokenErr{
 			got: p.tok,
-			want: token{
-				typ: tokenPropValue,
-			},
 		}
 	}
 	val := p.tok.val

diff --git a/parse_test.go b/parse_test.go
@@ -120,20 +120,15 @@ func TestParse(t *testing.T) {
 
 func TestParseProp(t *testing.T) {
 	tests := []struct {
-		input string
-		want  property
+		description string
+		input       string
+		want        property
+		shouldError bool
+		wantError   error
 	}{
 		{
-			input: "shell=/bin/bash",
-			want: property{
-				key: "shell",
-				vals: map[string][]string{
-					"": {"/bin/bash"},
-				},
-			},
-		},
-		{
-			input: "Greeting[en]=Hello\nGreeting[fr]=Bonjour",
+			description: "valid property",
+			input:       "Greeting[en]=Hello\nGreeting[fr]=Bonjour",
 			want: property{
 				key: "Greeting",
 				vals: map[string][]string{
@@ -142,85 +137,88 @@ func TestParseProp(t *testing.T) {
 				},
 			},
 		},
+		{
+			description: "unexpected token, missing property value",
+			input:       "Greeting=",
+			want:        property{"", map[string][]string{"": {}}},
+			shouldError: true,
+			wantError:   &unexpectedTokenErr{token{tokenError, `unexpected character: '\x00', an assignment must be followed by one or more alphanumeric characters`}},
+		},
 	}
 
 	for _, test := range tests {
-		p := newParser([]byte(test.input))
-		p.nextToken()
-		got := newProperty(p.tok.val)
-		for {
-			err := p.parseProperty(&got)
-			if err != nil {
-				t.Fatal(err)
-			}
+		t.Run(test.description, func(t *testing.T) {
+			var err error
+			p := newParser([]byte(test.input))
 			p.nextToken()
-			if p.tok.typ == tokenEOF {
-				break
+
+			got := newProperty(p.tok.val)
+			for {
+				err = p.parseProperty(&got)
+				if err != nil {
+					break
+				}
+				p.nextToken()
+				if p.tok.typ == tokenEOF {
+					break
+				}
 			}
-		}
-		if !cmp.Equal(got, test.want, cmp.Options{cmp.AllowUnexported(property{})}) {
-			t.Errorf("%v != %v", got, test.want)
-		}
+			if test.shouldError {
+				if !cmp.Equal(err, test.wantError, cmp.AllowUnexported(unexpectedTokenErr{}, token{})) {
+					t.Fatalf("parseProperty(%v) returned %v, want %v", test.input, err, test.wantError)
+				}
+			} else {
+				if err != nil {
+					t.Fatalf("parseProperty(%v) returned %v, want %v", test.input, err, test.wantError)
+				}
+				if !cmp.Equal(got, test.want, cmp.Options{cmp.AllowUnexported(property{})}) {
+					t.Errorf("parseProperty(%v) = %v, want %v\ndiff -want +got\n%v", test.input, got, test.want, cmp.Diff(test.want, got))
+				}
+			}
+		})
 	}
 }
 
 func TestParseSection(t *testing.T) {
 	tests := []struct {
-		input string
-		want  section
+		description string
+		input       string
+		want        section
+		shouldError bool
+		wantError   error
 	}{
 		{
-			input: "[user]\nname=root\nshell=/bin/bash",
-			want: section{
-				name: "user",
-				props: map[string]property{
-					"name": {
-						key: "name",
-						vals: map[string][]string{
-							"": {"root"},
-						},
-					},
-					"shell": {
-						key: "shell",
-						vals: map[string][]string{
-							"": {"/bin/bash"},
-						},
-					},
-				},
-			},
-		},
-		{
-			input: "[user]\n; UNIX user name\nname=root\n; Default shell\nshell=/bin/bash",
+			description: "valid",
+			input:       "[user]\n; UNIX user name\nname=root\n; Default shell\nshell=/bin/bash",
 			want: section{
 				name: "user",
 				props: map[string]property{
-					"name": {
-						key: "name",
-						vals: map[string][]string{
-							"": {"root"},
-						},
-					},
-					"shell": {
-						key: "shell",
-						vals: map[string][]string{
-							"": {"/bin/bash"},
-						},
-					},
+					"name":  {"name", map[string][]string{"": {"root"}}},
+					"shell": {"shell", map[string][]string{"": {"/bin/bash"}}},
 				},
 			},
 		},
 	}
 
 	for _, test := range tests {
-		p := newParser([]byte(test.input))
-		p.nextToken()
-		got := newSection(p.tok.val)
-		err := p.parseSection(&got)
-		if err != nil {
-			t.Fatal(err)
-		}
-		if !cmp.Equal(got, test.want, cmp.Options{cmp.AllowUnexported(property{}, section{})}) {
-			t.Errorf("%v != %v", got, test.want)
-		}
+		t.Run(test.description, func(t *testing.T) {
+			p := newParser([]byte(test.input))
+			p.nextToken()
+			got := newSection(p.tok.val)
+			err := p.parseSection(&got)
+
+			if test.shouldError {
+				if !cmp.Equal(err, test.wantError) {
+					t.Fatalf("parseSection(%v) returned %v, want %v", test.input, err, test.wantError)
+				}
+			} else {
+				if err != nil {
+					t.Fatalf("parseSection(%v) returned %v, want %v", test.input, err, test.wantError)
+				}
+				if !cmp.Equal(got, test.want, cmp.Options{cmp.AllowUnexported(property{}, section{})}) {
+					t.Errorf("parseSection(%v) = %v, want %v\ndiff -want +got\n%v", test.input, got, test.want, cmp.Diff(test.want, got))
+				}
+			}
+		})
 	}
 }