Repository: mikefarah/yq
Pull request #1148 (2022-03-20 01:53:57)

Boolean fix


Patch:
diff --git a/examples/data1.yaml b/examples/data1.yaml
index 1bf3a5cb..5b4dbd13 100644
--- a/examples/data1.yaml
+++ b/examples/data1.yaml
@@ -1,1 +1,2 @@
-cat: purrs
+a: mike
+b: [t, f]
\ No newline at end of file
diff --git a/pkg/yqlib/operator_booleans.go b/pkg/yqlib/operator_booleans.go
index cbc558c9..d919b9a6 100644
--- a/pkg/yqlib/operator_booleans.go
+++ b/pkg/yqlib/operator_booleans.go
@@ -27,41 +27,48 @@ func isTruthy(c *CandidateNode) (bool, error) {
 	return isTruthyNode(node)
 }
 
-type boolOp func(bool, bool) bool
+func getBoolean(candidate *CandidateNode) (bool, error) {
+	if candidate != nil {
+		candidate.Node = unwrapDoc(candidate.Node)
+		return isTruthy(candidate)
+	}
+	return false, nil
+}
 
-func performBoolOp(op boolOp) func(d *dataTreeNavigator, context Context, lhs *CandidateNode, rhs *CandidateNode) (*CandidateNode, error) {
-	return func(d *dataTreeNavigator, context Context, lhs *CandidateNode, rhs *CandidateNode) (*CandidateNode, error) {
-		owner := lhs
+func getOwner(lhs *CandidateNode, rhs *CandidateNode) *CandidateNode {
+	owner := lhs
 
-		if lhs == nil && rhs == nil {
-			owner = &CandidateNode{}
-		} else if lhs == nil {
-			owner = rhs
-		}
+	if lhs == nil && rhs == nil {
+		owner = &CandidateNode{}
+	} else if lhs == nil {
+		owner = rhs
+	}
+	return owner
+}
 
-		var errDecoding error
-		lhsTrue := false
-		if lhs != nil {
-			lhs.Node = unwrapDoc(lhs.Node)
-			lhsTrue, errDecoding = isTruthy(lhs)
+func returnRhsTruthy(d *dataTreeNavigator, context Context, lhs *CandidateNode, rhs *CandidateNode) (*CandidateNode, error) {
+	owner := getOwner(lhs, rhs)
+	rhsBool, err := getBoolean(rhs)
+	if err != nil {
+		return nil, err
+	}
 
-			if errDecoding != nil {
-				return nil, errDecoding
-			}
+	return createBooleanCandidate(owner, rhsBool), nil
+}
+
+func returnLHSWhen(targetBool bool) func(lhs *CandidateNode) (*CandidateNode, error) {
+	return func(lhs *CandidateNode) (*CandidateNode, error) {
+		var err error
+		var lhsBool bool
+
+		if lhsBool, err = getBoolean(lhs); err != nil || lhsBool != targetBool {
+			return nil, err
 		}
-		log.Debugf("-- lhsTrue", lhsTrue)
-
-		rhsTrue := false
-		if rhs != nil {
-			rhs.Node = unwrapDoc(rhs.Node)
-			rhsTrue, errDecoding = isTruthy(rhs)
-			if errDecoding != nil {
-				return nil, errDecoding
-			}
+		owner := &CandidateNode{}
+		if lhs != nil {
+			owner = lhs
 		}
-		log.Debugf("-- rhsTrue", rhsTrue)
-
-		return createBooleanCandidate(owner, op(lhsTrue, rhsTrue)), nil
+		return createBooleanCandidate(owner, targetBool), nil
 	}
 }
 
@@ -133,20 +140,21 @@ func anyOperator(d *dataTreeNavigator, context Context, expressionNode *Expressi
 }
 
 func orOperator(d *dataTreeNavigator, context Context, expressionNode *ExpressionNode) (Context, error) {
-	log.Debugf("-- orOp")
-	return crossFunction(d, context.ReadOnlyClone(), expressionNode, performBoolOp(
-		func(b1 bool, b2 bool) bool {
-			log.Debugf("-- peformingOrOp with %v and %v", b1, b2)
-			return b1 || b2
-		}), true)
+	prefs := crossFunctionPreferences{
+		CalcWhenEmpty:  true,
+		Calculation:    returnRhsTruthy,
+		LhsResultValue: returnLHSWhen(true),
+	}
+	return crossFunctionWithPrefs(d, context.ReadOnlyClone(), expressionNode, prefs)
 }
 
 func andOperator(d *dataTreeNavigator, context Context, expressionNode *ExpressionNode) (Context, error) {
-	log.Debugf("-- AndOp")
-	return crossFunction(d, context.ReadOnlyClone(), expressionNode, performBoolOp(
-		func(b1 bool, b2 bool) bool {
-			return b1 && b2
-		}), true)
+	prefs := crossFunctionPreferences{
+		CalcWhenEmpty:  true,
+		Calculation:    returnRhsTruthy,
+		LhsResultValue: returnLHSWhen(false),
+	}
+	return crossFunctionWithPrefs(d, context.ReadOnlyClone(), expressionNode, prefs)
 }
 
 func notOperator(d *dataTreeNavigator, context Context, expressionNode *ExpressionNode) (Context, error) {
diff --git a/pkg/yqlib/operators.go b/pkg/yqlib/operators.go
index 6ca50254..1a70db6a 100644
--- a/pkg/yqlib/operators.go
+++ b/pkg/yqlib/operators.go
@@ -54,10 +54,25 @@ func emptyOperator(d *dataTreeNavigator, context Context, expressionNode *Expres
 
 type crossFunctionCalculation func(d *dataTreeNavigator, context Context, lhs *CandidateNode, rhs *CandidateNode) (*CandidateNode, error)
 
-func resultsForRHS(d *dataTreeNavigator, context Context, lhsCandidate *CandidateNode, rhs Context, calculation crossFunctionCalculation, results *list.List, calcWhenEmpty bool) error {
+func resultsForRHS(d *dataTreeNavigator, context Context, lhsCandidate *CandidateNode, prefs crossFunctionPreferences, rhsExp *ExpressionNode, results *list.List) error {
 
-	if calcWhenEmpty && rhs.MatchingNodes.Len() == 0 {
-		resultCandidate, err := calculation(d, context, lhsCandidate, nil)
+	if prefs.LhsResultValue != nil {
+		result, err := prefs.LhsResultValue(lhsCandidate)
+		if err != nil {
+			return err
+		} else if result != nil {
+			results.PushBack(result)
+			return nil
+		}
+	}
+
+	rhs, err := d.GetMatchingNodes(context, rhsExp)
+	if err != nil {
+		return err
+	}
+
+	if prefs.CalcWhenEmpty && rhs.MatchingNodes.Len() == 0 {
+		resultCandidate, err := prefs.Calculation(d, context, lhsCandidate, nil)
 		if err != nil {
 			return err
 		}
@@ -70,7 +85,7 @@ func resultsForRHS(d *dataTreeNavigator, context Context, lhsCandidate *Candidat
 	for rightEl := rhs.MatchingNodes.Front(); rightEl != nil; rightEl = rightEl.Next() {
 		log.Debugf("Applying calc")
 		rhsCandidate := rightEl.Value.(*CandidateNode)
-		resultCandidate, err := calculation(d, context, lhsCandidate, rhsCandidate)
+		resultCandidate, err := prefs.Calculation(d, context, lhsCandidate, rhsCandidate)
 		if err != nil {
 			return err
 		}
@@ -81,7 +96,15 @@ func resultsForRHS(d *dataTreeNavigator, context Context, lhsCandidate *Candidat
 	return nil
 }
 
-func doCrossFunc(d *dataTreeNavigator, context Context, expressionNode *ExpressionNode, calculation crossFunctionCalculation, calcWhenEmpty bool) (Context, error) {
+type crossFunctionPreferences struct {
+	CalcWhenEmpty bool
+	// if this returns a result node,
+	// we wont bother calculating the RHS
+	LhsResultValue func(*CandidateNode) (*CandidateNode, error)
+	Calculation    crossFunctionCalculation
+}
+
+func doCrossFunc(d *dataTreeNavigator, context Context, expressionNode *ExpressionNode, prefs crossFunctionPreferences) (Context, error) {
 	var results = list.New()
 	lhs, err := d.GetMatchingNodes(context, expressionNode.LHS)
 	if err != nil {
@@ -89,14 +112,8 @@ func doCrossFunc(d *dataTreeNavigator, context Context, expressionNode *Expressi
 	}
 	log.Debugf("crossFunction LHS len: %v", lhs.MatchingNodes.Len())
 
-	rhs, err := d.GetMatchingNodes(context, expressionNode.RHS)
-
-	if err != nil {
-		return Context{}, err
-	}
-
-	if calcWhenEmpty && lhs.MatchingNodes.Len() == 0 {
-		err := resultsForRHS(d, context, nil, rhs, calculation, results, calcWhenEmpty)
+	if prefs.CalcWhenEmpty && lhs.MatchingNodes.Len() == 0 {
+		err := resultsForRHS(d, context, nil, prefs, expressionNode.RHS, results)
 		if err != nil {
 			return Context{}, err
 		}
@@ -105,7 +122,7 @@ func doCrossFunc(d *dataTreeNavigator, context Context, expressionNode *Expressi
 	for el := lhs.MatchingNodes.Front(); el != nil; el = el.Next() {
 		lhsCandidate := el.Value.(*CandidateNode)
 
-		err := resultsForRHS(d, context, lhsCandidate, rhs, calculation, results, calcWhenEmpty)
+		err = resultsForRHS(d, context, lhsCandidate, prefs, expressionNode.RHS, results)
 		if err != nil {
 			return Context{}, err
 		}
@@ -115,6 +132,11 @@ func doCrossFunc(d *dataTreeNavigator, context Context, expressionNode *Expressi
 }
 
 func crossFunction(d *dataTreeNavigator, context Context, expressionNode *ExpressionNode, calculation crossFunctionCalculation, calcWhenEmpty bool) (Context, error) {
+	prefs := crossFunctionPreferences{CalcWhenEmpty: calcWhenEmpty, Calculation: calculation}
+	return crossFunctionWithPrefs(d, context, expressionNode, prefs)
+}
+
+func crossFunctionWithPrefs(d *dataTreeNavigator, context Context, expressionNode *ExpressionNode, prefs crossFunctionPreferences) (Context, error) {
 	var results = list.New()
 
 	var evaluateAllTogether = true
@@ -124,15 +146,16 @@ func crossFunction(d *dataTreeNavigator, context Context, expressionNode *Expres
 			break
 		}
 	}
+
 	if evaluateAllTogether {
 		log.Debug("crossFunction evaluateAllTogether!")
-		return doCrossFunc(d, context, expressionNode, calculation, calcWhenEmpty)
+		return doCrossFunc(d, context, expressionNode, prefs)
 	}
 
 	log.Debug("crossFunction evaluate apart!")
 
 	for matchEl := context.MatchingNodes.Front(); matchEl != nil; matchEl = matchEl.Next() {
-		innerResults, err := doCrossFunc(d, context.SingleChildContext(matchEl.Value.(*CandidateNode)), expressionNode, calculation, calcWhenEmpty)
+		innerResults, err := doCrossFunc(d, context.SingleChildContext(matchEl.Value.(*CandidateNode)), expressionNode, prefs)
 		if err != nil {
 			return Context{}, err
 		}


Test patch:
diff --git a/pkg/yqlib/operator_booleans_test.go b/pkg/yqlib/operator_booleans_test.go
index e9b72f99..f5735bb2 100644
--- a/pkg/yqlib/operator_booleans_test.go
+++ b/pkg/yqlib/operator_booleans_test.go
@@ -44,6 +44,22 @@ var booleanOperatorScenarios = []expressionScenario{
 			"D0, P[], (doc)::b: hi\n",
 		},
 	},
+	{
+		skipDoc:     true,
+		description: "And should not run 2nd arg if first is false",
+		expression:  `false and test(3)`,
+		expected: []string{
+			"D0, P[], (!!bool)::false\n",
+		},
+	},
+	{
+		skipDoc:     true,
+		description: "Or should not run 2nd arg if first is true",
+		expression:  `true or test(3)`,
+		expected: []string{
+			"D0, P[], (!!bool)::true\n",
+		},
+	},
 	{
 		description: "`and` example",
 		expression:  `true and false`,
@@ -151,12 +167,21 @@ var booleanOperatorScenarios = []expressionScenario{
 		document:   `{a: true, b: false}`,
 		expression: `.[] or (false, true)`,
 		expected: []string{
-			"D0, P[a], (!!bool)::true\n",
 			"D0, P[a], (!!bool)::true\n",
 			"D0, P[b], (!!bool)::false\n",
 			"D0, P[b], (!!bool)::true\n",
 		},
 	},
+	{
+		skipDoc:    true,
+		document:   `{a: true, b: false}`,
+		expression: `.[] and (false, true)`,
+		expected: []string{
+			"D0, P[a], (!!bool)::false\n",
+			"D0, P[a], (!!bool)::true\n",
+			"D0, P[b], (!!bool)::false\n",
+		},
+	},
 	{
 		skipDoc:    true,
 		document:   `{}`,