Skip to content

Commit

Permalink
add a subcommand into diff processor to detect missing docs (#11156)
Browse files Browse the repository at this point in the history
  • Loading branch information
iyabchen authored Jan 10, 2025
1 parent 070ddb5 commit 4f7ee52
Show file tree
Hide file tree
Showing 8 changed files with 1,094 additions and 0 deletions.
79 changes: 79 additions & 0 deletions tools/diff-processor/cmd/detect_missing_docs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package cmd

import (
newProvider "google/provider/new/google/provider"
oldProvider "google/provider/old/google/provider"
"slices"
"sort"

"encoding/json"
"fmt"
"io"
"os"

"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"
)

const detectMissingDocDesc = `Compute list of fields missing documents`

type MissingDocsInfo struct {
Name string
FilePath string
Fields []string
}

type detectMissingDocsOptions struct {
rootOptions *rootOptions
computeSchemaDiff func() diff.SchemaDiff
newResourceSchema map[string]*schema.Resource
stdout io.Writer
}

func newDetectMissingDocsCmd(rootOptions *rootOptions) *cobra.Command {
o := &detectMissingDocsOptions{
rootOptions: rootOptions,
computeSchemaDiff: func() diff.SchemaDiff {
return diff.ComputeSchemaDiff(oldProvider.ResourceMap(), newProvider.ResourceMap())
},
stdout: os.Stdout,
}
cmd := &cobra.Command{
Use: "detect-missing-docs",
Short: detectMissingDocDesc,
Long: detectMissingDocDesc,
Args: cobra.ExactArgs(1),
RunE: func(c *cobra.Command, args []string) error {
return o.run(args)
},
}
return cmd
}
func (o *detectMissingDocsOptions) run(args []string) error {
schemaDiff := o.computeSchemaDiff()
detectedResources, err := detector.DetectMissingDocs(schemaDiff, args[0], o.newResourceSchema)
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,
})
}

if err := json.NewEncoder(o.stdout).Encode(info); err != nil {
return fmt.Errorf("error encoding json: %w", err)
}

return nil
}
91 changes: 91 additions & 0 deletions tools/diff-processor/cmd/detect_missing_docs_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package cmd

import (
"bytes"
"encoding/json"
"testing"

"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/diff"
"github.com/google/go-cmp/cmp"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

func TestDetectMissingDocs(t *testing.T) {
cases := []struct {
name string
oldResourceMap map[string]*schema.Resource
newResourceMap map[string]*schema.Resource
want []MissingDocsInfo
}{
{
name: "no new fields",
oldResourceMap: map[string]*schema.Resource{
"google_x": {
Schema: map[string]*schema.Schema{
"field-a": {Description: "beep", Computed: true, Optional: true},
"field-b": {Description: "beep", Computed: true},
},
},
},
newResourceMap: map[string]*schema.Resource{
"google_x": {
Schema: map[string]*schema.Schema{
"field-a": {Description: "beep", Computed: true, Optional: true},
"field-b": {Description: "beep", Computed: true},
},
},
},
want: []MissingDocsInfo{},
},
{
name: "multiple new fields missing doc",
oldResourceMap: map[string]*schema.Resource{},
newResourceMap: map[string]*schema.Resource{
"google_x": {
Schema: map[string]*schema.Schema{
"field-a": {Description: "beep", Computed: true, Optional: true},
"field-b": {Description: "beep", Computed: true},
},
},
},
want: []MissingDocsInfo{
{
Name: "google_x",
FilePath: "/website/docs/r/x.html.markdown",
Fields: []string{"field-a", "field-b"},
},
},
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
var buf bytes.Buffer
o := detectMissingDocsOptions{
computeSchemaDiff: func() diff.SchemaDiff {
return diff.ComputeSchemaDiff(tc.oldResourceMap, tc.newResourceMap)
},
newResourceSchema: tc.newResourceMap,
stdout: &buf,
}

err := o.run([]string{t.TempDir()})
if err != nil {
t.Fatalf("Error running command: %s", err)
}

out := make([]byte, buf.Len())
buf.Read(out)

var got []MissingDocsInfo
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)
}
})
}
}
98 changes: 98 additions & 0 deletions tools/diff-processor/detector/detector.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package detector

import (
"fmt"
"os"
"path/filepath"
"sort"
"strings"

"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/diff"
"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/documentparser"
"github.com/GoogleCloudPlatform/magic-modules/tools/test-reader/reader"
"github.com/hashicorp/hcl/v2/hclwrite"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -33,6 +37,12 @@ type Field struct {
Tested bool
}

// MissingDocDetails denotes the doc file path and the fields that are not shown up in the corresponding doc.
type MissingDocDetails struct {
FilePath string
Fields []string
}

// Detect missing tests for the given resource changes map in the given slice of tests.
// Return a map of resource names to missing test info about that resource.
func DetectMissingTests(schemaDiff diff.SchemaDiff, allTests []*reader.Test) (map[string]*MissingTestInfo, error) {
Expand Down Expand Up @@ -152,3 +162,91 @@ func suggestedTest(resourceName string, untested []string) string {
}
return strings.ReplaceAll(string(f.Bytes()), `"VALUE"`, "# value needed")
}

// 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) {
ret := make(map[string]MissingDocDetails)
for resource, resourceDiff := range schemaDiff {
fieldsInDoc := make(map[string]bool)

docFilePath, err := resourceToDocFile(resource, repoPath)
if err != nil {
fmt.Printf("Warning: %s.\n", err)
} else {
content, err := os.ReadFile(docFilePath)
if err != nil {
return nil, fmt.Errorf("failed to read resource doc %s: %w", docFilePath, err)
}
parser := documentparser.NewParser()
err = parser.Parse(content)
if err != nil {
return nil, fmt.Errorf("failed to parse document %s: %w", docFilePath, err)
}

argumentsInDoc := listToMap(parser.Arguments())
attributesInDoc := listToMap(parser.Attributes())
for _, m := range []map[string]bool{argumentsInDoc, attributesInDoc} {
for k, v := range m {
fieldsInDoc[k] = v
}
}
// for iam resource
if v, ok := fieldsInDoc["member/members"]; ok {
fieldsInDoc["member"] = v
fieldsInDoc["members"] = v
}
}
details := MissingDocDetails{
FilePath: strings.ReplaceAll(docFilePath, repoPath, ""),
}

for field, fieldDiff := range resourceDiff.Fields {
if !isNewField(fieldDiff) {
continue
}
if !fieldsInDoc[field] {
details.Fields = append(details.Fields, field)
}
}
if len(details.Fields) > 0 {
ret[resource] = details
}
}
return ret, nil
}

func isNewField(fieldDiff diff.FieldDiff) bool {
return fieldDiff.Old == nil && fieldDiff.New != nil
}

func resourceToDocFile(resource string, repoPath string) (string, error) {
baseNameOptions := []string{
strings.TrimPrefix(resource, "google_") + ".html.markdown",
resource + ".html.markdown",
}
suffix := []string{"_policy", "_binding", "_member"}
for _, s := range suffix {
if strings.HasSuffix(resource, "_iam"+s) {
iamName := strings.TrimSuffix(resource, s)
baseNameOptions = append(baseNameOptions, iamName+".html.markdown")
baseNameOptions = append(baseNameOptions, strings.TrimPrefix(iamName, "google_")+".html.markdown")
}
}
for _, baseName := range baseNameOptions {
fullPath := filepath.Join(repoPath, "website", "docs", "r", baseName)
_, err := os.ReadFile(fullPath)
if !os.IsNotExist(err) {
return fullPath, nil
}
}
return filepath.Join(repoPath, "website", "docs", "r", 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 {
m[item] = true
}
return m
}
Loading

0 comments on commit 4f7ee52

Please sign in to comment.