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

Require provider-defined function parameter naming #964

Merged
merged 13 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/unreleased/BREAKING CHANGES-20240320-095152.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: BREAKING CHANGES
body: 'function: All parameters must be explicitly named via the `Name` field'
time: 2024-03-20T09:51:52.869254Z
custom:
Issue: "964"
6 changes: 6 additions & 0 deletions .changes/unreleased/BREAKING CHANGES-20240320-171454.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: BREAKING CHANGES
body: 'function: `DefaultParameterNamePrefix` and `DefaultVariadicParameterName` constants
have been removed'
time: 2024-03-20T17:14:54.480412Z
custom:
Issue: "964"
52 changes: 40 additions & 12 deletions function/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,22 +90,22 @@ func (d Definition) Parameter(ctx context.Context, position int) (Parameter, dia
// implementation of the definition to prevent unexpected errors or panics. This
// logic runs during the GetProviderSchema RPC, or via provider-defined unit
// testing, and should never include false positives.
func (d Definition) ValidateImplementation(ctx context.Context) diag.Diagnostics {
func (d Definition) ValidateImplementation(ctx context.Context, req DefinitionValidateRequest, resp *DefinitionValidateResponse) {
var diags diag.Diagnostics

if d.Return == nil {
diags.AddError(
"Invalid Function Definition",
"When validating the function definition, an implementation issue was found. "+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
"Definition Return field is undefined",
fmt.Sprintf("Function %q - Definition Return field is undefined", req.FuncName),
)
} else if d.Return.GetType() == nil {
diags.AddError(
"Invalid Function Definition",
"When validating the function definition, an implementation issue was found. "+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
"Definition return data type is undefined",
fmt.Sprintf("Function %q - Definition return data type is undefined", req.FuncName),
)
} else if returnWithValidateImplementation, ok := d.Return.(fwfunction.ReturnWithValidateImplementation); ok {
req := fwfunction.ValidateReturnImplementationRequest{}
Expand All @@ -120,9 +120,14 @@ func (d Definition) ValidateImplementation(ctx context.Context) diag.Diagnostics
for pos, param := range d.Parameters {
parameterPosition := int64(pos)
name := param.GetName()
// If name is not set, default the param name based on position: "param1", "param2", etc.
// If name is not set, add an error diagnostic, parameter names are mandatory.
if name == "" {
name = fmt.Sprintf("%s%d", DefaultParameterNamePrefix, pos+1)
diags.AddError(
"Invalid Function Definition",
"When validating the function definition, an implementation issue was found. "+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
fmt.Sprintf("Function %q - Parameter at position %d does not have a name", req.FuncName, pos),
)
}

if paramWithValidateImplementation, ok := param.(fwfunction.ParameterWithValidateImplementation); ok {
Expand All @@ -138,13 +143,13 @@ func (d Definition) ValidateImplementation(ctx context.Context) diag.Diagnostics
}

conflictPos, exists := paramNames[name]
if exists {
if exists && name != "" {
diags.AddError(
"Invalid Function Definition",
"When validating the function definition, an implementation issue was found. "+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
"Parameter names must be unique. "+
fmt.Sprintf("Parameters at position %d and %d have the same name %q", conflictPos, pos, name),
fmt.Sprintf("Function %q - Parameters at position %d and %d have the same name %q", req.FuncName, conflictPos, pos, name),
)
continue
}
Expand All @@ -154,9 +159,14 @@ func (d Definition) ValidateImplementation(ctx context.Context) diag.Diagnostics

if d.VariadicParameter != nil {
name := d.VariadicParameter.GetName()
// If name is not set, default the variadic param name
// If name is not set, add an error diagnostic, parameter names are mandatory.
if name == "" {
name = DefaultVariadicParameterName
diags.AddError(
"Invalid Function Definition",
"When validating the function definition, an implementation issue was found. "+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
fmt.Sprintf("Function %q - The variadic parameter does not have a name", req.FuncName),
)
}

if paramWithValidateImplementation, ok := d.VariadicParameter.(fwfunction.ParameterWithValidateImplementation); ok {
Expand All @@ -171,18 +181,18 @@ func (d Definition) ValidateImplementation(ctx context.Context) diag.Diagnostics
}

conflictPos, exists := paramNames[name]
if exists {
if exists && name != "" {
diags.AddError(
"Invalid Function Definition",
"When validating the function definition, an implementation issue was found. "+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
"Parameter names must be unique. "+
fmt.Sprintf("Parameter at position %d and the variadic parameter have the same name %q", conflictPos, name),
fmt.Sprintf("Function %q - Parameter at position %d and the variadic parameter have the same name %q", req.FuncName, conflictPos, name),
)
}
}

return diags
resp.Diagnostics.Append(diags...)
}

// DefinitionRequest represents a request for the Function to return its
Expand All @@ -202,3 +212,21 @@ type DefinitionResponse struct {
// An empty slice indicates success, with no warnings or errors generated.
Diagnostics diag.Diagnostics
}

// DefinitionValidateRequest represents a request for the Function to validate its
// definition. An instance of this request struct is supplied as an argument to
// the Definition type ValidateImplementation method.
type DefinitionValidateRequest struct {
// FuncName is the name of the function definition being validated.
FuncName string
}

// DefinitionValidateResponse represents a response to a DefinitionValidateRequest.
// An instance of this response struct is supplied as an argument to the Definition
// type ValidateImplementation method.
type DefinitionValidateResponse struct {
// Diagnostics report errors or warnings related to validation of a function
// definition. An empty slice indicates success, with no warnings or errors
// generated.
Diagnostics diag.Diagnostics
}