From 9b06616fad5f464a5a8e1c3b4c0560101ea1ebce Mon Sep 17 00:00:00 2001
From: rhysd <lin90162@yahoo.co.jp>
Date: Fri, 3 May 2024 13:57:43 +0900
Subject: [PATCH] narrow type of logical operator expressions by assuming
 truthy/falsy value (fix #384)

---
 expr_sema.go                                 | 37 ++++++++++++-
 expr_sema_test.go                            | 55 ++++++++++++++++++++
 testdata/ok/issue-384_narrow-logical-op.yaml | 19 +++++++
 3 files changed, 109 insertions(+), 2 deletions(-)
 create mode 100644 testdata/ok/issue-384_narrow-logical-op.yaml

diff --git a/expr_sema.go b/expr_sema.go
index c6bf0a430..e2f138baf 100644
--- a/expr_sema.go
+++ b/expr_sema.go
@@ -867,10 +867,43 @@ func (sema *ExprSemanticsChecker) checkCompareOp(n *CompareOpNode) ExprType {
 	return BoolType{}
 }
 
+func (sema *ExprSemanticsChecker) checkWithAssuming(n ExprNode, isTruthy bool) ExprType {
+	switch n := n.(type) {
+	case *LogicalOpNode:
+		switch n.Kind {
+		case LogicalOpNodeKindAnd:
+			// When `L && R` is true, its type is R
+			if isTruthy {
+				sema.check(n.Left)
+				return sema.check(n.Right)
+			}
+		case LogicalOpNodeKindOr:
+			// When `L || R` is false, its type is R
+			if !isTruthy {
+				sema.check(n.Left)
+				return sema.check(n.Right)
+			}
+		}
+		return sema.checkLogicalOp(n)
+	case *NotOpNode:
+		return sema.checkWithAssuming(n.Operand, !isTruthy)
+	default:
+		return sema.check(n)
+	}
+}
+
 func (sema *ExprSemanticsChecker) checkLogicalOp(n *LogicalOpNode) ExprType {
-	lty := sema.check(n.Left)
 	rty := sema.check(n.Right)
-	return lty.Merge(rty)
+	switch n.Kind {
+	case LogicalOpNodeKindAnd:
+		// When L is false in L && R, its type is L. Otherwise R.
+		return sema.checkWithAssuming(n.Left, false).Merge(rty)
+	case LogicalOpNodeKindOr:
+		// When L is true in L || R, its type is L. Otherwise R.
+		return sema.checkWithAssuming(n.Left, true).Merge(rty)
+	default:
+		return AnyType{}
+	}
 }
 
 func (sema *ExprSemanticsChecker) check(expr ExprNode) ExprType {
diff --git a/expr_sema_test.go b/expr_sema_test.go
index bdbfefaae..092070368 100644
--- a/expr_sema_test.go
+++ b/expr_sema_test.go
@@ -664,6 +664,61 @@ func TestExprSemanticsCheckOK(t *testing.T) {
 			expected:   StringType{},
 			configVars: []string{"some_variable"},
 		},
+		{
+			what:     "narrow type of && operator by assumed value (#384)",
+			input:    "('foo' && 10) || 20",
+			expected: NumberType{},
+		},
+		{
+			what:     "narrow type of || operator by assumed value (#384)",
+			input:    "('foo' || 10) && 20",
+			expected: NumberType{},
+		},
+		{
+			what:     "narrow type of nested && operator",
+			input:    "((('foo' && true) || false) && 10) || 20",
+			expected: NumberType{},
+		},
+		{
+			what:     "narrow type of nested || operator",
+			input:    "((('foo' || true) && false) || 10) && 20",
+			expected: NumberType{},
+		},
+		{
+			what:     "don't narrow type nested && operator",
+			input:    "('foo' && 10) && 20",
+			expected: StringType{},
+		},
+		{
+			what:     "don't narrow type nested || operator",
+			input:    "('foo' || 10) || 20",
+			expected: StringType{},
+		},
+		{
+			what:     "narrowed || operator at LHS and && operator at RHS",
+			input:    "('foo' || 10) && (('foo' && 10) || 20)",
+			expected: NumberType{},
+		},
+		{
+			what:     "narrowed && operator at LHS and || operator at RHS",
+			input:    "('foo' && 10) || (('foo' || 10) && 20)",
+			expected: NumberType{},
+		},
+		{
+			what:     "not operator negates && operator type narrowing",
+			input:    "!('foo' && 10) && 20",
+			expected: NumberType{},
+		},
+		{
+			what:     "not operator negates || operator type narrowing",
+			input:    "!('foo' || 10) || 20",
+			expected: NumberType{},
+		},
+		{
+			what:     "double not operators does nothing on type narrowing",
+			input:    "!!('foo' || 10) && 20",
+			expected: NumberType{},
+		},
 	}
 
 	allSPFuncs := []string{}
diff --git a/testdata/ok/issue-384_narrow-logical-op.yaml b/testdata/ok/issue-384_narrow-logical-op.yaml
new file mode 100644
index 000000000..880e97611
--- /dev/null
+++ b/testdata/ok/issue-384_narrow-logical-op.yaml
@@ -0,0 +1,19 @@
+on: push
+
+jobs:
+  and_op_narrowing:
+    runs-on: ubuntu-latest
+    # `timeout-minutes` only allow number type value
+    timeout-minutes: ${{ '...' && 60 || 20 }}
+    steps:
+      - run: echo hello
+        # `timeout-minutes` only allow number type value
+        timeout-minutes: ${{ '...' && 60 || 20 }}
+  or_op_narrowing:
+    runs-on: ubuntu-latest
+    # `timeout-minutes` only allow number type value
+    timeout-minutes: ${{ ('...' || 60) && 20 }}
+    steps:
+      - run: echo hello
+        # `timeout-minutes` only allow number type value
+        timeout-minutes: ${{ ('...' || 60) && 20 }}