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

earlydecoder: avoid crash by validating provider names #130

Merged
merged 1 commit into from
Aug 5, 2022

Conversation

radeksimko
Copy link
Member

This is to address hashicorp/terraform-ls#1026

@radeksimko radeksimko added the bug Something isn't working label Aug 4, 2022
@radeksimko radeksimko requested a review from a team August 4, 2022 19:20
@radeksimko radeksimko self-assigned this Aug 4, 2022
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

This fixes the crash for me! 👍

But I never see the diagnostics published to VS Code which might be another bug?

@radeksimko
Copy link
Member Author

But I never see the diagnostics published to VS Code which might be another bug?

It is true that we never publish the diagnostics from earlydecoder, although we store them, so that dependent jobs can check those. We could publish them, but most of these diagnostics here can be little vague - so that would need to be addressed first.

There's also a question of duplication between some future diagnostics we end up producing from the main (version & schema specific) decoder + HCL diagnostics. So I'm not against publishing these diags, but I think it should be considered in the wider context in hashicorp/vscode-terraform#720

@radeksimko radeksimko merged commit 49cc6ae into main Aug 5, 2022
@radeksimko radeksimko deleted the b-validate-implied-provider-names branch August 5, 2022 10:26
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