Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formalize interpolation language into a real typed interpreted language #785

Merged
merged 31 commits into from
Jan 14, 2015

Conversation

mitchellh
Copy link
Contributor

This PR turns the syntax of interpolated configuration values into a real parsed language with real types. Before this, it was just strings all the way down (somehow everything turned into strings). This was really weird because it didn't allow math operations, we actually support some concept of maps in variables, etc. It also led to less than stellar errors.

I'm going to do a bit more work on this PR, but opening it for any feedback and to show to some folks.

This PR is completely backwards compatible. It also ends up fixing a lot of issues or introducing new functionality:

  • String concatenation just works, so you can do things like ${file("foo_${count.index}.txt")} now.
  • Escape characters work properly: ${"foo\nbar"} (newline, example)
  • Numbers are supported: ${function(42)}, also floating point
  • All function argument counts are semantically verified
  • Types are all verified (although they're still all strings for now since the terraform package still gives us all strings. That is the next step to fix after this PR. The language itself is fully typed and type checked).
  • Super well tested. The last interpolation regexp stuff was reasonably tested but not as well as it could've been.

The language itself is a very standard interpreted language with lex -> parse -> semantic check -> execute phases. Lexer goes to a token stream parser goes to an AST tree, semantic check and execution is done using the visitor pattern on the AST tree.

Fixes #744

@mitchellh
Copy link
Contributor Author

The tests pass, Travis is just sad for some reason.

@armon
Copy link
Member

armon commented Jan 13, 2015

@lamdor
Copy link
Contributor

lamdor commented Jan 13, 2015

💯 Thanks for doing this, it's appreciated!

@svanharmelen
Copy link
Contributor

Nice... Really nice... Your clearly back from the holidays 😉

@knuckolls
Copy link
Contributor

👍 Much needed. Thanks. I happened to find a bug and wrote a fix in the existing code late last week pertaining to doing element(resource.*.parameter, count.index) calls. Going to see if that still remains on this branch and report back.

@ceh
Copy link
Contributor

ceh commented Jan 14, 2015

Woah, a treat for the weekend. I will enjoy reading this. Cheers @mitchellh!

@knuckolls
Copy link
Contributor

The bug does still exist. I'll get it written up this afternoon.

@mitchellh
Copy link
Contributor Author

@knuckolls I'm guessing its not a parsing bug so much as a logic but in the function at this point. And also probably because we don't have proper types being sent into the configuration yet.

@knuckolls
Copy link
Contributor

@mitchellh wrote it up in a new issue #795

@mitchellh
Copy link
Contributor Author

Coming in!

@ghost
Copy link

ghost commented May 4, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variable interpolation syntax is confounding in non-trivial cases
6 participants