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

schema/decoder: Replace LiteralValue with LiteralType's Enum #260

Open
radeksimko opened this issue Apr 14, 2023 · 0 comments
Open

schema/decoder: Replace LiteralValue with LiteralType's Enum #260

radeksimko opened this issue Apr 14, 2023 · 0 comments
Labels
enhancement New feature or request technical-debt

Comments

@radeksimko
Copy link
Member

radeksimko commented Apr 14, 2023

Context

The primary known use case for LiteralValue is to serve as a form of "enum", i.e. a predictable list of literal values.

There is a lot of complexity currently embedded into LiteralValue and the schema also looks quite verbose + the LiteralValue alone isn't enough to express that common "enum", it takes OneOf{} and 1 LiteralValue per enum member.

https://github.com/search?q=repo%3Ahashicorp%2Fterraform-schema%20LiteralValue&type=code

It is also unclear whether this schema shape would be useful for providers for potentially communicating some dynamic enums, to support use cases such as the ones described in hashicorp/vscode-terraform#1235

Proposal

We should consider replacing roughly the following

Constraint: schema.OneOf{
	schema.LiteralValue{Value: cty.StringVal("one")},
	schema.LiteralValue{Value: cty.StringVal("two")},
	schema.LiteralValue{Value: cty.StringVal("three")},
}

with roughly this

Constraint: schema.LiteralType{
	Type: cty.String,
	Values: []cty.Value{
		cty.StringVal("one"),
		cty.StringVal("two"),
		cty.StringVal("three"),
	},
}

Implementation Notes

One potential design issue is how do we codify some more details about each value, such as:

  • IsDeprecated bool
  • Description lang.MarkupContent
  • anything else we may end up needing in the context of completion, hover, or e.g. documentLink

Embedding everything into LiteralType would make it easier to reuse all logic, which we mostly re-implement anyway.

It would also make it easier to e.g. render all possible values as part of hover data, do validation.

Lastly, it became clear that there's relatively little benefit in providing "accurate" value-comparison based hover, semantic token highlighting, go-to-* etc. which is where most of the extra complexity in LiteralValue comes from currently. In most cases treating values the same way as literal types may just be sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request technical-debt
Projects
None yet
Development

No branches or pull requests

1 participant