General improvements for maintainability (#52)

* Improvements to naming, godoc and logical flow

* Further improvements to naming, godoc and logical flow

* Fix up some flow control

* Further godoc, naming, flow and logic improvements

* Update actions

* Update actions

* Fix image ref

* Cleanup lint

* Cleanup template globals

* Add template and copybook godoc, minor tidy

.github/workflows/lint.yml

@@ -14,11 +14,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2.3.4
-
       - name: Run linter
         uses: golangci/golangci-lint-action@v2.5.2
         with:
-          version: v1.33.2
+          version: v1.55.0
           args: --timeout=5m
         env:
           GOFLAGS: "-mod=readonly"

.github/workflows/test.yml

@@ -21,7 +21,7 @@ jobs:
       - run: git fetch --prune --unshallow
 
       - name: "Run test coverage"
-        uses: "docker://golang:1.16.5-stretch"
+        uses: "docker://golang:1.21.6-bullseye"
         env:
           GO111MODULE: "on"
         with:

.golangci.yaml

@@ -145,7 +145,6 @@ linters:
   disable-all: true
   enable:
     - deadcode
-    - depguard
     - dupl
     - errcheck
     - gochecknoinits
@@ -180,6 +179,7 @@ linters:
     - scopelint
 
   # don't enable:
+  # - depguard
   # - gochecknoglobals
   # - gocognit
   # - prealloc
@@ -215,4 +215,4 @@ issues:
   # e.g. new-from-patch: path/to/patch/file
 
 service:
-  golangci-lint-version: 1.32.X
+  golangci-lint-version: 1.55.X

Makefile

@@ -6,7 +6,7 @@ COLOUR_GREEN=$(shell tput setaf 2)
 COVERAGE=$(shell cat THRESHOLD)
 DOCKER_REGISTRY?=library
 
-default: | clean vendor tidy lint cover
+default: | clean tidy lint cover
 	@if [[ -e .git/rebase-merge ]]; then git --no-pager log -1 --pretty='%h %s'; fi
 	@printf '%sSuccess%s\n' "${COLOUR_GREEN}" "${COLOUR_NORMAL}"
 
@@ -32,11 +32,6 @@ clean: ## Cleans up generated coverage files and binaries
 	rm -f cover.html
 	rm -rf `find . -type d -name "dist"`
 
-.PHONY: vendor
-vendor: ## Cleans up go mod dependencies and vendor's all dependencies
-	go mod tidy
-	go mod vendor
-
 .PHONY: build
 build: clean ## Builds the gopic struct generation tool
 	go build -v \
@@ -49,7 +44,7 @@ install: build ## Builds and installs gopic struct generation tool to your GOPAT
 
 .PHONY: tidy
 tidy: ## Reorders imports
-	goimports -v -w -e . ./cmd/*
+	go mod tidy
 
 .PHONY: lint
 lint: ## Runs the golangci-lint checker

THRESHOLD

@@ -1 +1 @@
-75.0
+76.1

cmd/pkg/cli/root.go

@@ -3,7 +3,6 @@ package cli
 import (
 	"fmt"
 	"io"
-	"io/ioutil"
 	"log"
 	"os"
 	"path/filepath"
@@ -51,10 +50,13 @@ var (
 
 // Execute executes the root command.
 func Execute() error {
-	return rootCmd.Execute()
+	if err := rootCmd.Execute(); err != nil {
+		return fmt.Errorf("execute: %w", err)
+	}
+	return nil
 }
 
-func init() { // nolint:gochecknoinits
+func init() { //nolint:gochecknoinits
 	dirCmd.Flags().BoolP(displayFlag, "d", false, displayHelp)
 	dirCmd.Flags().StringP(outFlag, "o", "", outputHelp)
 	dirCmd.Flags().StringP(inFlag, "i", "", inputHelp)
@@ -77,7 +79,7 @@ func init() { // nolint:gochecknoinits
 	rootCmd.AddCommand(fileCmd)
 }
 
-func dirRun(cmd *cobra.Command, _ []string) error { // nolint:gocyclo
+func dirRun(cmd *cobra.Command, _ []string) error { //nolint:gocyclo
 	out, err := cmd.Flags().GetString(outFlag)
 	if err != nil {
 		return fmt.Errorf("failed to extract value for flag %s: %w", outFlag, err)
@@ -95,14 +97,14 @@ func dirRun(cmd *cobra.Command, _ []string) error { // nolint:gocyclo
 
 	d, _ := cmd.Flags().GetBool(displayFlag)
 
-	fs, err := ioutil.ReadDir(in)
+	fs, err := os.ReadDir(in)
 	if err != nil {
 		return fmt.Errorf("failed to read files for dir %s: %w", in, err)
 	}
 
 	_, err = os.Stat(out)
 	if os.IsNotExist(err) {
-		errDir := os.MkdirAll(out, 0750)
+		errDir := os.MkdirAll(out, os.ModePerm)
 		if errDir != nil {
 			return fmt.Errorf("failed to create dir %s: %w", out, errDir)
 		}
@@ -112,13 +114,11 @@ func dirRun(cmd *cobra.Command, _ []string) error { // nolint:gocyclo
 		if ff.IsDir() {
 			continue
 		}
-
 		log.Printf("parsing copybook file %s", ff.Name())
-		f, err := os.Open(filepath.Join(in, ff.Name())) // nolint:gosec
+		f, err := os.Open(filepath.Join(in, ff.Name())) //nolint:gosec
 		if err != nil {
 			return fmt.Errorf("failed to open file %s: %w", ff.Name(), err)
 		}
-
 		if err := run(f, filepath.Join(out, ff.Name()), pkg, d); err != nil {
 			return err
 		}
@@ -132,50 +132,47 @@ func fileRun(cmd *cobra.Command, _ []string) error {
 	if err != nil {
 		return fmt.Errorf("failed to extract value for flag %s: %w", outFlag, err)
 	}
-
 	in, err := cmd.Flags().GetString(inFlag)
 	if err != nil {
 		return fmt.Errorf("failed to extract value for flag %s: %w", inFlag, err)
 	}
-
 	pkg, err := cmd.Flags().GetString(pkgFlag)
 	if err != nil {
 		return fmt.Errorf("failed to extract value for flag %s: %w", pkgFlag, err)
 	}
-
 	d, _ := cmd.Flags().GetBool(displayFlag)
 
 	log.Printf("parsing copybook file %s", in)
-	f, err := os.Open(in) // nolint:gosec
+	f, err := os.Open(in) //nolint:gosec
 	if err != nil {
 		return fmt.Errorf("failed to open file %s: %w", in, err)
 	}
-
 	return run(f, out, pkg, d)
 }
 
 func run(r io.Reader, output, pkg string, preview bool) error {
 	name := strings.TrimSuffix(output, filepath.Ext(output))
 	n := name[strings.LastIndex(name, "/")+1:]
-
 	c := copybook.New(n, pkg, template.Copybook())
-
-	b, err := ioutil.ReadAll(r)
+	b, err := io.ReadAll(r)
 	if err != nil {
 		return fmt.Errorf("failed to read input data: %w", err)
 	}
-
 	tree := lex.NewTree(lex.New(n, string(b)))
 	time.Sleep(time.Millisecond)
-	c.Root = tree.Parse()
+	c.Root, err = tree.Parse()
+	if err != nil {
+		return fmt.Errorf("failed to parse copybook: %w", err)
+	}
+
 	// TODO: (pgmitche) if record in tree is struct but has no children,
 	// it should probably be ignored entirely
 	if preview {
 		c.Preview()
 	}
 
 	name = fmt.Sprintf("%s.go", name)
-	newFile, err := os.Create(name)
+	newFile, err := os.Create(name) //nolint:gosec // intentionally creating file
 	if err != nil {
 		return fmt.Errorf("failed to create file %s: %w", name, err)
 	}
@@ -184,6 +181,8 @@ func run(r io.Reader, output, pkg string, preview bool) error {
 			log.Fatalln(err)
 		}
 	}()
-
-	return c.WriteToStruct(newFile)
+	if err := c.WriteToStruct(newFile); err != nil {
+		return fmt.Errorf("write to struct: %w", err)
+	}
+	return nil
 }

cmd/pkg/copybook/copybook.go

@@ -13,66 +13,68 @@ import (
 	"github.com/foundatn-io/go-pic/pkg/lex"
 )
 
+// Copybook represents a copybook.
 type Copybook struct {
-	Name    string
-	Root    *lex.Record
-	Package string
-	t       *template.Template
+	Name     string
+	Root     *lex.Record
+	Package  string
+	template *template.Template
 }
 
+// Record represents a record.
 type Record struct {
-	Num     int          // Num is not necessarily required
-	Level   int          // Level is not necessarily required
-	Name    string       // Name is required
-	Picture reflect.Kind // Picture is required
-	Length  int          // Length is required
-	Occurs  int          // Occurs is required if present
+	Number     int          // Number is not necessarily required
+	Level      int          // Level is not necessarily required
+	Name       string       // Name is required
+	Type       reflect.Kind // Type is required
+	Length     int          // Length is required
+	Occurrence int          // Occurrence is required if present
 }
 
-func New(name, pkg string, t *template.Template) *Copybook {
+// New creates a new copybook.
+func New(name, pkg string, tmpl *template.Template) *Copybook {
 	return &Copybook{
-		Name:    name,
-		Package: pkg,
-		t:       t,
+		Name:     name,
+		Package:  pkg,
+		template: tmpl,
 	}
 }
 
+// Preview prints a preview of the copybook.
 func (c *Copybook) Preview() {
 	log.Println("------ STRUCT PREVIEW -----")
-	treePrint(c.Root, 0)
+	printTree(c.Root, 0)
 	log.Println("------- END PREVIEW -------")
 }
 
-func treePrint(node *lex.Record, nest int) {
+// printTree prints a tree representation of a record.
+func printTree(node *lex.Record, nest int) {
 	if node.Typ == reflect.Struct {
 		indent := strings.Repeat(">", nest)
 		log.Printf("L%d: %s%s", nest, indent, node.Name)
 		nest++
-		for _, nn := range node.Children {
-			treePrint(nn, nest)
+		for _, nestedNode := range node.Children {
+			printTree(nestedNode, nest)
 		}
 	} else {
 		indent := strings.Repeat("-", nest)
 		log.Printf("L%d: %s%s", nest, indent, node.Name)
 	}
 }
 
+// WriteToStruct writes the copybook to a formatted struct.
 func (c *Copybook) WriteToStruct(writer io.Writer) error {
-	var b bytes.Buffer
-	if err := c.t.Execute(&b, c); err != nil {
-		return fmt.Errorf("failed template copybook data: %w", err)
+	var buffer bytes.Buffer
+	if err := c.template.Execute(&buffer, c); err != nil {
+		return fmt.Errorf("failed to execute template: %w", err)
 	}
-
-	bb, err := format.Source(b.Bytes())
+	formatted, err := format.Source(buffer.Bytes())
 	if err != nil {
-		return fmt.Errorf("failed to gofmt templated copybook data: %w", err)
+		return fmt.Errorf("failed to format source: %w", err)
 	}
-
-	if _, err = writer.Write(bb); err != nil {
-		return fmt.Errorf("failed to write templated copybook data: %w", err)
+	if _, err = writer.Write(formatted); err != nil {
+		return fmt.Errorf("failed to write to writer: %w", err)
 	}
-
-	b.Reset()
-
+	buffer.Reset()
 	return nil
 }

cmd/pkg/copybook/copybook_test.go

@@ -78,7 +78,8 @@ func Test_Build(t *testing.T) {
 
 			lxr := lex.New(tt.name, string(b))
 			tree := lex.NewTree(lxr)
-			c.Root = tree.Parse()
+			c.Root, err = tree.Parse()
+			require.NoError(t, err)
 
 			var buf bytes.Buffer
 			require.NoError(t, c.WriteToStruct(&buf))