function: Add validation for parameter name conflicts and update defaulting logic

.changes/unreleased/BUG FIXES-20240228-151959.yaml

@@ -0,0 +1,6 @@
+kind: BUG FIXES
+body: 'function: Added implementation validation to `function.Definition` to ensure
+  all parameter names (including the variadic parameter) are unique.'
+time: 2024-02-28T15:19:59.843244-05:00
+custom:
+  Issue: "926"

.changes/unreleased/BUG FIXES-20240228-152155.yaml

@@ -0,0 +1,6 @@
+kind: BUG FIXES
+body: 'function: Updated the default parameter name to include the position of the
+  parameter (i.e. `param1`, `param2`, etc.). Variadic parameters will default to `varparam`.'
+time: 2024-02-28T15:21:55.182389-05:00
+custom:
+  Issue: "926"

function/bool_parameter.go

@@ -60,8 +60,11 @@ type BoolParameter struct {
 
 	// Name is a short usage name for the parameter, such as "data". This name
 	// is used in documentation, such as generating a function signature,
-	// however its usage may be extended in the future. If no name is provided,
-	// this will default to "param".
+	// however its usage may be extended in the future.
+	//
+	// If no name is provided, this will default to "param" with a suffix of the
+	// position the parameter is in the function definition. ("param1", "param2", etc.)
+	// If the parameter is variadic, the default name will be "varparam".
 	//
 	// This must be a valid Terraform identifier, such as starting with an
 	// alphabetical character and followed by alphanumeric or underscore
@@ -91,11 +94,7 @@ func (p BoolParameter) GetMarkdownDescription() string {
 
 // GetName returns the parameter name.
 func (p BoolParameter) GetName() string {
-	if p.Name != "" {
-		return p.Name
-	}
-
-	return DefaultParameterName
+	return p.Name
 }
 
 // GetType returns the parameter data type.

function/bool_parameter_test.go

@@ -182,13 +182,7 @@ func TestBoolParameterGetName(t *testing.T) {
 	}{
 		"unset": {
 			parameter: function.BoolParameter{},
-			expected:  function.DefaultParameterName,
-		},
-		"Name-empty": {
-			parameter: function.BoolParameter{
-				Name: "",
-			},
-			expected: function.DefaultParameterName,
+			expected:  "",
 		},
 		"Name-nonempty": {
 			parameter: function.BoolParameter{

function/definition.go

@@ -108,6 +108,48 @@ func (d Definition) ValidateImplementation(ctx context.Context) diag.Diagnostics
 		)
 	}
 
+	paramNames := make(map[string]int, len(d.Parameters))
+	for pos, param := range d.Parameters {
+		name := param.GetName()
+		// If name is not set, default the param name based on position: "param1", "param2", etc.
+		if name == "" {
+			name = fmt.Sprintf("%s%d", DefaultParameterNamePrefix, pos+1)
+		}
+
+		conflictPos, exists := paramNames[name]
+		if exists {
+			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),
+			)
+			continue
+		}
+
+		paramNames[name] = pos
+	}
+
+	if d.VariadicParameter != nil {
+		name := d.VariadicParameter.GetName()
+		// If name is not set, default the variadic param name
+		if name == "" {
+			name = DefaultVariadicParameterName
+		}
+
+		conflictPos, exists := paramNames[name]
+		if exists {
+			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),
+			)
+		}
+	}
+
 	return diags
 }
 

function/definition_test.go

@@ -152,11 +152,35 @@ func TestDefinitionValidateImplementation(t *testing.T) {
 		definition function.Definition
 		expected   diag.Diagnostics
 	}{
-		"valid": {
+		"valid-no-params": {
 			definition: function.Definition{
 				Return: function.StringReturn{},
 			},
 		},
+		"valid-only-variadic": {
+			definition: function.Definition{
+				VariadicParameter: function.StringParameter{},
+				Return:            function.StringReturn{},
+			},
+		},
+		"valid-param-name-defaults": {
+			definition: function.Definition{
+				Parameters: []function.Parameter{
+					function.StringParameter{},
+					function.StringParameter{},
+				},
+				Return: function.StringReturn{},
+			},
+		},
+		"valid-param-names-defaults-with-variadic": {
+			definition: function.Definition{
+				Parameters: []function.Parameter{
+					function.StringParameter{},
+				},
+				VariadicParameter: function.NumberParameter{},
+				Return:            function.StringReturn{},
+			},
+		},
 		"result-missing": {
 			definition: function.Definition{},
 			expected: diag.Diagnostics{
@@ -168,6 +192,153 @@ func TestDefinitionValidateImplementation(t *testing.T) {
 				),
 			},
 		},
+		"conflicting-param-names": {
+			definition: function.Definition{
+				Parameters: []function.Parameter{
+					function.StringParameter{
+						Name: "string-param",
+					},
+					function.Float64Parameter{
+						Name: "float-param",
+					},
+					function.Int64Parameter{
+						Name: "param-dup",
+					},
+					function.NumberParameter{
+						Name: "number-param",
+					},
+					function.BoolParameter{
+						Name: "param-dup",
+					},
+				},
+				Return: function.StringReturn{},
+			},
+			expected: diag.Diagnostics{
+				diag.NewErrorDiagnostic(
+					"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. "+
+						"Parameters at position 2 and 4 have the same name \"param-dup\"",
+				),
+			},
+		},
+		"conflicting-param-name-with-default": {
+			definition: function.Definition{
+				Parameters: []function.Parameter{
+					function.StringParameter{
+						Name: "param2",
+					},
+					function.Float64Parameter{}, // defaults to param2
+				},
+				Return: function.StringReturn{},
+			},
+			expected: diag.Diagnostics{
+				diag.NewErrorDiagnostic(
+					"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. "+
+						"Parameters at position 0 and 1 have the same name \"param2\"",
+				),
+			},
+		},
+		"conflicting-param-names-variadic": {
+			definition: function.Definition{
+				Parameters: []function.Parameter{
+					function.Float64Parameter{
+						Name: "float-param",
+					},
+					function.Int64Parameter{
+						Name: "param-dup",
+					},
+					function.NumberParameter{
+						Name: "number-param",
+					},
+				},
+				VariadicParameter: function.BoolParameter{
+					Name: "param-dup",
+				},
+				Return: function.StringReturn{},
+			},
+			expected: diag.Diagnostics{
+				diag.NewErrorDiagnostic(
+					"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. "+
+						"Parameter at position 1 and the variadic parameter have the same name \"param-dup\"",
+				),
+			},
+		},
+		"conflicting-param-names-variadic-multiple": {
+			definition: function.Definition{
+				Parameters: []function.Parameter{
+					function.StringParameter{
+						Name: "param-dup",
+					},
+					function.Float64Parameter{
+						Name: "float-param",
+					},
+					function.Int64Parameter{
+						Name: "param-dup",
+					},
+					function.NumberParameter{
+						Name: "number-param",
+					},
+					function.BoolParameter{
+						Name: "param-dup",
+					},
+				},
+				VariadicParameter: function.BoolParameter{
+					Name: "param-dup",
+				},
+				Return: function.StringReturn{},
+			},
+			expected: diag.Diagnostics{
+				diag.NewErrorDiagnostic(
+					"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. "+
+						"Parameters at position 0 and 2 have the same name \"param-dup\"",
+				),
+				diag.NewErrorDiagnostic(
+					"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. "+
+						"Parameters at position 0 and 4 have the same name \"param-dup\"",
+				),
+				diag.NewErrorDiagnostic(
+					"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. "+
+						"Parameter at position 0 and the variadic parameter have the same name \"param-dup\"",
+				),
+			},
+		},
+		"conflicting-param-name-with-variadic-default": {
+			definition: function.Definition{
+				Parameters: []function.Parameter{
+					function.Float64Parameter{
+						Name: function.DefaultVariadicParameterName,
+					},
+				},
+				VariadicParameter: function.BoolParameter{}, // defaults to varparam
+				Return:            function.StringReturn{},
+			},
+			expected: diag.Diagnostics{
+				diag.NewErrorDiagnostic(
+					"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. "+
+						"Parameter at position 0 and the variadic parameter have the same name \"varparam\"",
+				),
+			},
+		},
 	}
 
 	for name, testCase := range testCases {

function/float64_parameter.go

@@ -57,8 +57,11 @@ type Float64Parameter struct {
 
 	// Name is a short usage name for the parameter, such as "data". This name
 	// is used in documentation, such as generating a function signature,
-	// however its usage may be extended in the future. If no name is provided,
-	// this will default to "param".
+	// however its usage may be extended in the future.
+	//
+	// If no name is provided, this will default to "param" with a suffix of the
+	// position the parameter is in the function definition. ("param1", "param2", etc.)
+	// If the parameter is variadic, the default name will be "varparam".
 	//
 	// This must be a valid Terraform identifier, such as starting with an
 	// alphabetical character and followed by alphanumeric or underscore
@@ -88,11 +91,7 @@ func (p Float64Parameter) GetMarkdownDescription() string {
 
 // GetName returns the parameter name.
 func (p Float64Parameter) GetName() string {
-	if p.Name != "" {
-		return p.Name
-	}
-
-	return DefaultParameterName
+	return p.Name
 }
 
 // GetType returns the parameter data type.

function/float64_parameter_test.go

@@ -182,13 +182,7 @@ func TestFloat64ParameterGetName(t *testing.T) {
 	}{
 		"unset": {
 			parameter: function.Float64Parameter{},
-			expected:  function.DefaultParameterName,
-		},
-		"Name-empty": {
-			parameter: function.Float64Parameter{
-				Name: "",
-			},
-			expected: function.DefaultParameterName,
+			expected:  "",
 		},
 		"Name-nonempty": {
 			parameter: function.Float64Parameter{

function/int64_parameter.go

@@ -56,8 +56,11 @@ type Int64Parameter struct {
 
 	// Name is a short usage name for the parameter, such as "data". This name
 	// is used in documentation, such as generating a function signature,
-	// however its usage may be extended in the future. If no name is provided,
-	// this will default to "param".
+	// however its usage may be extended in the future.
+	//
+	// If no name is provided, this will default to "param" with a suffix of the
+	// position the parameter is in the function definition. ("param1", "param2", etc.)
+	// If the parameter is variadic, the default name will be "varparam".
 	//
 	// This must be a valid Terraform identifier, such as starting with an
 	// alphabetical character and followed by alphanumeric or underscore
@@ -87,11 +90,7 @@ func (p Int64Parameter) GetMarkdownDescription() string {
 
 // GetName returns the parameter name.
 func (p Int64Parameter) GetName() string {
-	if p.Name != "" {
-		return p.Name
-	}
-
-	return DefaultParameterName
+	return p.Name
 }
 
 // GetType returns the parameter data type.

function/int64_parameter_test.go

@@ -182,13 +182,7 @@ func TestInt64ParameterGetName(t *testing.T) {
 	}{
 		"unset": {
 			parameter: function.Int64Parameter{},
-			expected:  function.DefaultParameterName,
-		},
-		"Name-empty": {
-			parameter: function.Int64Parameter{
-				Name: "",
-			},
-			expected: function.DefaultParameterName,
+			expected:  "",
 		},
 		"Name-nonempty": {
 			parameter: function.Int64Parameter{