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

Use upstreamed HCL typexpr package #168

Merged
merged 1 commit into from
Feb 7, 2023
Merged

Conversation

jpogran
Copy link
Contributor

@jpogran jpogran commented Dec 5, 2022

Replace internal copy of typeexpr package from Terraform core with the typeexpr package that was upstreamed back to HCL v2.14.0.

Currently we use a copy of typeexpr package from Terraform Core which represents Terraform 0.14 - 1.2.

The v1.2 functionality was upstreamed back to HCL v2.14.0 and Terraform Core switched back to that upstream and removed its own copy of the typeexpr package in hashicorp/terraform#31729

Part of hashicorp/terraform-ls#917

Replace internal copy of typeexpr package from Terraform core with the typeexpr package that was [upstreamed back to HCL v2.14.0](https://github.com/hashicorp/hcl/releases/tag/v2.14.0).

Currently we use a [copy of `typeexpr` package](https://github.com/hashicorp/terraform-schema/tree/8c89111ff59a5c1bbad6c54caf8440ec17f1e1a2/internal/typeexpr) from Terraform Core which represents Terraform `0.14 - 1.2`.

The v1.2 functionality was [upstreamed back to HCL v2.14.0](https://github.com/hashicorp/hcl/releases/tag/v2.14.0) and Terraform Core switched back to that upstream and removed its own copy of the `typeexpr` package in hashicorp/terraform#31729
@jpogran jpogran self-assigned this Dec 5, 2022
@jpogran jpogran marked this pull request as ready for review December 5, 2022 19:25
@jpogran jpogran linked an issue Dec 5, 2022 that may be closed by this pull request
@radeksimko radeksimko requested a review from a team December 6, 2022 10:13
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Just to clarify, for posterity - the expectation is that this change is no-op for the end-user, right?

Given that despite using the newer upstream implementation, the TypeConstraint method we use still represents the old <1.3 style type declaration which will continue to error on the defaults, such as in this snippet:

variable "test" {
  type = object({
    foo = string
    bar = optional(string, "toot")
  })
}

I am guessing the next step, which can be a separate PR, would be to leverage the new method TypeConstraintWithDefaults and fall back to TypeConstraint, which I think is what Terraform internally probably does. At least that's how the documentation suggests it works.

https://pkg.go.dev/github.com/hashicorp/hcl/[email protected]/ext/typeexpr#TypeConstraintWithDefaults

TypeConstraintWithDefaults attempts to parse the given expression as a type constraint which may include default values for object attributes. If successful both the resulting type and corresponding defaults are returned. If unsuccessful, error diagnostics are returned.

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.

Use upstreamed HCL typexpr package
2 participants