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

Create an error type for unknown function diags #657

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Feb 14, 2024

Now that we have namespaced functions, and implementations like Terraform can add functions based on configuration, the reason for an unknown function call name becomes a little less clear. Because functions are populated outside of the hcl package scope, there isn't enough context to provide a useful diagnostic to the user.

We can create a new Diagnostic.Extra value satisfying a new FunctionCallUnknownDiagExtra interface, indicating specifically when a diagnostic is created due to an unknown function name. This will carry back the namespace and function name for the caller to inspect, which will allow refinement of the diagnostic based on information only known to the caller.

// FunctionCallUnknown is an interface implemented by a value in the Extra
// field of some diagnostics to indicate when the error was caused by a call to
// an unknown function.
type FunctionCallUnknown interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

The existing "diagnostic extra" interface nearby here has "DiagExtra" at the end of its name to try to make it clear that it's there only for used in the "diagnostic extra" field. Should we do that here too?

(If we do end up finding another use of this interface that isn't related to "diagnostic extra" then we can presumably add another name covering the same methods to make it back-compatible, but I'm not imagining any specific reason why we might do that in future.)

Copy link
Member Author

@jbardin jbardin Feb 14, 2024

Choose a reason for hiding this comment

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

Yeah, that's a good point. I think I removed that at some point because this fell more natural to describe the error, but it does lose the context that it's only for the Extra field.

Now that we have namespaced functions, and implementations like
Terraform can add functions based on configuration, the reason for an
unknown function call name becomes a little less clear. Because
functions are populated outside of the hcl package scope, there isn't
enough context to provide a useful diagnostic to the user.

We can create a new Diagnostic.Extra value for
FunctionCallUnknownDiagExtra to indicate specifically when a diagnostic
is created due to an unknown function name. This will carry back the
namespace and function name for the caller to inspect, which will allow
refinement of the diagnostic based on information only known to the
caller.
@jbardin jbardin force-pushed the jbardin/unknown-function-extra branch from 67f9054 to 159a39d Compare February 14, 2024 20:55
@jbardin jbardin merged commit fe0951f into main Feb 14, 2024
7 checks passed
@jbardin jbardin deleted the jbardin/unknown-function-extra branch February 14, 2024 20:57
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.

2 participants