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

Skip inferring variable type from default values #338

Merged
merged 2 commits into from
Nov 2, 2023

Conversation

dbanck
Copy link
Member

@dbanck dbanck commented Oct 27, 2023

This PR removes the concept of inferring types from default values via AttributeValue.

See Radek's comment below for more information.

UX

Before
CleanShot 2023-11-01 at 17 28 59@2x

After
CleanShot 2023-11-01 at 17 29 21@2x


@dbanck dbanck added the bug Something isn't working label Oct 27, 2023
@dbanck dbanck self-assigned this Oct 27, 2023
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.

Given the conversation we had with TF Core team on this topic in Slack, I think we should consider entirely binning the concept of inferring type based on value - i.e. remove AsTypeOf entirely, because the behaviour of inferring the type based on default is not something present in Terraform.

We may revisit the treatment of default and generally evaluation of expressions as part of #132 or hashicorp/terraform-ls#495 but for now I think we should bin it.

We currently only use it for variable defaults anyway, so if we don't end up using it at all then we should treat it as dead code worth removing.

It does make our completion less helpful but it may as well just push module authors towards being more explicit and we may create further pressure as part of hashicorp/terraform-ls#1476

@dbanck dbanck force-pushed the b-update-variable-infer branch from 9dd29f7 to b5ae66d Compare November 1, 2023 16:24
@dbanck dbanck changed the title Skip inferring variable type from [] or {} default values Skip inferring variable type from default values Nov 1, 2023
@dbanck dbanck marked this pull request as ready for review November 1, 2023 16:30
@dbanck dbanck requested a review from a team as a code owner November 1, 2023 16:30
@dbanck dbanck merged commit caa96d6 into main Nov 2, 2023
5 checks passed
@dbanck dbanck deleted the b-update-variable-infer branch November 2, 2023 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants