Validate: refactor internal methods to use fs.FS interface for file handling (#413)

* Refactor `validateStaticDocs()` and `validateLegacyWebsite()` to use a `fs.FS` interface for walking.

* Refactor `validate_test.go` to use `fstest.MapFS`

* Refactor `directory_test.go` to use `fstest.MapFS`

* Add tests for file mismatch check

* Fix `unparam` lints

* Add changelog entries

* Use `filepath.Join` instead of `website/docs` for windows compatability.

* Revert "Use `filepath.Join` instead of `website/docs` for windows compatability."

This reverts commit 9c92c3e.

* Remove `filepath.Join()` usages and `/` separator conversions.

* Remove `filepath.fromSlash()` usages

* Use `path.Dir()` instead of `filepath.Dir()`

* Use `doublestar.Match()` instead of `doublestar.PathMatch()`

* Convert path slashes to os separators in error messages

* Convert path slashes to os separators in invalid directory check error messages

Author: SBGoods

.changes/unreleased/BUG FIXES-20241022-163805.yaml

@@ -0,0 +1,5 @@
+kind: BUG FIXES
+body: 'validate: File extension check now runs on `index.*` files instead of just `index.md` files.'
+time: 2024-10-22T16:38:05.530944-04:00
+custom:
+    Issue: "413"

.changes/unreleased/BUG FIXES-20241022-164013.yaml

@@ -0,0 +1,5 @@
+kind: BUG FIXES
+body: 'validate: File extension check now specifies the correct valid extensions in the error message.'
+time: 2024-10-22T16:40:13.832638-04:00
+custom:
+    Issue: "413"

.changes/unreleased/BUG FIXES-20241022-164107.yaml

@@ -0,0 +1,5 @@
+kind: BUG FIXES
+body: 'validate: Front matter check now runs with the correct options on legacy index files.'
+time: 2024-10-22T16:41:07.726856-04:00
+custom:
+    Issue: "413"

internal/check/directory.go

@@ -6,6 +6,7 @@ package check
 import (
 	"fmt"
 	"log"
+	"path"
 	"path/filepath"
 )
 
@@ -82,7 +83,7 @@ func InvalidDirectoriesCheck(dirPath string) error {
 		return nil
 	}
 
-	return fmt.Errorf("invalid Terraform Provider documentation directory found: %s", dirPath)
+	return fmt.Errorf("invalid Terraform Provider documentation directory found: %s", filepath.FromSlash(dirPath))
 
 }
 
@@ -92,7 +93,7 @@ func MixedDirectoriesCheck(docFiles []string) error {
 	err := fmt.Errorf("mixed Terraform Provider documentation directory layouts found, must use only legacy or registry layout")
 
 	for _, file := range docFiles {
-		directory := filepath.Dir(file)
+		directory := path.Dir(file)
 		log.Printf("[DEBUG] Found directory: %s", directory)
 
 		// Allow docs/ with other files
@@ -120,7 +121,7 @@ func MixedDirectoriesCheck(docFiles []string) error {
 
 func IsValidLegacyDirectory(directory string) bool {
 	for _, validLegacyDirectory := range ValidLegacyDirectories {
-		if directory == filepath.FromSlash(validLegacyDirectory) {
+		if directory == validLegacyDirectory {
 			return true
 		}
 	}
@@ -130,7 +131,7 @@ func IsValidLegacyDirectory(directory string) bool {
 
 func IsValidRegistryDirectory(directory string) bool {
 	for _, validRegistryDirectory := range ValidRegistryDirectories {
-		if directory == filepath.FromSlash(validRegistryDirectory) {
+		if directory == validRegistryDirectory {
 			return true
 		}
 	}
@@ -139,32 +140,32 @@ func IsValidRegistryDirectory(directory string) bool {
 }
 
 func IsValidCdktfDirectory(directory string) bool {
-	if directory == filepath.FromSlash(fmt.Sprintf("%s/%s", LegacyIndexDirectory, CdktfIndexDirectory)) {
+	if directory == fmt.Sprintf("%s/%s", LegacyIndexDirectory, CdktfIndexDirectory) {
 		return true
 	}
 
-	if directory == filepath.FromSlash(fmt.Sprintf("%s/%s", RegistryIndexDirectory, CdktfIndexDirectory)) {
+	if directory == fmt.Sprintf("%s/%s", RegistryIndexDirectory, CdktfIndexDirectory) {
 		return true
 	}
 
 	for _, validCdktfLanguage := range ValidCdktfLanguages {
 
-		if directory == filepath.FromSlash(fmt.Sprintf("%s/%s/%s", LegacyIndexDirectory, CdktfIndexDirectory, validCdktfLanguage)) {
+		if directory == fmt.Sprintf("%s/%s/%s", LegacyIndexDirectory, CdktfIndexDirectory, validCdktfLanguage) {
 			return true
 		}
 
-		if directory == filepath.FromSlash(fmt.Sprintf("%s/%s/%s", RegistryIndexDirectory, CdktfIndexDirectory, validCdktfLanguage)) {
+		if directory == fmt.Sprintf("%s/%s/%s", RegistryIndexDirectory, CdktfIndexDirectory, validCdktfLanguage) {
 			return true
 		}
 
 		for _, validLegacySubdirectory := range ValidLegacySubdirectories {
-			if directory == filepath.FromSlash(fmt.Sprintf("%s/%s/%s/%s", LegacyIndexDirectory, CdktfIndexDirectory, validCdktfLanguage, validLegacySubdirectory)) {
+			if directory == fmt.Sprintf("%s/%s/%s/%s", LegacyIndexDirectory, CdktfIndexDirectory, validCdktfLanguage, validLegacySubdirectory) {
 				return true
 			}
 		}
 
 		for _, validRegistrySubdirectory := range ValidRegistrySubdirectories {
-			if directory == filepath.FromSlash(fmt.Sprintf("%s/%s/%s/%s", RegistryIndexDirectory, CdktfIndexDirectory, validCdktfLanguage, validRegistrySubdirectory)) {
+			if directory == fmt.Sprintf("%s/%s/%s/%s", RegistryIndexDirectory, CdktfIndexDirectory, validCdktfLanguage, validRegistrySubdirectory) {
 				return true
 			}
 		}

internal/check/directory_test.go

@@ -4,26 +4,94 @@
 package check
 
 import (
-	"os"
-	"path/filepath"
+	"io/fs"
 	"testing"
+	"testing/fstest"
 
 	"github.com/bmatcuk/doublestar/v4"
 )
 
-var DocumentationGlobPattern = `{docs/index.md,docs/{,cdktf/}{data-sources,guides,resources,functions}/**/*,website/docs/**/*}`
+var DocumentationGlobPattern = `{docs/index.*,docs/{,cdktf/}{data-sources,guides,resources,functions}/**/*,website/docs/**/*}`
 
 func TestMixedDirectoriesCheck(t *testing.T) {
 	t.Parallel()
 	testCases := map[string]struct {
-		BasePath    string
+		ProviderFS  fs.FS
 		ExpectError bool
 	}{
 		"valid mixed directories": {
-			BasePath: filepath.Join("testdata", "valid-mixed-directories"),
+			ProviderFS: fstest.MapFS{
+				"docs/nonregistrydocs/thing.md": {},
+				"website/docs/index.md":         {},
+			},
 		},
-		"invalid mixed directories": {
-			BasePath:    filepath.Join("testdata", "invalid-mixed-directories"),
+		"valid mixed directories - cdktf": {
+			ProviderFS: fstest.MapFS{
+				"docs/cdktf/typescript/index.md": {},
+				"website/docs/index.md":          {},
+			},
+		},
+		"invalid mixed directories - registry data source": {
+			ProviderFS: fstest.MapFS{
+				"docs/data-sources/invalid.md": {},
+				"website/docs/index.md":        {},
+			},
+			ExpectError: true,
+		},
+		"invalid mixed directories - registry resource": {
+			ProviderFS: fstest.MapFS{
+				"docs/resources/invalid.md": {},
+				"website/docs/index.md":     {},
+			},
+			ExpectError: true,
+		},
+		"invalid mixed directories - registry guide": {
+			ProviderFS: fstest.MapFS{
+				"docs/guides/invalid.md": {},
+				"website/docs/index.md":  {},
+			},
+			ExpectError: true,
+		},
+		"invalid mixed directories - registry function": {
+			ProviderFS: fstest.MapFS{
+				"docs/functions/invalid.md": {},
+				"website/docs/index.md":     {},
+			},
+			ExpectError: true,
+		},
+		"invalid mixed directories - legacy data source": {
+			ProviderFS: fstest.MapFS{
+				"website/docs/d/invalid.html.markdown": {},
+				"docs/resources/thing.md":              {},
+			},
+			ExpectError: true,
+		},
+		"invalid mixed directories - legacy resource": {
+			ProviderFS: fstest.MapFS{
+				"website/docs/r/invalid.html.markdown": {},
+				"docs/resources/thing.md":              {},
+			},
+			ExpectError: true,
+		},
+		"invalid mixed directories - legacy guide": {
+			ProviderFS: fstest.MapFS{
+				"website/docs/guides/invalid.html.markdown": {},
+				"docs/resources/thing.md":                   {},
+			},
+			ExpectError: true,
+		},
+		"invalid mixed directories - legacy function": {
+			ProviderFS: fstest.MapFS{
+				"website/docs/functions/invalid.html.markdown": {},
+				"docs/resources/thing.md":                      {},
+			},
+			ExpectError: true,
+		},
+		"invalid mixed directories - legacy index": {
+			ProviderFS: fstest.MapFS{
+				"website/docs/index.html.markdown": {},
+				"docs/resources/thing.md":          {},
+			},
 			ExpectError: true,
 		},
 	}
@@ -34,9 +102,7 @@ func TestMixedDirectoriesCheck(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
-			providerFs := os.DirFS(testCase.BasePath)
-
-			files, err := doublestar.Glob(providerFs, DocumentationGlobPattern)
+			files, err := doublestar.Glob(testCase.ProviderFS, DocumentationGlobPattern)
 			if err != nil {
 				t.Fatalf("error finding documentation files: %s", err)
 			}

internal/check/file.go

@@ -5,8 +5,8 @@ package check
 
 import (
 	"fmt"
+	"io/fs"
 	"log"
-	"os"
 	"path/filepath"
 )
 
@@ -23,14 +23,14 @@ func (opts *FileOptions) FullPath(path string) string {
 }
 
 // FileSizeCheck verifies that documentation file is below the Terraform Registry storage limit.
-func FileSizeCheck(fullpath string) error {
-	fi, err := os.Stat(fullpath)
+func FileSizeCheck(providerFs fs.FS, path string) error {
+	fi, err := fs.Stat(providerFs, path)
 
 	if err != nil {
 		return err
 	}
 
-	log.Printf("[DEBUG] File %s size: %d (limit: %d)", fullpath, fi.Size(), RegistryMaximumSizeOfFile)
+	log.Printf("[DEBUG] File %s size: %d (limit: %d)", path, fi.Size(), RegistryMaximumSizeOfFile)
 	if fi.Size() >= int64(RegistryMaximumSizeOfFile) {
 		return fmt.Errorf("exceeded maximum (%d) size of documentation file for Terraform Registry: %d", RegistryMaximumSizeOfFile, fi.Size())
 	}

internal/check/file_extension.go

@@ -30,7 +30,7 @@ var ValidRegistryFileExtensions = []string{
 // FileExtensionCheck checks if the file extension of the given path is valid.
 func FileExtensionCheck(path string, validExtensions []string) error {
 	if !FilePathEndsWithExtensionFrom(path, validExtensions) {
-		return fmt.Errorf("file does not end with a valid extension, valid extensions: %v", ValidLegacyFileExtensions)
+		return fmt.Errorf("file does not end with a valid extension, valid extensions: %v", validExtensions)
 	}
 
 	return nil

internal/check/file_test.go

@@ -4,26 +4,40 @@
 package check
 
 import (
-	"os"
+	"io/fs"
 	"path/filepath"
 	"testing"
+	"testing/fstest"
 )
 
 func TestFileSizeCheck(t *testing.T) {
 	t.Parallel()
 	testCases := map[string]struct {
+		FileSystem  fs.FS
 		Size        int64
 		ExpectError bool
 	}{
 		"under limit": {
-			Size: RegistryMaximumSizeOfFile - 1,
+			FileSystem: fstest.MapFS{
+				"file.md": {
+					Data: make([]byte, RegistryMaximumSizeOfFile-1),
+				},
+			},
 		},
 		"on limit": {
-			Size:        RegistryMaximumSizeOfFile,
+			FileSystem: fstest.MapFS{
+				"file.md": {
+					Data: make([]byte, RegistryMaximumSizeOfFile),
+				},
+			},
 			ExpectError: true,
 		},
 		"over limit": {
-			Size:        RegistryMaximumSizeOfFile + 1,
+			FileSystem: fstest.MapFS{
+				"file.md": {
+					Data: make([]byte, RegistryMaximumSizeOfFile+1),
+				},
+			},
 			ExpectError: true,
 		},
 	}
@@ -34,15 +48,7 @@ func TestFileSizeCheck(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
-			file, _ := os.CreateTemp(t.TempDir(), "TestFileSizeCheck")
-
-			defer file.Close()
-
-			if err := file.Truncate(testCase.Size); err != nil {
-				t.Fatalf("error writing temporary file: %s", err)
-			}
-
-			got := FileSizeCheck(file.Name())
+			got := FileSizeCheck(testCase.FileSystem, "file.md")
 
 			if got == nil && testCase.ExpectError {
 				t.Errorf("expected error, got no error")

internal/check/provider_file.go

@@ -5,8 +5,9 @@ package check
 
 import (
 	"fmt"
+	"io/fs"
 	"log"
-	"os"
+	"path/filepath"
 )
 
 type ProviderFileOptions struct {
@@ -17,12 +18,14 @@ type ProviderFileOptions struct {
 }
 
 type ProviderFileCheck struct {
-	Options *ProviderFileOptions
+	Options    *ProviderFileOptions
+	ProviderFs fs.FS
 }
 
-func NewProviderFileCheck(opts *ProviderFileOptions) *ProviderFileCheck {
+func NewProviderFileCheck(providerFs fs.FS, opts *ProviderFileOptions) *ProviderFileCheck {
 	check := &ProviderFileCheck{
-		Options: opts,
+		Options:    opts,
+		ProviderFs: providerFs,
 	}
 
 	if check.Options == nil {
@@ -46,21 +49,21 @@ func (check *ProviderFileCheck) Run(path string) error {
 	log.Printf("[DEBUG] Checking file: %s", fullpath)
 
 	if err := FileExtensionCheck(path, check.Options.ValidExtensions); err != nil {
-		return fmt.Errorf("%s: error checking file extension: %w", path, err)
+		return fmt.Errorf("%s: error checking file extension: %w", filepath.FromSlash(path), err)
 	}
 
-	if err := FileSizeCheck(fullpath); err != nil {
-		return fmt.Errorf("%s: error checking file size: %w", path, err)
+	if err := FileSizeCheck(check.ProviderFs, path); err != nil {
+		return fmt.Errorf("%s: error checking file size: %w", filepath.FromSlash(path), err)
 	}
 
-	content, err := os.ReadFile(fullpath)
+	content, err := fs.ReadFile(check.ProviderFs, path)
 
 	if err != nil {
-		return fmt.Errorf("%s: error reading file: %w", path, err)
+		return fmt.Errorf("%s: error reading file: %w", filepath.FromSlash(path), err)
 	}
 
 	if err := NewFrontMatterCheck(check.Options.FrontMatter).Run(content); err != nil {
-		return fmt.Errorf("%s: error checking file frontmatter: %w", path, err)
+		return fmt.Errorf("%s: error checking file frontmatter: %w", filepath.FromSlash(path), err)
 	}
 
 	return nil