simplify eval as we can only concat lists (#111)

Co-authored-by: rgodden <rgodden@thoughtmachine.net>

eval/eval.go

@@ -1,6 +1,7 @@
 package eval
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/please-build/buildtools/build"
@@ -31,18 +32,10 @@ func LookLikeBuildLabel(l string) bool {
 }
 
 func (e *Eval) EvalGlobs(dir string, rule *build.Rule, attrName string) ([]string, error) {
-	files, err := e.evalGlobs(dir, rule.Attr(attrName))
-	if err != nil {
-		return nil, err
-	}
-	ret := make([]string, 0, len(files))
-	for f := range files {
-		ret = append(ret, f)
-	}
-	return ret, nil
+	return e.evalGlobs(dir, rule.Attr(attrName))
 }
 
-func (e *Eval) evalGlobs(dir string, val build.Expr) (map[string]struct{}, error) {
+func (e *Eval) evalGlobs(dir string, val build.Expr) ([]string, error) {
 	switch expr := val.(type) {
 	case *build.CallExpr:
 		globArgs := parseGlob(expr)
@@ -51,32 +44,20 @@ func (e *Eval) evalGlobs(dir string, val build.Expr) (map[string]struct{}, error
 		}
 		return e.globber.Glob(dir, globArgs)
 	case *build.BinaryExpr:
-		ret, err := e.evalGlobs(dir, expr.X)
+		if expr.Op != "+" {
+			return nil, fmt.Errorf("encountered a binary expression with operation %s. Only + is supported", expr.Op)
+		}
+		x, err := e.evalGlobs(dir, expr.X)
 		if err != nil {
 			return nil, err
 		}
 		y, err := e.evalGlobs(dir, expr.Y)
 		if err != nil {
 			return nil, err
 		}
-		switch expr.Op {
-		case "+":
-			for f := range y {
-				ret[f] = struct{}{}
-			}
-		case "-":
-			for f := range y {
-				delete(ret, f)
-			}
-		}
-		return ret, nil
+		return append(x, y...), nil
 	default:
-		str := build.Strings(expr)
-		ret := make(map[string]struct{}, len(str))
-		for _, s := range str {
-			ret[s] = struct{}{}
-		}
-		return ret, nil
+		return build.Strings(expr), nil
 	}
 }
 

eval/eval_test.go

@@ -35,7 +35,6 @@ func TestParseGlob(t *testing.T) {
 			include: []string{"*.go"},
 			exclude: []string{"*_test.go"},
 		},
-
 		{
 			name: "ignores other args",
 			// Please has some other stuff we don't care about.
@@ -85,22 +84,13 @@ func TestEvalGlob(t *testing.T) {
 			code:     `glob(["mai*.go"]) + ["bar.go"]`,
 			expected: []string{"main.go", "bar.go"},
 		},
-		{
-			name:     "glob - strings",
-			code:     `glob(["*.go"]) - ["bar.go"]`,
-			expected: []string{"main.go", "bar_test.go"},
-		},
 	}
 	for _, test := range testCases {
 		t.Run(test.name, func(t *testing.T) {
 			file, err := build.ParseBuild(test.name, []byte(test.code))
 			require.NoError(t, err)
 			require.Len(t, file.Stmt, 1)
-			ret, err := e.evalGlobs("test_project", file.Stmt[0])
-			got := make([]string, 0, len(ret))
-			for f := range ret {
-				got = append(got, f)
-			}
+			got, err := e.evalGlobs("test_project", file.Stmt[0])
 			require.NoError(t, err)
 			assert.ElementsMatch(t, test.expected, got)
 		})

glob/glob.go

@@ -25,7 +25,7 @@ func New() *Globber {
 // 1) globs should only match .go files as they're being used in go rules
 // 2) go rules will never depend on files outside the package dir, so we don't need to support **
 // 3) we don't want symlinks, directories and other non-regular files
-func (g *Globber) Glob(dir string, args *Args) (map[string]struct{}, error) {
+func (g *Globber) Glob(dir string, args *Args) ([]string, error) {
 	inc := map[string]struct{}{}
 	for _, i := range args.Include {
 		fs, err := g.glob(dir, i)
@@ -49,7 +49,11 @@ func (g *Globber) Glob(dir string, args *Args) (map[string]struct{}, error) {
 		}
 	}
 
-	return inc, nil
+	ret := make([]string, 0, len(inc))
+	for i := range inc {
+		ret = append(ret, i)
+	}
+	return ret, nil
 }
 
 // glob matches all regular files in a directory based on a glob pattern

glob/glob_test.go

@@ -15,7 +15,7 @@ func TestGlob(t *testing.T) {
 		})
 		require.NoError(t, err)
 
-		assert.Equal(t, map[string]struct{}{"bar_test.go": {}}, files)
+		assert.ElementsMatch(t, []string{"bar_test.go"}, files)
 	})
 
 	t.Run("excludes pattern", func(t *testing.T) {
@@ -25,6 +25,6 @@ func TestGlob(t *testing.T) {
 		})
 		require.NoError(t, err)
 
-		assert.Equal(t, map[string]struct{}{"main.go": {}, "bar.go": {}}, files)
+		assert.ElementsMatch(t, []string{"main.go", "bar.go"}, files)
 	})
 }