bartavelle / language-puppet

A library to work with Puppet manifests, test them and eventually replace everything ruby.
BSD 3-Clause "New" or "Revised" License
51 stars 8 forks source link

Cannot parse assert_type lambda #253

Closed PierreR closed 6 years ago

PierreR commented 6 years ago

I guess this is similar to #200.

Would it be possible to support the following idiom:

 assert_type(Pattern[/^(aufs|devicemapper|btrfs|overlay|overlay2|vfs|zfs)$/], $storage_driver) |$a, $b| {
...
}

It is used for instance in https://github.com/puppetlabs/puppetlabs-docker/blob/master/manifests/init.pp#L529

It is currently failing to parse with:

x swarm.worker.testing: cannot parse ./modules/docker/manifests/init.pp:492:68:
    |
492 |     assert_type(Pattern[/^(Debian|RedHat|windows)$/], $::osfamily) |$a, $b| {
    |                                                                    ^
unexpected '|'
expecting '}' or Statement
 at ./modules/profile/manifests/docker.pp:116:5
bartavelle commented 6 years ago

It is indeed similar to #200.

bartavelle commented 6 years ago

It is possible to support such functions in an ad-hoc way, but it would be nice to have a principled way.

bartavelle commented 6 years ago

Alright, I changed stuff in the parser, you should have a new error for this, and for #200 !

bartavelle commented 6 years ago

(Please tell me this is the case)

PierreR commented 6 years ago

Unless I am missing something, I have the same error:

x swarm.worker.testing: cannot parse ./modules/docker/manifests/init.pp:491:68:
    |
491 |     assert_type(Pattern[/^(Debian|RedHat|windows)$/], $::osfamily) |$a, $b| {
    |                                                                    ^
unexpected '|'
expecting '}' or Statement
PierreR commented 6 years ago

ouch that's wicked. The lexer spec tests are not running on travis for some reason.

When I do a cabal test these tests are failing ...

bartavelle commented 6 years ago

I think that if you remove the type as the first argument, you would have another parsing error. This is yet another parsing problem. I think it would be nice to have a formal grammar for the language, but this will probably not happen ...

PierreR commented 6 years ago

I am not so sure because this is failing the same way and there is not type annotation:

class application::role::swarm::test (
) {
  $fruit = with("apples", "oranges", "bananas") |$x, $y, $z| {
    "${x}, ${y}, and ${z}"
  }
  notify {"${fruit}":}
}
ERROR: (swarm.manager.dev) cannot parse ./modules/application/manifests/role/swarm/test.pp:3:49:
  |
3 |   $fruit = with("apples", "oranges", "bananas") |$x, $y, $z| {
  |                                                 ^
unexpected '|'
expecting "!=", "!~", "<<", "<=", "==", "=~", ">=", ">>", "and", "in", "or", '*', '+', '-', '.', '/', '<', '>', '[', '}', or Statement
 at ./modules/application/manifests/role/swarm/manager.pp:2:3
bartavelle commented 6 years ago

right, this is getting problematic :) I think it is time to rewrite the parser from scratch, using something like happy

PierreR commented 6 years ago

This is well above my head but I would be quite interested to know more precisely why megaparsec isn't a good fit anymore (as I find parser quite fascinating in general) ;-)

bartavelle commented 6 years ago

It is not that, it is just that all the parsers are applied in order, and it is really hard for my feeble mind to understand what it means and what their interactions are.

A tool like happy can tell you where you have "conflicts", that is parts of your parsers that parse the "same thing".

bartavelle commented 6 years ago

But in theory megaparsec does the job just fine!

PierreR commented 6 years ago

So the plan would be the rewrite the parser with megaparsec with the help of something like happy to understand the interaction more.

bartavelle commented 6 years ago

I don't know at all. The plan right now would be to find some description of the language, so that I am not chasing randomly using only examples.

PierreR commented 6 years ago

This might be helpful ?

PierreR commented 6 years ago

I suspect the problem comes from the lambdaCall parser particularly the line that parses the with argument (list of expression separated by a comma) ? https://github.com/bartavelle/language-puppet/blob/master/src/Puppet/Parser.hs What do you think ?

PierreR commented 6 years ago

Maybe _hoLambdaExpr should be Vector Expression ?

PierreR commented 6 years ago

https://github.com/puppetlabs/puppet-specifications/blob/4e54d7a8c1e8f16c1cdbc44d4b5537c27784e7cd/language/calls.md#arguments

bartavelle commented 6 years ago

It is very helpful!

bartavelle commented 6 years ago

Oh right, that is the problem!

bartavelle commented 6 years ago

Do you want to try fixing this?

PierreR commented 6 years ago

No really ;-) I have had a go fixing the parser but then there are still many bits to change in resolve and I am not sure I know fully what I am doing. But if you don't have the time, I can give it a proper try if you prefer.

bartavelle commented 6 years ago

I just added the function. But I did not write tests for it :/

PierreR commented 6 years ago

It works !