internal/telemetry/cmd/stacks: test predicates

Factor out parsing and evaluation of #stacks predicates.

Add a test.

For golang/go#71045.

Change-Id: I677e34e555a1f1ebb0722088d55e0c9edd3b3f40
Reviewed-on: https://go-review.googlesource.com/c/tools/+/643776
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: jba

gopls/internal/telemetry/cmd/stacks/stacks.go

@@ -404,81 +404,97 @@ func readIssues(pcfg ProgramConfig) ([]*Issue, error) {
 	for _, issue := range issues {
 		block := findPredicateBlock(issue.Body)
 		if block != "" {
-			expr, err := parser.ParseExpr(block)
+			pred, err := parsePredicate(block)
 			if err != nil {
 				log.Printf("invalid predicate in issue #%d: %v\n<<%s>>",
 					issue.Number, err, block)
 				continue
 			}
-			var validate func(ast.Expr) error
-			validate = func(e ast.Expr) error {
-				switch e := e.(type) {
-				case *ast.UnaryExpr:
-					if e.Op != token.NOT {
-						return fmt.Errorf("invalid op: %s", e.Op)
-					}
-					return validate(e.X)
-
-				case *ast.BinaryExpr:
-					if e.Op != token.LAND && e.Op != token.LOR {
-						return fmt.Errorf("invalid op: %s", e.Op)
-					}
-					if err := validate(e.X); err != nil {
-						return err
-					}
-					return validate(e.Y)
+			issue.predicateText = block
+			issue.predicate = pred
+		}
+	}
 
-				case *ast.ParenExpr:
-					return validate(e.X)
+	return issues, nil
+}
 
-				case *ast.BasicLit:
-					if e.Kind != token.STRING {
-						return fmt.Errorf("invalid literal (%s)", e.Kind)
-					}
-					if _, err := strconv.Unquote(e.Value); err != nil {
-						return err
-					}
+// parsePredicate parses a predicate expression, returning a function that evaluates
+// the predicate on a stack.
+// The expression must match this grammar:
+//
+//	expr = "string literal"
+//	     | ( expr )
+//	     | ! expr
+//	     | expr && expr
+//	     | expr || expr
+func parsePredicate(s string) (func(string) bool, error) {
+	expr, err := parser.ParseExpr(s)
+	if err != nil {
+		return nil, fmt.Errorf("parse error: %w", err)
+	}
+	var validate func(ast.Expr) error
+	validate = func(e ast.Expr) error {
+		switch e := e.(type) {
+		case *ast.UnaryExpr:
+			if e.Op != token.NOT {
+				return fmt.Errorf("invalid op: %s", e.Op)
+			}
+			return validate(e.X)
 
-				default:
-					return fmt.Errorf("syntax error (%T)", e)
-				}
-				return nil
+		case *ast.BinaryExpr:
+			if e.Op != token.LAND && e.Op != token.LOR {
+				return fmt.Errorf("invalid op: %s", e.Op)
 			}
-			if err := validate(expr); err != nil {
-				log.Printf("invalid predicate in issue #%d: %v\n<<%s>>",
-					issue.Number, err, block)
-				continue
+			if err := validate(e.X); err != nil {
+				return err
 			}
-			issue.predicateText = block
-			issue.predicate = func(stack string) bool {
-				var eval func(ast.Expr) bool
-				eval = func(e ast.Expr) bool {
-					switch e := e.(type) {
-					case *ast.UnaryExpr:
-						return !eval(e.X)
-
-					case *ast.BinaryExpr:
-						if e.Op == token.LAND {
-							return eval(e.X) && eval(e.Y)
-						} else {
-							return eval(e.X) || eval(e.Y)
-						}
+			return validate(e.Y)
 
-					case *ast.ParenExpr:
-						return eval(e.X)
+		case *ast.ParenExpr:
+			return validate(e.X)
 
-					case *ast.BasicLit:
-						substr, _ := strconv.Unquote(e.Value)
-						return strings.Contains(stack, substr)
-					}
-					panic("unreachable")
-				}
-				return eval(expr)
+		case *ast.BasicLit:
+			if e.Kind != token.STRING {
+				return fmt.Errorf("invalid literal (%s)", e.Kind)
+			}
+			if _, err := strconv.Unquote(e.Value); err != nil {
+				return err
 			}
+
+		default:
+			return fmt.Errorf("syntax error (%T)", e)
 		}
+		return nil
+	}
+	if err := validate(expr); err != nil {
+		return nil, err
 	}
 
-	return issues, nil
+	return func(stack string) bool {
+		var eval func(ast.Expr) bool
+		eval = func(e ast.Expr) bool {
+			switch e := e.(type) {
+			case *ast.UnaryExpr:
+				return !eval(e.X)
+
+			case *ast.BinaryExpr:
+				if e.Op == token.LAND {
+					return eval(e.X) && eval(e.Y)
+				} else {
+					return eval(e.X) || eval(e.Y)
+				}
+
+			case *ast.ParenExpr:
+				return eval(e.X)
+
+			case *ast.BasicLit:
+				substr, _ := strconv.Unquote(e.Value)
+				return strings.Contains(stack, substr)
+			}
+			panic("unreachable")
+		}
+		return eval(expr)
+	}, nil
 }
 
 // claimStack maps each stack ID to its issue (if any).

gopls/internal/telemetry/cmd/stacks/stacks_test.go

@@ -75,3 +75,53 @@ func TestReadPCLineTable(t *testing.T) {
 		})
 	}
 }
+
+func TestParsePredicate(t *testing.T) {
+	for _, tc := range []struct {
+		expr string
+		arg  string
+		want bool
+	}{
+		{`"x"`, `"x"`, true},
+		{`"x"`, `"axe"`, true}, // literals match by strings.Contains
+		{`"x"`, `"y"`, false},
+		{`!"x"`, "x", false},
+		{`!"x"`, "y", true},
+		{`"x" && "y"`, "xy", true},
+		{`"x" && "y"`, "x", false},
+		{`"x" && "y"`, "y", false},
+		{`"xz" && "zy"`, "xzy", true}, // matches need not be disjoint
+		{`"x" || "y"`, "xy", true},
+		{`"x" || "y"`, "x", true},
+		{`"x" || "y"`, "y", true},
+		{`"x" || "y"`, "z", false},
+	} {
+		eval, err := parsePredicate(tc.expr)
+		if err != nil {
+			t.Fatal(err)
+		}
+		got := eval(tc.arg)
+		if got != tc.want {
+			t.Errorf("%s applied to %q: got %t, want %t", tc.expr, tc.arg, got, tc.want)
+		}
+	}
+}
+
+func TestParsePredicateError(t *testing.T) {
+	// Validate that bad predicates return errors.
+	for _, expr := range []string{
+		``,
+		`1`,
+		`foo`, // an identifier, not a literal
+		`"x" + "y"`,
+		`"x" &&`,
+		`~"x"`,
+		`f(1)`,
+	} {
+		if _, err := parsePredicate(expr); err == nil {
+			t.Errorf("%s: got nil, want error", expr)
+		} else {
+			t.Logf("%s: %v", expr, err)
+		}
+	}
+}