Skip to content

Commit

Permalink
Merge branch 'fromjson' (fix #464)
Browse files Browse the repository at this point in the history
  • Loading branch information
rhysd committed Nov 14, 2024
2 parents 5aaa4ce + cae9c3e commit 919b72a
Show file tree
Hide file tree
Showing 20 changed files with 333 additions and 60 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ ronn ./man/actionlint.1.ronn
or

```sh
make ./man/actionlint.1
make man
```

## How to develop playground
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ all: build test lint

t test: .testtimestamp

.linttimestamp: $(TESTS) $(SRCS) $(TOOL)
.linttimestamp: $(TESTS) $(SRCS) $(TOOL) docs/checks.md
go vet ./...
staticcheck ./...
GOOS=js GOARCH=wasm staticcheck ./playground
Expand Down
17 changes: 3 additions & 14 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Features:
- **Other several useful checks**; [glob syntax][filter-pattern-doc] validation, dependencies check for `needs:`,
runner label validation, cron syntax validation, ...

See [the full list][checks] of checks done by actionlint.
See the [full list][checks] of checks done by actionlint.

<img src="https://github.com/rhysd/ss/blob/master/actionlint/main.gif?raw=true" alt="actionlint reports 7 errors" width="806" height="492"/>

Expand Down Expand Up @@ -82,18 +82,6 @@ test.yaml:22:17: receiver of object dereference "permissions" must be type of ob
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
## Why?
- **Running a workflow is time consuming.** You need to push the changes and wait until the workflow runs on GitHub even if
it contains some trivial mistakes. [act][] is useful to debug the workflow locally. But it is not suitable for CI and still
time consuming when your workflow gets larger.
- **Checks of workflow files by GitHub are very loose.** It reports no error even if unexpected keys are in mappings
(meant that some typos in keys). And also it reports no error when accessing to property which is actually not existing.
For example `matrix.foo` when no `foo` is defined in `matrix:` section, it is evaluated to `null` and causes no error.
- **Some mistakes silently break a workflow.** Most common case I saw is specifying missing property to cache key. In the
case cache silently does not work properly but a workflow itself runs without error. So you might not notice the mistake
forever.

## Quick start
Install `actionlint` command by downloading [the released binary][releases] or by Homebrew or by `go install`. See
Expand Down Expand Up @@ -133,6 +121,8 @@ See [the usage document][usage] for more details.
When you see some bugs or false positives, it is helpful to [file a new issue][issue-form] with a minimal example
of input. Giving me some feedbacks like feature requests or ideas of additional checks is also welcome.

See the [contribution guide](./CONTRIBUTING.md) for more details.

## License

actionlint is distributed under [the MIT license](./LICENSE.txt).
Expand All @@ -145,7 +135,6 @@ actionlint is distributed under [the MIT license](./LICENSE.txt).
[playground]: https://rhysd.github.io/actionlint/
[shellcheck]: https://github.com/koalaman/shellcheck
[pyflakes]: https://github.com/PyCQA/pyflakes
[act]: https://github.com/nektos/act
[syntax-doc]: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions
[filter-pattern-doc]: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#filter-pattern-cheat-sheet
[script-injection-doc]: https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions#understanding-the-risk-of-script-injections
Expand Down
65 changes: 53 additions & 12 deletions docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ List of checks:
- [Permissions](#permissions)
- [Reusable workflows](#check-reusable-workflows)
- [ID naming convention](#id-naming-convention)
- [Contexts and special functions availability](#ctx-spfunc-availability)
- [Availability of contexts and special functions](#ctx-spfunc-availability)
- [Deprecated workflow commands](#check-deprecated-workflow-commands)
- [Conditions always evaluated to true at `if:`](#if-cond-always-true)
- [Action metadata syntax validation](#action-metadata-syntax)
Expand Down Expand Up @@ -395,8 +395,6 @@ jobs:
# Function overloads can be handled properly. contains() has string version and array version
- run: echo "${{ contains('hello, world', 'lo,') }}"
- run: echo "${{ contains(github.event.labels.*.name, 'enhancement') }}"
# format() has a special check for formatting string
- run: echo "${{ format('{0}{1}', 1, 2, 3) }}"
```
Output:
Expand All @@ -422,13 +420,9 @@ test.yaml:15:51: 2nd argument of function call is not assignable. "object" canno
|
15 | - run: echo "${{ startsWith('hello, world', github.event) }}"
| ^~~~~~~~~~~~~
test.yaml:20:24: format string "{0}{1}" does not contain placeholder {2}. remove argument which is unused in the format string [expression]
|
20 | - run: echo "${{ format('{0}{1}', 1, 2, 3) }}"
| ^~~~~~~~~~~~~~~~
```
[Playground](https://rhysd.github.io/actionlint/#eNqckMFOwzAQRO/9ilGF5IKciMItP8IROWHBAWe3yq4pUuR/Ry4SAqnNoScf5r3xaIU7HLLGzbv02m0AI7X6AnNmbWqe+8yWmxRqdorU6KA/FNBUsgMNUeBulgWZP1iO/DwIG30ZSnGX0LfRYu5b+iQ2vQBuK6gWZnsaLe5cpJTE4yhzenEeLol3tyhlu+rqGfk6y/9bvdpRLxBG1itG/6p/P2tT6Clpe9dymMjDEcfAA03Etl73KvMUbOeW+7Lsi/PYezx4PJ6k7wAAAP//nfWd6A==)
[Playground](https://rhysd.github.io/actionlint/#eNqckEFKxjAQhfc9xVCEqKQ5QC/iUpI6mGo6UzoTK5TcXWJB/OFvF11l8b7v5TFMPcxZYvPBQfoGQFG0vgBLJulqnkMmzV3yNfuNRHGWnQLoKtkDDpHBPGwbZPokXul1YFL8VijFHKHvo8YcHH4hqRyAbQVF/aIvo8ZHEzEltrDykt6MBZPYmicopT115Y58zbI3q0876gX8SHJh9J/6/zOXfMAk7tmRn9CCQYqeBpyQdK/7CQAA//9h6o/Y)
[Contexts][contexts-doc] and [built-in functions][funcs-doc] are strongly typed. Typos in property access of contexts and
function names can be checked. And invalid function calls like wrong number of arguments or type mismatch at parameter also
Expand All @@ -440,11 +434,58 @@ The semantics checker can properly handle that
- some parameters are optional (e.g. `join(strings, sep)` and `join(strings)`)
- some parameters are repeatable (e.g. `hashFiles(file1, file2, ...)`)

In addition, `format()` function has a special check for placeholders in the first parameter which represents the formatting
string.

Note that context names and function names are case-insensitive. For example, `toJSON` and `toJson` are the same function.

In addition, actionlint performs special checks on some built-in functions.

- `format()`: Checks placeholders in the first parameter which represents the format string.
- `fromJSON()`: Checks the JSON string is valid and the return value is strongly typed.

Example input:

```yaml
on: push
jobs:
test:
# ERROR: Key 'mac' does not exist in the object returned by the fromJSON()
runs-on: ${{ fromJSON('{"win":"windows-latest","linux":"ubuntul-latest"}')['mac'] }}
steps:
# ERROR: {2} is missing in the first argument of format()
- run: echo "${{ format('{0}{1}', 1, 2, 3) }}"
# ERROR: Argument for {2} is missing in the arguments of format()
- run: echo "${{ format('{0}{1}{2}', 1, 2) }}"
- run: echo This is a special branch!
# ERROR: Broken JSON string. Special check for fromJSON()
if: contains(fromJson('["main","release","dev"'), github.ref_name)
```

Output:

```
test.yaml:6:18: property "mac" is not defined in object type {linux: string; win: string} [expression]
|
6 | runs-on: ${{ fromJSON('{"win":"windows-latest","linux":"ubuntul-latest"}')['mac'] }}
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test.yaml:9:24: format string "{0}{1}" does not contain placeholder {2}. remove argument which is unused in the format string [expression]
|
9 | - run: echo "${{ format('{0}{1}', 1, 2, 3) }}"
| ^~~~~~~~~~~~~~~~
test.yaml:11:24: format string "{0}{1}{2}" contains placeholder {2} but only 2 arguments are given to format [expression]
|
11 | - run: echo "${{ format('{0}{1}{2}', 1, 2) }}"
| ^~~~~~~~~~~~~~~~~~~
test.yaml:14:31: broken JSON string is passed to fromJSON() at offset 23: unexpected end of JSON input [expression]
|
14 | if: contains(fromJson('["main","release","dev"'), github.ref_name)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
```

[Playground](https://rhysd.github.io/actionlint/#eNqMj0FL9DAQhu/7K95v+CAtZMVdb/kJHvSgt0Uk7aY20k5KJnGFkP8ure5R8DJzmPeZhzewwZJl3O3eQydmByQnad1AzCz7NfC/FAwxzPdPjw+NKnTxTGad53CR/WRXhDRNnvMnGcpd5pSn66Gq9qRm26sX1Lo9luQW+XYA+9Vj4PoxgDZTiLNNjSq3tRyq0jhoHDXuWtRKf4PK8cr9Bj2PXuAFFrK43tsJXbTcj/9+soAfDPrAyXqWZmsvgRt1otl6Jk3RTc6KI01n90Gq1XjzaczdTXTDK9vZtV8BAAD//8ITaRA=)

GitHub Actions does not provide the syntax to create an array or object constant. It [is popular](https://github.com/search?q=fromJSON%28%27+lang%3Ayaml&type=code)
to create such constants via `fromJSON()`.

<a id="check-contextual-step-object"></a>
## Contextual typing for `steps.<step_id>` objects

Expand Down Expand Up @@ -2578,7 +2619,7 @@ IDs must start with a letter or `_` and contain only alphanumeric characters, `-
convention, and reports invalid IDs as errors.

<a id="ctx-spfunc-availability"></a>
## Contexts and special functions availability
## Availability of contexts and special functions

Example input:

Expand Down
23 changes: 19 additions & 4 deletions expr_sema.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package actionlint

import (
"encoding/json"
"fmt"
"strconv"
"strings"
Expand Down Expand Up @@ -794,15 +795,15 @@ func checkFuncSignature(n *FuncCallNode, sig *FuncSignature, args []ExprType) *E
return nil
}

func (sema *ExprSemanticsChecker) checkBuiltinFunctionCall(n *FuncCallNode, _ *FuncSignature) {
func (sema *ExprSemanticsChecker) checkBuiltinFuncCall(n *FuncCallNode, sig *FuncSignature) ExprType {
sema.checkSpecialFunctionAvailability(n)

// Special checks for specific built-in functions
switch strings.ToLower(n.Callee) {
case "format":
lit, ok := n.Args[0].(*StringNode)
if !ok {
return
return sig.Ret
}
l := len(n.Args) - 1 // -1 means removing first format string argument

Expand All @@ -819,7 +820,22 @@ func (sema *ExprSemanticsChecker) checkBuiltinFunctionCall(n *FuncCallNode, _ *F
for i := range holders {
sema.errorf(n, "format string %q contains placeholder {%d} but only %d arguments are given to format", lit.Value, i, l)
}
case "fromjson":
lit, ok := n.Args[0].(*StringNode)
if !ok {
return sig.Ret
}
var v any
err := json.Unmarshal([]byte(lit.Value), &v)
if err == nil {
return typeOfJSONValue(v)
}
if s, ok := err.(*json.SyntaxError); ok {
sema.errorf(lit, "broken JSON string is passed to fromJSON() at offset %d: %s", s.Offset, s)
}
}

return sig.Ret
}

func (sema *ExprSemanticsChecker) checkFuncCall(n *FuncCallNode) ExprType {
Expand All @@ -846,8 +862,7 @@ func (sema *ExprSemanticsChecker) checkFuncCall(n *FuncCallNode) ExprType {
err := checkFuncSignature(n, sig, tys)
if err == nil {
// When one of overload pass type check, overload was resolved correctly
sema.checkBuiltinFunctionCall(n, sig)
return sig.Ret
return sema.checkBuiltinFuncCall(n, sig)
}
errs = append(errs, err)
}
Expand Down
20 changes: 18 additions & 2 deletions expr_sema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,8 @@ func TestExprSemanticsCheckOK(t *testing.T) {
},
{
what: "non-special function",
input: "fromJSON('{}')",
expected: AnyType{},
input: "contains('hello, world', 'o, w')",
expected: BoolType{},
availSPFuncs: []string{"always"},
},
{
Expand Down Expand Up @@ -729,6 +729,15 @@ func TestExprSemanticsCheckOK(t *testing.T) {
input: "format('hello {{{0}', 'world')", // First {{ is escaped. {0} is not escaped
expected: StringType{},
},
{
what: "fromJSON with JSON constant value",
input: `fromJSON('{"foo":true,"bar":["foo", 12.3],"piyo":null}')`,
expected: NewStrictObjectType(map[string]ExprType{
"foo": BoolType{},
"bar": &ArrayType{Elem: StringType{}}, // Element type was merged
"piyo": NullType{},
}),
},
}

allSPFuncs := []string{}
Expand Down Expand Up @@ -1255,6 +1264,13 @@ func TestExprSemanticsCheckError(t *testing.T) {
"must not start with the GITHUB_ prefix",
},
},
{
what: "broken JSON value at fromJSON argument",
input: `fromJSON('{"foo": true')`,
expected: []string{
"broken JSON string is passed to fromJSON() at offset 12",
},
},
}

allSP := []string{}
Expand Down
45 changes: 45 additions & 0 deletions expr_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,3 +427,48 @@ func (ty *ArrayType) DeepCopy() ExprType {
func EqualTypes(l, r ExprType) bool {
return l.Assignable(r) && r.Assignable(l)
}

// typeOfJSONValue returns the type of the given JSON value. The JSON value is an any value decoded by json.Unmarshal.
// https://pkg.go.dev/encoding/json#Unmarshal
//
// To unmarshal JSON into an interface value, Unmarshal stores one of these in the interface value:
// - bool, for JSON booleans
// - float64, for JSON numbers
// - string, for JSON strings
// - []interface{}, for JSON arrays
// - map[string]interface{}, for JSON objects
// - nil for JSON null
func typeOfJSONValue(v any) ExprType {
switch v := v.(type) {
case bool:
return BoolType{}
case float64:
return NumberType{}
case string:
return StringType{}
case []any:
var elem ExprType
for _, e := range v {
t := typeOfJSONValue(e)
if elem == nil {
elem = t
} else {
elem = elem.Merge(t)
}
}
if elem == nil {
elem = AnyType{}
}
return &ArrayType{Elem: elem}
case map[string]any:
props := make(map[string]ExprType, len(v))
for k, v := range v {
props[k] = typeOfJSONValue(v)
}
return NewStrictObjectType(props)
case nil:
return NullType{}
default:
panic(v) // Unreachable
}
}
Loading

0 comments on commit 919b72a

Please sign in to comment.