Make bpfix not modify the input tree

Make a new object called Fixer to hold the state of the tree, and
make a copy of the input tree so the original doesn't get modified.

Bug: 73724997
Test: bpfix_test.go
Change-Id: I1dc6fd99158c8b0e1db029df99e6cf72699a5e63
Merged-In: I1dc6fd99158c8b0e1db029df99e6cf72699a5e63
(cherry picked from commit adee968a4bace28bd26253bc436043eb513f6117)

4 files changed, 68 insertions, 24 deletions

diff --git a/androidmk/cmd/androidmk/androidmk.go b/androidmk/cmd/androidmk/androidmk.go
index 6e0b474b..b6a973c0 100644
--- a/androidmk/cmd/androidmk/androidmk.go
+++ b/androidmk/cmd/androidmk/androidmk.go
@@ -239,7 +239,8 @@ func convertFile(filename string, buffer *bytes.Buffer) (string, []error) {
 	}
 
 	// check for common supported but undesirable structures and clean them up
-	err := bpfix.FixTree(tree, bpfix.NewFixRequest().AddAll())
+	fixer := bpfix.NewFixer(tree)
+	tree, err := fixer.Fix(bpfix.NewFixRequest().AddAll())
 	if err != nil {
 		return "", []error{err}
 	}
diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go
index 2c3cc6cd..4300e81b 100644
--- a/bpfix/bpfix/bpfix.go
+++ b/bpfix/bpfix/bpfix.go
@@ -18,7 +18,9 @@ package bpfix
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
+	"io"
 	"path/filepath"
 
 	"github.com/google/blueprint/parser"
@@ -42,21 +44,34 @@ func (r FixRequest) AddAll() (result FixRequest) {
 	return result
 }
 
-// FixTree repeatedly applies the fixes listed in the given FixRequest to the given File
+type Fixer struct {
+	tree *parser.File
+}
+
+func NewFixer(tree *parser.File) *Fixer {
+	fixer := &Fixer{tree}
+
+	// make a copy of the tree
+	fixer.reparse()
+
+	return fixer
+}
+
+// Fix repeatedly applies the fixes listed in the given FixRequest to the given File
 // until there is no fix that affects the tree
-func FixTree(tree *parser.File, config FixRequest) error {
-	prevIdentifier, err := fingerprint(tree)
+func (f *Fixer) Fix(config FixRequest) (*parser.File, error) {
+	prevIdentifier, err := f.fingerprint()
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	maxNumIterations := 20
 	i := 0
 	for {
-		err = fixTreeOnce(tree, config)
-		newIdentifier, err := fingerprint(tree)
+		err = f.fixTreeOnce(config)
+		newIdentifier, err := f.fingerprint()
 		if err != nil {
-			return err
+			return nil, err
 		}
 		if bytes.Equal(newIdentifier, prevIdentifier) {
 			break
@@ -67,31 +82,56 @@ func FixTree(tree *parser.File, config FixRequest) error {
 		// detect infinite loop
 		i++
 		if i >= maxNumIterations {
-			return fmt.Errorf("Applied fixes %d times and yet the tree continued to change. Is there an infinite loop?", i)
+			return nil, fmt.Errorf("Applied fixes %d times and yet the tree continued to change. Is there an infinite loop?", i)
 			break
 		}
 	}
-	return err
+	return f.tree, err
 }
 
 // returns a unique identifier for the given tree that can be used to determine whether the tree changed
-func fingerprint(tree *parser.File) (fingerprint []byte, err error) {
-	bytes, err := parser.Print(tree)
+func (f *Fixer) fingerprint() (fingerprint []byte, err error) {
+	bytes, err := parser.Print(f.tree)
 	if err != nil {
 		return nil, err
 	}
 	return bytes, nil
 }
 
-func fixTreeOnce(tree *parser.File, config FixRequest) error {
+func (f *Fixer) reparse() ([]byte, error) {
+	buf, err := parser.Print(f.tree)
+	if err != nil {
+		return nil, err
+	}
+	newTree, err := parse(f.tree.Name, bytes.NewReader(buf))
+	if err != nil {
+		return nil, err
+	}
+	f.tree = newTree
+	return buf, nil
+}
+
+func parse(name string, r io.Reader) (*parser.File, error) {
+	tree, errs := parser.Parse(name, r, parser.NewScope(nil))
+	if errs != nil {
+		s := "parse error: "
+		for _, err := range errs {
+			s += "\n" + err.Error()
+		}
+		return nil, errors.New(s)
+	}
+	return tree, nil
+}
+
+func (f *Fixer) fixTreeOnce(config FixRequest) error {
 	if config.simplifyKnownRedundantVariables {
-		err := simplifyKnownPropertiesDuplicatingEachOther(tree)
+		err := f.simplifyKnownPropertiesDuplicatingEachOther()
 		if err != nil {
 			return err
 		}
 	}
 	if config.rewriteIncorrectAndroidmkPrebuilts {
-		err := rewriteIncorrectAndroidmkPrebuilts(tree)
+		err := f.rewriteIncorrectAndroidmkPrebuilts()
 		if err != nil {
 			return err
 		}
@@ -99,13 +139,13 @@ func fixTreeOnce(tree *parser.File, config FixRequest) error {
 	return nil
 }
 
-func simplifyKnownPropertiesDuplicatingEachOther(tree *parser.File) error {
+func (f *Fixer) simplifyKnownPropertiesDuplicatingEachOther() error {
 	// remove from local_include_dirs anything in export_include_dirs
-	return removeMatchingModuleListProperties(tree, "export_include_dirs", "local_include_dirs")
+	return f.removeMatchingModuleListProperties("export_include_dirs", "local_include_dirs")
 }
 
-func rewriteIncorrectAndroidmkPrebuilts(tree *parser.File) error {
-	for _, def := range tree.Defs {
+func (f *Fixer) rewriteIncorrectAndroidmkPrebuilts() error {
+	for _, def := range f.tree.Defs {
 		mod, ok := def.(*parser.Module)
 		if !ok {
 			continue
@@ -163,8 +203,8 @@ func filterExpressionList(items *parser.List, removals *parser.List) {
 }
 
 // Remove each modules[i].Properties[<legacyName>][j] that matches a modules[i].Properties[<canonicalName>][k]
-func removeMatchingModuleListProperties(tree *parser.File, canonicalName string, legacyName string) error {
-	for _, def := range tree.Defs {
+func (f *Fixer) removeMatchingModuleListProperties(canonicalName string, legacyName string) error {
+	for _, def := range f.tree.Defs {
 		mod, ok := def.(*parser.Module)
 		if !ok {
 			continue
diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go
index e17e815a..1e1e6933 100644
--- a/bpfix/bpfix/bpfix_test.go
+++ b/bpfix/bpfix/bpfix_test.go
@@ -62,14 +62,16 @@ func implFilterListTest(t *testing.T, local_include_dirs []string, export_includ
 		t.Fatalf("%d parse errors", len(errs))
 	}
 
+	fixer := NewFixer(tree)
+
 	// apply simplifications
-	err := simplifyKnownPropertiesDuplicatingEachOther(tree)
+	err := fixer.simplifyKnownPropertiesDuplicatingEachOther()
 	if len(errs) > 0 {
 		t.Fatal(err)
 	}
 
 	// lookup legacy property
-	mod := tree.Defs[0].(*parser.Module)
+	mod := fixer.tree.Defs[0].(*parser.Module)
 	_, found := mod.GetProperty("local_include_dirs")
 	if !found {
 		t.Fatalf("failed to include key local_include_dirs in parse tree")
diff --git a/bpfix/cmd/bpfix.go b/bpfix/cmd/bpfix.go
index 461f41dd..2fde3836 100644
--- a/bpfix/cmd/bpfix.go
+++ b/bpfix/cmd/bpfix.go
@@ -75,7 +75,8 @@ func processFile(filename string, in io.Reader, out io.Writer, fixRequest bpfix.
 	}
 
 	// compute and apply any requested fixes
-	err = bpfix.FixTree(file, fixRequest)
+	fixer := bpfix.NewFixer(file)
+	file, err = fixer.Fix(fixRequest)
 	if err != nil {
 		return err
 	}