Skip to content

Commit

Permalink
improve if-cond rule example
Browse files Browse the repository at this point in the history
to explain how expression at condition of `if:` works
  • Loading branch information
rhysd committed Jul 12, 2023
1 parent 8cfb90b commit 1b9da1c
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 14 deletions.
27 changes: 20 additions & 7 deletions docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -2553,6 +2553,10 @@ jobs:
- run: echo 'Commit is pushed'
# OK
if: ${{ github.event_name == 'push' }}
- run: echo 'Commit is pushed'
# OK
if: |
github.event_name == 'push'
- run: echo 'Commit is pushed'
# ERROR: It is always evaluated to true
if: |
Expand All @@ -2571,21 +2575,21 @@ jobs:
Output:

```
test.yaml:12:13: if: condition "${{ github.event_name == 'push' }}\n" is always evaluated to true because extra characters are around ${{ }} [if-cond]
test.yaml:16:13: if: condition "${{ github.event_name == 'push' }}\n" is always evaluated to true because extra characters are around ${{ }} [if-cond]
|
12 | if: |
16 | if: |
| ^
test.yaml:16:13: if: condition "${{ github.event_name == 'push' }} " is always evaluated to true because extra characters are around ${{ }} [if-cond]
test.yaml:20:13: if: condition "${{ github.event_name == 'push' }} " is always evaluated to true because extra characters are around ${{ }} [if-cond]
|
16 | if: "${{ github.event_name == 'push' }} "
20 | if: "${{ github.event_name == 'push' }} "
| ^~~~
test.yaml:22:13: if: condition "${{ github.event_name == 'push' }} && ${{ github.ref_name == 'main' }}" is always evaluated to true because extra characters are around ${{ }} [if-cond]
test.yaml:26:13: if: condition "${{ github.event_name == 'push' }} && ${{ github.ref_name == 'main' }}" is always evaluated to true because extra characters are around ${{ }} [if-cond]
|
22 | if: ${{ github.event_name == 'push' }} && ${{ github.ref_name == 'main' }}
26 | if: ${{ github.event_name == 'push' }} && ${{ github.ref_name == 'main' }}
| ^~~
```

[Playground](https://rhysd.github.io/actionlint#eJy1j0sKwjAUReddxaVIM6oLCHTkQqTVVxsxSWlenNTu3aT+KoJWwVF4nMO5xBqJ1rsmSfa2cjIBmBzHF+i8cbkNgq+8YZ8fyshG5Jhad7GAPJoStGksxMpqrRjKjVnaiqsEqFpi0ffYKW58taQjGV6bUhOKAiLaAsPwffN0v/CXfvo5inROFmyhS2We8+/KWXbDHdUPOI38sDjjP2F4Yr2OB+cMjO6qVQ==)
[Playground](https://rhysd.github.io/actionlint#eJy1j00OgjAQhfec4oUYusIDNGHlQQzoIDW2JXTqBrm7FP8wJogaV5PJ+/K9GWskau+qKNrbwskIYHIcJtB441LbA77whn16yEM2RI6pdhcKSAMpQZvKQqys1oqh3KClrbhCgColFm2LneLKF0s6kuG1yTUhyyACLdB1nztP9w1T7t/E/zg8fi9FPEcLttC5Ms/6KXOS3OKGykc4lnzROOOfvnhEvZb3zBmiAMLK)

Evaluation of `${{ }}` at `if:` condition is tricky. When the expression in `${{ }}` is evaluated to boolean value and there is
no extra characters around the `${{ }}`, the condition is evaluated to the boolean value. Otherwise the condition is treated as
Expand All @@ -2605,6 +2609,15 @@ is equivalent to
if: "${{ false }}\n"
```

Unlike using `${{ }}`, putting an expression directly ignores white spaces around it. It's the reason why

```yaml
if: |
false
```

works as intended.

actionlint checks all `if:` conditions in workflow and reports error when some condition is always evaluated to true due to extra
characters around `${{ }}`.

Expand Down
8 changes: 4 additions & 4 deletions rule_if_cond_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ func TestRuleIfCond(t *testing.T) {

errs := r.Errs()
if tc.valid && len(errs) > 0 {
t.Fatalf("wanted no error but have %q", errs)
t.Fatalf("wanted no error but have %q for condition %q", errs, tc.cond)
}
if !tc.valid && len(errs) != 1 {
t.Fatalf("wanted one error but have %q", errs)
t.Fatalf("wanted one error but have %q for condition %q", errs, tc.cond)
}
})
}
Expand All @@ -56,10 +56,10 @@ func TestRuleIfCond(t *testing.T) {

errs := r.Errs()
if tc.valid && len(errs) > 0 {
t.Fatalf("wanted no error but have %q", errs)
t.Fatalf("wanted no error but have %q for condition %q", errs, tc.cond)
}
if !tc.valid && len(errs) != 1 {
t.Fatalf("wanted one error but have %q", errs)
t.Fatalf("wanted one error but have %q for condition %q", errs, tc.cond)
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions testdata/examples/if_cond_always_true.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
test.yaml:12:13: if: condition "${{ github.event_name == 'push' }}\n" is always evaluated to true because extra characters are around ${{ }} [if-cond]
test.yaml:16:13: if: condition "${{ github.event_name == 'push' }} " is always evaluated to true because extra characters are around ${{ }} [if-cond]
test.yaml:22:13: if: condition "${{ github.event_name == 'push' }} && ${{ github.ref_name == 'main' }}" is always evaluated to true because extra characters are around ${{ }} [if-cond]
test.yaml:16:13: if: condition "${{ github.event_name == 'push' }}\n" is always evaluated to true because extra characters are around ${{ }} [if-cond]
test.yaml:20:13: if: condition "${{ github.event_name == 'push' }} " is always evaluated to true because extra characters are around ${{ }} [if-cond]
test.yaml:26:13: if: condition "${{ github.event_name == 'push' }} && ${{ github.ref_name == 'main' }}" is always evaluated to true because extra characters are around ${{ }} [if-cond]
4 changes: 4 additions & 0 deletions testdata/examples/if_cond_always_true.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ jobs:
- run: echo 'Commit is pushed'
# OK
if: ${{ github.event_name == 'push' }}
- run: echo 'Commit is pushed'
# OK
if: |
github.event_name == 'push'
- run: echo 'Commit is pushed'
# ERROR: It is always evaluated to true
if: |
Expand Down

0 comments on commit 1b9da1c

Please sign in to comment.