ListResource: revisit handling of null ListResult fields

internal/fwserver/server_listresource.go

@@ -127,18 +127,22 @@ func processListResult(req list.ListRequest, result list.ListResult) ListResult
 		return ListResult(result)
 	}
 
-	if result.Identity == nil { // TODO: is result.Identity.Raw.IsNull() a practical concern?
+	if result.Identity == nil || result.Identity.Raw.IsNull() {
 		return ListResultError(
 			"Incomplete List Result",
-			"The provider did not populate the Identity field in the ListResourceResult. This may be due to an error in the provider's implementation.",
+			"When listing resources, an implementation issue was found. "+
+				"This is always a problem with the provider. Please report this to the provider developers.\n\n"+
+				"The \"Identity\" field is nil.\n\n",
 		)
 	}
 
 	if req.IncludeResource {
-		if result.Resource == nil { // TODO: is result.Resource.Raw.IsNull() a practical concern?
+		if result.Resource == nil || result.Resource.Raw.IsNull() {
 			result.Diagnostics.AddWarning(
 				"Incomplete List Result",
-				"The provider did not populate the Resource field in the ListResourceResult. This may be due to the provider not supporting this functionality or an error in the provider's implementation.",
+				"When listing resources, an implementation issue was found. "+
+					"This is always a problem with the provider. Please report this to the provider developers.\n\n"+
+					"The \"IncludeResource\" field in the ListRequest is true, but the \"Resource\" field in the ListResult is nil.\n\n",
 			)
 		}
 	}

internal/fwserver/server_listresource_test.go

@@ -206,10 +206,36 @@ func TestServerListResource(t *testing.T) {
 			expectedStreamEvents: []fwserver.ListResult{
 				{
 					Diagnostics: diag.Diagnostics{
-						diag.NewErrorDiagnostic(
-							"Incomplete List Result",
-							"The provider did not populate the Identity field in the ListResourceResult. This may be due to an error in the provider's implementation.",
-						),
+						diag.NewErrorDiagnostic("Incomplete List Result", "..."),
+					},
+				},
+			},
+		},
+		"error-on-null-resource-identity": {
+			server: &fwserver.Server{
+				Provider: &testprovider.Provider{},
+			},
+			request: &fwserver.ListRequest{
+				Config: &tfsdk.Config{},
+				ListResource: &testprovider.ListResource{
+					ListMethod: func(ctx context.Context, req list.ListRequest, resp *list.ListResultsStream) {
+						resp.Results = slices.Values([]list.ListResult{
+							{
+								Identity: &tfsdk.ResourceIdentity{},
+								Resource: &tfsdk.Resource{
+									Schema: testSchema,
+									Raw:    testResourceValue1,
+								},
+								DisplayName: "Test Resource 1",
+							},
+						})
+					},
+				},
+			},
+			expectedStreamEvents: []fwserver.ListResult{
+				{
+					Diagnostics: diag.Diagnostics{
+						diag.NewErrorDiagnostic("Incomplete List Result", "..."),
 					},
 				},
 			},
@@ -244,10 +270,43 @@ func TestServerListResource(t *testing.T) {
 					},
 					DisplayName: "Test Resource 1",
 					Diagnostics: diag.Diagnostics{
-						diag.NewWarningDiagnostic(
-							"Incomplete List Result",
-							"The provider did not populate the Resource field in the ListResourceResult. This may be due to the provider not supporting this functionality or an error in the provider's implementation.",
-						),
+						diag.NewWarningDiagnostic("Incomplete List Result", "..."),
+					},
+				},
+			},
+		},
+		"warning-on-null-resource": {
+			server: &fwserver.Server{
+				Provider: &testprovider.Provider{},
+			},
+			request: &fwserver.ListRequest{
+				Config:          &tfsdk.Config{},
+				IncludeResource: true,
+				ListResource: &testprovider.ListResource{
+					ListMethod: func(ctx context.Context, req list.ListRequest, resp *list.ListResultsStream) {
+						resp.Results = slices.Values([]list.ListResult{
+							{
+								Identity: &tfsdk.ResourceIdentity{
+									Schema: testIdentitySchema,
+									Raw:    testIdentityValue1,
+								},
+								Resource:    &tfsdk.Resource{},
+								DisplayName: "Test Resource 1",
+							},
+						})
+					},
+				},
+			},
+			expectedStreamEvents: []fwserver.ListResult{
+				{
+					Identity: &tfsdk.ResourceIdentity{
+						Schema: testIdentitySchema,
+						Raw:    testIdentityValue1,
+					},
+					Resource:    &tfsdk.Resource{},
+					DisplayName: "Test Resource 1",
+					Diagnostics: diag.Diagnostics{
+						diag.NewWarningDiagnostic("Incomplete List Result", "..."),
 					},
 				},
 			},
@@ -264,8 +323,19 @@ func TestServerListResource(t *testing.T) {
 				t.Fatalf("unexpected error: %s", err)
 			}
 
+			opts := cmp.Options{
+				cmp.Comparer(func(a, b diag.Diagnostics) bool {
+					// Differences in Detail() are not relevant to correctness of logic
+					for i := range a {
+						if a[i].Severity() != b[i].Severity() || a[i].Summary() != b[i].Summary() {
+							return false
+						}
+					}
+					return true
+				}),
+			}
 			events := slices.AppendSeq([]fwserver.ListResult{}, response.Results)
-			if diff := cmp.Diff(events, testCase.expectedStreamEvents); diff != "" {
+			if diff := cmp.Diff(events, testCase.expectedStreamEvents, opts); diff != "" {
 				t.Errorf("unexpected difference: %s", diff)
 			}
 		})

internal/proto5server/server_listresource_test.go

@@ -31,7 +31,6 @@ func TestServerListResource(t *testing.T) {
 	}
 
 	type ThingResource struct {
-		// TODO: how do we feel about this?
 		ThingResourceIdentity
 		Name string `tfsdk:"name"`
 	}
@@ -106,7 +105,15 @@ func TestServerListResource(t *testing.T) {
 						continue
 					}
 
-					result := req.ToListResult(ctx, resources[name].ThingResourceIdentity, resources[name], name)
+					result := req.NewListResult()
+					result.DisplayName = name
+
+					diags = result.Identity.Set(ctx, resources[name].ThingResourceIdentity)
+					result.Diagnostics.Append(diags...)
+
+					diags = result.Resource.Set(ctx, resources[name])
+					result.Diagnostics.Append(diags...)
+
 					results = append(results, result)
 				}
 				resp.Results = slices.Values(results)
@@ -117,21 +124,6 @@ func TestServerListResource(t *testing.T) {
 		}
 	}
 
-	listResourceThatDoesNotPopulateResource := func() list.ListResource {
-		r, ok := listResource().(*testprovider.ListResource)
-		if !ok {
-			t.Fatal("listResourceThatDoesNotPopulateResource must be a testprovider.ListResource")
-		}
-
-		r.ListMethod = func(ctx context.Context, req list.ListRequest, resp *list.ListResultsStream) {
-			result := req.ToListResult(ctx, resources["plateau"].ThingResourceIdentity, nil, "plateau")
-
-			resp.Results = slices.Values([]list.ListResult{result})
-		}
-
-		return r
-	}
-
 	managedResource := func() resource.Resource {
 		return &testprovider.ResourceWithIdentity{
 			IdentitySchemaMethod: func(ctx context.Context, req resource.IdentitySchemaRequest, resp *resource.IdentitySchemaResponse) {
@@ -251,29 +243,6 @@ func TestServerListResource(t *testing.T) {
 				},
 			},
 		},
-		"result-with-include-resource-warning": {
-			server: server(listResourceThatDoesNotPopulateResource, managedResource),
-			request: &tfprotov5.ListResourceRequest{
-				TypeName:        "test_resource",
-				Config:          plateau,
-				IncludeResource: true,
-			},
-			expectedError:       nil,
-			expectedDiagnostics: diag.Diagnostics{},
-			expectedResults: []tfprotov5.ListResourceResult{
-				{
-					DisplayName: "plateau",
-					Identity:    expectedResourceIdentities["plateau"],
-					Diagnostics: []*tfprotov5.Diagnostic{
-						{
-							Severity: tfprotov5.DiagnosticSeverityWarning,
-							Summary:  "Incomplete List Result",
-							Detail:   "The provider did not populate the Resource field in the ListResourceResult. This may be due to the provider not supporting this functionality or an error in the provider's implementation.",
-						},
-					},
-				},
-			},
-		},
 	}
 
 	for name, testCase := range testCases {

internal/proto6server/server_listresource_test.go

@@ -31,7 +31,6 @@ func TestServerListResource(t *testing.T) {
 	}
 
 	type ThingResource struct {
-		// TODO: how do we feel about this?
 		ThingResourceIdentity
 		Name string `tfsdk:"name"`
 	}
@@ -106,7 +105,15 @@ func TestServerListResource(t *testing.T) {
 						continue
 					}
 
-					result := req.ToListResult(ctx, resources[name].ThingResourceIdentity, resources[name], name)
+					result := req.NewListResult()
+					result.DisplayName = name
+
+					diags = result.Identity.Set(ctx, resources[name].ThingResourceIdentity)
+					result.Diagnostics = append(result.Diagnostics, diags...)
+
+					diags = result.Resource.Set(ctx, resources[name])
+					result.Diagnostics = append(result.Diagnostics, diags...)
+
 					results = append(results, result)
 				}
 				resp.Results = slices.Values(results)
@@ -124,9 +131,14 @@ func TestServerListResource(t *testing.T) {
 		}
 
 		r.ListMethod = func(ctx context.Context, req list.ListRequest, resp *list.ListResultsStream) {
-			result := req.ToListResult(ctx, resources["plateau"].ThingResourceIdentity, nil, "plateau")
+			result := req.NewListResult()
+			result.DisplayName = "plateau"
+
+			diags := result.Identity.Set(ctx, resources["plateau"].ThingResourceIdentity)
+			result.Diagnostics = append(result.Diagnostics, diags...)
 
-			resp.Results = slices.Values([]list.ListResult{result})
+			results := []list.ListResult{result}
+			resp.Results = slices.Values(results)
 		}
 
 		return r
@@ -264,6 +276,7 @@ func TestServerListResource(t *testing.T) {
 				{
 					DisplayName: "plateau",
 					Identity:    expectedResourceIdentities["plateau"],
+					Resource:    &tfprotov6.DynamicValue{MsgPack: []uint8{0xc0}},
 					Diagnostics: []*tfprotov6.Diagnostic{
 						{
 							Severity: tfprotov6.DiagnosticSeverityWarning,

list/list_resource.go

@@ -103,6 +103,20 @@ type ListRequest struct {
 	ResourceIdentitySchema fwschema.Schema
 }
 
+// NewListResult creates a new [ListResult] with convenient defaults
+// for each field.
+func (r ListRequest) NewListResult() ListResult {
+	identity := &tfsdk.ResourceIdentity{Schema: r.ResourceIdentitySchema}
+	resource := &tfsdk.Resource{Schema: r.ResourceSchema}
+
+	return ListResult{
+		DisplayName: "",
+		Resource:    resource,
+		Identity:    identity,
+		Diagnostics: diag.Diagnostics{},
+	}
+}
+
 // ListResultsStream represents a streaming response to a [ListRequest].  An
 // instance of this struct is supplied as an argument to the provider's
 // [ListResource.List] function. The provider should set a Results iterator
@@ -120,9 +134,20 @@ type ListResultsStream struct {
 }
 
 // NoListResults is an iterator that pushes zero results.
-var NoListResults = func(func(ListResult) bool) {}
+var NoListResults = func(push func(ListResult) bool) {}
+
+// ListResultsStreamDiagnostics returns a function that yields a single
+// [ListResult] with the given Diagnostics
+func ListResultsStreamDiagnostics(diags diag.Diagnostics) iter.Seq[ListResult] {
+	return func(push func(ListResult) bool) {
+		if !push(ListResult{Diagnostics: diags}) {
+			return
+		}
+	}
+}
 
-// ListResult represents a listed managed resource instance.
+// ListResult represents a listed managed resource instance. For convenience,
+// create new values using [NewListResult] instead of struct literals.
 type ListResult struct {
 	// Identity is the identity of the managed resource instance.
 	//