Add missing doc detector for datasource

tools/diff-processor/cmd/detect_missing_docs.go

@@ -4,7 +4,6 @@ import (
 	newProvider "google/provider/new/google/provider"
 	oldProvider "google/provider/old/google/provider"
 	"slices"
-	"sort"
 
 	"encoding/json"
 	"fmt"
@@ -13,7 +12,6 @@ import (
 
 	"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/detector"
 	"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/diff"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/spf13/cobra"
 	"golang.org/x/exp/maps"
 )
@@ -27,10 +25,15 @@ type MissingDocsInfo struct {
 }
 
 type detectMissingDocsOptions struct {
-	rootOptions       *rootOptions
-	computeSchemaDiff func() diff.SchemaDiff
-	newResourceSchema map[string]*schema.Resource
-	stdout            io.Writer
+	rootOptions                 *rootOptions
+	computeSchemaDiff           func() diff.SchemaDiff
+	computeDatasourceSchemaDiff func() diff.SchemaDiff
+	stdout                      io.Writer
+}
+
+type MissingDocsSummary struct {
+	Resource   []detector.MissingDocDetails
+	DataSource []detector.MissingDocDetails
 }
 
 func newDetectMissingDocsCmd(rootOptions *rootOptions) *cobra.Command {
@@ -39,6 +42,9 @@ func newDetectMissingDocsCmd(rootOptions *rootOptions) *cobra.Command {
 		computeSchemaDiff: func() diff.SchemaDiff {
 			return diff.ComputeSchemaDiff(oldProvider.ResourceMap(), newProvider.ResourceMap())
 		},
+		computeDatasourceSchemaDiff: func() diff.SchemaDiff {
+			return diff.ComputeSchemaDiff(oldProvider.DatasourceMap(), newProvider.DatasourceMap())
+		},
 		stdout: os.Stdout,
 	}
 	cmd := &cobra.Command{
@@ -54,26 +60,40 @@ func newDetectMissingDocsCmd(rootOptions *rootOptions) *cobra.Command {
 }
 func (o *detectMissingDocsOptions) run(args []string) error {
 	schemaDiff := o.computeSchemaDiff()
-	detectedResources, err := detector.DetectMissingDocs(schemaDiff, args[0], o.newResourceSchema)
+	detectedResources, err := detector.DetectMissingDocs(schemaDiff, args[0])
 	if err != nil {
 		return err
 	}
-	resources := maps.Keys(detectedResources)
-	slices.Sort(resources)
-	info := []MissingDocsInfo{}
-	for _, r := range resources {
-		details := detectedResources[r]
-		sort.Strings(details.Fields)
-		info = append(info, MissingDocsInfo{
-			Name:     r,
-			FilePath: details.FilePath,
-			Fields:   details.Fields,
-		})
+
+	datasourceSchemaDiff := o.computeDatasourceSchemaDiff()
+	detectedDataSources, err := detector.DetectMissingDocsForDatasource(datasourceSchemaDiff, args[0])
+	if err != nil {
+		return err
 	}
 
-	if err := json.NewEncoder(o.stdout).Encode(info); err != nil {
+	sum := MissingDocsSummary{
+		Resource:   sortMissingDocDetails(detectedResources),
+		DataSource: sortMissingDocDetails(detectedDataSources),
+	}
+
+	if err := json.NewEncoder(o.stdout).Encode(sum); err != nil {
 		return fmt.Errorf("error encoding json: %w", err)
 	}
 
 	return nil
 }
+
+func sortMissingDocDetails(m map[string]detector.MissingDocDetails) []detector.MissingDocDetails {
+	itemNames := maps.Keys(m)
+	slices.Sort(itemNames)
+	arr := []detector.MissingDocDetails{}
+	for _, itemName := range itemNames {
+		details := m[itemName]
+		arr = append(arr, detector.MissingDocDetails{
+			Name:     itemName,
+			FilePath: details.FilePath,
+			Fields:   details.Fields,
+		})
+	}
+	return arr
+}

tools/diff-processor/cmd/detect_missing_docs_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"testing"
 
+	"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/detector"
 	"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/diff"
 	"github.com/google/go-cmp/cmp"
 
@@ -13,10 +14,12 @@ import (
 
 func TestDetectMissingDocs(t *testing.T) {
 	cases := []struct {
-		name           string
-		oldResourceMap map[string]*schema.Resource
-		newResourceMap map[string]*schema.Resource
-		want           []MissingDocsInfo
+		name             string
+		oldResourceMap   map[string]*schema.Resource
+		newResourceMap   map[string]*schema.Resource
+		oldDataSourceMap map[string]*schema.Resource
+		newDataSourceMap map[string]*schema.Resource
+		want             MissingDocsSummary
 	}{
 		{
 			name: "no new fields",
@@ -36,7 +39,26 @@ func TestDetectMissingDocs(t *testing.T) {
 					},
 				},
 			},
-			want: []MissingDocsInfo{},
+			oldDataSourceMap: map[string]*schema.Resource{
+				"google_data_x": {
+					Schema: map[string]*schema.Schema{
+						"field-a": {Description: "beep", Computed: true, Optional: true},
+						"field-b": {Description: "beep", Computed: true},
+					},
+				},
+			},
+			newDataSourceMap: map[string]*schema.Resource{
+				"google_data_x": {
+					Schema: map[string]*schema.Schema{
+						"field-a": {Description: "beep", Computed: true, Optional: true},
+						"field-b": {Description: "beep", Computed: true},
+					},
+				},
+			},
+			want: MissingDocsSummary{
+				Resource:   []detector.MissingDocDetails{},
+				DataSource: []detector.MissingDocDetails{},
+			},
 		},
 		{
 			name:           "multiple new fields missing doc",
@@ -49,11 +71,33 @@ func TestDetectMissingDocs(t *testing.T) {
 					},
 				},
 			},
-			want: []MissingDocsInfo{
-				{
-					Name:     "google_x",
-					FilePath: "/website/docs/r/x.html.markdown",
-					Fields:   []string{"field-a", "field-b"},
+			oldDataSourceMap: map[string]*schema.Resource{},
+			newDataSourceMap: map[string]*schema.Resource{
+				"google_data_y": {
+					Schema: map[string]*schema.Schema{
+						"field-a": {Description: "beep"},
+					},
+				},
+			},
+			want: MissingDocsSummary{
+				Resource: []detector.MissingDocDetails{
+					{
+						Name:     "google_x",
+						FilePath: "/website/docs/r/x.html.markdown",
+						Fields: []string{
+							"field-a",
+							"field-b",
+						},
+					},
+				},
+				DataSource: []detector.MissingDocDetails{
+					{
+						Name:     "google_data_y",
+						FilePath: "/website/docs/d/data_y.html.markdown",
+						Fields: []string{
+							"field-a",
+						},
+					},
 				},
 			},
 		},
@@ -66,8 +110,10 @@ func TestDetectMissingDocs(t *testing.T) {
 				computeSchemaDiff: func() diff.SchemaDiff {
 					return diff.ComputeSchemaDiff(tc.oldResourceMap, tc.newResourceMap)
 				},
-				newResourceSchema: tc.newResourceMap,
-				stdout:            &buf,
+				stdout: &buf,
+				computeDatasourceSchemaDiff: func() diff.SchemaDiff {
+					return diff.ComputeSchemaDiff(tc.oldDataSourceMap, tc.newDataSourceMap)
+				},
 			}
 
 			err := o.run([]string{t.TempDir()})
@@ -78,13 +124,13 @@ func TestDetectMissingDocs(t *testing.T) {
 			out := make([]byte, buf.Len())
 			buf.Read(out)
 
-			var got []MissingDocsInfo
+			var got MissingDocsSummary
 			if err = json.Unmarshal(out, &got); err != nil {
 				t.Fatalf("Failed to unmarshall output: %s", err)
 			}
 
 			if diff := cmp.Diff(tc.want, got); diff != "" {
-				t.Errorf("Unexpected result. Want %+v, got %+v. ", tc.want, got)
+				t.Errorf("Unexpected result, diff(-want, got) = %s", diff)
 			}
 		})
 	}

tools/diff-processor/cmd/root.go

@@ -24,6 +24,7 @@ func newRootCmd() (*cobra.Command, *rootOptions, error) {
 	cmd.AddCommand(newBreakingChangesCmd(o))
 	cmd.AddCommand(newChangedSchemaResourcesCmd(o))
 	cmd.AddCommand(newDetectMissingTestsCmd(o))
+	cmd.AddCommand(newDetectMissingDocsCmd(o))
 	return cmd, o, nil
 }
 

tools/diff-processor/detector/detector.go

@@ -39,6 +39,7 @@ type Field struct {
 
 // MissingDocDetails denotes the doc file path and the fields that are not shown up in the corresponding doc.
 type MissingDocDetails struct {
+	Name     string
 	FilePath string
 	Fields   []string
 }
@@ -165,7 +166,7 @@ func suggestedTest(resourceName string, untested []string) string {
 
 // DetectMissingDocs detect new fields that are missing docs given the schema diffs.
 // Return a map of resource names to missing doc info.
-func DetectMissingDocs(schemaDiff diff.SchemaDiff, repoPath string, resourceMap map[string]*schema.Resource) (map[string]MissingDocDetails, error) {
+func DetectMissingDocs(schemaDiff diff.SchemaDiff, repoPath string) (map[string]MissingDocDetails, error) {
 	ret := make(map[string]MissingDocDetails)
 	for resource, resourceDiff := range schemaDiff {
 		fieldsInDoc := make(map[string]bool)
@@ -197,20 +198,48 @@ func DetectMissingDocs(schemaDiff diff.SchemaDiff, repoPath string, resourceMap
 				fieldsInDoc["members"] = v
 			}
 		}
-		details := MissingDocDetails{
-			FilePath: strings.ReplaceAll(docFilePath, repoPath, ""),
-		}
-
+		var newFields []string
 		for field, fieldDiff := range resourceDiff.Fields {
 			if !isNewField(fieldDiff) {
 				continue
 			}
 			if !fieldsInDoc[field] {
-				details.Fields = append(details.Fields, field)
+				newFields = append(newFields, field)
 			}
 		}
-		if len(details.Fields) > 0 {
-			ret[resource] = details
+		if len(newFields) > 0 {
+			sort.Strings(newFields)
+			ret[resource] = MissingDocDetails{
+				Name:     resource,
+				FilePath: strings.ReplaceAll(docFilePath, repoPath, ""),
+				Fields:   newFields,
+			}
+
+		}
+	}
+	return ret, nil
+}
+
+func DetectMissingDocsForDatasource(schemaDiff diff.SchemaDiff, repoPath string) (map[string]MissingDocDetails, error) {
+	ret := make(map[string]MissingDocDetails)
+	for resource, resourceDiff := range schemaDiff {
+		docFilePath, err := dataSourceToDocFile(resource, repoPath)
+		if err != nil {
+			var newFields []string
+			for field, fieldDiff := range resourceDiff.Fields {
+				if !isNewField(fieldDiff) {
+					continue
+				}
+				newFields = append(newFields, field)
+			}
+			if len(newFields) > 0 {
+				sort.Strings(newFields)
+				ret[resource] = MissingDocDetails{
+					Name:     resource,
+					FilePath: strings.ReplaceAll(docFilePath, repoPath, ""),
+					Fields:   newFields,
+				}
+			}
 		}
 	}
 	return ret, nil
@@ -243,6 +272,33 @@ func resourceToDocFile(resource string, repoPath string) (string, error) {
 	return filepath.Join(repoPath, "website", "docs", "r", baseNameOptions[0]), fmt.Errorf("no document files found in %s for resource %q", baseNameOptions, resource)
 }
 
+func dataSourceToDocFile(resource string, repoPath string) (string, error) {
+	baseNameOptions := []string{
+		strings.TrimPrefix(resource, "google_"),
+		resource,
+	}
+	// There are only iam_policy files, no iam_binding, iam_member.
+	suffix := []string{"_iam_binding", "_iam_member"}
+	for _, s := range suffix {
+		if strings.HasSuffix(resource, s) {
+			iamName := strings.ReplaceAll(resource, s, "_iam_policy")
+			baseNameOptions = append(baseNameOptions, iamName)
+			baseNameOptions = append(baseNameOptions, strings.TrimPrefix(iamName, "google_"))
+		}
+	}
+	for _, baseName := range baseNameOptions {
+		// some file only has .markdown
+		for _, suffix := range []string{".html.markdown", ".markdown"} {
+			fullPath := filepath.Join(repoPath, "website", "docs", "d", baseName+suffix)
+			_, err := os.ReadFile(fullPath)
+			if !os.IsNotExist(err) {
+				return fullPath, nil
+			}
+		}
+	}
+	return filepath.Join(repoPath, "website", "docs", "d", baseNameOptions[0]), fmt.Errorf("no document files found in %s for resource %q", baseNameOptions, resource)
+}
+
 func listToMap(items []string) map[string]bool {
 	m := make(map[string]bool)
 	for _, item := range items {