Skip to content

decoder: Add support for LiteralType as Constraint #186

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.

Sign up for GitHub

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

Jump to bottom
Merged
radeksimko merged 8 commits into from
Mar 20, 2023
Merged

decoder: Add support for LiteralType as Constraint #186

radeksimko merged 8 commits into from
Mar 20, 2023

Conversation

@radeksimko
Copy link
Member

@radeksimko radeksimko commented Jan 16, 2023 •
edited
Loading

Part of hashicorp/terraform-ls#496

Existing tests were updated as part of 6990693 (not part of this PR) and yield either comparable or better results, except for reference collection, which:

  • we agreed to leave for a separate PR
  • involves other constraints which aren't implemented yet

UX

2023-03-16 20 12 50

@radeksimko radeksimko added the enhancement New feature or request label Jan 16, 2023
@radeksimko radeksimko force-pushed the f-expr-cons-literal-type branch 4 times, most recently from e7e068d to fb1fc7a Compare January 23, 2023 13:53
@radeksimko radeksimko self-assigned this Jan 23, 2023
@radeksimko radeksimko force-pushed the f-expr-cons-literal-type branch 5 times, most recently from 3dc35cf to 0358cd7 Compare January 27, 2023 14:08
@radeksimko radeksimko force-pushed the f-expr-cons-literal-type branch from 0358cd7 to f3bcd05 Compare February 16, 2023 15:10
@radeksimko radeksimko removed their assignment Feb 16, 2023
@radeksimko radeksimko mentioned this pull request Feb 17, 2023
Merged
@radeksimko radeksimko changed the title decoder/schema: Add support for LiteralType as Constraint decoder: Add support for LiteralType as Constraint Mar 7, 2023
@radeksimko radeksimko mentioned this pull request Mar 13, 2023
Closed
15 tasks
@radeksimko radeksimko self-assigned this Mar 15, 2023
@radeksimko radeksimko force-pushed the f-expr-cons-literal-type branch 7 times, most recently from cfdd98f to 680c316 Compare March 15, 2023 14:17
@radeksimko radeksimko force-pushed the f-expr-cons-literal-type branch 5 times, most recently from b7b2a80 to 25ca427 Compare March 16, 2023 13:53
@radeksimko radeksimko force-pushed the f-expr-cons-literal-type branch 2 times, most recently from cdd0ea8 to f5566d6 Compare March 16, 2023 18:08
@radeksimko radeksimko marked this pull request as ready for review March 16, 2023 20:20
@radeksimko radeksimko requested a review from a team as a code owner March 16, 2023 20:20
@radeksimko radeksimko force-pushed the f-expr-cons-literal-type branch from f5566d6 to 1909c29 Compare March 20, 2023 10:36
radeksimko
radeksimko commented Mar 20, 2023
View reviewed changes
jpogran
jpogran approved these changes Mar 20, 2023
View reviewed changes
Copy link
Contributor

@jpogran jpogran left a comment

Choose a reason for hiding this comment

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

Small concerns. Great work!

@radeksimko
Copy link
Member Author

radeksimko commented Mar 20, 2023 •
edited
Loading

I added LiteralType validation for nil Type and also another one to ensure there's either Constraint or Expr, so that should ensure integrity on a higher level and ensure there's no nil Constraint.

If we still somehow end up with nil constraint further down then I would call that a really bad bug which needs to be fixed in the place that produces the schema, not down here. In other words, we shouldn't be producing invalid schema in the first place anywhere and then hiding that fact, so crash is the right thing to do in such a case.

@radeksimko radeksimko mentioned this pull request Mar 20, 2023
Open
@radeksimko radeksimko merged commit 050d866 into main Mar 20, 2023
@radeksimko radeksimko deleted the f-expr-cons-literal-type branch March 20, 2023 16:50
This was referenced Apr 25, 2023
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jpogran jpogran jpogran approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@radeksimko radeksimko

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@radeksimko @jpogran