Write build definitions directly to output writer

buildDef.WriteTo was calling valueList to convert all the build
parameter ninjaStrings into strings, which uses ValueWithEscaper
to build a strings.Builder.  This results in building a string
only to immediately copy it into the output writer's buffer.

Instead, pass an io.StringWriter to ValueWithEscaper so it can
build the string directly into the output writer's buffer.  This
requires converting ninjaWriterWithWrap into an io.StringWriter.

Test: ninja_writer_test.go
Change-Id: I02e1cf8259306267b9d2d0ebe8c81e13dd443725

ninja_defs.go

@@ -392,20 +392,20 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string)
 	var (
 		comment       = b.Comment
 		rule          = b.Rule.fullName(pkgNames)
-		outputs       = valueList(b.Outputs, pkgNames, outputEscaper)
+		outputs       = b.Outputs
-		implicitOuts  = valueList(b.ImplicitOutputs, pkgNames, outputEscaper)
+		implicitOuts  = b.ImplicitOutputs
-		explicitDeps  = valueList(b.Inputs, pkgNames, inputEscaper)
+		explicitDeps  = b.Inputs
-		implicitDeps  = valueList(b.Implicits, pkgNames, inputEscaper)
+		implicitDeps  = b.Implicits
-		orderOnlyDeps = valueList(b.OrderOnly, pkgNames, inputEscaper)
+		orderOnlyDeps = b.OrderOnly
-		validations   = valueList(b.Validations, pkgNames, inputEscaper)
+		validations   = b.Validations
 	)
 
 	if b.RuleDef != nil {
-		implicitDeps = append(valueList(b.RuleDef.CommandDeps, pkgNames, inputEscaper), implicitDeps...)
+		implicitDeps = append(b.RuleDef.CommandDeps, implicitDeps...)
-		orderOnlyDeps = append(valueList(b.RuleDef.CommandOrderOnly, pkgNames, inputEscaper), orderOnlyDeps...)
+		orderOnlyDeps = append(b.RuleDef.CommandOrderOnly, orderOnlyDeps...)
 	}
 
-	err := nw.Build(comment, rule, outputs, implicitOuts, explicitDeps, implicitDeps, orderOnlyDeps, validations)
+	err := nw.Build(comment, rule, outputs, implicitOuts, explicitDeps, implicitDeps, orderOnlyDeps, validations, pkgNames)
 	if err != nil {
 		return err
 	}
@@ -435,7 +435,7 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string)
 	}
 
 	if !b.Optional {
-		err = nw.Default(outputs...)
+		err = nw.Default(pkgNames, outputs...)
 		if err != nil {
 			return err
 		}
@@ -444,16 +444,6 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string)
 	return nw.BlankLine()
 }
 
-func valueList(list []ninjaString, pkgNames map[*packageContext]string,
-	escaper *strings.Replacer) []string {
-
-	result := make([]string, len(list))
-	for i, ninjaStr := range list {
-		result[i] = ninjaStr.ValueWithEscaper(pkgNames, escaper)
-	}
-	return result
-}
-
 func writeVariables(nw *ninjaWriter, variables map[string]ninjaString,
 	pkgNames map[*packageContext]string) error {
 	var keys []string

ninja_strings.go

@@ -17,6 +17,7 @@ package blueprint
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"strings"
 )
 
@@ -36,7 +37,7 @@ var (
 
 type ninjaString interface {
 	Value(pkgNames map[*packageContext]string) string
-	ValueWithEscaper(pkgNames map[*packageContext]string, escaper *strings.Replacer) string
+	ValueWithEscaper(w io.StringWriter, pkgNames map[*packageContext]string, escaper *strings.Replacer)
 	Eval(variables map[Variable]ninjaString) (string, error)
 	Variables() []Variable
 }
@@ -284,26 +285,24 @@ func parseNinjaStrings(scope scope, strs []string) ([]ninjaString,
 }
 
 func (n varNinjaString) Value(pkgNames map[*packageContext]string) string {
-	return n.ValueWithEscaper(pkgNames, defaultEscaper)
+	if len(n.strings) == 1 {
+		return defaultEscaper.Replace(n.strings[0])
+	}
+	str := &strings.Builder{}
+	n.ValueWithEscaper(str, pkgNames, defaultEscaper)
+	return str.String()
 }
 
-func (n varNinjaString) ValueWithEscaper(pkgNames map[*packageContext]string,
+func (n varNinjaString) ValueWithEscaper(w io.StringWriter, pkgNames map[*packageContext]string,
-	escaper *strings.Replacer) string {
+	escaper *strings.Replacer) {
 
-	if len(n.strings) == 1 {
+	w.WriteString(escaper.Replace(n.strings[0]))
-		return escaper.Replace(n.strings[0])
-	}
-
-	str := strings.Builder{}
-	str.WriteString(escaper.Replace(n.strings[0]))
 	for i, v := range n.variables {
-		str.WriteString("${")
+		w.WriteString("${")
-		str.WriteString(v.fullName(pkgNames))
+		w.WriteString(v.fullName(pkgNames))
-		str.WriteString("}")
+		w.WriteString("}")
-		str.WriteString(escaper.Replace(n.strings[i+1]))
+		w.WriteString(escaper.Replace(n.strings[i+1]))
 	}
-
-	return str.String()
 }
 
 func (n varNinjaString) Eval(variables map[Variable]ninjaString) (string, error) {
@@ -327,12 +326,12 @@ func (n varNinjaString) Variables() []Variable {
 }
 
 func (l literalNinjaString) Value(pkgNames map[*packageContext]string) string {
-	return l.ValueWithEscaper(pkgNames, defaultEscaper)
+	return defaultEscaper.Replace(string(l))
 }
 
-func (l literalNinjaString) ValueWithEscaper(pkgNames map[*packageContext]string,
+func (l literalNinjaString) ValueWithEscaper(w io.StringWriter, pkgNames map[*packageContext]string,
-	escaper *strings.Replacer) string {
+	escaper *strings.Replacer) {
-	return escaper.Replace(string(l))
+	w.WriteString(escaper.Replace(string(l)))
 }
 
 func (l literalNinjaString) Eval(variables map[Variable]ninjaString) (string, error) {

ninja_writer.go

@@ -114,14 +114,15 @@ func (n *ninjaWriter) Rule(name string) error {
 }
 
 func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
-	explicitDeps, implicitDeps, orderOnlyDeps, validations []string) error {
+	explicitDeps, implicitDeps, orderOnlyDeps, validations []ninjaString,
+	pkgNames map[*packageContext]string) error {
 
 	n.justDidBlankLine = false
 
 	const lineWrapLen = len(" $")
 	const maxLineLen = lineWidth - lineWrapLen
 
-	wrapper := ninjaWriterWithWrap{
+	wrapper := &ninjaWriterWithWrap{
 		ninjaWriter: n,
 		maxLineLen:  maxLineLen,
 	}
@@ -136,14 +137,16 @@ func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
 	wrapper.WriteString("build")
 
 	for _, output := range outputs {
-		wrapper.WriteStringWithSpace(output)
+		wrapper.Space()
+		output.ValueWithEscaper(wrapper, pkgNames, outputEscaper)
 	}
 
 	if len(implicitOuts) > 0 {
 		wrapper.WriteStringWithSpace("|")
 
 		for _, out := range implicitOuts {
-			wrapper.WriteStringWithSpace(out)
+			wrapper.Space()
+			out.ValueWithEscaper(wrapper, pkgNames, outputEscaper)
 		}
 	}
 
@@ -152,14 +155,16 @@ func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
 	wrapper.WriteStringWithSpace(rule)
 
 	for _, dep := range explicitDeps {
-		wrapper.WriteStringWithSpace(dep)
+		wrapper.Space()
+		dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
 	}
 
 	if len(implicitDeps) > 0 {
 		wrapper.WriteStringWithSpace("|")
 
 		for _, dep := range implicitDeps {
-			wrapper.WriteStringWithSpace(dep)
+			wrapper.Space()
+			dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
 		}
 	}
 
@@ -167,7 +172,8 @@ func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
 		wrapper.WriteStringWithSpace("||")
 
 		for _, dep := range orderOnlyDeps {
-			wrapper.WriteStringWithSpace(dep)
+			wrapper.Space()
+			dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
 		}
 	}
 
@@ -175,7 +181,8 @@ func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
 		wrapper.WriteStringWithSpace("|@")
 
 		for _, dep := range validations {
-			wrapper.WriteStringWithSpace(dep)
+			wrapper.Space()
+			dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
 		}
 	}
 
@@ -228,13 +235,13 @@ func (n *ninjaWriter) ScopedAssign(name, value string) error {
 	return nil
 }
 
-func (n *ninjaWriter) Default(targets ...string) error {
+func (n *ninjaWriter) Default(pkgNames map[*packageContext]string, targets ...ninjaString) error {
 	n.justDidBlankLine = false
 
 	const lineWrapLen = len(" $")
 	const maxLineLen = lineWidth - lineWrapLen
 
-	wrapper := ninjaWriterWithWrap{
+	wrapper := &ninjaWriterWithWrap{
 		ninjaWriter: n,
 		maxLineLen:  maxLineLen,
 	}
@@ -242,7 +249,8 @@ func (n *ninjaWriter) Default(targets ...string) error {
 	wrapper.WriteString("default")
 
 	for _, target := range targets {
-		wrapper.WriteString(" " + target)
+		wrapper.Space()
+		target.ValueWithEscaper(wrapper, pkgNames, outputEscaper)
 	}
 
 	return wrapper.Flush()
@@ -278,24 +286,56 @@ func (n *ninjaWriter) writeStatement(directive, name string) error {
 	return nil
 }
 
+// ninjaWriterWithWrap is an io.StringWriter that writes through to a ninjaWriter, but supports
+// user-readable line wrapping on boundaries when ninjaWriterWithWrap.Space is called.
+// It collects incoming calls to WriteString until either the line length is exceeded, in which case
+// it inserts a wrap before the pending strings and then writes them, or the next call to Space, in
+// which case it writes out the pending strings.
+//
+// WriteString never returns an error, all errors are held until Flush is called.  Once an error has
+// occurred all writes become noops.
 type ninjaWriterWithWrap struct {
 	*ninjaWriter
+	// pending lists the strings that have been written since the last call to Space.
+	pending []string
+
+	// pendingLen accumulates the lengths of the strings in pending.
+	pendingLen int
+
+	// lineLen accumulates the number of bytes on the current line.
+	lineLen int
+
+	// maxLineLen is the length of the line before wrapping.
 	maxLineLen int
-	writtenLen int
+
+	// space is true if the strings in pending should be preceded by a space.
+	space bool
+
+	// err holds any error that has occurred to return in Flush.
 	err error
 }
 
-func (n *ninjaWriterWithWrap) writeString(s string, space bool) {
+// WriteString writes the string to buffer, wrapping on a previous Space call if necessary.
+// It never returns an error, all errors are held until Flush is called.
+func (n *ninjaWriterWithWrap) WriteString(s string) (written int, noError error) {
+	// Always return the full length of the string and a nil error.
+	// ninjaWriterWithWrap doesn't return errors to the caller, it saves them until Flush()
+	written = len(s)
+
 	if n.err != nil {
 		return
 	}
 
-	spaceLen := 0
+	const spaceLen = 1
-	if space {
+	if !n.space {
-		spaceLen = 1
+		// No space is pending, so a line wrap can't be inserted before this, so just write
-	}
+		// the string.
-
+		n.lineLen += len(s)
-	if n.writtenLen+len(s)+spaceLen > n.maxLineLen {
+		_, n.err = n.writer.WriteString(s)
+	} else if n.lineLen+len(s)+spaceLen > n.maxLineLen {
+		// A space is pending, and the pending strings plus the current string would exceed the
+		// maximum line length.  Wrap and indent before the pending space and strings, then write
+		// the pending and current strings.
 		_, n.err = n.writer.WriteString(" $\n")
 		if n.err != nil {
 			return
@@ -304,29 +344,68 @@ func (n *ninjaWriterWithWrap) writeString(s string, space bool) {
 		if n.err != nil {
 			return
 		}
-		n.writtenLen = indentWidth * 2
+		n.lineLen = indentWidth*2 + n.pendingLen
 		s = strings.TrimLeftFunc(s, unicode.IsSpace)
-	} else if space {
+		n.pending = append(n.pending, s)
-		_, n.err = n.writer.WriteString(" ")
+		n.writePending()
+
+		n.space = false
+	} else {
+		// A space is pending but the current string would not reach the maximum line length,
+		// add it to the pending list.
+		n.pending = append(n.pending, s)
+		n.pendingLen += len(s)
+		n.lineLen += len(s)
+	}
+
+	return
+}
+
+// Space inserts a space that is also a possible wrapping point into the string.
+func (n *ninjaWriterWithWrap) Space() {
 	if n.err != nil {
 		return
 	}
-		n.writtenLen++
+	if n.space {
+		// A space was already pending, and the space plus any strings written after the space did
+		// not reach the maxmimum line length, so write out the old space and pending strings.
+		_, n.err = n.writer.WriteString(" ")
+		n.lineLen++
+		n.writePending()
 	}
-
+	n.space = true
-	_, n.err = n.writer.WriteString(s)
-	n.writtenLen += len(s)
 }
 
-func (n *ninjaWriterWithWrap) WriteString(s string) {
+// writePending writes out all the strings stored in pending and resets it.
-	n.writeString(s, false)
+func (n *ninjaWriterWithWrap) writePending() {
+	if n.err != nil {
+		return
+	}
+	for _, pending := range n.pending {
+		_, n.err = n.writer.WriteString(pending)
+		if n.err != nil {
+			return
+		}
+	}
+	// Reset the length of pending back to 0 without reducing its capacity to avoid reallocating
+	// the backing array.
+	n.pending = n.pending[:0]
+	n.pendingLen = 0
 }
 
+// WriteStringWithSpace is a helper that calls Space and WriteString.
 func (n *ninjaWriterWithWrap) WriteStringWithSpace(s string) {
-	n.writeString(s, true)
+	n.Space()
+	_, _ = n.WriteString(s)
 }
 
+// Flush writes out any pending space or strings and then a newline.  It also returns any errors
+// that have previously occurred.
 func (n *ninjaWriterWithWrap) Flush() error {
+	if n.space {
+		_, n.err = n.writer.WriteString(" ")
+	}
+	n.writePending()
 	if n.err != nil {
 		return n.err
 	}

ninja_writer_test.go

@@ -16,6 +16,7 @@ package blueprint
 
 import (
 	"bytes"
+	"strings"
 	"testing"
 )
 
@@ -49,14 +50,26 @@ var ninjaWriterTestCases = []struct {
 	},
 	{
 		input: func(w *ninjaWriter) {
-			ck(w.Build("foo comment", "foo", []string{"o1", "o2"}, []string{"io1", "io2"},
+			ck(w.Build("foo comment", "foo", testNinjaStrings("o1", "o2"),
-				[]string{"e1", "e2"}, []string{"i1", "i2"}, []string{"oo1", "oo2"}, []string{"v1", "v2"}))
+				testNinjaStrings("io1", "io2"), testNinjaStrings("e1", "e2"),
+				testNinjaStrings("i1", "i2"), testNinjaStrings("oo1", "oo2"),
+				testNinjaStrings("v1", "v2"), nil))
 		},
 		output: "# foo comment\nbuild o1 o2 | io1 io2: foo e1 e2 | i1 i2 || oo1 oo2 |@ v1 v2\n",
 	},
 	{
 		input: func(w *ninjaWriter) {
-			ck(w.Default("foo"))
+			ck(w.Build("foo comment", "foo",
+				testNinjaStrings(strings.Repeat("o", lineWidth)),
+				nil,
+				testNinjaStrings(strings.Repeat("i", lineWidth)),
+				nil, nil, nil, nil))
+		},
+		output: "# foo comment\nbuild $\n        " + strings.Repeat("o", lineWidth) + ": foo $\n        " + strings.Repeat("i", lineWidth) + "\n",
+	},
+	{
+		input: func(w *ninjaWriter) {
+			ck(w.Default(nil, testNinjaStrings("foo")...))
 		},
 		output: "default foo\n",
 	},
@@ -94,7 +107,8 @@ var ninjaWriterTestCases = []struct {
 			ck(w.ScopedAssign("command", "echo out: $out in: $in _arg: $_arg"))
 			ck(w.ScopedAssign("pool", "p"))
 			ck(w.BlankLine())
-			ck(w.Build("r comment", "r", []string{"foo.o"}, nil, []string{"foo.in"}, nil, nil, nil))
+			ck(w.Build("r comment", "r", testNinjaStrings("foo.o"),
+				nil, testNinjaStrings("foo.in"), nil, nil, nil, nil))
 			ck(w.ScopedAssign("_arg", "arg value"))
 		},
 		output: `pool p
@@ -124,3 +138,8 @@ func TestNinjaWriter(t *testing.T) {
 		}
 	}
 }
+
+func testNinjaStrings(s ...string) []ninjaString {
+	ret, _ := parseNinjaStrings(nil, s)
+	return ret
+}