Rewrite contexts before evaluating them

[badouralix]

Description

This PR aims to fix #104. In a nutshell, act uses a javascript virtual machine to interpret and evaluate expressions in github actions. It works pretty well, except for expressions like object.property-with-hyphens which are correctly handled by github action runners, but not by the javascript vm.

This solution is heavily inspired by #104 (comment) written by @Shadowfiend, although the implementation diverged a little bit from the initial proposal. The idea is to tweak just a little bit the expression and transform it into object['property-with-hyphens'] which is both github actions and javascript compliant.

To do so, we introduce the Rewrite method which job is exactly the one described above, and we call it just before evaluating the expression in the javascript vm.

Explanations

⚠️ The current implementation relies on a regex hackery ( see here to test it online ), which is fairly ugly and hard to maintain. In the future, we should invest some time to implement a proper lexer/parser and a proper term rewriter. But in the short-term, this regex works.

That being said, using regex for this rewriting is really hard to get right !

Here is a snapshot of what I tried...

^[a-zA-Z][a-zA-Z0-9_]*(\.[a-zA-Z][a-zA-Z0-9_-]*|\[.+\])*$
^\w+(?:\[.+\])*(\.[\w-]+)?
^(\w+(\[.+\])*)(\.[\w-]+)?
^(\w+(?:\[.+\])*)(\.[\w-]+)?
^(\w+(?:\[.+\])*)(?:\.([\w-]+))?
^(\w+(?:\[.+\])*)(?:\.([\w-]+))?(.*)$
^(?P<object>\w+(\[.+\])*)(\.(?P<property>[\w-]+))?(?P<trailing>.*)$

One thing to keep in mind is that not all . nor not all - are bad. For instance we do not want to rewrite filenames with extension.
But also, we would need to identify strings, escaped characters, already-bracketed expressions, recursive expressions, etc.

So let's keep it simple for now. We choose to only rewrite contexts, which is a subset of expressions
https://help.github.com/en/actions/reference/context-and-expression-syntax-for-github-actions#contexts

The pattern defines three groups, as described below :

^(\w+(?:\[.+\])*)(?:\.([\w-]+))?(.*)$
  \            /       \    /    \/
   '-----.----'         \  /      Remaining characters
         |               \/
         |                First level of property using dots
         |
         Current object
         Potentially nested object, but using the bracket notation

Here is the execution of the method step by step with in := "ecole.centrale.paris". See also https://play.golang.org/p/bu2rO817ycU

0. Before entering the loop

   Variable	Value
   re	"ecole.centrale.paris"

1. First iteration

   Variable	Value
   matches[0]	"ecole.centrale.paris"
   matches[1]	"ecole"
   matches[2]	"centrale"
   matches[3]	".paris"
   re	"ecole['centrale'].paris"

2. Second iteration

   Variable	Value
   matches[0]	"ecole['centrale'].paris"
   matches[1]	"ecole['centrale']"
   matches[2]	"paris"
   matches[3]	""
   re	"ecole['centrale']['paris']"

3. Third iteration

   Variable	Value
   matches[0]	"ecole['centrale']['paris']"
   matches[1]	"ecole['centrale']['paris']"
   matches[2]	""
   matches[3]	""
   re	"ecole['centrale']['paris']"

   Here we trigger the break condition and the function returns

Testing

Before the fix :

$ act --version
act version 0.2.9

$ act -P ubuntu-latest=node:12.6-buster-slim -W pkg/runner/testdata/issue-104
[Test stuff/Testing Testing] 🚀  Start image=node:12.6-buster-slim
[Test stuff/Testing Testing]   🐳  docker run image=node:12.6-buster-slim entrypoint=["/usr/bin/tail" "-f" "/dev/null"] cmd=[]
[Test stuff/Testing Testing] ⭐  Run hello
[Test stuff/Testing Testing]   ☁  git clone 'https://github.com/actions/hello-world-docker-action' # ref=master
ERRO[0001] Unable to interpolate string '${{ inputs.who-to-greet }}' - [ReferenceError: 'to' is not defined]
[Test stuff/Testing Testing]   🐳  docker build -t act-actions-hello-world-docker-action-master:latest /Users/ayaz.badouraly/.cache/act/actions-hello-world-docker-action@master
[Test stuff/Testing Testing]   🐳  docker run image=act-actions-hello-world-docker-action-master:latest entrypoint=[] cmd=[""]
| Hello
[Test stuff/Testing Testing]   ⚙  ::set-output:: time=Sun Jun 21 17:15:37 UTC 2020
[Test stuff/Testing Testing]   ✅  Success - hello

After the fix :

$ go run . -P ubuntu-latest=node:12.6-buster-slim -W pkg/runner/testdata/issue-104
[Test stuff/Testing Testing] 🚀  Start image=node:12.6-buster-slim
[Test stuff/Testing Testing]   🐳  docker run image=node:12.6-buster-slim entrypoint=["/usr/bin/tail" "-f" "/dev/null"] cmd=[]
[Test stuff/Testing Testing] ⭐  Run hello
[Test stuff/Testing Testing]   ☁  git clone 'https://github.com/actions/hello-world-docker-action' # ref=master
[Test stuff/Testing Testing]   🐳  docker build -t act-actions-hello-world-docker-action-master:latest /Users/ayaz.badouraly/.cache/act/actions-hello-world-docker-action@master
[Test stuff/Testing Testing]   🐳  docker run image=act-actions-hello-world-docker-action-master:latest entrypoint=[] cmd=["World"]
| Hello World
[Test stuff/Testing Testing]   ⚙  ::set-output:: time=Sun Jun 21 17:16:33 UTC 2020
[Test stuff/Testing Testing]   ✅  Success - hello

References

• Expressions in github actions
• Dot notation vs. bracket notation
• Test the regex online
• Test the rewrite function online
• École Centrale Paris

[cplee]

@badouralix - this is freaking awesome...how can i help?

[cplee]

thanks for the contribution!

[Shadowfiend]

🙌 thank you!

[justincy]

Can you cut a new release with this?

[myselfhimself]

yes please release
ERRO[0012] Unable to interpolate string '${{ matrix.python-version }}' - [ReferenceError: 'version' is not defined]

[cplee]

@justincy @myselfhimself - new release 0.2.12 is now available!

[myselfhimself]

Perfect I will test against expressions like matrix.python-version. Thanks! Let me come back and tell next week

[myselfhimself]

Fix confirmed thanks!