back out some non-idiomatic one lining but add helper functions to make some stuff easier to read

Signed-off-by: Callum Styan <[email protected]>

Author: cstyan

cmd/readmevalidation/coderresources.go

@@ -56,15 +56,21 @@ func validateCoderResourceDescription(description string) error {
 	return nil
 }
 
+func isPermittedRelativeURL(checkURL string) bool {
+	// Would normally be skittish about having relative paths like this, but it should be safe because we have
+	// guarantees about the structure of the repo, and where this logic will run.
+	return strings.HasPrefix(checkURL, "./") || strings.HasPrefix(checkURL, "/") || strings.HasPrefix(checkURL, "../../../../.icons")
+}
+
 func validateCoderResourceIconURL(iconURL string) []error {
 	if iconURL == "" {
 		return []error{xerrors.New("icon URL cannot be empty")}
 	}
 
 	errs := []error{}
 
-	isAbsoluteURL := !strings.HasPrefix(iconURL, ".") && !strings.HasPrefix(iconURL, "/")
-	if isAbsoluteURL {
+	// If the URL does not have a relative path.
+	if !strings.HasPrefix(iconURL, ".") && !strings.HasPrefix(iconURL, "/") {
 		if _, err := url.ParseRequestURI(iconURL); err != nil {
 			errs = append(errs, xerrors.New("absolute icon URL is not correctly formatted"))
 		}
@@ -74,12 +80,8 @@ func validateCoderResourceIconURL(iconURL string) []error {
 		return errs
 	}
 
-	// Would normally be skittish about having relative paths like this, but it should be safe because we have guarantees
-	// about the structure of the repo, and where this logic will run.
-	isPermittedRelativeURL := strings.HasPrefix(iconURL, "./") ||
-		strings.HasPrefix(iconURL, "/") ||
-		strings.HasPrefix(iconURL, "../../../../.icons")
-	if !isPermittedRelativeURL {
+	// If the URL has a relative path.
+	if !isPermittedRelativeURL(iconURL) {
 		errs = append(errs, xerrors.Errorf("relative icon URL %q must either be scoped to that module's directory, or the top-level /.icons directory (this can usually be done by starting the path with \"../../../.icons\")", iconURL))
 	}
 
@@ -271,7 +273,7 @@ func parseCoderResourceReadmeFiles(resourceType string, rms []readme) (map[strin
 
 // Todo: Need to beef up this function by grabbing each image/video URL from
 // the body's AST.
-func validateCoderResourceRelativeUrls(_ map[string]coderResourceReadme) error {
+func validateCoderResourceRelativeURLs(_ map[string]coderResourceReadme) error {
 	return nil
 }
 
@@ -283,14 +285,13 @@ func aggregateCoderResourceReadmeFiles(resourceType string) ([]readme, error) {
 
 	var allReadmeFiles []readme
 	var errs []error
-	var resourceDirs []os.DirEntry
 	for _, rf := range registryFiles {
 		if !rf.IsDir() {
 			continue
 		}
 
 		resourceRootPath := path.Join(rootRegistryPath, rf.Name(), resourceType)
-		resourceDirs, err = os.ReadDir(resourceRootPath)
+		resourceDirs, err := os.ReadDir(resourceRootPath)
 		if err != nil {
 			if !errors.Is(err, os.ErrNotExist) {
 				errs = append(errs, err)
@@ -343,7 +344,7 @@ func validateAllCoderResourceFilesOfType(resourceType string) error {
 	}
 	logger.Info(context.Background(), "rocessed README files as valid Coder resources", "num_files", len(resources), "type", resourceType)
 
-	if err = validateCoderResourceRelativeUrls(resources); err != nil {
+	if err := validateCoderResourceRelativeURLs(resources); err != nil {
 		return err
 	}
 	logger.Info(context.Background(), "all relative URLs for READMEs are valid", "type", resourceType)

cmd/readmevalidation/contributors.go

@@ -128,7 +128,7 @@ func validateContributorAvatarURL(avatarURL *string) []error {
 		errs = append(errs, xerrors.New("avatar URL is not allowed to contain search parameters"))
 	}
 
-	matched := false
+	var matched bool
 	for _, ff := range supportedAvatarFileFormats {
 		matched = strings.HasSuffix(*avatarURL, ff)
 		if matched {
@@ -243,7 +243,6 @@ func aggregateContributorReadmeFiles() ([]readme, error) {
 		}
 
 		dirPath = path.Join(rootRegistryPath, e.Name())
-
 		readmePath := path.Join(dirPath, "README.md")
 		rmBytes, err := os.ReadFile(readmePath)
 		if err != nil {
@@ -288,8 +287,7 @@ func validateContributorRelativeURLs(contributors map[string]contributorProfileR
 			continue
 		}
 
-		absolutePath := strings.TrimSuffix(con.filePath, "README.md") +
-			*con.frontmatter.AvatarURL
+		absolutePath := strings.TrimSuffix(con.filePath, "README.md") + *con.frontmatter.AvatarURL
 		if _, err := os.ReadFile(absolutePath); err != nil {
 			errs = append(errs, xerrors.Errorf("%q: relative avatar path %q does not point to image in file system", con.filePath, absolutePath))
 		}
@@ -317,7 +315,7 @@ func validateAllContributorFiles() error {
 	}
 	logger.Info(context.Background(), "processed README files as valid contributor profiles", "num_contributors", len(contributors))
 
-	if err = validateContributorRelativeURLs(contributors); err != nil {
+	if err := validateContributorRelativeURLs(contributors); err != nil {
 		return err
 	}
 	logger.Info(context.Background(), "all relative URLs for READMEs are valid")

cmd/readmevalidation/readmefiles.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bufio"
+	"fmt"
 	"regexp"
 	"strings"
 
@@ -21,15 +22,15 @@ const (
 	// system as expected.
 	validationPhaseStructure validationPhase = "File structure validation"
 
-	// validationPhaseFile indicates when README files are being read from
+	// ValidationPhaseFile indicates when README files are being read from
 	// the file system.
 	validationPhaseFile validationPhase = "Filesystem reading"
 
-	// validationPhaseReadme indicates when a README's frontmatter is
+	// ValidationPhaseReadme indicates when a README's frontmatter is
 	// being parsed as YAML. This phase does not include YAML validation.
 	validationPhaseReadme validationPhase = "README parsing"
 
-	// validationPhaseCrossReference indicates when a README's frontmatter
+	// ValidationPhaseCrossReference indicates when a README's frontmatter
 	// is having all its relative URLs be validated for whether they point to
 	// valid resources.
 	validationPhaseCrossReference validationPhase = "Cross-referencing relative asset URLs"
@@ -58,14 +59,13 @@ func separateFrontmatter(readmeText string) (readmeFrontmatter string, readmeBod
 
 	const fence = "---"
 
-	fm := ""
-	body := ""
-	nextLine := ""
+	var fm strings.Builder
+	var body strings.Builder
 	fenceCount := 0
 
 	lineScanner := bufio.NewScanner(strings.NewReader(strings.TrimSpace(readmeText)))
 	for lineScanner.Scan() {
-		nextLine = lineScanner.Text()
+		nextLine := lineScanner.Text()
 		if fenceCount < 2 && nextLine == fence {
 			fenceCount++
 			continue
@@ -79,19 +79,19 @@ func separateFrontmatter(readmeText string) (readmeFrontmatter string, readmeBod
 		// extra meaning attached to the indentation. The same does NOT apply to the README; best we can do is gather
 		// all the lines and then trim around it.
 		if inReadmeBody := fenceCount >= 2; inReadmeBody {
-			body += nextLine + "\n"
+			fmt.Fprintf(&body, "%s\n", nextLine)
 		} else {
-			fm += strings.TrimSpace(nextLine) + "\n"
+			fmt.Fprintf(&fm, "%s\n", strings.TrimSpace(nextLine))
 		}
 	}
 	if fenceCount < 2 {
 		return "", "", xerrors.New("README does not have two sets of frontmatter fences")
 	}
-	if fm == "" {
+	if fm.Len() == 0 {
 		return "", "", xerrors.New("readme has frontmatter fences but no frontmatter content")
 	}
 
-	return fm, strings.TrimSpace(body), nil
+	return fm.String(), strings.TrimSpace(body.String()), nil
 }
 
 // TODO: This seems to work okay for now, but the really proper way of doing this is by parsing this as an AST, and then
@@ -113,11 +113,10 @@ func validateReadmeBody(body string) []error {
 	latestHeaderLevel := 0
 	foundFirstH1 := false
 	isInCodeBlock := false
-	nextLine := ""
 
 	lineScanner := bufio.NewScanner(strings.NewReader(trimmed))
 	for lineScanner.Scan() {
-		nextLine = lineScanner.Text()
+		nextLine := lineScanner.Text()
 
 		// Have to check this because a lot of programming languages support # comments (including Terraform), and
 		// without any context, there's no way to tell the difference between a markdown header and code comment.

cmd/readmevalidation/repostructure.go

@@ -13,12 +13,8 @@ import (
 var supportedUserNameSpaceDirectories = append(supportedResourceTypes, ".icons", ".images")
 
 func validateCoderResourceSubdirectory(dirPath string) []error {
-	var (
-		err    error
-		subDir os.FileInfo
-	)
-
-	if subDir, err = os.Stat(dirPath); err != nil {
+	subDir, err := os.Stat(dirPath)
+	if err != nil {
 		// It's valid for a specific resource directory not to exist. It's just that if it does exist, it must follow specific rules.
 		if !errors.Is(err, os.ErrNotExist) {
 			return []error{addFilePathToError(dirPath, err)}
@@ -43,7 +39,7 @@ func validateCoderResourceSubdirectory(dirPath string) []error {
 		}
 
 		resourceReadmePath := path.Join(dirPath, f.Name(), "README.md")
-		if _, err = os.Stat(resourceReadmePath); err != nil {
+		if _, err := os.Stat(resourceReadmePath); err != nil {
 			if errors.Is(err, os.ErrNotExist) {
 				errs = append(errs, xerrors.Errorf("%q: 'README.md' does not exist", resourceReadmePath))
 			} else {
@@ -52,7 +48,7 @@ func validateCoderResourceSubdirectory(dirPath string) []error {
 		}
 
 		mainTerraformPath := path.Join(dirPath, f.Name(), "main.tf")
-		if _, err = os.Stat(mainTerraformPath); err != nil {
+		if _, err := os.Stat(mainTerraformPath); err != nil {
 			if errors.Is(err, os.ErrNotExist) {
 				errs = append(errs, xerrors.Errorf("%q: 'main.tf' file does not exist", mainTerraformPath))
 			} else {
@@ -64,16 +60,12 @@ func validateCoderResourceSubdirectory(dirPath string) []error {
 }
 
 func validateRegistryDirectory() []error {
-	var (
-		userDirs []os.DirEntry
-		err      error
-	)
-	if userDirs, err = os.ReadDir(rootRegistryPath); err != nil {
+	userDirs, err := os.ReadDir(rootRegistryPath)
+	if err != nil {
 		return []error{err}
 	}
 
 	allErrs := []error{}
-	var iterFiles []os.DirEntry
 	for _, d := range userDirs {
 		dirPath := path.Join(rootRegistryPath, d.Name())
 		if !d.IsDir() {
@@ -82,16 +74,17 @@ func validateRegistryDirectory() []error {
 		}
 
 		contributorReadmePath := path.Join(dirPath, "README.md")
-		if _, err = os.Stat(contributorReadmePath); err != nil {
+		if _, err := os.Stat(contributorReadmePath); err != nil {
 			allErrs = append(allErrs, err)
 		}
 
-		if iterFiles, err = os.ReadDir(dirPath); err != nil {
+		files, err := os.ReadDir(dirPath)
+		if err != nil {
 			allErrs = append(allErrs, err)
 			continue
 		}
 
-		for _, f := range iterFiles {
+		for _, f := range files {
 			// TODO: Decide if there's anything more formal that we want to ensure about non-directories scoped to user namespaces.
 			if !f.IsDir() {
 				continue