Add Validate() to all collections

[Doridian]

This also fixes a bug in internal/reflect/map.go where it used the map's own type to Validate elements, which does not seem correct at all.

Fixes #467

[hashicorp-cla]

All committers have signed the CLA.

[bflad]

Hi @Doridian 👋 Thank you for submitting this. Overall this is looking pretty good, it could just use some additional unit testing and small adjustments. If you have any questions about adding that testing, please reach out.

Also, if you could, it would be great to include a changelog entry which can be done by adding a .changelog/481.txt file with contents such as:

```release-note:bug
types: Ensured `List`, `Map`, and `Set` types with `xattr.TypeWithValidate` elements run validation on those elements
```

[bflad]

We should ensure there is a covering unit test for this change. 👍

[bflad]

We should ensure there is a covering unit test for this new conditional. 👍

[Doridian]

Addresses all suggestions but unit tests.

I was looking at the unit tests and could not really find any that seem to test the current validation framework, so I was unsure how or where to add those.

I also saw tests for collections spread over the local list_test/map_test and then the big struct_test file.

I will gladly take any suggestion for where to best add those tests and if I maybe just missed validation tests I could base off of.

[Doridian]

It seems that AtSetValue expects an attr.Value, not tftypes.Value. Pushed a fix for that.

[bflad]

@Doridian since we are adding a method onto the type, we can unit test the method itself, e.g.

func TestListTypeValidate(t *testing.T) {
	t.Parallel()

	testCases := map[string]struct {
		listType      ListType
		tfValue       tftypes.Value
		path          path.Path
		expectedDiags diag.Diagnostics
	}{
		"wrong-value-type": {
			listType: ListType{
				ElemType: StringType,
			},
			tfValue: tftypes.NewValue(tftypes.Set{
				ElementType: tftypes.String,
			}, []tftypes.Value{
				tftypes.NewValue(tftypes.String, "testvalue"),
			}),
			path: path.Root("test"),
			expectedDiags: diag.Diagnostics{
				diag.NewAttributeErrorDiagnostic(
					path.Root("test"),
					"List Type Validation Error",
					"An unexpected error was encountered trying to validate an attribute value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+
						"expected List value, received tftypes.Value with value: tftypes.Set[tftypes.String]<tftypes.String<\"testvalue\">>",
				),
			},
		},
		"no-validation": {
			listType: ListType{
				ElemType: StringType,
			},
			tfValue: tftypes.NewValue(tftypes.List{
				ElementType: tftypes.String,
			}, []tftypes.Value{
				tftypes.NewValue(tftypes.String, "testvalue"),
			}),
			path: path.Root("test"),
		},
	}

	for name, testCase := range testCases {
		name, testCase := name, testCase

		t.Run(name, func(t *testing.T) {
			t.Parallel()

			diags := testCase.listType.Validate(context.Background(), testCase.tfValue, testCase.path)

			if diff := cmp.Diff(diags, testCase.expectedDiags); diff != "" {
				t.Errorf("unexpected diagnostics difference: %s", diff)
			}
		})
	}
}

Rinse and repeat for MapType and SetType.

This is okay for now as it would unfortunately take a lot more effort to import the existing internal/testing/types.StringTypeWithValidateError to verify the element validation diagnostics are passed back, as importing that right now would cause an import cycle and updating the testing file to package types_test and putting types. in front of everything in the types package is a lot to ask. 😅

[bflad]

(We could also create an unexported type with validation errors in the test files too, but it is totally fine.)

[Doridian]

I have added tests for list and map exactly as you described.
For map, as it already was validatable, I have just appended the wrong-value-type test case.

[Doridian]

This is okay for now as it would unfortunately take a lot more effort to import the existing internal/testing/types.StringTypeWithValidateError to verify the element validation diagnostics are passed back, as importing that right now would cause an import cycle and updating the testing file to package types_test and putting types. in front of everything in the types package is a lot to ask. 😅

Maybe in another PR if I find the time 😉

[bflad]

This looks great, @Doridian, thank you so much for your efforts here 🚀

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.