Skip to content

Commit

Permalink
narrow type of logical operator expressions by assuming truthy/falsy …
Browse files Browse the repository at this point in the history
…value (fix #384)
  • Loading branch information
rhysd committed May 3, 2024
1 parent ef68695 commit 9b06616
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 2 deletions.
37 changes: 35 additions & 2 deletions expr_sema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
55 changes: 55 additions & 0 deletions expr_sema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
19 changes: 19 additions & 0 deletions testdata/ok/issue-384_narrow-logical-op.yaml
Original file line number Diff line number Diff line change
@@ -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 }}

0 comments on commit 9b06616

Please sign in to comment.