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

Syntax Rewrite for v0.12.x #39

Merged
merged 106 commits into from
Jan 20, 2020
Merged

Conversation

patrickrgaffney
Copy link
Collaborator

@patrickrgaffney patrickrgaffney commented Sep 5, 2019

HCL2

Since v0.12, the underlying HCL syntax has undergone a lot of changes. This PR reworks the sublime-syntax to better support the new language features.

Rationale

I've only been updating the .sublime-syntax file — as I only use Sublime. A quick Google search shows a few different converter tools to turn it back into a PList-style .tmLanguage file if that's still desired.

While reworking the syntax I've been trying to update the scopes to match the official conventions better (if possible). All of the changes made so far can be seen in the changes.md file. I can remove that file when this is complete — I've been trying to keep a thorough list there.

The major goal of this PR was to update the syntax for the new HCL2 syntax. In addition to Terraform, other Hashicorp projects are using HCL for configuation now: Vault, Nomad, (and soon 🤞) Packer. The changes in this PR should better support non-Terraform-specific HCL files going forward.

I added a lot of documentation to the sublime-syntax and re-ordered it to better follow the HCL spec. I also added fairly extensive unit testing. 🎉

Merging this PR would close the following open issues:

Progress / TODO

  • Comments
    • inline #
    • inline //
    • inline /* ... */
    • block /* ... */
  • Language Constants
    • true / false
    • null
  • Numeric Constants
    • integers
    • floating-point
    • exponents
    • signed exponents
  • Strings
    • double quotes: string.quoted.double.terraform
    • punctuation scopes
    • character escapes
    • string interpolation
      • ${}
      • %{}
      • left-trim and right-trim ~
      • include operators
      • include attributes
      • include builtin variables
      • include functions
      • include builtin functions
      • include objects
      • include tuples
      • if directives
      • for directives
    • heredocs
      • << and <<- operators
      • allow multiline
      • allow identifier before first newline
      • allow identifier on last line
  • Operators
    • comparison
    • arithmetic
    • logic
    • conditional
    • ellipsis ...
    • parenthesis
    • splat operators .* and [*]
    • : in for-expressions
    • => in for-expressions
  • Attribute-Access
    • identifiers
    • definitions
    • dot-access
    • match subscripts
  • Tuples
    • match bracket [] syntax
    • include literals
    • match over multi-line
    • include comments
    • include operators
    • include attribute-access
    • allow nested tuples
    • match comma ,
    • include built-in variables
    • include objects
    • include function calls
    • for expressions
  • Objects
    • match curly {} syntax
    • include literals
    • match over multi-line
    • include comments
    • include operators
    • include tuples
    • include function calls
    • allow nested objects
    • attribute definition
    • attribute access
    • strings as keys
    • for expressions
    • include built-in variables
    • computed attribute names ()
  • Functions
    • match identifier function name
    • match named parameter values
    • include literals in parameter list
    • include tuples in parameter list
    • include objects in parameter list
    • include operators in parameter list
    • match comma ,
    • allow nested function calls
    • include comments
    • include built-in variables
    • for expressions
    • allow over multiple lines
    • meta: meta.function
  • for Expressions
    • match inside tuple
    • match inside object
    • for keyword
    • if keyword
    • in keyword
    • : operator
    • => operator
    • include literals
    • include operators
    • include function calls
    • include brackets
    • include builtin variables
    • include attribute-access
    • allow over multiple lines
    • include comments
  • Blocks
    • match type
    • match optional number of string labels
    • match optional number of identifier labels
    • meta_scope for entire block
    • allow nested blocks
    • include operators
    • include literals
    • include functions
    • include objects
    • include tuples
    • include attribute access
    • include builtin variables
    • include builtin block-types
    • include attribute definition
    • include constants
    • allow multi-line
    • include comments
    • meta: meta.block
  • Terraform specific
    • built-in functions (as of v0.12.8)
    • built-in variables
    • built-in block types
    • built-in primitive types (string, number, bool, any)
    • import statements

- CHANGED: scope `comment.line.number-sign.terraform` to `comment.line.terraform`
    - Better reflects scoping conventions
- FIXED: previously incorrect used `number-sign` even for `//` inline comments
- ADDED: syntax tests
- ADDED: syntax tests
- FIXED: removed invalid constants `yes`, `no`, `on`, `off`.
- ADDED: `null` value from v0.12
- ADDED: syntax tests
Numbers

- Separated into `Integers` and `Floats`
- REMOVED: unit of measurement suffixes
- REMOVED: hexadecimal literals

Integers

- CHANGED: scope `constant.numeric.terraform` is now `constant.numeric.integer.terraform`
    - Better reflects scoping conventions
- CHANGED: scope `constant.numeric.terraform` is now `constant.numeric.float.terraform`
    - Better reflects scoping conventions
- ADDED: exponents in float literals
- ADDED: syntax tests
More closely matches what is in the spec.
- REMOVED: the extra `string.terraform` scope
- ADDED: character escapes
  - newline
  - carriage return
  - backslash
  - quote
  - unicode points
- CHANGED: scope `entity.tag.embedded.[begin|end]` is now `punctuation.section.interpolation.[begin|end]`
    - Better reflects scoping conventions
- FIXED: use `clear_scopes` to remove the `string.*` scope inside interpolation
- ADDED: the trim-left and trim-right operators inside interpolation
- ADDED: template if/for directives
- ADDED: match literals inside string interpolation
- ADDED: comparison operators `==`, `!=`, `<`, `>`, `<=`, `>=`
- ADDED: arithmetic operators `+`, `-`, `*`, `/`, `%`
- ADDED: logic operators `&&`, `||`, `!`
- ADDED: conditional operator `predicate ? expression : expression`
- ADDED: first-class array syntax
- ADDED: matches index operations
- ADDED: `variable.other.member` scope on attribute access
- ADDED: `punctuation.accessor.dot` scope on dot-operator
- ADDED: attribute-only splat operator
- ADDED: matches attribute-access inside string interpolation
- ADDED: syntax-tests
- ADDED: allow `-` after first alpha character in identifiers
- CHANGED: `variable.assignment` to `variable.other.readwrite`
    - Better reflects scoping conventions
- ADDED: paren-matching for computed attr names
- CHANGED `keyword.operator` to `keyword.operator.assignment` for `=`
    - Better reflects scoping conventions
- ADDED: match outside of string interpolation
- CHANGED: `keyword.other.function.inline` to `variable.function`
    - Better reflects scoping conventions
- ADDED: match operators, literals, comments, attributes, etc inside parameter list
- ADDED: match variable parameters as `variable.parameter`
- ADDED: match commas in parameter list
- ADDED: syntax tests
@patrickrgaffney
Copy link
Collaborator Author

patrickrgaffney commented Oct 15, 2019

Okay, just pushed up some changes:

  • Changed attribute-access by . scope to match JS.
  • From what I can tell, it appears we are matching string interpolation punctuation the same as JS — via punctuation.section.interpolation.[begin|end] plus the meta.interpolation scope. This could be different depending on which version of Sublime you're on. I'm running Build 3210 from the Dev channel.
  • I'm cool with adding some illegal scopes to the definitions. Maybe they could come in a separate PR? I can create an issue to track it if that works for you.
  • I agree with you on the parameter highlighting. Looking through other standard language packages it looks like they only match parameter names on definition and for named parameters. Since HCL currently doesn't allow you provide named parameters or define your own functions, I removed the variable.parameter matching.

Object keys seem to be a bit of a mixed bag, here's what some of the standard languages do:

  • JS: meta.object-literal.key.js
  • Python: for dictionaries, keys are quoted, and thus are highlighted as strings with meta.mapping.key.python.
  • Go: for maps, keys can be any type that is comparable — which is quite a few of them. They're highlighted as a literal of that type, without a meta_scope.

We're currently using entity.name.tag, which is part of the "minimal color scheme coverage" in the Sublime docs. I left it as that for now as I didn't have a better idea, but I'm open to other suggestions.

I went through and added additional tests to cover the issues in #28, #26, #21, and #18. The tests we're already covering #23.

@patrickrgaffney
Copy link
Collaborator Author

I did some more digging into the object key syntax scopes today. It seems like most of the official sublime-syntaxes use a meta scope of meta.mapping.key on the entire key along with string.unquoted or string.quoted. Seems like this will be the standard way forward per this RFC.

Found in the following syntaxes: JS, Git Config, Lua, Python, Erlang, JSON.

I'm good with updating this package to use that scope, sound good @alexlouden?

@alexlouden
Copy link
Owner

I'm good with updating this package to use that scope, sound good @alexlouden?

Perfect, thank you!

From what I can tell, it appears we are matching string interpolation punctuation the same as JS — via punctuation.section.interpolation.[begin|end] plus the meta.interpolation scope.

Ah sorry, my previous comment was using babel-sublime (imo big improvement over the built-in JS one), which uses keyword:

image

The ${ is keyword.other.substitution.begin and the period is keyword.operator.accessor

if I just use the JS plugin then it doesn't highlight the ${ or the period at all (just white). Are you okay with using these?

@alexlouden
Copy link
Owner

Also again - thank you so much for doing all this!

@alexlouden
Copy link
Owner

I've just set up CircleCI to unit test the repo, failing at the moment since there aren't any tests but maybe will test this PR next commit?

@patrickrgaffney
Copy link
Collaborator Author

Interesting. I used to use babel-sublime pretty much exclusively, but recently the builtin-in JS package has gotten a lot better — any version after v3.1 has most of the newer TC-39 features. 😁

It does look like the built-in Monokai color scheme doesn't handle the recommended scopes for interpolation very well — which is odd since the other built-ins (Breakers, Celeste, Mariana, Sixteen) all seem to be catching those scopes.

Although I think this is a relatively "new" standard — looks like the RFC for interpolation scopes was accepted back in February. I can update them to use the keyword. prefix.

Per the latest scope naming RFC and the standard packages, we use
meta.mapping.key as the meta_scope, and either string.quoted or
string.unquoted around the key name. Computed keys only get the
meta_scope and are matched for expressions.
@patrickrgaffney
Copy link
Collaborator Author

Regarding CI — I have access to the GitHub actions beta. I could take a stab at adding one of those in a later PR if you want. I played around with it on another repo I maintain and it's pretty cool, plus you don't have to leave the GH UI.

@alexlouden
Copy link
Owner

the builtin-in JS package has gotten a lot better

Ah, that might be it! I switched over to primarily VS Code about two years ago :)

Regarding CI — I have access to the GitHub actions beta. I could take a stab at adding one of those in a later PR if you want. I played around with it on another repo I maintain and it's pretty cool, plus you don't have to leave the GH UI.

I had a quick poke around at creating a GH Action for it, but thought since it requires downloading and installing Sublime Text I thought maybe CircleCI's caching would help speed up tests a bit? By all means if you'd like to convert it to then go for it! The linting you set up in the other repo looks really neat

It was updated to use "punctuation.accessor", but the support for
this new scope appears to be hit or miss. It was changed to
"keyword.operator.accessor"
The scope was updated to be "punctuation.section.interpolation",
but that is a newer scope with limited color scheme support at this
time, so it was changed to be "keyword.other.interpolation".
@patrickrgaffney
Copy link
Collaborator Author

Updated the scopes for the attribute-access and string interpolation operators per our previous conversation. Sorry this took so long, been a long week.

@patrickrgaffney
Copy link
Collaborator Author

@alexlouden friendly ping 🙂 Are you okay with merging this?

@alexlouden
Copy link
Owner

Oh I'm sorry I missed this, if you're happy with it then let's merge it in!

@patrickrgaffney
Copy link
Collaborator Author

Yeah, I'm good with it. I'll let you merge and cut the new release. 😎

We should probably change the ## Unreleased in the CHANGELOG.md to the new version number (## vX.X.X) before the new release is made.

@jeffbyrnes
Copy link

For my own projects, I typically finalize the CHANGELOG & update the version number as a single commit, and tag that commit to use for the release.

@patrickrgaffney
Copy link
Collaborator Author

Yeah, I do something similar as well for my projects. Just didn't want to step on Alex's toes. 😁

@patrickrgaffney
Copy link
Collaborator Author

@alexlouden Did you want me to merge this?

@grigorov
Copy link

grigorov commented Jan 9, 2020

Hey, 2020 year... when merge?

@TaysirTayyab
Copy link

@alexlouden ping. Would love to get this merged in.

@plasticine
Copy link

This is amazing — thanks so much for all your work on this @patrickrgaffney & @alexlouden 👏

@alexlouden alexlouden merged commit 34d6a1d into alexlouden:master Jan 20, 2020
@alexlouden
Copy link
Owner

Sorry everyone, I've just merged it in and made a new release (hope I did it right!).

Thanks again for this @patrickrgaffney - please consider yourself a co-owner of the repo and do whatever you think is best with it going forward 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unit testing new terraform block isn't highlighted Add support for tfvars+tfstate
7 participants