Keep parser.Directive internal to parser

Signed-off-by: Daniel Nephin <dnephin@docker.com>

rewritten from github.com/moby/moby 64c4c1c3d5e0fe364d83db5a8dc99a24fd121754

frontend/dockerfile/parser/dumper/main.go

@@ -5,6 +5,7 @@ import (
 	"os"
 
 	"github.com/docker/docker/builder/dockerfile/parser"
+	"go/ast"
 )
 
 func main() {
@@ -23,12 +24,10 @@ func main() {
 		}
 		defer f.Close()
 
-		d := parser.NewDefaultDirective()
+		result, err := parser.Parse(f)
-		ast, err := parser.Parse(f, d)
 		if err != nil {
 			panic(err)
-		} else {
-			fmt.Println(ast.Dump())
 		}
+		fmt.Println(result.AST.Dump())
 	}
 }

frontend/dockerfile/parser/json_test.go

@@ -28,10 +28,9 @@ var validJSONArraysOfStrings = map[string][]string{
 
 func TestJSONArraysOfStrings(t *testing.T) {
 	for json, expected := range validJSONArraysOfStrings {
-		d := Directive{}
+		d := NewDefaultDirective()
-		d.SetEscapeToken(DefaultEscapeToken)
 
-		if node, _, err := parseJSON(json, &d); err != nil {
+		if node, _, err := parseJSON(json, d); err != nil {
 			t.Fatalf("%q should be a valid JSON array of strings, but wasn't! (err: %q)", json, err)
 		} else {
 			i := 0
@@ -51,10 +50,9 @@ func TestJSONArraysOfStrings(t *testing.T) {
 		}
 	}
 	for _, json := range invalidJSONArraysOfStrings {
-		d := Directive{}
+		d := NewDefaultDirective()
-		d.SetEscapeToken(DefaultEscapeToken)
 
-		if _, _, err := parseJSON(json, &d); err != errDockerfileNotStringArray {
+		if _, _, err := parseJSON(json, d); err != errDockerfileNotStringArray {
 			t.Fatalf("%q should be an invalid JSON array of strings, but wasn't!", json)
 		}
 	}

frontend/dockerfile/parser/parser.go

@@ -34,7 +34,7 @@ type Node struct {
 	Original   string          // original line used before parsing
 	Flags      []string        // only top Node should have this set
 	StartLine  int             // the line in the original dockerfile where the node begins
-	EndLine    int             // the line in the original dockerfile where the node ends
+	endLine    int             // the line in the original dockerfile where the node ends
 }
 
 // Dump dumps the AST defined by `node` as a list of sexps.
@@ -70,7 +70,7 @@ var (
 )
 
 // DefaultEscapeToken is the default escape token
-const DefaultEscapeToken = "\\"
+const DefaultEscapeToken = '\\'
 
 // Directive is the structure used during a build run to hold the state of
 // parsing directives.
@@ -81,8 +81,8 @@ type Directive struct {
 	escapeSeen            bool           // Whether the escape directive has been seen
 }
 
-// SetEscapeToken sets the default token for escaping characters in a Dockerfile.
+// setEscapeToken sets the default token for escaping characters in a Dockerfile.
-func (d *Directive) SetEscapeToken(s string) error {
+func (d *Directive) setEscapeToken(s string) error {
 	if s != "`" && s != "\\" {
 		return fmt.Errorf("invalid ESCAPE '%s'. Must be ` or \\", s)
 	}
@@ -91,18 +91,13 @@ func (d *Directive) SetEscapeToken(s string) error {
 	return nil
 }
 
-// EscapeToken returns the escape token
-func (d *Directive) EscapeToken() rune {
-	return d.escapeToken
-}
-
 // NewDefaultDirective returns a new Directive with the default escapeToken token
 func NewDefaultDirective() *Directive {
 	directive := Directive{
 		escapeSeen:           false,
 		lookingForDirectives: true,
 	}
-	directive.SetEscapeToken(DefaultEscapeToken)
+	directive.setEscapeToken(string(DefaultEscapeToken))
 	return &directive
 }
 
@@ -200,7 +195,7 @@ func handleParserDirective(line string, d *Directive) (bool, error) {
 	}
 	for i, n := range tokenEscapeCommand.SubexpNames() {
 		if n == "escapechar" {
-			if err := d.SetEscapeToken(tecMatch[i]); err != nil {
+			if err := d.setEscapeToken(tecMatch[i]); err != nil {
 				return false, err
 			}
 			return true, nil
@@ -209,9 +204,16 @@ func handleParserDirective(line string, d *Directive) (bool, error) {
 	return false, nil
 }
 
-// Parse is the main parse routine.
+// Result is the result of parsing a Dockerfile
-// It handles an io.ReadWriteCloser and returns the root of the AST.
+type Result struct {
-func Parse(rwc io.Reader, d *Directive) (*Node, error) {
+	AST         *Node
+	EscapeToken rune
+}
+
+// Parse reads lines from a Reader, parses the lines into an AST and returns
+// the AST and escape token
+func Parse(rwc io.Reader) (*Result, error) {
+	d := NewDefaultDirective()
 	currentLine := 0
 	root := &Node{}
 	root.StartLine = -1
@@ -267,18 +269,18 @@ func Parse(rwc io.Reader, d *Directive) (*Node, error) {
 		if child != nil {
 			// Update the line information for the current child.
 			child.StartLine = startLine
-			child.EndLine = currentLine
+			child.endLine = currentLine
 			// Update the line information for the root. The starting line of the root is always the
 			// starting line of the first child and the ending line is the ending line of the last child.
 			if root.StartLine < 0 {
 				root.StartLine = currentLine
 			}
-			root.EndLine = currentLine
+			root.endLine = currentLine
 			root.Children = append(root.Children, child)
 		}
 	}
 
-	return root, nil
+	return &Result{AST: root, EscapeToken: d.escapeToken}, nil
 }
 
 // covers comments and empty lines. Lines should be trimmed before passing to

frontend/dockerfile/parser/parser_test.go

@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"runtime"
 	"testing"
+
+	"github.com/docker/docker/pkg/testutil/assert"
 )
 
 const testDir = "testfiles"
@@ -16,17 +18,11 @@ const testFileLineInfo = "testfile-line/Dockerfile"
 
 func getDirs(t *testing.T, dir string) []string {
 	f, err := os.Open(dir)
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatal(err)
-	}
-
 	defer f.Close()
 
 	dirs, err := f.Readdirnames(0)
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatal(err)
-	}
-
 	return dirs
 }
 
@@ -35,15 +31,11 @@ func TestTestNegative(t *testing.T) {
 		dockerfile := filepath.Join(negativeTestDir, dir, "Dockerfile")
 
 		df, err := os.Open(dockerfile)
-		if err != nil {
+		assert.NilError(t, err)
-			t.Fatalf("Dockerfile missing for %s: %v", dir, err)
-		}
 		defer df.Close()
 
-		_, err = Parse(df, NewDefaultDirective())
+		_, err = Parse(df)
-		if err == nil {
+		assert.Error(t, err, "")
-			t.Fatalf("No error parsing broken dockerfile for %s", dir)
-		}
 	}
 }
 
@@ -53,31 +45,21 @@ func TestTestData(t *testing.T) {
 		resultfile := filepath.Join(testDir, dir, "result")
 
 		df, err := os.Open(dockerfile)
-		if err != nil {
+		assert.NilError(t, err)
-			t.Fatalf("Dockerfile missing for %s: %v", dir, err)
-		}
 		defer df.Close()
 
-		ast, err := Parse(df, NewDefaultDirective())
+		result, err := Parse(df)
-		if err != nil {
+		assert.NilError(t, err)
-			t.Fatalf("Error parsing %s's dockerfile: %v", dir, err)
-		}
 
 		content, err := ioutil.ReadFile(resultfile)
-		if err != nil {
+		assert.NilError(t, err)
-			t.Fatalf("Error reading %s's result file: %v", dir, err)
-		}
 
 		if runtime.GOOS == "windows" {
 			// CRLF --> CR to match Unix behavior
 			content = bytes.Replace(content, []byte{'\x0d', '\x0a'}, []byte{'\x0a'}, -1)
 		}
 
-		if ast.Dump()+"\n" != string(content) {
+		assert.Equal(t, result.AST.Dump()+"\n", string(content))
-			fmt.Fprintln(os.Stderr, "Result:\n"+ast.Dump())
-			fmt.Fprintln(os.Stderr, "Expected:\n"+string(content))
-			t.Fatalf("%s: AST dump of dockerfile does not match result", dir)
-		}
 	}
 }
 
@@ -119,46 +101,33 @@ func TestParseWords(t *testing.T) {
 
 	for _, test := range tests {
 		words := parseWords(test["input"][0], NewDefaultDirective())
-		if len(words) != len(test["expect"]) {
+		assert.DeepEqual(t, words, test["expect"])
-			t.Fatalf("length check failed. input: %v, expect: %q, output: %q", test["input"][0], test["expect"], words)
-		}
-		for i, word := range words {
-			if word != test["expect"][i] {
-				t.Fatalf("word check failed for word: %q. input: %q, expect: %q, output: %q", word, test["input"][0], test["expect"], words)
-			}
-		}
 	}
 }
 
 func TestLineInformation(t *testing.T) {
 	df, err := os.Open(testFileLineInfo)
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatalf("Dockerfile missing for %s: %v", testFileLineInfo, err)
-	}
 	defer df.Close()
 
-	ast, err := Parse(df, NewDefaultDirective())
+	result, err := Parse(df)
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatalf("Error parsing dockerfile %s: %v", testFileLineInfo, err)
-	}
 
-	if ast.StartLine != 5 || ast.EndLine != 31 {
+	ast := result.AST
-		fmt.Fprintf(os.Stderr, "Wrong root line information: expected(%d-%d), actual(%d-%d)\n", 5, 31, ast.StartLine, ast.EndLine)
+	if ast.StartLine != 5 || ast.endLine != 31 {
+		fmt.Fprintf(os.Stderr, "Wrong root line information: expected(%d-%d), actual(%d-%d)\n", 5, 31, ast.StartLine, ast.endLine)
 		t.Fatal("Root line information doesn't match result.")
 	}
-	if len(ast.Children) != 3 {
+	assert.Equal(t, len(ast.Children), 3)
-		fmt.Fprintf(os.Stderr, "Wrong number of child: expected(%d), actual(%d)\n", 3, len(ast.Children))
-		t.Fatalf("Root line information doesn't match result for %s", testFileLineInfo)
-	}
 	expected := [][]int{
 		{5, 5},
 		{11, 12},
 		{17, 31},
 	}
 	for i, child := range ast.Children {
-		if child.StartLine != expected[i][0] || child.EndLine != expected[i][1] {
+		if child.StartLine != expected[i][0] || child.endLine != expected[i][1] {
 			t.Logf("Wrong line information for child %d: expected(%d-%d), actual(%d-%d)\n",
-				i, expected[i][0], expected[i][1], child.StartLine, child.EndLine)
+				i, expected[i][0], expected[i][1], child.StartLine, child.endLine)
 			t.Fatal("Root line information doesn't match result.")
 		}
 	}