diff --git a/pkg/ghexpr/builder.go b/pkg/ghexpr/builder.go new file mode 100644 index 00000000000..88c0637f79c --- /dev/null +++ b/pkg/ghexpr/builder.go @@ -0,0 +1,98 @@ +package ghexpr + +import ( + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var builderLog = logger.New("ghexpr:builder") + +// BuildConditionTree creates a condition tree from an existing condition string and a +// new draft condition string. When existingCondition is empty, the draft is returned +// as a standalone [ExpressionNode]; otherwise the two are combined with AND. +func BuildConditionTree(existingCondition string, draftCondition string) ConditionNode { + builderLog.Printf("Building condition tree: existing=%q, draft=%q", existingCondition, draftCondition) + draftNode := &ExpressionNode{Expression: draftCondition} + if existingCondition == "" { + return draftNode + } + existingNode := &ExpressionNode{Expression: existingCondition} + return &AndNode{Left: existingNode, Right: draftNode} +} + +// BuildOr creates an OR node combining two conditions. +func BuildOr(left ConditionNode, right ConditionNode) ConditionNode { + return &OrNode{Left: left, Right: right} +} + +// BuildAnd creates an AND node combining two conditions. +func BuildAnd(left ConditionNode, right ConditionNode) ConditionNode { + builderLog.Print("Building AND condition node") + return &AndNode{Left: left, Right: right} +} + +// BuildPropertyAccess creates a property-access node for a dotted path. +// Example: BuildPropertyAccess("github.event.action") +func BuildPropertyAccess(path string) *PropertyAccessNode { + return &PropertyAccessNode{PropertyPath: path} +} + +// BuildStringLiteral creates a single-quoted string literal node. +func BuildStringLiteral(value string) *StringLiteralNode { + return &StringLiteralNode{Value: value} +} + +// BuildBooleanLiteral creates a boolean literal node (true or false). +func BuildBooleanLiteral(value bool) *BooleanLiteralNode { + return &BooleanLiteralNode{Value: value} +} + +// BuildNullLiteral creates a null literal node. +func BuildNullLiteral() *ExpressionNode { + return &ExpressionNode{Expression: "null"} +} + +// BuildComparison creates a comparison node with an explicit operator string +// such as "==", "!=", "<", ">", "<=", or ">=". +func BuildComparison(left ConditionNode, operator string, right ConditionNode) *ComparisonNode { + return &ComparisonNode{Left: left, Operator: operator, Right: right} +} + +// BuildEquals creates an equality comparison node (==). +func BuildEquals(left ConditionNode, right ConditionNode) *ComparisonNode { + return BuildComparison(left, "==", right) +} + +// BuildNotEquals creates an inequality comparison node (!=). +func BuildNotEquals(left ConditionNode, right ConditionNode) *ComparisonNode { + return BuildComparison(left, "!=", right) +} + +// BuildFunctionCall creates a function-call node. +// Example: BuildFunctionCall("contains", BuildPropertyAccess("github.event_name"), BuildStringLiteral("push")) +func BuildFunctionCall(functionName string, args ...ConditionNode) *FunctionCallNode { + return &FunctionCallNode{FunctionName: functionName, Arguments: args} +} + +// BuildDisjunction creates a multi-term OR node. +// When multiline is true, [DisjunctionNode.RenderMultiline] is used. +func BuildDisjunction(multiline bool, terms ...ConditionNode) *DisjunctionNode { + return &DisjunctionNode{Terms: terms, Multiline: multiline} +} + +// RenderCondition runs [OptimizeExpression] on node and returns the rendered string. +func RenderCondition(node ConditionNode) string { + return OptimizeExpression(node).Render() +} + +// RenderConditionAsIf writes a YAML `if:` block scalar entry to yaml using the given +// indentation prefix for each line of the rendered condition. The condition is first +// optimized with [OptimizeExpression]. +func RenderConditionAsIf(yaml *strings.Builder, condition ConditionNode, indent string) { + yaml.WriteString(" if: |\n") + conditionStr := RenderCondition(condition) + for line := range strings.SplitSeq(conditionStr, "\n") { + yaml.WriteString(indent + line + "\n") + } +} diff --git a/pkg/ghexpr/builder_test.go b/pkg/ghexpr/builder_test.go new file mode 100644 index 00000000000..150ebc3a37f --- /dev/null +++ b/pkg/ghexpr/builder_test.go @@ -0,0 +1,79 @@ +//go:build !integration + +package ghexpr_test + +import ( + "testing" + + "github.com/github/gh-aw/pkg/ghexpr" + "github.com/stretchr/testify/assert" +) + +// TestBuildBasicNodes verifies that all primitive builders produce the expected Render output. +func TestBuildBasicNodes(t *testing.T) { + assert.Equal(t, "a.b.c", ghexpr.BuildPropertyAccess("a.b.c").Render()) + assert.Equal(t, "'hello'", ghexpr.BuildStringLiteral("hello").Render()) + assert.Equal(t, "'it''s'", ghexpr.BuildStringLiteral("it's").Render(), "embedded single quote must be doubled") + assert.Equal(t, "true", ghexpr.BuildBooleanLiteral(true).Render()) + assert.Equal(t, "false", ghexpr.BuildBooleanLiteral(false).Render()) + assert.Equal(t, "null", ghexpr.BuildNullLiteral().Render()) +} + +// TestBuildComparison verifies comparison builders. +func TestBuildComparison(t *testing.T) { + left := ghexpr.BuildPropertyAccess("github.event_name") + right := ghexpr.BuildStringLiteral("push") + + assert.Equal(t, "github.event_name == 'push'", ghexpr.BuildEquals(left, right).Render()) + assert.Equal(t, "github.event_name != 'push'", ghexpr.BuildNotEquals(left, right).Render()) + assert.Equal(t, "github.event_name < 'push'", ghexpr.BuildComparison(left, "<", right).Render()) +} + +// TestBuildFunctionCall verifies function-call builder. +func TestBuildFunctionCall(t *testing.T) { + node := ghexpr.BuildFunctionCall("contains", + ghexpr.BuildPropertyAccess("github.event.label.name"), + ghexpr.BuildStringLiteral("deploy"), + ) + assert.Equal(t, "contains(github.event.label.name, 'deploy')", node.Render()) +} + +// TestBuildLogical verifies AND / OR builders. +func TestBuildLogical(t *testing.T) { + a := ghexpr.BuildPropertyAccess("a") + b := ghexpr.BuildPropertyAccess("b") + + // PropertyAccessNode operands of AND don't need parentheses. + assert.Equal(t, "a && b", ghexpr.BuildAnd(a, b).Render()) + assert.Equal(t, "a || b", ghexpr.BuildOr(a, b).Render()) +} + +// TestBuildConditionTree verifies condition tree composition. +func TestBuildConditionTree(t *testing.T) { + assert.Equal(t, "draft", + ghexpr.BuildConditionTree("", "draft").Render(), + "empty existing condition returns draft-only node") + + tree := ghexpr.BuildConditionTree("existing", "draft") + assert.Equal(t, "(existing) && (draft)", tree.Render()) +} + +// TestBuildDisjunction verifies multi-term OR builder. +func TestBuildDisjunction(t *testing.T) { + node := ghexpr.BuildDisjunction(false, + ghexpr.BuildPropertyAccess("a"), + ghexpr.BuildPropertyAccess("b"), + ghexpr.BuildPropertyAccess("c"), + ) + assert.Equal(t, "a || b || c", node.Render()) +} + +// TestRenderCondition verifies RenderCondition optimises and renders. +func TestRenderCondition(t *testing.T) { + // A && true should optimise to just A. + node := ghexpr.BuildAnd( + ghexpr.BuildPropertyAccess("github.event_name"), + ghexpr.BuildBooleanLiteral(true), + ) + assert.Equal(t, "github.event_name", ghexpr.RenderCondition(node)) +} diff --git a/pkg/ghexpr/doc.go b/pkg/ghexpr/doc.go new file mode 100644 index 00000000000..88b5621d163 --- /dev/null +++ b/pkg/ghexpr/doc.go @@ -0,0 +1,73 @@ +// Package ghexpr implements a parser, optimizer, and builder for GitHub Actions +// expression syntax (${{ ... }}). +// +// # Grammar +// +// The following grammar describes the subset of GitHub Actions expression +// syntax handled by this package (adapted from the official spec at +// https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions): +// +// expression = or_expr +// or_expr = and_expr ( "||" and_expr )* +// and_expr = unary_expr ( "&&" unary_expr )* +// unary_expr = "!" unary_expr | primary_expr +// primary_expr = "(" expression ")" +// | function_call +// | literal +// | property_access +// function_call = IDENT "(" [ expression ( "," expression )* ] ")" +// property_access= IDENT ( "." IDENT )* +// | IDENT "[" expression "]" +// literal = STRING | NUMBER | "true" | "false" | "null" +// STRING = "'" [^']* "'" | '"' [^"]* '"' +// NUMBER = "-"? DIGIT+ ( "." DIGIT+ )? +// IDENT = [a-zA-Z_] [a-zA-Z0-9_-]* +// +// Note: GitHub Actions expressions are always wrapped in ${{ and }} markers +// when embedded in YAML values. This package operates on the inner content +// (without the ${{ }} wrappers) unless otherwise documented. +// +// # AST +// +// Parsed expressions are represented as a tree of [ConditionNode] values: +// +// - [ExpressionNode] – an opaque leaf expression +// - [AndNode] – left && right +// - [OrNode] – left || right +// - [NotNode] – !child +// - [ComparisonNode] – left op right (==, !=, <, >, <=, >=) +// - [FunctionCallNode] – func(arg, …) +// - [PropertyAccessNode]– a.b.c property path +// - [StringLiteralNode] – 'value' +// - [BooleanLiteralNode]– true / false +// - [DisjunctionNode] – multi-term OR, for rendering as GitHub's multi-line +// +// # Optimizer +// +// [OptimizeExpression] applies boolean-algebra simplifications (idempotent law, +// absorption, De Morgan, etc.) to produce a shorter equivalent expression. +// Status-function calls (always, success, failure, cancelled) are treated as +// opaque so the optimizer never removes them. +// +// # Builders +// +// Composable [Build…] functions construct condition trees without touching raw +// strings. [RenderCondition] renders a tree to its GitHub Actions syntax. +// +// # Patterns +// +// Pre-compiled [regexp.Regexp] values are exposed for common matching tasks: +// +// - [ExpressionPattern] – matches ${{ … }} +// - [ExpressionPatternDotAll] – same with dot-all flag +// - [StringLiteralPattern] – matches 'x', "x", or `x` +// - [NumberLiteralPattern] – matches numeric literals +// - [OrPattern] – splits left || right +// - [ComparisonExtractionPattern] – extracts the left side of comparisons +// - [InlineExpressionPattern] – matches inline ${{ … }} +// - [TemplateIfPattern] – matches {{#if … }} +// - [TemplateElseIfPattern] – matches {{#elseif … }} +// - [RangePattern] – matches numeric ranges (e.g. "1-10") +// +// Predicate helpers: [HasExpressionMarker], [ContainsExpression], [IsExpression]. +package ghexpr diff --git a/pkg/ghexpr/nodes.go b/pkg/ghexpr/nodes.go new file mode 100644 index 00000000000..b0e1b74c917 --- /dev/null +++ b/pkg/ghexpr/nodes.go @@ -0,0 +1,222 @@ +package ghexpr + +import ( + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var nodesLog = logger.New("ghexpr:nodes") + +// ConditionNode represents a node in a condition expression tree. +// All concrete types satisfy this interface; use type assertions to distinguish them. +type ConditionNode interface { + Render() string +} + +// Compile-time assertions: all ConditionNode implementation types must satisfy the interface. +var ( + _ ConditionNode = (*ExpressionNode)(nil) + _ ConditionNode = (*AndNode)(nil) + _ ConditionNode = (*OrNode)(nil) + _ ConditionNode = (*NotNode)(nil) + _ ConditionNode = (*DisjunctionNode)(nil) + _ ConditionNode = (*FunctionCallNode)(nil) + _ ConditionNode = (*PropertyAccessNode)(nil) + _ ConditionNode = (*StringLiteralNode)(nil) + _ ConditionNode = (*BooleanLiteralNode)(nil) + _ ConditionNode = (*ComparisonNode)(nil) +) + +// ExpressionNode represents a leaf expression — an opaque string that is valid +// GitHub Actions expression syntax (without the ${{ }} wrapper). +type ExpressionNode struct { + Expression string + Description string // Optional comment/description for the expression +} + +// Render implements ConditionNode. +func (e *ExpressionNode) Render() string { + return e.Expression +} + +// AndNode represents an AND operation between two conditions. +type AndNode struct { + Left, Right ConditionNode +} + +// needsParensAsAndOperand returns true when child must be wrapped in parentheses +// when it appears as an operand of an && expression. Or-level nodes and opaque +// ExpressionNodes must be wrapped to preserve operator precedence. +// NotNode is also wrapped to prevent the leading ! from becoming a YAML type-tag +// indicator when the full expression is placed in an `if:` YAML value. +func needsParensAsAndOperand(child ConditionNode) bool { + switch child.(type) { + case *OrNode, *DisjunctionNode, *ExpressionNode, *NotNode: + return true + } + return false +} + +// Render implements ConditionNode. +func (a *AndNode) Render() string { + leftStr := a.Left.Render() + if needsParensAsAndOperand(a.Left) { + leftStr = "(" + leftStr + ")" + } + rightStr := a.Right.Render() + if needsParensAsAndOperand(a.Right) { + rightStr = "(" + rightStr + ")" + } + return leftStr + " && " + rightStr +} + +// OrNode represents an OR operation between two conditions. +// +// || has the lowest precedence of any boolean operator, so no child of an OR +// expression ever needs explicit parentheses to preserve evaluation order. +type OrNode struct { + Left, Right ConditionNode +} + +// Render implements ConditionNode. +func (o *OrNode) Render() string { + return o.Left.Render() + " || " + o.Right.Render() +} + +// NotNode represents a NOT operation on a condition. +type NotNode struct { + Child ConditionNode +} + +// Render implements ConditionNode. +func (n *NotNode) Render() string { + // For simple function calls like cancelled(), render as !cancelled() instead of !(cancelled()) + // This prevents GitHub Actions from interpreting the extra parentheses as an object structure. + if _, isFunctionCall := n.Child.(*FunctionCallNode); isFunctionCall { + return "!" + n.Child.Render() + } + return "!(" + n.Child.Render() + ")" +} + +// DisjunctionNode represents an OR operation with multiple terms to avoid deep nesting. +// When Multiline is true, each term is rendered on its own line. +type DisjunctionNode struct { + Terms []ConditionNode + Multiline bool +} + +// Render implements ConditionNode. +func (d *DisjunctionNode) Render() string { + if len(d.Terms) == 0 { + return "" + } + if len(d.Terms) == 1 { + return d.Terms[0].Render() + } + + if d.Multiline { + return d.RenderMultiline() + } + + nodesLog.Printf("Rendering inline disjunction with %d terms", len(d.Terms)) + var parts []string + for _, term := range d.Terms { + parts = append(parts, term.Render()) + } + return strings.Join(parts, " || ") +} + +// RenderMultiline renders the disjunction with each term on a separate line, +// including comments for ExpressionNode values that carry a Description. +func (d *DisjunctionNode) RenderMultiline() string { + if len(d.Terms) == 0 { + return "" + } + if len(d.Terms) == 1 { + return d.Terms[0].Render() + } + + nodesLog.Printf("Rendering multiline disjunction with %d terms", len(d.Terms)) + + var lines []string + for i, term := range d.Terms { + var line string + + if expr, ok := term.(*ExpressionNode); ok && expr.Description != "" { + line = "# " + expr.Description + "\n" + } + + if i < len(d.Terms)-1 { + line += term.Render() + " ||" + } else { + line += term.Render() + } + + lines = append(lines, line) + } + + return strings.Join(lines, "\n") +} + +// FunctionCallNode represents a function call such as contains(array, value). +type FunctionCallNode struct { + FunctionName string + Arguments []ConditionNode +} + +// Render implements ConditionNode. +func (f *FunctionCallNode) Render() string { + var args []string + for _, arg := range f.Arguments { + args = append(args, arg.Render()) + } + return f.FunctionName + "(" + strings.Join(args, ", ") + ")" +} + +// PropertyAccessNode represents a property-access path such as github.event.action. +type PropertyAccessNode struct { + PropertyPath string +} + +// Render implements ConditionNode. +func (p *PropertyAccessNode) Render() string { + return p.PropertyPath +} + +// StringLiteralNode represents a single-quoted string literal. +// Embedded single quotes are escaped by doubling them per the GitHub Actions spec. +type StringLiteralNode struct { + Value string +} + +// Render implements ConditionNode. +func (s *StringLiteralNode) Render() string { + escaped := strings.ReplaceAll(s.Value, "'", "''") + return "'" + escaped + "'" +} + +// BooleanLiteralNode represents the literals true or false. +type BooleanLiteralNode struct { + Value bool +} + +// Render implements ConditionNode. +func (b *BooleanLiteralNode) Render() string { + if b.Value { + return "true" + } + return "false" +} + +// ComparisonNode represents a binary comparison: ==, !=, <, >, <=, or >=. +type ComparisonNode struct { + Left ConditionNode + Operator string + Right ConditionNode +} + +// Render implements ConditionNode. +func (c *ComparisonNode) Render() string { + return c.Left.Render() + " " + c.Operator + " " + c.Right.Render() +} diff --git a/pkg/ghexpr/optimizer.go b/pkg/ghexpr/optimizer.go new file mode 100644 index 00000000000..712240fc3c5 --- /dev/null +++ b/pkg/ghexpr/optimizer.go @@ -0,0 +1,463 @@ +package ghexpr + +import ( + "slices" + + "github.com/github/gh-aw/pkg/logger" +) + +var optimizerLog = logger.New("ghexpr:optimizer") + +// OptimizeExpression applies boolean-algebra simplifications to a ConditionNode tree, +// returning an equivalent but potentially simpler and shorter expression. +// +// Rules applied (bottom-up, fixpoint iteration): +// +// Constant folding: !true → false, !false → true +// Double negation: !!A → A +// Boolean identity: A && true → A, A || false → A +// Boolean annihilation: A && false → false, A || true → true +// Idempotent law: A && A → A, A || A → A +// Complement law: A && !A → false, A || !A → true +// De Morgan (AND): !(A && B) → !A || !B +// De Morgan (OR): !(A || B) → !A && !B +// Absorption (AND): A && (A || B) → A +// Absorption (OR): A || (A && B) → A +// Subsumption (disj): disj(A, A&&B, …) → disj(A, …) [A&&B subsumed by A] +// DisjunctionNode: deduplication, false-filtering, true short-circuit +// +// SAFETY: GitHub Actions status functions (always, success, failure, cancelled) +// have semantics beyond plain booleans. The optimizer never eliminates a status +// function call from an expression; it only applies rules when both operands of +// && / || are free of status functions. +// +// Execution is bounded: at most maxOptimizationPasses bottom-up passes are +// performed so the optimizer always terminates. +func OptimizeExpression(node ConditionNode) ConditionNode { + if node == nil { + return nil + } + + const maxOptimizationPasses = 10 + + current := node + for pass := range maxOptimizationPasses { + next := optimizeNode(current) + if next.Render() == current.Render() { + optimizerLog.Printf("Expression stabilised after %d pass(es)", pass+1) + break + } + current = next + } + return current +} + +// optimizeNode performs a single bottom-up optimisation pass. +func optimizeNode(node ConditionNode) ConditionNode { + switch n := node.(type) { + case *AndNode: + return optimizeAndNode(n) + case *OrNode: + return optimizeOrNode(n) + case *NotNode: + return optimizeNotNode(n) + case *DisjunctionNode: + return optimizeDisjunctionNode(n) + default: + return node + } +} + +// --- helper predicates ------------------------------------------------------- + +func isBoolLiteral(node ConditionNode, value bool) bool { + lit, ok := node.(*BooleanLiteralNode) + return ok && lit.Value == value +} + +// isStatusFunc returns true when node is a call to one of the GitHub Actions +// status-check functions: always(), success(), failure(), cancelled(). +func isStatusFunc(node ConditionNode) bool { + fn, ok := node.(*FunctionCallNode) + if !ok { + return false + } + switch fn.FunctionName { + case "always", "success", "failure", "cancelled": + return true + } + return false +} + +// nodesEqual returns true when a and b render to identical strings. +func nodesEqual(a, b ConditionNode) bool { + if a == nil || b == nil { + return a == nil && b == nil + } + return a.Render() == b.Render() +} + +// isNegationOf returns true when b is the logical negation of a or vice versa. +func isNegationOf(a, b ConditionNode) bool { + if notB, ok := b.(*NotNode); ok && nodesEqual(a, notB.Child) { + return true + } + if notA, ok := a.(*NotNode); ok && nodesEqual(notA.Child, b) { + return true + } + return false +} + +// containsStatusFunc returns true when any node in the tree is a status function. +func containsStatusFunc(node ConditionNode) bool { + if isStatusFunc(node) { + return true + } + switch n := node.(type) { + case *AndNode: + return containsStatusFunc(n.Left) || containsStatusFunc(n.Right) + case *OrNode: + return containsStatusFunc(n.Left) || containsStatusFunc(n.Right) + case *NotNode: + return containsStatusFunc(n.Child) + case *DisjunctionNode: + return slices.ContainsFunc(n.Terms, containsStatusFunc) + case *FunctionCallNode: + return slices.ContainsFunc(n.Arguments, containsStatusFunc) + } + return false +} + +// collectOrTerms recursively flattens a chain of OrNode / DisjunctionNode into a flat slice. +func collectOrTerms(node ConditionNode) []ConditionNode { + switch n := node.(type) { + case *OrNode: + return append(collectOrTerms(n.Left), collectOrTerms(n.Right)...) + case *DisjunctionNode: + terms := make([]ConditionNode, 0, len(n.Terms)) + for _, t := range n.Terms { + terms = append(terms, collectOrTerms(t)...) + } + return terms + } + return []ConditionNode{node} +} + +// collectAndTerms recursively flattens a chain of AndNode into a flat slice. +func collectAndTerms(node ConditionNode) []ConditionNode { + if and, ok := node.(*AndNode); ok { + return append(collectAndTerms(and.Left), collectAndTerms(and.Right)...) + } + return []ConditionNode{node} +} + +// rebuildAndChain assembles a left-folded AndNode chain from a non-empty slice. +func rebuildAndChain(terms []ConditionNode) ConditionNode { + if len(terms) == 1 { + return terms[0] + } + result := ConditionNode(&AndNode{Left: terms[0], Right: terms[1]}) + for _, t := range terms[2:] { + result = &AndNode{Left: result, Right: t} + } + return result +} + +// termSubsumedBy returns true when cand is subsumed by sub in a disjunction. +func termSubsumedBy(cand, sub ConditionNode) bool { + if nodesEqual(cand, sub) { + return false + } + if containsStatusFunc(cand) || containsStatusFunc(sub) { + return false + } + for _, ct := range collectAndTerms(cand) { + if nodesEqual(ct, sub) { + return true + } + } + return false +} + +// --- node-specific optimisers ------------------------------------------------ + +func optimizeAndNode(n *AndNode) ConditionNode { + left := optimizeNode(n.Left) + right := optimizeNode(n.Right) + + if isBoolLiteral(left, false) || isBoolLiteral(right, false) { + optimizerLog.Printf("AND annihilation: %s && %s → false", left.Render(), right.Render()) + return &BooleanLiteralNode{Value: false} + } + + terms := collectAndTerms(&AndNode{Left: left, Right: right}) + + for _, t := range terms { + if isBoolLiteral(t, false) { + return &BooleanLiteralNode{Value: false} + } + } + + hasStatusFuncInTerms := slices.ContainsFunc(terms, containsStatusFunc) + filtered := make([]ConditionNode, 0, len(terms)) + for _, t := range terms { + if isBoolLiteral(t, true) && !hasStatusFuncInTerms { + optimizerLog.Printf("AND identity (flatten): removed true literal") + continue + } + filtered = append(filtered, t) + } + if len(filtered) == 0 { + return &BooleanLiteralNode{Value: true} + } + + seen := make(map[string]struct{}, len(filtered)) + deduped := make([]ConditionNode, 0, len(filtered)) + for _, t := range filtered { + key := t.Render() + if _, exists := seen[key]; !exists { + seen[key] = struct{}{} + deduped = append(deduped, t) + } else { + optimizerLog.Printf("AND dedup: removing duplicate term %q", key) + } + } + if len(deduped) == 1 { + return deduped[0] + } + + if !hasStatusFuncInTerms { + for i := range deduped { + for j := i + 1; j < len(deduped); j++ { + if isNegationOf(deduped[i], deduped[j]) { + optimizerLog.Printf("AND complement (flatten): %s && %s → false", deduped[i].Render(), deduped[j].Render()) + return &BooleanLiteralNode{Value: false} + } + } + } + } + + if !hasStatusFuncInTerms { + absorbed := make([]bool, len(deduped)) + for i, ti := range deduped { + orTerms := collectOrTerms(ti) + if len(orTerms) < 2 { + continue + } + for j, tj := range deduped { + if i == j || absorbed[i] { + continue + } + for _, ot := range orTerms { + if nodesEqual(ot, tj) { + optimizerLog.Printf("AND absorption: (%s) && (%s) → %s (absorbed)", tj.Render(), ti.Render(), tj.Render()) + absorbed[i] = true + break + } + } + } + } + if slices.Contains(absorbed, true) { + surviving := make([]ConditionNode, 0, len(deduped)) + for i, t := range deduped { + if !absorbed[i] { + surviving = append(surviving, t) + } + } + if len(surviving) == 0 { + return &BooleanLiteralNode{Value: true} + } + return optimizeNode(rebuildAndChain(surviving)) + } + } + + return rebuildAndChain(deduped) +} + +func optimizeOrNode(n *OrNode) ConditionNode { + left := optimizeNode(n.Left) + right := optimizeNode(n.Right) + + _, leftIsOr := left.(*OrNode) + _, leftIsDisj := left.(*DisjunctionNode) + _, rightIsOr := right.(*OrNode) + _, rightIsDisj := right.(*DisjunctionNode) + if leftIsOr || leftIsDisj || rightIsOr || rightIsDisj { + terms := append(collectOrTerms(left), collectOrTerms(right)...) + optimizerLog.Printf("OR flatten: collected %d terms", len(terms)) + return optimizeDisjunctionNode(&DisjunctionNode{Terms: terms}) + } + + if isBoolLiteral(left, true) || isBoolLiteral(right, true) { + optimizerLog.Printf("OR annihilation: %s || %s → true", left.Render(), right.Render()) + return &BooleanLiteralNode{Value: true} + } + + if isBoolLiteral(right, false) { + return left + } + if isBoolLiteral(left, false) { + return right + } + + if containsStatusFunc(left) || containsStatusFunc(right) { + return &OrNode{Left: left, Right: right} + } + + if nodesEqual(left, right) { + optimizerLog.Printf("OR idempotent: %s || %s → %s", left.Render(), right.Render(), left.Render()) + return left + } + + if isNegationOf(left, right) { + optimizerLog.Printf("OR complement: %s || %s → true", left.Render(), right.Render()) + return &BooleanLiteralNode{Value: true} + } + + for _, pair := range [][2]ConditionNode{{left, right}, {right, left}} { + simple, complex := pair[0], pair[1] + if termSubsumedBy(complex, simple) { + optimizerLog.Printf("OR absorption: %s || (%s) → %s (absorbed)", simple.Render(), complex.Render(), simple.Render()) + return simple + } + } + + return &OrNode{Left: left, Right: right} +} + +func optimizeNotNode(n *NotNode) ConditionNode { + child := optimizeNode(n.Child) + + if lit, ok := child.(*BooleanLiteralNode); ok { + optimizerLog.Printf("NOT constant folding: !%v → %v", lit.Value, !lit.Value) + return &BooleanLiteralNode{Value: !lit.Value} + } + + if notChild, ok := child.(*NotNode); ok { + optimizerLog.Printf("NOT double negation: !!%s → %s", notChild.Child.Render(), notChild.Child.Render()) + return optimizeNode(notChild.Child) + } + + if andChild, ok := child.(*AndNode); ok && !containsStatusFunc(andChild) { + optimizerLog.Printf("NOT De Morgan (AND): !(%s && %s) → !%s || !%s", + andChild.Left.Render(), andChild.Right.Render(), + andChild.Left.Render(), andChild.Right.Render()) + return optimizeNode(&OrNode{ + Left: &NotNode{Child: andChild.Left}, + Right: &NotNode{Child: andChild.Right}, + }) + } + + if orChild, ok := child.(*OrNode); ok && !containsStatusFunc(orChild) { + optimizerLog.Printf("NOT De Morgan (OR): !(%s || %s) → !%s && !%s", + orChild.Left.Render(), orChild.Right.Render(), + orChild.Left.Render(), orChild.Right.Render()) + return optimizeNode(&AndNode{ + Left: &NotNode{Child: orChild.Left}, + Right: &NotNode{Child: orChild.Right}, + }) + } + + if disjChild, ok := child.(*DisjunctionNode); ok { + if len(disjChild.Terms) == 0 { + return &NotNode{Child: child} + } + if !containsStatusFunc(disjChild) { + optimizerLog.Printf("NOT De Morgan (Disjunction): !(disjunction[%d]) → AND chain of negations", len(disjChild.Terms)) + negations := make([]ConditionNode, len(disjChild.Terms)) + for i, term := range disjChild.Terms { + negations[i] = &NotNode{Child: term} + } + return optimizeNode(rebuildAndChain(negations)) + } + } + + return &NotNode{Child: child} +} + +func optimizeDisjunctionNode(n *DisjunctionNode) ConditionNode { + if len(n.Terms) == 0 { + return n + } + + optimised := make([]ConditionNode, 0, len(n.Terms)) + for _, term := range n.Terms { + optimised = append(optimised, optimizeNode(term)) + } + + for _, term := range optimised { + if isBoolLiteral(term, true) { + optimizerLog.Printf("Disjunction short-circuit on true") + return &BooleanLiteralNode{Value: true} + } + } + + filtered := make([]ConditionNode, 0, len(optimised)) + for _, term := range optimised { + if !isBoolLiteral(term, false) { + filtered = append(filtered, term) + } + } + if len(filtered) == 0 { + optimizerLog.Printf("Disjunction all-false → false") + return &BooleanLiteralNode{Value: false} + } + + seen := make(map[string]struct{}, len(filtered)) + deduped := make([]ConditionNode, 0, len(filtered)) + for _, term := range filtered { + key := term.Render() + if _, exists := seen[key]; !exists { + seen[key] = struct{}{} + deduped = append(deduped, term) + } else { + optimizerLog.Printf("Disjunction dedup: removing duplicate term %q", key) + } + } + + if len(deduped) == 1 { + return deduped[0] + } + + if !slices.ContainsFunc(deduped, containsStatusFunc) { + for i := range deduped { + for j := i + 1; j < len(deduped); j++ { + if isNegationOf(deduped[i], deduped[j]) { + optimizerLog.Printf("Disjunction complement: %s || %s → true", deduped[i].Render(), deduped[j].Render()) + return &BooleanLiteralNode{Value: true} + } + } + } + + subsumed := make([]bool, len(deduped)) + for i, cand := range deduped { + for j, sub := range deduped { + if i == j { + continue + } + if termSubsumedBy(cand, sub) { + optimizerLog.Printf("Disjunction subsumption: %s subsumed by %s", cand.Render(), sub.Render()) + subsumed[i] = true + break + } + } + } + if slices.Contains(subsumed, true) { + surviving := make([]ConditionNode, 0, len(deduped)) + for i, t := range deduped { + if !subsumed[i] { + surviving = append(surviving, t) + } + } + if len(surviving) == 0 { + return &BooleanLiteralNode{Value: false} + } + if len(surviving) == 1 { + return surviving[0] + } + return optimizeNode(&DisjunctionNode{Terms: surviving, Multiline: n.Multiline}) + } + } + + return &DisjunctionNode{Terms: deduped, Multiline: n.Multiline} +} diff --git a/pkg/ghexpr/optimizer_test.go b/pkg/ghexpr/optimizer_test.go new file mode 100644 index 00000000000..b5ac5232921 --- /dev/null +++ b/pkg/ghexpr/optimizer_test.go @@ -0,0 +1,129 @@ +//go:build !integration + +package ghexpr_test + +import ( + "testing" + + "github.com/github/gh-aw/pkg/ghexpr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// helpers for building nodes concisely in tests. +func prop(path string) *ghexpr.PropertyAccessNode { + return &ghexpr.PropertyAccessNode{PropertyPath: path} +} +func str(v string) *ghexpr.StringLiteralNode { return &ghexpr.StringLiteralNode{Value: v} } +func blit(v bool) *ghexpr.BooleanLiteralNode { return &ghexpr.BooleanLiteralNode{Value: v} } +func expr(e string) *ghexpr.ExpressionNode { return &ghexpr.ExpressionNode{Expression: e} } +func fn(name string, args ...ghexpr.ConditionNode) *ghexpr.FunctionCallNode { + return &ghexpr.FunctionCallNode{FunctionName: name, Arguments: args} +} + +func assertOptimized(t *testing.T, want string, node ghexpr.ConditionNode) { + t.Helper() + result := ghexpr.OptimizeExpression(node) + require.NotNil(t, result) + assert.Equal(t, want, result.Render()) +} + +// --- nil / leaf --- + +func TestOptimizeExpression_NilInput(t *testing.T) { + assert.Nil(t, ghexpr.OptimizeExpression(nil)) +} + +func TestOptimizeExpression_Leaf(t *testing.T) { + assertOptimized(t, "github.event_name == 'issues'", expr("github.event_name == 'issues'")) + assertOptimized(t, "github.event_name", prop("github.event_name")) + assertOptimized(t, "'push'", str("push")) + assertOptimized(t, "true", blit(true)) + assertOptimized(t, "false", blit(false)) +} + +// --- NOT constant folding --- + +func TestOptimizeExpression_NOT_ConstantFolding(t *testing.T) { + assertOptimized(t, "false", &ghexpr.NotNode{Child: blit(true)}) + assertOptimized(t, "true", &ghexpr.NotNode{Child: blit(false)}) +} + +// --- double negation --- + +func TestOptimizeExpression_DoubleNegation(t *testing.T) { + assertOptimized(t, "a", &ghexpr.NotNode{Child: &ghexpr.NotNode{Child: expr("a")}}) +} + +// --- AND identity and annihilation --- + +func TestOptimizeExpression_AND_Identity(t *testing.T) { + assertOptimized(t, "a", &ghexpr.AndNode{Left: expr("a"), Right: blit(true)}) + assertOptimized(t, "a", &ghexpr.AndNode{Left: blit(true), Right: expr("a")}) +} + +func TestOptimizeExpression_AND_Annihilation(t *testing.T) { + assertOptimized(t, "false", &ghexpr.AndNode{Left: expr("a"), Right: blit(false)}) + assertOptimized(t, "false", &ghexpr.AndNode{Left: blit(false), Right: expr("a")}) +} + +// --- OR identity and annihilation --- + +func TestOptimizeExpression_OR_Identity(t *testing.T) { + assertOptimized(t, "a", &ghexpr.OrNode{Left: expr("a"), Right: blit(false)}) + assertOptimized(t, "a", &ghexpr.OrNode{Left: blit(false), Right: expr("a")}) +} + +func TestOptimizeExpression_OR_Annihilation(t *testing.T) { + assertOptimized(t, "true", &ghexpr.OrNode{Left: expr("a"), Right: blit(true)}) +} + +// --- idempotent --- + +func TestOptimizeExpression_Idempotent(t *testing.T) { + assertOptimized(t, "a", &ghexpr.AndNode{Left: expr("a"), Right: expr("a")}) + assertOptimized(t, "a", &ghexpr.OrNode{Left: expr("a"), Right: expr("a")}) +} + +// --- De Morgan --- + +func TestOptimizeExpression_DeMorgan_AND(t *testing.T) { + node := &ghexpr.NotNode{Child: &ghexpr.AndNode{Left: expr("a"), Right: expr("b")}} + assertOptimized(t, "!(a) || !(b)", node) +} + +func TestOptimizeExpression_DeMorgan_OR(t *testing.T) { + node := &ghexpr.NotNode{Child: &ghexpr.OrNode{Left: expr("a"), Right: expr("b")}} + assertOptimized(t, "(!(a)) && (!(b))", node) +} + +// --- Status functions are preserved --- + +func TestOptimizeExpression_StatusFunctionPreserved(t *testing.T) { + // always() && true should simplify but always() must survive + node := &ghexpr.AndNode{Left: fn("always"), Right: blit(true)} + result := ghexpr.OptimizeExpression(node) + require.NotNil(t, result) + rendered := result.Render() + assert.Contains(t, rendered, "always()", "status function must be preserved") +} + +// --- Disjunction short-circuit and dedup --- + +func TestOptimizeExpression_Disjunction_TrueShortCircuit(t *testing.T) { + node := &ghexpr.DisjunctionNode{Terms: []ghexpr.ConditionNode{expr("a"), blit(true), expr("b")}} + assertOptimized(t, "true", node) +} + +func TestOptimizeExpression_Disjunction_FalseFilter(t *testing.T) { + node := &ghexpr.DisjunctionNode{Terms: []ghexpr.ConditionNode{blit(false), expr("a"), blit(false)}} + assertOptimized(t, "a", node) +} + +func TestOptimizeExpression_Disjunction_Dedup(t *testing.T) { + node := &ghexpr.DisjunctionNode{Terms: []ghexpr.ConditionNode{expr("a"), expr("b"), expr("a")}} + result := ghexpr.OptimizeExpression(node) + require.NotNil(t, result) + rendered := result.Render() + assert.Equal(t, "a || b", rendered) +} diff --git a/pkg/ghexpr/optimizer_white_box_test.go b/pkg/ghexpr/optimizer_white_box_test.go new file mode 100644 index 00000000000..2a81cad7088 --- /dev/null +++ b/pkg/ghexpr/optimizer_white_box_test.go @@ -0,0 +1,175 @@ +//go:build !integration + +// White-box tests for unexported optimizer helpers. Must use package ghexpr +// (not ghexpr_test) to access unexported symbols. +package ghexpr + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Helpers identical to those in optimizer_test.go but re-declared here for +// access to unexported symbols (different test package). +func wbProp(path string) *PropertyAccessNode { return &PropertyAccessNode{PropertyPath: path} } +func wbStr(v string) *StringLiteralNode { return &StringLiteralNode{Value: v} } +func wbBool(v bool) *BooleanLiteralNode { return &BooleanLiteralNode{Value: v} } +func wbExpr(e string) *ExpressionNode { return &ExpressionNode{Expression: e} } +func wbFn(name string, args ...ConditionNode) *FunctionCallNode { + return &FunctionCallNode{FunctionName: name, Arguments: args} +} +func wbAnd(l, r ConditionNode) *AndNode { return &AndNode{Left: l, Right: r} } +func wbOr(l, r ConditionNode) *OrNode { return &OrNode{Left: l, Right: r} } +func wbNot(c ConditionNode) *NotNode { return &NotNode{Child: c} } +func wbCmp(l ConditionNode, op string, r ConditionNode) *ComparisonNode { + return &ComparisonNode{Left: l, Operator: op, Right: r} +} +func wbDisj(terms ...ConditionNode) *DisjunctionNode { + return &DisjunctionNode{Terms: terms, Multiline: false} +} + +// --------------------------------------------------------------------------- +// isBoolLiteral +// --------------------------------------------------------------------------- + +func TestIsBoolLiteral(t *testing.T) { + assert.True(t, isBoolLiteral(wbBool(true), true)) + assert.True(t, isBoolLiteral(wbBool(false), false)) + assert.False(t, isBoolLiteral(wbBool(true), false)) + assert.False(t, isBoolLiteral(wbExpr("x"), true)) +} + +// --------------------------------------------------------------------------- +// isStatusFunc +// --------------------------------------------------------------------------- + +func TestIsStatusFunc(t *testing.T) { + assert.True(t, isStatusFunc(wbFn("always")), "always is a status func") + assert.True(t, isStatusFunc(wbFn("success")), "success is a status func") + assert.True(t, isStatusFunc(wbFn("failure")), "failure is a status func") + assert.True(t, isStatusFunc(wbFn("cancelled")), "cancelled is a status func") + assert.False(t, isStatusFunc(wbFn("contains")), "contains is not a status func") + assert.False(t, isStatusFunc(wbFn("startsWith")), "startsWith is not a status func") + assert.False(t, isStatusFunc(wbExpr("x")), "ExpressionNode is not a status func") +} + +// --------------------------------------------------------------------------- +// nodesEqual +// --------------------------------------------------------------------------- + +func TestNodesEqual(t *testing.T) { + a := wbExpr("github.event_name == 'issues'") + b := wbExpr("github.event_name == 'issues'") + c := wbExpr("github.event_name == 'push'") + assert.True(t, nodesEqual(a, b), "identical renders are equal") + assert.False(t, nodesEqual(a, c), "different renders are not equal") + assert.True(t, nodesEqual(nil, nil), "nil == nil") + assert.False(t, nodesEqual(a, nil), "non-nil != nil") + assert.False(t, nodesEqual(nil, a), "nil != non-nil") +} + +// --------------------------------------------------------------------------- +// containsStatusFunc +// --------------------------------------------------------------------------- + +func TestContainsStatusFunc(t *testing.T) { + assert.True(t, containsStatusFunc(wbFn("always")), "direct always()") + assert.True(t, containsStatusFunc(wbNot(wbFn("cancelled"))), "!cancelled() contains status func") + assert.True(t, containsStatusFunc(wbAnd(wbFn("success"), wbExpr("x"))), "success() in AND") + assert.False(t, containsStatusFunc(wbExpr("github.event_name == 'issues'")), "plain expr has no status func") + assert.False(t, containsStatusFunc(wbFn("contains", wbProp("x"), wbStr("y"))), "contains has no status func") + assert.False(t, containsStatusFunc(wbAnd(wbExpr("a"), wbExpr("b"))), "plain AND has no status func") +} + +// --------------------------------------------------------------------------- +// isNegationOf +// --------------------------------------------------------------------------- + +func TestIsNegationOf(t *testing.T) { + a := wbExpr("github.event_name == 'issues'") + b := wbNot(a) + assert.True(t, isNegationOf(a, b), "A and !A are negations") + assert.True(t, isNegationOf(b, a), "!A and A are negations (symmetric)") + c := wbExpr("github.actor != 'bot'") + assert.False(t, isNegationOf(a, c), "different expressions are not negations") +} + +// --------------------------------------------------------------------------- +// collectOrTerms +// --------------------------------------------------------------------------- + +func TestCollectOrTerms_Flat(t *testing.T) { + a := wbCmp(wbProp("a"), "==", wbStr("1")) + terms := collectOrTerms(a) + require.Len(t, terms, 1, "leaf should produce one term") + assert.Equal(t, "a == '1'", terms[0].Render()) +} + +func TestCollectOrTerms_OrNode(t *testing.T) { + a := wbCmp(wbProp("a"), "==", wbStr("1")) + b := wbCmp(wbProp("b"), "==", wbStr("2")) + c := wbCmp(wbProp("c"), "==", wbStr("3")) + terms := collectOrTerms(wbOr(wbOr(a, b), c)) + require.Len(t, terms, 3, "should collect 3 terms from nested OR") +} + +func TestCollectOrTerms_DisjunctionNode(t *testing.T) { + a := wbCmp(wbProp("a"), "==", wbStr("1")) + b := wbCmp(wbProp("b"), "==", wbStr("2")) + terms := collectOrTerms(wbDisj(a, b)) + require.Len(t, terms, 2, "should collect 2 terms from DisjunctionNode") +} + +func TestCollectOrTerms_MixedOrAndDisj(t *testing.T) { + a := wbCmp(wbProp("a"), "==", wbStr("1")) + b := wbCmp(wbProp("b"), "==", wbStr("2")) + c := wbCmp(wbProp("c"), "==", wbStr("3")) + terms := collectOrTerms(wbOr(wbDisj(a, b), c)) + require.Len(t, terms, 3, "should collect 3 terms from OrNode with DisjunctionNode child") +} + +// --------------------------------------------------------------------------- +// collectAndTerms +// --------------------------------------------------------------------------- + +func TestCollectAndTerms_Flat(t *testing.T) { + a := wbCmp(wbProp("a"), "==", wbStr("1")) + terms := collectAndTerms(a) + require.Len(t, terms, 1, "leaf should produce one term") + assert.Equal(t, "a == '1'", terms[0].Render()) +} + +func TestCollectAndTerms_AndNode(t *testing.T) { + a := wbCmp(wbProp("a"), "==", wbStr("1")) + b := wbCmp(wbProp("b"), "==", wbStr("2")) + c := wbCmp(wbProp("c"), "==", wbStr("3")) + terms := collectAndTerms(wbAnd(a, wbAnd(b, c))) + require.Len(t, terms, 3, "should collect 3 terms from nested AND") +} + +// --------------------------------------------------------------------------- +// rebuildAndChain +// --------------------------------------------------------------------------- + +func TestRebuildAndChain_Single(t *testing.T) { + a := wbCmp(wbProp("a"), "==", wbStr("1")) + result := rebuildAndChain([]ConditionNode{a}) + assert.Equal(t, "a == '1'", result.Render(), "single term rebuilds without AND") +} + +func TestRebuildAndChain_Two(t *testing.T) { + a := wbCmp(wbProp("a"), "==", wbStr("1")) + b := wbCmp(wbProp("b"), "==", wbStr("2")) + result := rebuildAndChain([]ConditionNode{a, b}) + assert.Equal(t, "a == '1' && b == '2'", result.Render(), "two terms rebuild as binary AND") +} + +func TestRebuildAndChain_Three(t *testing.T) { + a := wbCmp(wbProp("a"), "==", wbStr("1")) + b := wbCmp(wbProp("b"), "==", wbStr("2")) + c := wbCmp(wbProp("c"), "==", wbStr("3")) + result := rebuildAndChain([]ConditionNode{a, b, c}) + assert.Equal(t, "a == '1' && b == '2' && c == '3'", result.Render(), "three terms rebuild as left-folded AND chain") +} diff --git a/pkg/ghexpr/parser.go b/pkg/ghexpr/parser.go new file mode 100644 index 00000000000..4102729ed8c --- /dev/null +++ b/pkg/ghexpr/parser.go @@ -0,0 +1,460 @@ +package ghexpr + +import ( + "errors" + "fmt" + "strings" + "unicode" + + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/logger" +) + +var parserLog = logger.New("ghexpr:parser") + +// StripExpressionWrapper removes the ${{ }} wrapper from s if present, trimming +// inner whitespace. If s is not wrapped, it is returned unchanged. +func StripExpressionWrapper(expression string) string { + expr := strings.TrimSpace(expression) + if strings.HasPrefix(expr, "${{") && strings.HasSuffix(expr, "}}") { + return strings.TrimSpace(expr[3 : len(expr)-2]) + } + return expr +} + +// ExpressionParser holds parser state for a single parse call. +type ExpressionParser struct { + tokens []token + pos int +} + +type token struct { + kind tokenKind + value string + pos int +} + +type tokenKind int + +const ( + tokenLiteral tokenKind = iota + tokenAnd + tokenOr + tokenNot + tokenLeftParen + tokenRightParen + tokenEOF +) + +// ParseExpression parses expression (without ${{ }} wrappers) into a ConditionNode tree. +// Supports && (AND), || (OR), ! (NOT), and parentheses for grouping. +// +// Example: +// +// node, err := ghexpr.ParseExpression("condition1 && (condition2 || !condition3)") +func ParseExpression(expression string) (ConditionNode, error) { + parserLog.Printf("Parsing expression: %s", expression) + + if strings.TrimSpace(expression) == "" { + return nil, errors.New("empty expression") + } + + p := &ExpressionParser{} + tokens, err := p.tokenize(expression) + if err != nil { + parserLog.Printf("Failed to tokenize expression: %v", err) + return nil, err + } + p.tokens = tokens + p.pos = 0 + + result, err := p.parseOrExpression() + if err != nil { + parserLog.Printf("Failed to parse expression: %v", err) + return nil, err + } + + if p.current().kind != tokenEOF { + return nil, fmt.Errorf("unexpected token '%s' at position %d", p.current().value, p.current().pos) + } + + parserLog.Printf("Successfully parsed expression with %d tokens", len(tokens)) + return result, nil +} + +// tokenize breaks the expression string into tokens. +func (p *ExpressionParser) tokenize(expression string) ([]token, error) { + parserLog.Printf("Tokenizing expression of length %d", len(expression)) + var tokens []token + i := 0 + + for i < len(expression) { + if unicode.IsSpace(rune(expression[i])) { + i++ + continue + } + + switch { + case i+1 < len(expression) && expression[i:i+2] == "&&": + tokens = append(tokens, token{tokenAnd, "&&", i}) + i += 2 + case i+1 < len(expression) && expression[i:i+2] == "||": + tokens = append(tokens, token{tokenOr, "||", i}) + i += 2 + case expression[i] == '!' && (i+1 >= len(expression) || expression[i+1] != '='): + tokens = append(tokens, token{tokenNot, "!", i}) + i++ + case expression[i] == '(': + tokens = append(tokens, token{tokenLeftParen, "(", i}) + i++ + case expression[i] == ')': + tokens = append(tokens, token{tokenRightParen, ")", i}) + i++ + default: + start := i + parenCount := 0 + + for i < len(expression) { + ch := expression[i] + + if ch == '\'' || ch == '"' || ch == '`' { + quote := ch + i++ + for i < len(expression) { + if expression[i] == quote { + i++ + break + } + if expression[i] == '\\' && i+1 < len(expression) { + i += 2 + } else { + i++ + } + } + continue + } + + if ch == '(' { + parenCount++ + i++ + continue + } else if ch == ')' { + if parenCount > 0 { + parenCount-- + i++ + continue + } + break + } + + if parenCount == 0 { + if i+1 < len(expression) { + next := expression[i : i+2] + if next == "&&" || next == "||" { + break + } + } + if ch == '!' && (i+1 >= len(expression) || expression[i+1] != '=') { + break + } + } + + i++ + } + + literal := strings.TrimSpace(expression[start:i]) + if literal == "" { + return nil, fmt.Errorf("unexpected empty literal at position %d", start) + } + tokens = append(tokens, token{tokenLiteral, literal, start}) + } + } + + tokens = append(tokens, token{tokenEOF, "", i}) + return tokens, nil +} + +func (p *ExpressionParser) parseOrExpression() (ConditionNode, error) { + left, err := p.parseAndExpression() + if err != nil { + return nil, err + } + for p.current().kind == tokenOr { + p.advance() + right, err := p.parseAndExpression() + if err != nil { + return nil, err + } + left = &OrNode{Left: left, Right: right} + } + return left, nil +} + +func (p *ExpressionParser) parseAndExpression() (ConditionNode, error) { + left, err := p.parseUnaryExpression() + if err != nil { + return nil, err + } + for p.current().kind == tokenAnd { + p.advance() + right, err := p.parseUnaryExpression() + if err != nil { + return nil, err + } + left = &AndNode{Left: left, Right: right} + } + return left, nil +} + +func (p *ExpressionParser) parseUnaryExpression() (ConditionNode, error) { + if p.current().kind == tokenNot { + p.advance() + operand, err := p.parseUnaryExpression() + if err != nil { + return nil, err + } + return &NotNode{Child: operand}, nil + } + return p.parsePrimaryExpression() +} + +func (p *ExpressionParser) parsePrimaryExpression() (ConditionNode, error) { + if parserLog.Enabled() { + parserLog.Printf("Parsing primary expression at token: %s", p.current().value) + } + switch p.current().kind { + case tokenLeftParen: + p.advance() + expr, err := p.parseOrExpression() + if err != nil { + return nil, err + } + if p.current().kind != tokenRightParen { + return nil, fmt.Errorf("expected ')' at position %d", p.current().pos) + } + p.advance() + return expr, nil + + case tokenLiteral: + literal := p.current().value + p.advance() + return &ExpressionNode{Expression: literal}, nil + + default: + return nil, fmt.Errorf("unexpected token '%s' at position %d", p.current().value, p.current().pos) + } +} + +func (p *ExpressionParser) current() token { + if p.pos >= len(p.tokens) { + return token{tokenEOF, "", -1} + } + return p.tokens[p.pos] +} + +func (p *ExpressionParser) advance() { + if p.pos < len(p.tokens) { + p.pos++ + } +} + +// VisitExpressionTree walks a ConditionNode tree and calls visitor for each +// [ExpressionNode] leaf found in the tree. Walking stops and the error is +// returned immediately if visitor returns a non-nil error. +func VisitExpressionTree(node ConditionNode, visitor func(expr *ExpressionNode) error) error { + if node == nil { + return nil + } + switch n := node.(type) { + case *ExpressionNode: + return visitor(n) + case *AndNode: + if err := VisitExpressionTree(n.Left, visitor); err != nil { + return err + } + return VisitExpressionTree(n.Right, visitor) + case *OrNode: + if err := VisitExpressionTree(n.Left, visitor); err != nil { + return err + } + return VisitExpressionTree(n.Right, visitor) + case *NotNode: + return VisitExpressionTree(n.Child, visitor) + case *DisjunctionNode: + for _, term := range n.Terms { + if err := VisitExpressionTree(term, visitor); err != nil { + return err + } + } + } + // ComparisonNode, PropertyAccessNode, FunctionCallNode etc. are opaque leaves. + return nil +} + +// BreakLongExpression breaks a long expression string into multiple lines at +// logical operators (|| and &&) for readability. Strings inside quote +// characters are never split. +func BreakLongExpression(expression string) []string { + if len(expression) <= int(constants.MaxExpressionLineLength) { + return []string{expression} + } + + parserLog.Printf("Breaking long expression: length=%d", len(expression)) + + var lines []string + var current strings.Builder + i := 0 + + for i < len(expression) { + char := expression[i] + + if char == '\'' || char == '"' || char == '`' { + quote := char + current.WriteByte(char) + i++ + for i < len(expression) { + current.WriteByte(expression[i]) + if expression[i] == quote { + i++ + break + } + if expression[i] == '\\' && i+1 < len(expression) { + i++ + if i < len(expression) { + current.WriteByte(expression[i]) + } + } + i++ + } + continue + } + + if i+2 <= len(expression) { + next2 := expression[i : i+2] + if next2 == "||" || next2 == "&&" { + current.WriteString(next2) + i += 2 + if trimmed := strings.TrimSpace(current.String()); len(trimmed) > int(constants.ExpressionBreakThreshold) { + lines = append(lines, trimmed) + current.Reset() + for i < len(expression) && (expression[i] == ' ' || expression[i] == '\t') { + i++ + } + continue + } + continue + } + } + + current.WriteByte(char) + i++ + } + + if trimmed := strings.TrimSpace(current.String()); trimmed != "" { + lines = append(lines, trimmed) + } + + var finalLines []string + for _, line := range lines { + if len(line) > int(constants.MaxExpressionLineLength) { + finalLines = append(finalLines, BreakAtParentheses(line)...) + } else { + finalLines = append(finalLines, line) + } + } + + return finalLines +} + +// BreakAtParentheses attempts to break long lines at parentheses after function calls. +func BreakAtParentheses(expression string) []string { + if len(expression) <= int(constants.MaxExpressionLineLength) { + return []string{expression} + } + + parserLog.Printf("Breaking expression at parentheses: length=%d", len(expression)) + + var lines []string + var current strings.Builder + parenDepth := 0 + + for i := 0; i < len(expression); i++ { + char := expression[i] + current.WriteByte(char) + + switch char { + case '(': + parenDepth++ + case ')': + parenDepth-- + if parenDepth == 0 && current.Len() > 80 && i < len(expression)-1 { + j := i + 1 + for j < len(expression) && (expression[j] == ' ' || expression[j] == '\t') { + j++ + } + if j+1 < len(expression) && (expression[j:j+2] == "||" || expression[j:j+2] == "&&") { + current.WriteString(expression[i+1 : j+2]) + lines = append(lines, strings.TrimSpace(current.String())) + current.Reset() + i = j + 2 - 1 + for i+1 < len(expression) && (expression[i+1] == ' ' || expression[i+1] == '\t') { + i++ + } + } + } + } + } + + if trimmed := strings.TrimSpace(current.String()); trimmed != "" { + lines = append(lines, trimmed) + } + + return lines +} + +// HasNewlineInStringLiteral returns true if s contains an actual newline +// character inside a single-quoted GitHub Actions expression string literal. +// This is used to determine whether the YAML if: value needs special encoding. +func HasNewlineInStringLiteral(s string) bool { + inString := false + i := 0 + for i < len(s) { + ch := s[i] + if ch == '\'' { + if inString && i+1 < len(s) && s[i+1] == '\'' { + i += 2 + continue + } + inString = !inString + } else if ch == '\n' && inString { + return true + } + i++ + } + return false +} + +// EscapeForYAMLDoubleQuoted escapes s so it can be placed inside a YAML +// double-quoted scalar ("..."). YAML double-quoted scalars interpret \n, \r, +// \t, \\ and \" escape sequences, so the corresponding actual characters are +// converted to their two-character representations. +func EscapeForYAMLDoubleQuoted(s string) string { + var b strings.Builder + for i := range len(s) { + switch s[i] { + case '\\': + b.WriteString(`\\`) + case '"': + b.WriteString(`\"`) + case '\n': + b.WriteString(`\n`) + case '\r': + b.WriteString(`\r`) + case '\t': + b.WriteString(`\t`) + default: + b.WriteByte(s[i]) + } + } + return b.String() +} diff --git a/pkg/ghexpr/parser_test.go b/pkg/ghexpr/parser_test.go new file mode 100644 index 00000000000..544677417bd --- /dev/null +++ b/pkg/ghexpr/parser_test.go @@ -0,0 +1,103 @@ +//go:build !integration + +package ghexpr_test + +import ( + "testing" + + "github.com/github/gh-aw/pkg/ghexpr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestParseExpression covers basic parsing behavior. +func TestParseExpression(t *testing.T) { + tests := []struct { + name string + input string + rendered string + wantErr bool + }{ + {"single literal", "github.event_name", "github.event_name", false}, + {"AND", "a && b", "(a) && (b)", false}, + {"OR", "a || b", "a || b", false}, + {"NOT", "!a", "!(a)", false}, + {"AND higher precedence than OR", "a || b && c", "a || (b) && (c)", false}, + {"NOT with function call (parsed as opaque literal)", "!cancelled()", "!(cancelled())", false}, + {"parentheses override precedence", "(a || b) && c", "(a || b) && (c)", false}, + {"empty input", "", "", true}, + {"empty expression", " ", "", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + node, err := ghexpr.ParseExpression(tt.input) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.NotNil(t, node) + assert.Equal(t, tt.rendered, node.Render()) + }) + } +} + +// TestStripExpressionWrapper tests the wrapper-stripping helper. +func TestStripExpressionWrapper(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"${{ github.actor }}", "github.actor"}, + {"${{github.actor}}", "github.actor"}, + {"github.actor", "github.actor"}, + {"", ""}, + {"${{ }}", ""}, + {" ${{ foo }} ", "foo"}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + assert.Equal(t, tt.expected, ghexpr.StripExpressionWrapper(tt.input)) + }) + } +} + +// TestBreakLongExpression tests line-breaking behavior. +func TestBreakLongExpression(t *testing.T) { + short := "github.event_name == 'push'" + result := ghexpr.BreakLongExpression(short) + assert.Len(t, result, 1, "short expression should not be broken") + assert.Equal(t, short, result[0]) +} + +// TestVisitExpressionTree tests the tree-visitor utility. +func TestVisitExpressionTree(t *testing.T) { + node := &ghexpr.AndNode{ + Left: &ghexpr.ExpressionNode{Expression: "a"}, + Right: &ghexpr.OrNode{ + Left: &ghexpr.ExpressionNode{Expression: "b"}, + Right: &ghexpr.ExpressionNode{Expression: "c"}, + }, + } + + var visited []string + err := ghexpr.VisitExpressionTree(node, func(e *ghexpr.ExpressionNode) error { + visited = append(visited, e.Expression) + return nil + }) + require.NoError(t, err) + assert.Equal(t, []string{"a", "b", "c"}, visited) +} + +// TestHasNewlineInStringLiteral tests the newline-in-string-literal detector. +func TestHasNewlineInStringLiteral(t *testing.T) { + assert.True(t, ghexpr.HasNewlineInStringLiteral("contains(github.event.comment.body, 'text\nmore')")) + assert.False(t, ghexpr.HasNewlineInStringLiteral("github.event_name == 'push'")) +} + +// TestEscapeForYAMLDoubleQuoted tests YAML escaping. +func TestEscapeForYAMLDoubleQuoted(t *testing.T) { + assert.Equal(t, `line1\nline2`, ghexpr.EscapeForYAMLDoubleQuoted("line1\nline2")) + assert.Equal(t, `a\\b`, ghexpr.EscapeForYAMLDoubleQuoted(`a\b`)) + assert.Equal(t, `say \"hi\"`, ghexpr.EscapeForYAMLDoubleQuoted(`say "hi"`)) +} diff --git a/pkg/ghexpr/parser_white_box_test.go b/pkg/ghexpr/parser_white_box_test.go new file mode 100644 index 00000000000..f947a9ec466 --- /dev/null +++ b/pkg/ghexpr/parser_white_box_test.go @@ -0,0 +1,38 @@ +//go:build !integration + +// White-box tests for unexported parser types and methods. Must use package +// ghexpr (not ghexpr_test) to access unexported symbols. +package ghexpr + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestExpressionParserCurrentWithEmptyTokens tests the current() method when +// the token slice is empty. +func TestExpressionParserCurrentWithEmptyTokens(t *testing.T) { + p := &ExpressionParser{ + tokens: []token{}, + pos: 0, + } + + result := p.current() + assert.Equal(t, tokenEOF, result.kind, "current() with empty tokens should return EOF token") + assert.Equal(t, -1, result.pos, "current() with empty tokens should return pos -1") +} + +// TestExpressionParserCurrentBeyondLength tests the current() method when pos +// is past the end of the token slice. +func TestExpressionParserCurrentBeyondLength(t *testing.T) { + p := &ExpressionParser{ + tokens: []token{ + {tokenLiteral, "test", 0}, + }, + pos: 5, // Beyond array length + } + + result := p.current() + assert.Equal(t, tokenEOF, result.kind, "current() with pos beyond length should return EOF token") +} diff --git a/pkg/ghexpr/patterns.go b/pkg/ghexpr/patterns.go new file mode 100644 index 00000000000..6dfc0deb86a --- /dev/null +++ b/pkg/ghexpr/patterns.go @@ -0,0 +1,89 @@ +package ghexpr + +import ( + "regexp" + "strings" +) + +// HasExpressionMarker reports whether s contains a GitHub Actions expression +// opening marker ("${{"). This is a permissive check used when partial +// expressions should be treated as dynamic values. +func HasExpressionMarker(s string) bool { + return strings.Contains(s, "${{") +} + +// ContainsExpression reports whether s contains a complete, non-empty GitHub +// Actions expression — i.e. a "${{" marker followed by at least one character +// before a closing "}}". +func ContainsExpression(s string) bool { + _, afterOpen, found := strings.Cut(s, "${{") + if !found { + return false + } + closeIdx := strings.Index(afterOpen, "}}") + return closeIdx > 0 +} + +// IsExpression reports whether the entire string s is a GitHub Actions +// expression (starts with "${{" and ends with "}}"). +func IsExpression(s string) bool { + return strings.HasPrefix(s, "${{") && strings.HasSuffix(s, "}}") +} + +// Core Expression Patterns + +var ( + // ExpressionPattern matches GitHub Actions expressions: ${{ ... }} + // Uses non-greedy matching to handle multiple expressions in one string. + ExpressionPattern = regexp.MustCompile(`\$\{\{(.*?)\}\}`) + + // ExpressionPatternDotAll matches expressions with dotall mode enabled so + // that "." also matches newlines. Useful for multi-line expression bodies. + ExpressionPatternDotAll = regexp.MustCompile(`(?s)\$\{\{(.*?)\}\}`) +) + +// Template Patterns + +var ( + // InlineExpressionPattern matches inline ${{ … }} expressions in template strings. + InlineExpressionPattern = regexp.MustCompile(`\$\{\{[^}]+\}\}`) + + // TemplateIfPattern matches {{#if condition }} template conditionals. + // Captures the condition expression (which may itself contain ${{ … }}). + // + // Expression group: (?:\$\{\{[^\}]*\}\}|[^\}\{]|\{[^\{])* + // - \$\{\{[^\}]*\}\} — already-wrapped ${{ ... }} expression + // - [^\}\{] — any character that is not } or { + // - \{[^\{] — a { not immediately followed by another { (handles ${ env refs) + TemplateIfPattern = regexp.MustCompile(`\{\{#if\s+((?:\$\{\{[^\}]*\}\}|[^\}\{]|\{[^\{])*)\s*\}\}`) + + // TemplateElseIfPattern matches elseif/else-if/else_if template conditionals in all + // supported syntax variants: + // {{#elseif expr}} {{#else-if expr}} {{#else_if expr}} + // {{elseif expr}} {{else-if expr}} {{else_if expr}} + TemplateElseIfPattern = regexp.MustCompile(`\{\{#?else[-_]?if\s+((?:\$\{\{[^\}]*\}\}|[^\}\{]|\{[^\{])*)\s*\}\}`) +) + +// Literal and Operator Patterns + +var ( + // StringLiteralPattern matches string literals in single quotes, double quotes, + // or backticks. Example: 'hello', "world", `template`. + StringLiteralPattern = regexp.MustCompile(`^'[^']*'$|^"[^"]*"$|^` + "`[^`]*`$") + + // NumberLiteralPattern matches integer and decimal numeric literals (with + // optional leading minus). Example: 42, -3.14, 0.5. + NumberLiteralPattern = regexp.MustCompile(`^-?\d+(\.\d+)?$`) + + // RangePattern matches numeric range strings such as "1-10" or "100-200". + RangePattern = regexp.MustCompile(`^\d+-\d+$`) + + // OrPattern matches a logical OR expression, capturing both sides. + // Example: "value1 || value2". + OrPattern = regexp.MustCompile(`^(.+?)\s*\|\|\s*(.+)$`) + + // ComparisonExtractionPattern extracts the left-hand property access from a + // comparison expression. + // Example: "github.workflow == 'CI'" → captures "github.workflow". + ComparisonExtractionPattern = regexp.MustCompile(`([a-zA-Z_][a-zA-Z0-9_.]*)\s*(?:==|!=|<|>|<=|>=)\s*`) +) diff --git a/pkg/ghexpr/patterns_test.go b/pkg/ghexpr/patterns_test.go new file mode 100644 index 00000000000..8113472b02b --- /dev/null +++ b/pkg/ghexpr/patterns_test.go @@ -0,0 +1,83 @@ +//go:build !integration + +package ghexpr_test + +import ( + "testing" + + "github.com/github/gh-aw/pkg/ghexpr" + "github.com/stretchr/testify/assert" +) + +// TestHasExpressionMarker tests the permissive opening-marker check. +func TestHasExpressionMarker(t *testing.T) { + assert.True(t, ghexpr.HasExpressionMarker("${{ github.actor }}")) + assert.True(t, ghexpr.HasExpressionMarker("prefix ${{ x }}")) + assert.False(t, ghexpr.HasExpressionMarker("no expression here")) + assert.False(t, ghexpr.HasExpressionMarker("")) +} + +// TestContainsExpression tests the complete-expression check. +func TestContainsExpression(t *testing.T) { + assert.True(t, ghexpr.ContainsExpression("${{ github.actor }}")) + assert.True(t, ghexpr.ContainsExpression("text ${{ a }} text")) + assert.False(t, ghexpr.ContainsExpression("${{}}"), "empty inner content") + assert.False(t, ghexpr.ContainsExpression("plain")) + assert.False(t, ghexpr.ContainsExpression("${{ no closing")) +} + +// TestIsExpression tests the whole-string expression check. +func TestIsExpression(t *testing.T) { + assert.True(t, ghexpr.IsExpression("${{ github.actor }}")) + assert.True(t, ghexpr.IsExpression("${{github.actor}}")) + assert.False(t, ghexpr.IsExpression("prefix ${{ github.actor }}")) + assert.False(t, ghexpr.IsExpression("${{ github.actor }} suffix")) + assert.False(t, ghexpr.IsExpression("plain text")) +} + +// TestExpressionPattern verifies the compiled regex matches correctly. +func TestExpressionPattern(t *testing.T) { + m := ghexpr.ExpressionPattern.FindStringSubmatch("${{ github.actor }}") + assert.Len(t, m, 2) + assert.Equal(t, " github.actor ", m[1]) + + assert.False(t, ghexpr.ExpressionPattern.MatchString("plain text")) +} + +// TestStringLiteralPattern covers string-literal pattern matching. +func TestStringLiteralPattern(t *testing.T) { + assert.True(t, ghexpr.StringLiteralPattern.MatchString("'hello'")) + assert.True(t, ghexpr.StringLiteralPattern.MatchString(`"world"`)) + assert.False(t, ghexpr.StringLiteralPattern.MatchString("no quotes")) +} + +// TestNumberLiteralPattern covers number-literal pattern matching. +func TestNumberLiteralPattern(t *testing.T) { + assert.True(t, ghexpr.NumberLiteralPattern.MatchString("42")) + assert.True(t, ghexpr.NumberLiteralPattern.MatchString("-3.14")) + assert.True(t, ghexpr.NumberLiteralPattern.MatchString("0")) + assert.False(t, ghexpr.NumberLiteralPattern.MatchString("abc")) +} + +// TestOrPattern verifies OR-pattern captures both sides. +func TestOrPattern(t *testing.T) { + m := ghexpr.OrPattern.FindStringSubmatch("left || right") + assert.Len(t, m, 3) + assert.Equal(t, "left", m[1]) + assert.Equal(t, "right", m[2]) +} + +// TestTemplateIfPattern verifies the template-if pattern. +func TestTemplateIfPattern(t *testing.T) { + m := ghexpr.TemplateIfPattern.FindStringSubmatch("{{#if github.actor }}") + assert.NotNil(t, m) + assert.Contains(t, m[1], "github.actor") +} + +// TestRangePattern verifies range pattern matching. +func TestRangePattern(t *testing.T) { + assert.True(t, ghexpr.RangePattern.MatchString("1-10")) + assert.True(t, ghexpr.RangePattern.MatchString("100-200")) + assert.False(t, ghexpr.RangePattern.MatchString("abc")) + assert.False(t, ghexpr.RangePattern.MatchString("1")) +} diff --git a/pkg/workflow/expression_coverage_test.go b/pkg/workflow/expression_coverage_test.go index 5067f55102c..c851c83df78 100644 --- a/pkg/workflow/expression_coverage_test.go +++ b/pkg/workflow/expression_coverage_test.go @@ -148,31 +148,6 @@ func TestVisitExpressionTreeWithDifferentNodeTypes(t *testing.T) { } } -// TestExpressionParserCurrentWithEmptyTokens tests the current() method edge case -func TestExpressionParserCurrentWithEmptyTokens(t *testing.T) { - parser := &ExpressionParser{ - tokens: []token{}, - pos: 0, - } - - result := parser.current() - assert.Equal(t, tokenEOF, result.kind, "current() with empty tokens should return EOF token") - assert.Equal(t, -1, result.pos, "current() with empty tokens should return pos -1") -} - -// TestExpressionParserCurrentBeyondLength tests the current() method when pos >= len(tokens) -func TestExpressionParserCurrentBeyondLength(t *testing.T) { - parser := &ExpressionParser{ - tokens: []token{ - {tokenLiteral, "test", 0}, - }, - pos: 5, // Beyond array length - } - - result := parser.current() - assert.Equal(t, tokenEOF, result.kind, "current() with pos beyond length should return EOF token") -} - // TestParseExpressionEmptyString tests ParseExpression with empty string func TestParseExpressionEmptyString(t *testing.T) { tests := []struct { diff --git a/pkg/workflow/expression_nodes.go b/pkg/workflow/expression_nodes.go index 313674a96b8..c0c1ef22b07 100644 --- a/pkg/workflow/expression_nodes.go +++ b/pkg/workflow/expression_nodes.go @@ -1,211 +1,39 @@ +// This file re-exports the GitHub Actions expression AST types from pkg/ghexpr +// as type aliases so that all existing code in pkg/workflow continues to compile +// without modification. New code should import pkg/ghexpr directly. package workflow -import ( - "strings" +import "github.com/github/gh-aw/pkg/ghexpr" - "github.com/github/gh-aw/pkg/logger" -) +// ConditionNode re-exports [ghexpr.ConditionNode]. +type ConditionNode = ghexpr.ConditionNode -var expressionNodesLog = logger.New("workflow:expression_nodes") +// ExpressionNode re-exports [ghexpr.ExpressionNode]. +type ExpressionNode = ghexpr.ExpressionNode -// ConditionNode represents a node in a condition expression tree -type ConditionNode interface { - Render() string -} +// AndNode re-exports [ghexpr.AndNode]. +type AndNode = ghexpr.AndNode -// Compile-time assertions: all ConditionNode implementation types must satisfy the interface. -var ( - _ ConditionNode = (*ExpressionNode)(nil) - _ ConditionNode = (*AndNode)(nil) - _ ConditionNode = (*OrNode)(nil) - _ ConditionNode = (*NotNode)(nil) - _ ConditionNode = (*DisjunctionNode)(nil) - _ ConditionNode = (*FunctionCallNode)(nil) - _ ConditionNode = (*PropertyAccessNode)(nil) - _ ConditionNode = (*StringLiteralNode)(nil) - _ ConditionNode = (*BooleanLiteralNode)(nil) - _ ConditionNode = (*ComparisonNode)(nil) -) +// OrNode re-exports [ghexpr.OrNode]. +type OrNode = ghexpr.OrNode -// ExpressionNode represents a leaf expression -type ExpressionNode struct { - Expression string - Description string // Optional comment/description for the expression -} +// NotNode re-exports [ghexpr.NotNode]. +type NotNode = ghexpr.NotNode -func (e *ExpressionNode) Render() string { - return e.Expression -} +// DisjunctionNode re-exports [ghexpr.DisjunctionNode]. +type DisjunctionNode = ghexpr.DisjunctionNode -// AndNode represents an AND operation between two conditions -type AndNode struct { - Left, Right ConditionNode -} +// FunctionCallNode re-exports [ghexpr.FunctionCallNode]. +type FunctionCallNode = ghexpr.FunctionCallNode -// needsParensAsAndOperand returns true when child must be wrapped in parentheses -// when it appears as an operand of an && expression. Or-level nodes and opaque -// ExpressionNodes must be wrapped to preserve operator precedence. -// NotNode is also wrapped to prevent the leading ! from becoming a YAML type-tag -// indicator when the full expression is placed in an `if:` YAML value. -func needsParensAsAndOperand(child ConditionNode) bool { - switch child.(type) { - case *OrNode, *DisjunctionNode, *ExpressionNode, *NotNode: - return true - } - return false -} +// PropertyAccessNode re-exports [ghexpr.PropertyAccessNode]. +type PropertyAccessNode = ghexpr.PropertyAccessNode -func (a *AndNode) Render() string { - leftStr := a.Left.Render() - if needsParensAsAndOperand(a.Left) { - leftStr = "(" + leftStr + ")" - } - rightStr := a.Right.Render() - if needsParensAsAndOperand(a.Right) { - rightStr = "(" + rightStr + ")" - } - return leftStr + " && " + rightStr -} +// StringLiteralNode re-exports [ghexpr.StringLiteralNode]. +type StringLiteralNode = ghexpr.StringLiteralNode -// OrNode represents an OR operation between two conditions -type OrNode struct { - Left, Right ConditionNode -} +// BooleanLiteralNode re-exports [ghexpr.BooleanLiteralNode]. +type BooleanLiteralNode = ghexpr.BooleanLiteralNode -// || has the lowest precedence of any boolean operator, so no child of an OR -// expression ever needs explicit parentheses to preserve evaluation order. -func (o *OrNode) Render() string { - return o.Left.Render() + " || " + o.Right.Render() -} - -// NotNode represents a NOT operation on a condition -type NotNode struct { - Child ConditionNode -} - -func (n *NotNode) Render() string { - // For simple function calls like cancelled(), render as !cancelled() instead of !(cancelled()) - // This prevents GitHub Actions from interpreting the extra parentheses as an object structure - if _, isFunctionCall := n.Child.(*FunctionCallNode); isFunctionCall { - return "!" + n.Child.Render() - } - return "!(" + n.Child.Render() + ")" -} - -// DisjunctionNode represents an OR operation with multiple terms to avoid deep nesting -type DisjunctionNode struct { - Terms []ConditionNode - Multiline bool // If true, render each term on separate line with comments -} - -func (d *DisjunctionNode) Render() string { - if len(d.Terms) == 0 { - return "" - } - if len(d.Terms) == 1 { - return d.Terms[0].Render() - } - - // Use multiline rendering if enabled - if d.Multiline { - return d.RenderMultiline() - } - - expressionNodesLog.Printf("Rendering inline disjunction with %d terms", len(d.Terms)) - var parts []string - for _, term := range d.Terms { - parts = append(parts, term.Render()) - } - return strings.Join(parts, " || ") -} - -// RenderMultiline renders the disjunction with each term on a separate line, -// including comments for expressions that have descriptions -func (d *DisjunctionNode) RenderMultiline() string { - if len(d.Terms) == 0 { - return "" - } - if len(d.Terms) == 1 { - return d.Terms[0].Render() - } - - expressionNodesLog.Printf("Rendering multiline disjunction with %d terms", len(d.Terms)) - - var lines []string - for i, term := range d.Terms { - var line string - - // Add comment if this is an ExpressionNode with a description - if expr, ok := term.(*ExpressionNode); ok && expr.Description != "" { - line = "# " + expr.Description + "\n" - } - - // Add the expression with OR operator (except for the last term) - if i < len(d.Terms)-1 { - line += term.Render() + " ||" - } else { - line += term.Render() - } - - lines = append(lines, line) - } - - return strings.Join(lines, "\n") -} - -// FunctionCallNode represents a function call expression like contains(array, value) -type FunctionCallNode struct { - FunctionName string - Arguments []ConditionNode -} - -func (f *FunctionCallNode) Render() string { - var args []string - for _, arg := range f.Arguments { - args = append(args, arg.Render()) - } - return f.FunctionName + "(" + strings.Join(args, ", ") + ")" -} - -// PropertyAccessNode represents property access like github.event.action -type PropertyAccessNode struct { - PropertyPath string -} - -func (p *PropertyAccessNode) Render() string { - return p.PropertyPath -} - -// StringLiteralNode represents a string literal value -type StringLiteralNode struct { - Value string -} - -func (s *StringLiteralNode) Render() string { - // GitHub Actions single-quoted strings escape embedded single quotes by doubling them. - escaped := strings.ReplaceAll(s.Value, "'", "''") - return "'" + escaped + "'" -} - -// BooleanLiteralNode represents a boolean literal value -type BooleanLiteralNode struct { - Value bool -} - -func (b *BooleanLiteralNode) Render() string { - if b.Value { - return "true" - } - return "false" -} - -// ComparisonNode represents comparison operations like ==, !=, <, >, <=, >= -type ComparisonNode struct { - Left ConditionNode - Operator string - Right ConditionNode -} - -func (c *ComparisonNode) Render() string { - return c.Left.Render() + " " + c.Operator + " " + c.Right.Render() -} +// ComparisonNode re-exports [ghexpr.ComparisonNode]. +type ComparisonNode = ghexpr.ComparisonNode diff --git a/pkg/workflow/expression_optimizer.go b/pkg/workflow/expression_optimizer.go index 60c976f1fcb..616a97e9457 100644 --- a/pkg/workflow/expression_optimizer.go +++ b/pkg/workflow/expression_optimizer.go @@ -1,541 +1,9 @@ +// This file re-exports the GitHub Actions expression optimizer from pkg/ghexpr +// so that existing code in pkg/workflow continues to compile without modification. +// New code should import pkg/ghexpr directly. package workflow -import ( - "slices" +import "github.com/github/gh-aw/pkg/ghexpr" - "github.com/github/gh-aw/pkg/logger" -) - -var expressionOptimizerLog = logger.New("workflow:expression_optimizer") - -// OptimizeExpression applies boolean algebra simplifications to a ConditionNode tree, -// returning an equivalent but potentially simpler and shorter expression. -// -// Rules applied (bottom-up, fixpoint iteration): -// -// Constant folding: !true → false, !false → true -// Double negation: !!A → A -// Boolean identity: A && true → A, A || false → A -// Boolean annihilation: A && false → false, A || true → true -// Idempotent law: A && A → A, A || A → A -// Complement law: A && !A → false, A || !A → true -// De Morgan (AND): !(A && B) → !A || !B -// De Morgan (OR): !(A || B) → !A && !B -// Absorption (AND): A && (A || B) → A -// Absorption (OR): A || (A && B) → A -// Subsumption (disj): disj(A, A&&B, …) → disj(A, …) [A&&B subsumed by A] -// DisjunctionNode: deduplication, false-filtering, true short-circuit -// -// SAFETY: GitHub Actions status functions (always, success, failure, cancelled) -// have semantics beyond plain booleans – they control step execution based on -// prior step/job status. The optimizer therefore never eliminates a status -// function call from an expression; it only applies rules when both operands of -// && / || are free of status functions. -// -// Execution is bounded: at most maxOptimizationPasses bottom-up passes are -// performed so the optimizer always terminates in O(n * maxOptimizationPasses) -// time relative to the number of nodes in the tree. -func OptimizeExpression(node ConditionNode) ConditionNode { - if node == nil { - return nil - } - - const maxOptimizationPasses = 10 - - current := node - for pass := range maxOptimizationPasses { - next := optimizeNode(current) - // Stop early when the rendered form has stabilised (fixed point). - if next.Render() == current.Render() { - expressionOptimizerLog.Printf("Expression stabilised after %d pass(es)", pass+1) - break - } - current = next - } - return current -} - -// optimizeNode performs a single bottom-up optimisation pass over the tree. -// It recurses into children first so that simplifications at lower levels can -// unlock further simplifications at higher levels in the same pass. -func optimizeNode(node ConditionNode) ConditionNode { - switch n := node.(type) { - case *AndNode: - return optimizeAndNode(n) - case *OrNode: - return optimizeOrNode(n) - case *NotNode: - return optimizeNotNode(n) - case *DisjunctionNode: - return optimizeDisjunctionNode(n) - default: - // Leaf nodes (ExpressionNode, ComparisonNode, FunctionCallNode, - // PropertyAccessNode, StringLiteralNode, BooleanLiteralNode) are returned - // unchanged. - return node - } -} - -// --- helper predicates ------------------------------------------------------- - -// isBoolLiteral returns true when node is a BooleanLiteralNode with the given value. -func isBoolLiteral(node ConditionNode, value bool) bool { - lit, ok := node.(*BooleanLiteralNode) - return ok && lit.Value == value -} - -// isStatusFunc returns true when node is a call to one of the GitHub Actions -// status-check functions: always(), success(), failure(), cancelled(). -// These functions change the execution status of a step/job and must not be -// removed from an expression by boolean-algebra rules. -func isStatusFunc(node ConditionNode) bool { - fn, ok := node.(*FunctionCallNode) - if !ok { - return false - } - switch fn.FunctionName { - case "always", "success", "failure", "cancelled": - return true - } - return false -} - -// nodesEqual returns true when a and b render to identical strings. -// This is used as a conservative structural-equality test: if two nodes -// render identically they are semantically equivalent in the expression. -func nodesEqual(a, b ConditionNode) bool { - if a == nil || b == nil { - return a == nil && b == nil - } - return a.Render() == b.Render() -} - -// isNegationOf returns true when b is the logical negation of a or a is the -// logical negation of b (handles both A / !A and !A / A cases). -func isNegationOf(a, b ConditionNode) bool { - if notB, ok := b.(*NotNode); ok && nodesEqual(a, notB.Child) { - return true - } - if notA, ok := a.(*NotNode); ok && nodesEqual(notA.Child, b) { - return true - } - return false -} - -// containsStatusFunc returns true when any node in the tree is a status function. -// Used to gate complement / idempotent rules that must not fire on expressions -// containing status functions. -func containsStatusFunc(node ConditionNode) bool { - if isStatusFunc(node) { - return true - } - switch n := node.(type) { - case *AndNode: - return containsStatusFunc(n.Left) || containsStatusFunc(n.Right) - case *OrNode: - return containsStatusFunc(n.Left) || containsStatusFunc(n.Right) - case *NotNode: - return containsStatusFunc(n.Child) - case *DisjunctionNode: - return slices.ContainsFunc(n.Terms, containsStatusFunc) - case *FunctionCallNode: - return slices.ContainsFunc(n.Arguments, containsStatusFunc) - } - return false -} - -// collectOrTerms recursively flattens a chain of OrNode / DisjunctionNode into -// a flat slice of leaf terms. This allows the DisjunctionNode optimiser (which -// already performs dedup, false-filtering and true short-circuit) to operate on -// the entire or-chain in a single pass. -func collectOrTerms(node ConditionNode) []ConditionNode { - switch n := node.(type) { - case *OrNode: - return append(collectOrTerms(n.Left), collectOrTerms(n.Right)...) - case *DisjunctionNode: - terms := make([]ConditionNode, 0, len(n.Terms)) - for _, t := range n.Terms { - terms = append(terms, collectOrTerms(t)...) - } - return terms - } - return []ConditionNode{node} -} - -// collectAndTerms recursively flattens a chain of AndNode into a flat slice of -// leaf terms so that cross-chain deduplication can be performed in a single pass. -func collectAndTerms(node ConditionNode) []ConditionNode { - if and, ok := node.(*AndNode); ok { - return append(collectAndTerms(and.Left), collectAndTerms(and.Right)...) - } - return []ConditionNode{node} -} - -// rebuildAndChain assembles a left-folded AndNode chain from a non-empty slice. -func rebuildAndChain(terms []ConditionNode) ConditionNode { - if len(terms) == 1 { - return terms[0] - } - result := ConditionNode(&AndNode{Left: terms[0], Right: terms[1]}) - for _, t := range terms[2:] { - result = &AndNode{Left: result, Right: t} - } - return result -} - -// termSubsumedBy returns true when cand is subsumed by sub, meaning sub |= cand -// (cand is "more specific"). In a disjunction this makes cand redundant: -// disj(sub, cand) = sub. Only applies when neither term contains a status func. -// -// Example: sub=A, cand=A&&B → every model satisfying A&&B also satisfies A, -// so A already covers A&&B in a disjunction. -func termSubsumedBy(cand, sub ConditionNode) bool { - if nodesEqual(cand, sub) { - return false // identical terms are handled by dedup, not subsumption - } - if containsStatusFunc(cand) || containsStatusFunc(sub) { - return false - } - for _, ct := range collectAndTerms(cand) { - if nodesEqual(ct, sub) { - return true - } - } - return false -} - -// --- node-specific optimisers ------------------------------------------------ - -func optimizeAndNode(n *AndNode) ConditionNode { - // Bottom-up: optimise children first. - left := optimizeNode(n.Left) - right := optimizeNode(n.Right) - - // Annihilation: A && false → false (before flattening for early exit). - if isBoolLiteral(left, false) || isBoolLiteral(right, false) { - expressionOptimizerLog.Printf("AND annihilation: %s && %s → false", left.Render(), right.Render()) - return &BooleanLiteralNode{Value: false} - } - - // Flatten the entire AND chain so that rules can operate across nesting levels. - // e.g. A && (A && B) → [A, A, B] → dedup → [A, B] → A && B - terms := collectAndTerms(&AndNode{Left: left, Right: right}) - - // Annihilation within the flat list (covers cases after child optimisation). - for _, t := range terms { - if isBoolLiteral(t, false) { - expressionOptimizerLog.Printf("AND annihilation (flatten): false term → false") - return &BooleanLiteralNode{Value: false} - } - } - - // Identity: filter out `true` literals, but keep them when any term is a - // status function (to preserve status-function semantics). - hasStatusFuncInTerms := slices.ContainsFunc(terms, containsStatusFunc) - filtered := make([]ConditionNode, 0, len(terms)) - for _, t := range terms { - if isBoolLiteral(t, true) && !hasStatusFuncInTerms { - expressionOptimizerLog.Printf("AND identity (flatten): removed true literal") - continue - } - filtered = append(filtered, t) - } - if len(filtered) == 0 { - return &BooleanLiteralNode{Value: true} - } - - // Deduplicate terms by rendered form (safe even for status functions). - seen := make(map[string]struct{}, len(filtered)) - deduped := make([]ConditionNode, 0, len(filtered)) - for _, t := range filtered { - key := t.Render() - if _, exists := seen[key]; !exists { - seen[key] = struct{}{} - deduped = append(deduped, t) - } else { - expressionOptimizerLog.Printf("AND dedup: removing duplicate term %q", key) - } - } - if len(deduped) == 1 { - return deduped[0] - } - - // Complement: A && !A → false (skip when status functions present). - if !hasStatusFuncInTerms { - for i := range deduped { - for j := i + 1; j < len(deduped); j++ { - if isNegationOf(deduped[i], deduped[j]) { - expressionOptimizerLog.Printf("AND complement (flatten): %s && %s → false", deduped[i].Render(), deduped[j].Render()) - return &BooleanLiteralNode{Value: false} - } - } - } - } - - // Absorption (AND): A && (A || B) → A - // If any term is an OR/Disjunction that contains another conjunct as a - // sub-term, the OR term is absorbed: the simpler conjunct subsumes it. - if !hasStatusFuncInTerms { - absorbed := make([]bool, len(deduped)) - for i, ti := range deduped { - orTerms := collectOrTerms(ti) - if len(orTerms) < 2 { - continue // ti is not an OR expression - } - for j, tj := range deduped { - if i == j || absorbed[i] { - continue - } - for _, ot := range orTerms { - if nodesEqual(ot, tj) { - expressionOptimizerLog.Printf("AND absorption: (%s) && (%s) → %s (absorbed)", - tj.Render(), ti.Render(), tj.Render()) - absorbed[i] = true - break - } - } - } - } - anyAbsorbed := slices.Contains(absorbed, true) - if anyAbsorbed { - surviving := make([]ConditionNode, 0, len(deduped)) - for i, t := range deduped { - if !absorbed[i] { - surviving = append(surviving, t) - } - } - if len(surviving) == 0 { - return &BooleanLiteralNode{Value: true} - } - return optimizeNode(rebuildAndChain(surviving)) - } - } - - return rebuildAndChain(deduped) -} - -func optimizeOrNode(n *OrNode) ConditionNode { - // Bottom-up: optimise children first. - left := optimizeNode(n.Left) - right := optimizeNode(n.Right) - - // Flatten OR chains: when either child is already an OrNode or DisjunctionNode, - // collect all terms and delegate to the DisjunctionNode optimiser, which - // performs dedup, false-filtering and true short-circuit across the whole chain. - _, leftIsOr := left.(*OrNode) - _, leftIsDisj := left.(*DisjunctionNode) - _, rightIsOr := right.(*OrNode) - _, rightIsDisj := right.(*DisjunctionNode) - if leftIsOr || leftIsDisj || rightIsOr || rightIsDisj { - terms := append(collectOrTerms(left), collectOrTerms(right)...) - expressionOptimizerLog.Printf("OR flatten: collected %d terms", len(terms)) - return optimizeDisjunctionNode(&DisjunctionNode{Terms: terms}) - } - - // Annihilation: A || true → true - if isBoolLiteral(left, true) || isBoolLiteral(right, true) { - expressionOptimizerLog.Printf("OR annihilation: %s || %s → true", left.Render(), right.Render()) - return &BooleanLiteralNode{Value: true} - } - - // Identity: A || false → A - if isBoolLiteral(right, false) { - expressionOptimizerLog.Printf("OR identity (right false): %s || false → %s", left.Render(), left.Render()) - return left - } - if isBoolLiteral(left, false) { - expressionOptimizerLog.Printf("OR identity (left false): false || %s → %s", right.Render(), right.Render()) - return right - } - - // Skip idempotent / complement rules when status functions are present. - if containsStatusFunc(left) || containsStatusFunc(right) { - return &OrNode{Left: left, Right: right} - } - - // Idempotent: A || A → A - if nodesEqual(left, right) { - expressionOptimizerLog.Printf("OR idempotent: %s || %s → %s", left.Render(), right.Render(), left.Render()) - return left - } - - // Complement: A || !A → true - if isNegationOf(left, right) { - expressionOptimizerLog.Printf("OR complement: %s || %s → true", left.Render(), right.Render()) - return &BooleanLiteralNode{Value: true} - } - - // Absorption (OR): A || (A && B) → A - // If one side is an AND-chain that contains the other side as a conjunct, - // the AND-chain is absorbed by the simpler operand. - for _, pair := range [][2]ConditionNode{{left, right}, {right, left}} { - simple, complex := pair[0], pair[1] - if termSubsumedBy(complex, simple) { - expressionOptimizerLog.Printf("OR absorption: %s || (%s) → %s (absorbed)", - simple.Render(), complex.Render(), simple.Render()) - return simple - } - } - - return &OrNode{Left: left, Right: right} -} - -func optimizeNotNode(n *NotNode) ConditionNode { - // Bottom-up: optimise child first. - child := optimizeNode(n.Child) - - // Constant folding: !true → false, !false → true - if lit, ok := child.(*BooleanLiteralNode); ok { - expressionOptimizerLog.Printf("NOT constant folding: !%v → %v", lit.Value, !lit.Value) - return &BooleanLiteralNode{Value: !lit.Value} - } - - // Double negation: !!A → A - if notChild, ok := child.(*NotNode); ok { - expressionOptimizerLog.Printf("NOT double negation: !!%s → %s", notChild.Child.Render(), notChild.Child.Render()) - // Recurse so that the result of eliminating the double negation is - // itself a candidate for further simplification. - return optimizeNode(notChild.Child) - } - - // De Morgan: !(A && B) → !A || !B - // Only applied when neither operand contains a status function, since - // rearranging status functions changes execution semantics. - if andChild, ok := child.(*AndNode); ok && !containsStatusFunc(andChild) { - expressionOptimizerLog.Printf("NOT De Morgan (AND): !(%s && %s) → !%s || !%s", - andChild.Left.Render(), andChild.Right.Render(), - andChild.Left.Render(), andChild.Right.Render()) - return optimizeNode(&OrNode{ - Left: &NotNode{Child: andChild.Left}, - Right: &NotNode{Child: andChild.Right}, - }) - } - - // De Morgan: !(A || B) → !A && !B - if orChild, ok := child.(*OrNode); ok && !containsStatusFunc(orChild) { - expressionOptimizerLog.Printf("NOT De Morgan (OR): !(%s || %s) → !%s && !%s", - orChild.Left.Render(), orChild.Right.Render(), - orChild.Left.Render(), orChild.Right.Render()) - return optimizeNode(&AndNode{ - Left: &NotNode{Child: orChild.Left}, - Right: &NotNode{Child: orChild.Right}, - }) - } - - // De Morgan: !(A || B || ...) → !A && !B && ... (DisjunctionNode form) - // Move the empty-terms guard before the containsStatusFunc call to avoid - // an unnecessary tree walk when the disjunction is empty. - if disjChild, ok := child.(*DisjunctionNode); ok { - if len(disjChild.Terms) == 0 { - return &NotNode{Child: child} - } - if !containsStatusFunc(disjChild) { - expressionOptimizerLog.Printf("NOT De Morgan (Disjunction): !(disjunction[%d]) → AND chain of negations", len(disjChild.Terms)) - negations := make([]ConditionNode, len(disjChild.Terms)) - for i, term := range disjChild.Terms { - negations[i] = &NotNode{Child: term} - } - return optimizeNode(rebuildAndChain(negations)) - } - } - - return &NotNode{Child: child} -} - -func optimizeDisjunctionNode(n *DisjunctionNode) ConditionNode { - if len(n.Terms) == 0 { - return n - } - - // Bottom-up: optimise each term first. - optimised := make([]ConditionNode, 0, len(n.Terms)) - for _, term := range n.Terms { - optimised = append(optimised, optimizeNode(term)) - } - - // Short-circuit: if any term is true the whole disjunction is true. - for _, term := range optimised { - if isBoolLiteral(term, true) { - expressionOptimizerLog.Printf("Disjunction short-circuit on true") - return &BooleanLiteralNode{Value: true} - } - } - - // Filter out false terms (identity: A || false → A). - filtered := make([]ConditionNode, 0, len(optimised)) - for _, term := range optimised { - if !isBoolLiteral(term, false) { - filtered = append(filtered, term) - } - } - if len(filtered) == 0 { - expressionOptimizerLog.Printf("Disjunction all-false → false") - return &BooleanLiteralNode{Value: false} - } - - // Deduplicate terms by rendered form. - seen := make(map[string]struct{}, len(filtered)) - deduped := make([]ConditionNode, 0, len(filtered)) - for _, term := range filtered { - key := term.Render() - if _, exists := seen[key]; !exists { - seen[key] = struct{}{} - deduped = append(deduped, term) - } else { - expressionOptimizerLog.Printf("Disjunction dedup: removing duplicate term %q", key) - } - } - - if len(deduped) == 1 { - return deduped[0] - } - - // Complement: A || !A → true (mirrors the OrNode complement rule). - // Guard: skip when any term contains a status function so that - // status-function expressions are never silently eliminated. - if !slices.ContainsFunc(deduped, containsStatusFunc) { - for i := range deduped { - for j := i + 1; j < len(deduped); j++ { - if isNegationOf(deduped[i], deduped[j]) { - expressionOptimizerLog.Printf("Disjunction complement: %s || %s → true", deduped[i].Render(), deduped[j].Render()) - return &BooleanLiteralNode{Value: true} - } - } - } - - // Subsumption: disj(A, A&&B, …) → disj(A, …) - // A term cand is subsumed (and thus redundant in a disjunction) when - // a simpler term sub is also present such that sub |= cand, i.e. every - // assignment that satisfies sub also satisfies cand. - subsumed := make([]bool, len(deduped)) - for i, cand := range deduped { - for j, sub := range deduped { - if i == j { - continue - } - if termSubsumedBy(cand, sub) { - expressionOptimizerLog.Printf("Disjunction subsumption: %s subsumed by %s", cand.Render(), sub.Render()) - subsumed[i] = true - break - } - } - } - if slices.Contains(subsumed, true) { - surviving := make([]ConditionNode, 0, len(deduped)) - for i, t := range deduped { - if !subsumed[i] { - surviving = append(surviving, t) - } - } - if len(surviving) == 0 { - return &BooleanLiteralNode{Value: false} - } - if len(surviving) == 1 { - return surviving[0] - } - return optimizeNode(&DisjunctionNode{Terms: surviving, Multiline: n.Multiline}) - } - } - - return &DisjunctionNode{Terms: deduped, Multiline: n.Multiline} -} +// OptimizeExpression re-exports [ghexpr.OptimizeExpression]. +var OptimizeExpression = ghexpr.OptimizeExpression diff --git a/pkg/workflow/expression_optimizer_test.go b/pkg/workflow/expression_optimizer_test.go index daa3d860f1f..dcf570a302e 100644 --- a/pkg/workflow/expression_optimizer_test.go +++ b/pkg/workflow/expression_optimizer_test.go @@ -746,60 +746,6 @@ func TestRealWorld_NestedAndFlatRendering(t *testing.T) { assert.Equal(t, "(!cancelled()) && needs.agent.result != 'skipped' && needs.agent.outputs.detection_success == 'true'", result) } -// --------------------------------------------------------------------------- -// Helper utilities -// --------------------------------------------------------------------------- - -// --------------------------------------------------------------------------- -// Helper predicate tests (white-box) -// --------------------------------------------------------------------------- - -func TestIsBoolLiteral(t *testing.T) { - assert.True(t, isBoolLiteral(boolLit(true), true)) - assert.True(t, isBoolLiteral(boolLit(false), false)) - assert.False(t, isBoolLiteral(boolLit(true), false)) - assert.False(t, isBoolLiteral(expr("x"), true)) -} - -func TestIsStatusFunc(t *testing.T) { - assert.True(t, isStatusFunc(fn("always")), "always is a status func") - assert.True(t, isStatusFunc(fn("success")), "success is a status func") - assert.True(t, isStatusFunc(fn("failure")), "failure is a status func") - assert.True(t, isStatusFunc(fn("cancelled")), "cancelled is a status func") - assert.False(t, isStatusFunc(fn("contains")), "contains is not a status func") - assert.False(t, isStatusFunc(fn("startsWith")), "startsWith is not a status func") - assert.False(t, isStatusFunc(expr("x")), "ExpressionNode is not a status func") -} - -func TestNodesEqual(t *testing.T) { - a := expr("github.event_name == 'issues'") - b := expr("github.event_name == 'issues'") - c := expr("github.event_name == 'push'") - assert.True(t, nodesEqual(a, b), "identical renders are equal") - assert.False(t, nodesEqual(a, c), "different renders are not equal") - assert.True(t, nodesEqual(nil, nil), "nil == nil") - assert.False(t, nodesEqual(a, nil), "non-nil != nil") - assert.False(t, nodesEqual(nil, a), "nil != non-nil") -} - -func TestContainsStatusFunc(t *testing.T) { - assert.True(t, containsStatusFunc(fn("always")), "direct always()") - assert.True(t, containsStatusFunc(not1(fn("cancelled"))), "!cancelled() contains status func") - assert.True(t, containsStatusFunc(and2(fn("success"), expr("x"))), "success() in AND") - assert.False(t, containsStatusFunc(expr("github.event_name == 'issues'")), "plain expr has no status func") - assert.False(t, containsStatusFunc(fn("contains", prop("x"), str("y"))), "contains has no status func") - assert.False(t, containsStatusFunc(and2(expr("a"), expr("b"))), "plain AND has no status func") -} - -func TestIsNegationOf(t *testing.T) { - a := expr("github.event_name == 'issues'") - b := not1(a) - assert.True(t, isNegationOf(a, b), "A and !A are negations") - assert.True(t, isNegationOf(b, a), "!A and A are negations (symmetric)") - c := expr("github.actor != 'bot'") - assert.False(t, isNegationOf(a, c), "different expressions are not negations") -} - // --------------------------------------------------------------------------- // Edge cases // --------------------------------------------------------------------------- @@ -1014,83 +960,6 @@ func TestOptimizeDisjunction_ComplementWithStatusFunc_Preserved(t *testing.T) { assert.Contains(t, rendered, "always()", "always() must be preserved") } -// --------------------------------------------------------------------------- -// Utility function tests (white-box) -// --------------------------------------------------------------------------- - -func TestCollectOrTerms_Flat(t *testing.T) { - // collectOrTerms on a leaf returns just the leaf. - a := cmp(prop("a"), "==", str("1")) - terms := collectOrTerms(a) - require.Len(t, terms, 1, "leaf should produce one term") - assert.Equal(t, "a == '1'", terms[0].Render()) -} - -func TestCollectOrTerms_OrNode(t *testing.T) { - // collectOrTerms flattens a two-level OR. - a := cmp(prop("a"), "==", str("1")) - b := cmp(prop("b"), "==", str("2")) - c := cmp(prop("c"), "==", str("3")) - terms := collectOrTerms(or2(or2(a, b), c)) - require.Len(t, terms, 3, "should collect 3 terms from nested OR") -} - -func TestCollectOrTerms_DisjunctionNode(t *testing.T) { - // collectOrTerms flattens a DisjunctionNode. - a := cmp(prop("a"), "==", str("1")) - b := cmp(prop("b"), "==", str("2")) - terms := collectOrTerms(disj(a, b)) - require.Len(t, terms, 2, "should collect 2 terms from DisjunctionNode") -} - -func TestCollectOrTerms_MixedOrAndDisj(t *testing.T) { - // collectOrTerms flattens OrNode{DisjunctionNode{A,B}, C}. - a := cmp(prop("a"), "==", str("1")) - b := cmp(prop("b"), "==", str("2")) - c := cmp(prop("c"), "==", str("3")) - terms := collectOrTerms(or2(disj(a, b), c)) - require.Len(t, terms, 3, "should collect 3 terms from OrNode with DisjunctionNode child") -} - -func TestCollectAndTerms_Flat(t *testing.T) { - // collectAndTerms on a leaf returns just the leaf. - a := cmp(prop("a"), "==", str("1")) - terms := collectAndTerms(a) - require.Len(t, terms, 1, "leaf should produce one term") - assert.Equal(t, "a == '1'", terms[0].Render()) -} - -func TestCollectAndTerms_AndNode(t *testing.T) { - // collectAndTerms flattens A && (B && C). - a := cmp(prop("a"), "==", str("1")) - b := cmp(prop("b"), "==", str("2")) - c := cmp(prop("c"), "==", str("3")) - terms := collectAndTerms(and2(a, and2(b, c))) - require.Len(t, terms, 3, "should collect 3 terms from nested AND") -} - -func TestRebuildAndChain_Single(t *testing.T) { - a := cmp(prop("a"), "==", str("1")) - result := rebuildAndChain([]ConditionNode{a}) - assert.Equal(t, "a == '1'", result.Render(), "single term rebuilds without AND") -} - -func TestRebuildAndChain_Two(t *testing.T) { - a := cmp(prop("a"), "==", str("1")) - b := cmp(prop("b"), "==", str("2")) - result := rebuildAndChain([]ConditionNode{a, b}) - assert.Equal(t, "a == '1' && b == '2'", result.Render(), "two terms rebuild as binary AND") -} - -func TestRebuildAndChain_Three(t *testing.T) { - a := cmp(prop("a"), "==", str("1")) - b := cmp(prop("b"), "==", str("2")) - c := cmp(prop("c"), "==", str("3")) - result := rebuildAndChain([]ConditionNode{a, b, c}) - // Left-folded: (a && b) && c - assert.Equal(t, "a == '1' && b == '2' && c == '3'", result.Render(), "three terms rebuild as left-folded AND chain") -} - // --------------------------------------------------------------------------- // Z3-inspired rules: Absorption, Subsumption, Resolution, Factoring // --------------------------------------------------------------------------- diff --git a/pkg/workflow/expression_parser.go b/pkg/workflow/expression_parser.go index eeebc88e47f..3c68ccbdcbb 100644 --- a/pkg/workflow/expression_parser.go +++ b/pkg/workflow/expression_parser.go @@ -1,510 +1,34 @@ +// This file re-exports GitHub Actions expression parsing functions from pkg/ghexpr +// so that existing code in pkg/workflow continues to compile without modification. +// New code should import pkg/ghexpr directly. package workflow -import ( - "errors" - "fmt" - "strings" - "unicode" +import "github.com/github/gh-aw/pkg/ghexpr" - "github.com/github/gh-aw/pkg/constants" - "github.com/github/gh-aw/pkg/logger" -) +// ParseExpression re-exports [ghexpr.ParseExpression]. +var ParseExpression = ghexpr.ParseExpression -var expressionsLog = logger.New("workflow:expressions") +// VisitExpressionTree re-exports [ghexpr.VisitExpressionTree]. +var VisitExpressionTree = ghexpr.VisitExpressionTree -// stripExpressionWrapper removes the ${{ }} wrapper from an expression if present -func stripExpressionWrapper(expression string) string { - // Trim whitespace - expr := strings.TrimSpace(expression) - // Check if it starts with ${{ and ends with }} - if strings.HasPrefix(expr, "${{") && strings.HasSuffix(expr, "}}") { - // Remove the wrapper and trim inner whitespace - return strings.TrimSpace(expr[3 : len(expr)-2]) - } - return expr -} - -// ExpressionParser handles parsing of expression strings into ConditionNode trees -type ExpressionParser struct { - tokens []token - pos int -} - -type token struct { - kind tokenKind - value string - pos int -} - -type tokenKind int - -const ( - tokenLiteral tokenKind = iota - tokenAnd - tokenOr - tokenNot - tokenLeftParen - tokenRightParen - tokenEOF -) - -// ParseExpression parses a string expression into a ConditionNode tree -// Supports && (AND), || (OR), ! (NOT), and parentheses for grouping -// Example: "condition1 && (condition2 || !condition3)" -func ParseExpression(expression string) (ConditionNode, error) { - expressionsLog.Printf("Parsing expression: %s", expression) - - if strings.TrimSpace(expression) == "" { - return nil, errors.New("empty expression") - } - - parser := &ExpressionParser{} - tokens, err := parser.tokenize(expression) - if err != nil { - expressionsLog.Printf("Failed to tokenize expression: %v", err) - return nil, err - } - parser.tokens = tokens - parser.pos = 0 - - result, err := parser.parseOrExpression() - if err != nil { - expressionsLog.Printf("Failed to parse expression: %v", err) - return nil, err - } - - // Check that all tokens were consumed - if parser.current().kind != tokenEOF { - return nil, fmt.Errorf("unexpected token '%s' at position %d", parser.current().value, parser.current().pos) - } - - expressionsLog.Printf("Successfully parsed expression with %d tokens", len(tokens)) - return result, nil -} - -// tokenize breaks the expression string into tokens -func (p *ExpressionParser) tokenize(expression string) ([]token, error) { - expressionsLog.Printf("Tokenizing expression of length %d", len(expression)) - var tokens []token - i := 0 - - for i < len(expression) { - // Skip whitespace - if unicode.IsSpace(rune(expression[i])) { - i++ - continue - } - - switch { - case i+1 < len(expression) && expression[i:i+2] == "&&": - tokens = append(tokens, token{tokenAnd, "&&", i}) - i += 2 - case i+1 < len(expression) && expression[i:i+2] == "||": - tokens = append(tokens, token{tokenOr, "||", i}) - i += 2 - case expression[i] == '!' && (i+1 >= len(expression) || expression[i+1] != '='): - // Only treat ! as NOT if not followed by = (to avoid conflicting with !=) - tokens = append(tokens, token{tokenNot, "!", i}) - i++ - case expression[i] == '(': - tokens = append(tokens, token{tokenLeftParen, "(", i}) - i++ - case expression[i] == ')': - tokens = append(tokens, token{tokenRightParen, ")", i}) - i++ - default: - // Parse literal expression - everything until we hit a logical operator or paren - start := i - parenCount := 0 - - for i < len(expression) { - ch := expression[i] - - // Handle quoted strings - skip everything inside quotes - // Support single quotes ('), double quotes ("), and backticks (`) - if ch == '\'' || ch == '"' || ch == '`' { - quote := ch - i++ // skip opening quote - for i < len(expression) { - if expression[i] == quote { - i++ // skip closing quote - break - } - if expression[i] == '\\' && i+1 < len(expression) { - i += 2 // skip escaped character - } else { - i++ - } - } - continue - } - - // Track parentheses that are part of the expression (e.g., function calls) - if ch == '(' { - parenCount++ - i++ - continue - } else if ch == ')' { - if parenCount > 0 { - parenCount-- - i++ - continue - } else { - // This closes our group expression, stop here - break - } - } - - // Check for logical operators when not inside parentheses - if parenCount == 0 { - // Check for && or || - if i+1 < len(expression) { - next := expression[i : i+2] - if next == "&&" || next == "||" { - break - } - } - - // Check for logical NOT that's not part of != - if ch == '!' && (i+1 >= len(expression) || expression[i+1] != '=') { - break - } - } - - i++ - } - - literal := strings.TrimSpace(expression[start:i]) - if literal == "" { - return nil, fmt.Errorf("unexpected empty literal at position %d", start) - } - tokens = append(tokens, token{tokenLiteral, literal, start}) - } - } - - tokens = append(tokens, token{tokenEOF, "", i}) - return tokens, nil -} - -// parseOrExpression parses OR expressions (lowest precedence) -func (p *ExpressionParser) parseOrExpression() (ConditionNode, error) { - left, err := p.parseAndExpression() - if err != nil { - return nil, err - } - - for p.current().kind == tokenOr { - p.advance() // consume || - right, err := p.parseAndExpression() - if err != nil { - return nil, err - } - left = &OrNode{Left: left, Right: right} - } - - return left, nil -} - -// parseAndExpression parses AND expressions (higher precedence than OR) -func (p *ExpressionParser) parseAndExpression() (ConditionNode, error) { - left, err := p.parseUnaryExpression() - if err != nil { - return nil, err - } - - for p.current().kind == tokenAnd { - p.advance() // consume && - right, err := p.parseUnaryExpression() - if err != nil { - return nil, err - } - left = &AndNode{Left: left, Right: right} - } - - return left, nil -} - -// parseUnaryExpression parses NOT expressions and primary expressions -func (p *ExpressionParser) parseUnaryExpression() (ConditionNode, error) { - if p.current().kind == tokenNot { - p.advance() // consume ! - operand, err := p.parseUnaryExpression() - if err != nil { - return nil, err - } - return &NotNode{Child: operand}, nil - } +// BreakLongExpression re-exports [ghexpr.BreakLongExpression]. +var BreakLongExpression = ghexpr.BreakLongExpression - return p.parsePrimaryExpression() -} - -// parsePrimaryExpression parses literals and parenthesized expressions -func (p *ExpressionParser) parsePrimaryExpression() (ConditionNode, error) { - if expressionsLog.Enabled() { - expressionsLog.Printf("Parsing primary expression at token: %s", p.current().value) - } - switch p.current().kind { - case tokenLeftParen: - p.advance() // consume ( - expr, err := p.parseOrExpression() - if err != nil { - return nil, err - } - if p.current().kind != tokenRightParen { - return nil, fmt.Errorf("expected ')' at position %d", p.current().pos) - } - p.advance() // consume ) - return expr, nil - - case tokenLiteral: - literal := p.current().value - p.advance() - return &ExpressionNode{Expression: literal}, nil - - default: - return nil, fmt.Errorf("unexpected token '%s' at position %d", p.current().value, p.current().pos) - } -} - -// current returns the current token -func (p *ExpressionParser) current() token { - if p.pos >= len(p.tokens) { - return token{tokenEOF, "", -1} - } - return p.tokens[p.pos] -} - -// advance moves to the next token -func (p *ExpressionParser) advance() { - if p.pos < len(p.tokens) { - p.pos++ - } -} - -// VisitExpressionTree walks through an expression tree and calls the visitor function -// for each ExpressionNode (literal expression) found in the tree -func VisitExpressionTree(node ConditionNode, visitor func(expr *ExpressionNode) error) error { - if node == nil { - expressionsLog.Print("VisitExpressionTree called with nil node") - return nil - } - - switch n := node.(type) { - case *ExpressionNode: - return visitor(n) - case *AndNode: - if err := VisitExpressionTree(n.Left, visitor); err != nil { - return err - } - return VisitExpressionTree(n.Right, visitor) - case *OrNode: - if err := VisitExpressionTree(n.Left, visitor); err != nil { - return err - } - return VisitExpressionTree(n.Right, visitor) - case *NotNode: - return VisitExpressionTree(n.Child, visitor) - case *DisjunctionNode: - for _, term := range n.Terms { - if err := VisitExpressionTree(term, visitor); err != nil { - return err - } - } - default: - // For other node types (ComparisonNode, PropertyAccessNode, etc.) - // we don't recurse since they represent complete literal expressions - return nil - } - - return nil -} - -// BreakLongExpression breaks a long expression into multiple lines at logical points -// such as after || and && operators for better readability -func BreakLongExpression(expression string) []string { - // If the expression is not too long, return as-is - if len(expression) <= int(constants.MaxExpressionLineLength) { - return []string{expression} - } - - expressionsLog.Printf("Breaking long expression: length=%d", len(expression)) - - var lines []string - var current strings.Builder - i := 0 - - for i < len(expression) { - char := expression[i] - - // Handle quoted strings - don't break inside quotes - // Support single quotes ('), double quotes ("), and backticks (`) - if char == '\'' || char == '"' || char == '`' { - quote := char - current.WriteByte(char) - i++ +// BreakAtParentheses re-exports [ghexpr.BreakAtParentheses]. +var BreakAtParentheses = ghexpr.BreakAtParentheses - // Continue until closing quote - for i < len(expression) { - current.WriteByte(expression[i]) - if expression[i] == quote { - i++ - break - } - if expression[i] == '\\' && i+1 < len(expression) { - i++ // Skip escaped character - if i < len(expression) { - current.WriteByte(expression[i]) - } - } - i++ - } - continue - } - - // Look for logical operators as break points - if i+2 <= len(expression) { - next2 := expression[i : i+2] - if next2 == "||" || next2 == "&&" { - current.WriteString(next2) - i += 2 - - // If the current line is getting long (>ExpressionBreakThreshold chars), break here - if trimmed := strings.TrimSpace(current.String()); len(trimmed) > int(constants.ExpressionBreakThreshold) { - lines = append(lines, trimmed) - current.Reset() - // Skip whitespace after operator - for i < len(expression) && (expression[i] == ' ' || expression[i] == '\t') { - i++ - } - continue - } - continue - } - } - - current.WriteByte(char) - i++ - } - - // Add the remaining part - if trimmed := strings.TrimSpace(current.String()); trimmed != "" { - lines = append(lines, trimmed) - } - - // If we still have very long lines, try to break at parentheses - var finalLines []string - for _, line := range lines { - if len(line) > int(constants.MaxExpressionLineLength) { - subLines := BreakAtParentheses(line) - finalLines = append(finalLines, subLines...) - } else { - finalLines = append(finalLines, line) - } - } - - return finalLines -} - -// BreakAtParentheses attempts to break long lines at parentheses for function calls -func BreakAtParentheses(expression string) []string { - if len(expression) <= int(constants.MaxExpressionLineLength) { - return []string{expression} - } - - expressionsLog.Printf("Breaking expression at parentheses: length=%d", len(expression)) - - var lines []string - var current strings.Builder - parenDepth := 0 - - for i := 0; i < len(expression); i++ { - char := expression[i] - current.WriteByte(char) - - switch char { - case '(': - parenDepth++ - case ')': - parenDepth-- - - // If we're back to zero depth and the line is getting long, consider a break - if parenDepth == 0 && current.Len() > 80 && i < len(expression)-1 { - // Look ahead to see if there's a logical operator - j := i + 1 - for j < len(expression) && (expression[j] == ' ' || expression[j] == '\t') { - j++ - } - - if j+1 < len(expression) && (expression[j:j+2] == "||" || expression[j:j+2] == "&&") { - // Add the operator to current line and break - current.WriteString(expression[i+1 : j+2]) - lines = append(lines, strings.TrimSpace(current.String())) - current.Reset() - i = j + 2 - 1 // Set to j+2-1 so the loop increment makes i = j+2 - - // Skip whitespace after operator - for i+1 < len(expression) && (expression[i+1] == ' ' || expression[i+1] == '\t') { - i++ - } - } - } - } - } - - // Add remaining part - if trimmed := strings.TrimSpace(current.String()); trimmed != "" { - lines = append(lines, trimmed) - } - - return lines +// stripExpressionWrapper removes the ${{ }} wrapper from an expression if present. +func stripExpressionWrapper(expression string) string { + return ghexpr.StripExpressionWrapper(expression) } -// hasNewlineInStringLiteral returns true if s contains an actual newline character (\n) -// that appears inside a single-quoted GitHub Actions expression string literal. -// This is used to determine whether the YAML `if:` value needs special encoding to -// preserve the newline (e.g. for matching bot comments that append metadata after a newline). +// hasNewlineInStringLiteral reports whether s contains a newline inside a +// single-quoted GitHub Actions expression string literal. func hasNewlineInStringLiteral(s string) bool { - inString := false - i := 0 - for i < len(s) { - ch := s[i] - if ch == '\'' { - // Handle escaped single-quote inside a string: '' - if inString && i+1 < len(s) && s[i+1] == '\'' { - i += 2 // skip both quotes, stay in string - continue - } - inString = !inString - } else if ch == '\n' && inString { - return true - } - i++ - } - return false + return ghexpr.HasNewlineInStringLiteral(s) } -// escapeForYAMLDoubleQuoted escapes a string so it can be safely placed inside a YAML -// double-quoted scalar (i.e. wrapped with "..."). YAML double-quoted scalars interpret -// \n, \r, \t, \\ and \" escape sequences, so we convert the corresponding actual characters -// to their two-character backslash representations. After YAML parsing, the values are -// restored, so GitHub Actions receives the expression with the real characters intact. +// escapeForYAMLDoubleQuoted escapes s so it can be placed inside a YAML double-quoted scalar. func escapeForYAMLDoubleQuoted(s string) string { - var b strings.Builder - for i := range len(s) { - switch s[i] { - case '\\': - b.WriteString(`\\`) - case '"': - b.WriteString(`\"`) - case '\n': - b.WriteString(`\n`) - case '\r': - b.WriteString(`\r`) - case '\t': - b.WriteString(`\t`) - default: - b.WriteByte(s[i]) - } - } - return b.String() + return ghexpr.EscapeForYAMLDoubleQuoted(s) } diff --git a/pkg/workflow/expression_patterns.go b/pkg/workflow/expression_patterns.go index 86c338e0372..cc958ae5ce7 100644 --- a/pkg/workflow/expression_patterns.go +++ b/pkg/workflow/expression_patterns.go @@ -1,102 +1,78 @@ // This file provides centralized regex patterns for GitHub Actions expression matching. // -// # Expression Patterns -// -// This file consolidates regular expression patterns used across multiple validation and -// extraction files to provide a single source of truth for expression matching logic. +// Core language-level patterns and predicate helpers are defined in pkg/ghexpr. +// This file re-exports them as package-level vars for backward compatibility and +// adds gh-aw–specific context patterns (secrets, inputs, aw-inputs, etc.). // // # Available Pattern Categories // -// ## Core Expression Patterns -// - ExpressionPattern - Matches GitHub Actions expressions: ${{ ... }} -// - ExpressionPatternDotAll - Matches expressions with dotall mode (multiline) +// ## Core Expression Patterns (from pkg/ghexpr) +// - ExpressionPattern - Matches GitHub Actions expressions: ${{ ... }} +// - ExpressionPatternDotAll - Matches expressions with dotall mode (multiline) +// - InlineExpressionPattern - Matches inline ${{ ... }} in template strings +// - TemplateIfPattern - Matches {{#if ...}} template conditionals +// - TemplateElseIfPattern - Matches {{#elseif ...}} template conditionals +// - StringLiteralPattern - Matches string literals ('...', "...", `...`) +// - NumberLiteralPattern - Matches numeric literals +// - OrPattern - Splits logical-OR expressions +// - ComparisonExtractionPattern - Extracts LHS of comparisons +// - RangePattern - Matches numeric ranges (e.g., "1-10") +// +// ## Predicate helpers (from pkg/ghexpr) +// - hasExpressionMarker - reports ${{ present +// - containsExpression - reports complete ${{ ... }} present +// - isExpression - reports whole string is ${{ ... }} // -// ## Context Access Patterns -// - NeedsStepsPattern - Matches needs.* and steps.* patterns -// - InputsPattern - Matches github.event.inputs.* patterns -// - WorkflowCallInputsPattern - Matches inputs.* patterns (workflow_call) -// - AWInputsPattern - Matches github.aw.inputs.* patterns -// - EnvPattern - Matches env.* patterns +// ## gh-aw Context Access Patterns +// - NeedsStepsPattern - Matches needs.* and steps.* patterns +// - InputsPattern - Matches github.event.inputs.* patterns +// - WorkflowCallInputsPattern - Matches inputs.* patterns (workflow_call) +// - AWInputsPattern - Matches github.aw.inputs.* patterns +// - AWInputsExpressionPattern - Matches full ${{ github.aw.inputs.* }} expressions +// - AWImportInputsPattern - Matches github.aw.import-inputs.* patterns +// - AWImportInputsExpressionPattern - Matches full ${{ github.aw.import-inputs.* }} expressions +// - EnvPattern - Matches env.* patterns // // ## Secret Patterns -// - SecretExpressionPattern - Matches ${{ secrets.SECRET_NAME }} expressions +// - SecretExpressionPattern - Matches ${{ secrets.SECRET_NAME }} expressions // - SecretsExpressionPattern - Validates secrets expression syntax // -// ## Template Patterns -// - InlineExpressionPattern - Matches inline expressions in templates +// ## Template Patterns (gh-aw specific) // - UnsafeContextPattern - Matches potentially unsafe context patterns -// - TemplateIfPattern - Matches {{#if ...}} template conditionals -// -// ## Utility Patterns -// - ComparisonExtractionPattern - Extracts properties from comparison expressions -// - StringLiteralPattern - Matches string literals ('...', "...", `...`) -// - NumberLiteralPattern - Matches numeric literals -// - RangePattern - Matches numeric ranges (e.g., "1-10") -// -// # Design Rationale -// -// Centralizing regex patterns provides several benefits: -// - Single source of truth for expression matching logic -// - Consistent behavior across validation and extraction -// - Easier to maintain and update patterns -// - Better performance through pre-compilation -// - Reduced code duplication across files -// -// # Migration Notes -// -// This file consolidates patterns previously scattered across: -// - expression_validation.go -// - expression_extraction.go -// - secret_extraction.go -// - secrets_validation.go -// - template.go -// - template_injection_validation.go -// -// Files are gradually being migrated to use these centralized patterns. package workflow import ( "regexp" - "strings" + + "github.com/github/gh-aw/pkg/ghexpr" ) -// hasExpressionMarker reports whether s contains a GitHub Actions expression opening marker. -// This is a permissive check used in scenarios where partial expressions should be treated -// as dynamic values. -func hasExpressionMarker(s string) bool { - return strings.Contains(s, "${{") -} - -// containsExpression reports whether s contains a complete non-empty GitHub Actions expression. -// A complete expression has a "${{" marker that appears before a closing "}}" marker -// with at least one character between them. -func containsExpression(s string) bool { - _, afterOpen, found := strings.Cut(s, "${{") - if !found { - return false - } - closeIdx := strings.Index(afterOpen, "}}") - return closeIdx > 0 -} - -// isExpression reports whether the entire string s is a GitHub Actions expression. -func isExpression(s string) bool { - return strings.HasPrefix(s, "${{") && strings.HasSuffix(s, "}}") -} - -// Core Expression Patterns -var ( - // ExpressionPattern matches GitHub Actions expressions: ${{ ... }} - // Uses non-greedy matching to handle nested braces properly - ExpressionPattern = regexp.MustCompile(`\$\{\{(.*?)\}\}`) +// hasExpressionMarker re-exports [ghexpr.HasExpressionMarker] with the unexported +// name used throughout pkg/workflow. +func hasExpressionMarker(s string) bool { return ghexpr.HasExpressionMarker(s) } + +// containsExpression re-exports [ghexpr.ContainsExpression]. +func containsExpression(s string) bool { return ghexpr.ContainsExpression(s) } - // ExpressionPatternDotAll matches expressions with dotall mode enabled - // The (?s) flag enables dotall mode where . matches newlines - ExpressionPatternDotAll = regexp.MustCompile(`(?s)\$\{\{(.*?)\}\}`) +// isExpression re-exports [ghexpr.IsExpression]. +func isExpression(s string) bool { return ghexpr.IsExpression(s) } + +// Core Expression Patterns — sourced from pkg/ghexpr. +var ( + ExpressionPattern = ghexpr.ExpressionPattern + ExpressionPatternDotAll = ghexpr.ExpressionPatternDotAll + InlineExpressionPattern = ghexpr.InlineExpressionPattern + TemplateIfPattern = ghexpr.TemplateIfPattern + TemplateElseIfPattern = ghexpr.TemplateElseIfPattern + ComparisonExtractionPattern = ghexpr.ComparisonExtractionPattern + OrPattern = ghexpr.OrPattern + StringLiteralPattern = ghexpr.StringLiteralPattern + NumberLiteralPattern = ghexpr.NumberLiteralPattern + RangePattern = ghexpr.RangePattern ) -// Context Access Patterns +// gh-aw Context Access Patterns var ( // NeedsStepsPattern matches needs.* and steps.* context patterns // Example: needs.build.outputs.version, steps.setup.outputs.path @@ -145,55 +121,9 @@ var ( SecretsExpressionPattern = regexp.MustCompile(`^\$\{\{\s*secrets\.[A-Za-z_][A-Za-z0-9_]*(\s*\|\|\s*secrets\.[A-Za-z_][A-Za-z0-9_]*)*\s*\}\}$`) ) -// Template Patterns +// Template Patterns (gh-aw specific) var ( - // InlineExpressionPattern matches inline ${{ ... }} expressions in templates - InlineExpressionPattern = regexp.MustCompile(`\$\{\{[^}]+\}\}`) - - // UnsafeContextPattern matches potentially unsafe context patterns - // These patterns may allow injection attacks in templates + // UnsafeContextPattern matches potentially unsafe context patterns. + // These patterns may allow injection attacks in templates. UnsafeContextPattern = regexp.MustCompile(`\$\{\{\s*(github\.event\.|steps\.[^}]+\.outputs\.|inputs\.)[^}]+\}\}`) - - // TemplateIfPattern matches {{#if condition }} template conditionals - // Captures the condition expression (which may contain ${{ ... }}) - // - // Expression group: (?:\$\{\{[^\}]*\}\}|[^\}\{]|\{[^\{])* - // - \$\{\{[^\}]*\}\} — already-wrapped ${{ ... }} expression - // - [^\}\{] — any character that is not } or { - // - \{[^\{] — a { not immediately followed by another { (handles ${ env refs) - // Using [^\}\{] (instead of [^\}]) prevents the pattern from greedily consuming - // {{ sequences that start a nested template tag, which would embed a raw {{elseif - // or similar token inside the wrapped ${{ }} expression and confuse later validation. - TemplateIfPattern = regexp.MustCompile(`\{\{#if\s+((?:\$\{\{[^\}]*\}\}|[^\}\{]|\{[^\{])*)\s*\}\}`) - - // TemplateElseIfPattern matches elseif/else-if/else_if template conditionals in all supported - // syntax variants: - // {{#elseif expr}} {{#else-if expr}} {{#else_if expr}} - // {{elseif expr}} {{else-if expr}} {{else_if expr}} - // Captures the condition expression (which may contain ${{ ... }}) - // See TemplateIfPattern for the expression group design rationale. - TemplateElseIfPattern = regexp.MustCompile(`\{\{#?else[-_]?if\s+((?:\$\{\{[^\}]*\}\}|[^\}\{]|\{[^\{])*)\s*\}\}`) -) - -// Comparison and Literal Patterns -var ( - // ComparisonExtractionPattern extracts property accesses from comparison expressions - // Matches patterns like "github.workflow == 'value'" and extracts "github.workflow" - ComparisonExtractionPattern = regexp.MustCompile(`([a-zA-Z_][a-zA-Z0-9_.]*)\s*(?:==|!=|<|>|<=|>=)\s*`) - - // OrPattern matches logical OR expressions - // Example: value1 || value2 - OrPattern = regexp.MustCompile(`^(.+?)\s*\|\|\s*(.+)$`) - - // StringLiteralPattern matches string literals in single quotes, double quotes, or backticks - // Example: 'hello', "world", `template` - StringLiteralPattern = regexp.MustCompile(`^'[^']*'$|^"[^"]*"$|^` + "`[^`]*`$") - - // NumberLiteralPattern matches numeric literals (integers and decimals) - // Example: 42, -3.14, 0.5 - NumberLiteralPattern = regexp.MustCompile(`^-?\d+(\.\d+)?$`) - - // RangePattern matches numeric range patterns - // Example: 1-10, 100-200 - RangePattern = regexp.MustCompile(`^\d+-\d+$`) ) diff --git a/pkg/workflow/expression_safety_validation.go b/pkg/workflow/expression_safety_validation.go index 674b29d5851..5c2488e2454 100644 --- a/pkg/workflow/expression_safety_validation.go +++ b/pkg/workflow/expression_safety_validation.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/ghexpr" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" ) @@ -27,8 +28,6 @@ var ( // comparisonExpressionPattern matches a full comparison expression so both sides can be // validated recursively instead of allowing a safe-looking prefix to bypass validation. comparisonExpressionPattern = regexp.MustCompile(`^(.+?)\s*(?:==|!=|<|>|<=|>=)\s*(.+)$`) - // orExpressionPattern matches "left || right" for fallback literal/expression checking - orExpressionPattern = regexp.MustCompile(`^(.+?)\s*\|\|\s*(.+)$`) ) // validateExpressionSafety checks that all GitHub Actions expressions in the markdown content @@ -182,8 +181,8 @@ func validateSingleExpression(expression string, opts ExpressionValidationOption expression = strings.TrimSpace(expression) // Allow literal values (string, number, boolean) — safe leaf nodes in compound expressions. - if stringLiteralRegex.MatchString(expression) || - numberLiteralRegex.MatchString(expression) || + if ghexpr.StringLiteralPattern.MatchString(expression) || + ghexpr.NumberLiteralPattern.MatchString(expression) || expression == "true" || expression == "false" { return nil } @@ -214,7 +213,7 @@ func validateSingleExpression(expression string, opts ExpressionValidationOption // Check for OR expressions with literals (e.g., "inputs.repository || 'default'") if !allowed { - orMatch := orExpressionPattern.FindStringSubmatch(expression) + orMatch := ghexpr.OrPattern.FindStringSubmatch(expression) if len(orMatch) > 2 { leftExpr := strings.TrimSpace(orMatch[1]) rightExpr := strings.TrimSpace(orMatch[2]) @@ -225,9 +224,9 @@ func validateSingleExpression(expression string, opts ExpressionValidationOption if leftIsSafe { // Check if right side is a literal string (single, double, or backtick quotes) // Note: Using (?:) for non-capturing group and checking each quote type separately - isStringLiteral := stringLiteralRegex.MatchString(rightExpr) + isStringLiteral := ghexpr.StringLiteralPattern.MatchString(rightExpr) // Check if right side is a number literal - isNumberLiteral := numberLiteralRegex.MatchString(rightExpr) + isNumberLiteral := ghexpr.NumberLiteralPattern.MatchString(rightExpr) // Check if right side is a boolean literal isBooleanLiteral := rightExpr == "true" || rightExpr == "false" diff --git a/pkg/workflow/expression_syntax_validation.go b/pkg/workflow/expression_syntax_validation.go index 2a6d1c67261..b55d127b5cd 100644 --- a/pkg/workflow/expression_syntax_validation.go +++ b/pkg/workflow/expression_syntax_validation.go @@ -18,11 +18,6 @@ var expressionBracesPattern = regexp.MustCompile(`\$\{\{([^}]*)\}\}`) // Pre-compiled regexes shared between syntax and safety validation var ( - // stringLiteralRegex matches single-quoted, double-quoted, or backtick-quoted string literals. - // Note: escape sequences inside strings are not handled; GitHub Actions uses '' for literal quotes. - stringLiteralRegex = regexp.MustCompile(`^'[^']*'$|^"[^"]*"$|^` + "`[^`]*`$") - // numberLiteralRegex matches integer and decimal number literals (with optional leading minus) - numberLiteralRegex = regexp.MustCompile(`^-?\d+(\.\d+)?$`) // exprPartSplitRe splits expression strings on dot and bracket characters exprPartSplitRe = regexp.MustCompile(`[.\[\]]+`) // exprNumericPartRe matches purely numeric expression parts (array indices) diff --git a/pkg/workflow/publish_assets.go b/pkg/workflow/publish_assets.go index bd4a2b3d9f3..1f3d3847929 100644 --- a/pkg/workflow/publish_assets.go +++ b/pkg/workflow/publish_assets.go @@ -3,20 +3,18 @@ package workflow import ( "errors" "fmt" - "regexp" "strings" "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/ghexpr" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/typeutil" ) var publishAssetsLog = logger.New("workflow:publish_assets") -var githubExpressionPattern = regexp.MustCompile(`(?s)^\$\{\{.*\}\}$`) func isGitHubExpression(value string) bool { - trimmed := strings.TrimSpace(value) - return githubExpressionPattern.MatchString(trimmed) + return ghexpr.IsExpression(strings.TrimSpace(value)) } func normalizeAllowedExtension(extension string) string {