Vet and improve source code using NilAway

Add NilAway (<https://github.com/uber-go/nilaway>) as a new vet check in
order to detect and address potential runtime errors due to nil values.

All 4 violations it detected have been addressed. In `analyze.go` this
simply constitutes checking that received pointers aren't nil. In
`main.go` this constites, first, explicitly relying on the `ok` check of
a map lookup when trying to use said key (instead of implicitly relying
on the control flow being such that said key is injected if it isn't
there already), and second, rewriting to avoid using `strings.Split` and
instead using index lookup - in this case the finding by NilAway was a
false positive (`nil` can never be returned by `strings.Split`, even if
the internal function used may return `nil` in some cases) but rewriting
reduces memory usage.

The modifications are partially tested, covering only `analyze.go`
because `main.go` isn't covered by unit tests.

Signed-off-by: Eric Cornelissen <ericornelissen@gmail.com>

Makefile

@@ -79,6 +79,7 @@ vet: ## Vet the source code
 	@go run github.com/tetafro/godot/cmd/godot@v1.4.15 .
 	@go run github.com/tomarrell/wrapcheck/v2/cmd/wrapcheck@v2.8.1 .
 	@go run golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow@2f9d82f .
+	@go run go.uber.org/nilaway/cmd/nilaway@a267567 .
 	@go run honnef.co/go/tools/cmd/staticcheck@v0.4.6 .
 	@go run mvdan.cc/unparam@3ee2d22 .
 

analyze.go

@@ -38,7 +38,7 @@ type violation struct {
 var r = regexp.MustCompile(`\$\{\{.*?\}\}`)
 
 func analyzeManifest(manifest *Manifest) []violation {
-	if manifest.Runs.Using == "composite" {
+	if manifest != nil && manifest.Runs.Using == "composite" {
 		return analyzeSteps(manifest.Runs.Steps)
 	} else {
 		return make([]violation, 0)
@@ -47,6 +47,10 @@ func analyzeManifest(manifest *Manifest) []violation {
 
 func analyzeWorkflow(workflow *Workflow) []violation {
 	violations := make([]violation, 0)
+	if workflow == nil {
+		return violations
+	}
+
 	for id, job := range workflow.Jobs {
 		job := job
 		violations = append(violations, analyzeJob(id, &job)...)

analyze_test.go

@@ -35,6 +35,12 @@ func TestAnalyzeManifest(t *testing.T) {
 			manifest: Manifest{
 				Runs: ManifestRuns{
 					Using: "node16",
+					Steps: []JobStep{
+						{
+							Name: "Example unsafe",
+							Run:  "echo ${{ inputs.value }}",
+						},
+					},
 				},
 			},
 			want: 0,
@@ -123,6 +129,13 @@ func TestAnalyzeManifest(t *testing.T) {
 			}
 		})
 	}
+
+	t.Run("nil pointer", func(t *testing.T) {
+		violations := analyzeManifest(nil)
+		if got, want := len(violations), 0; got != want {
+			t.Fatalf("Unexpected number of violations (got %d, want %d)", got, want)
+		}
+	})
 }
 
 func TestAnalyzeWorkflow(t *testing.T) {
@@ -244,6 +257,13 @@ func TestAnalyzeWorkflow(t *testing.T) {
 			}
 		})
 	}
+
+	t.Run("nil pointer", func(t *testing.T) {
+		violations := analyzeWorkflow(nil)
+		if got, want := len(violations), 0; got != want {
+			t.Fatalf("Unexpected number of violations (got %d, want %d)", got, want)
+		}
+	})
 }
 
 func TestAnalyzeJob(t *testing.T) {

main.go

@@ -93,11 +93,13 @@ func run() int {
 
 			for file, fileViolations := range targetViolations {
 				if len(fileViolations) > 0 {
-					if _, ok := violations[target]; !ok {
-						violations[target] = make(map[string][]violation)
+					targetViolations, ok := violations[target]
+					if !ok {
+						targetViolations = make(map[string][]violation)
+						violations[target] = targetViolations
 					}
 
-					violations[target][file] = fileViolations
+					targetViolations[file] = fileViolations
 				}
 			}
 		} else {
@@ -310,8 +312,8 @@ func printViolation(v *violation) {
 }
 
 func getVariableNameForExpression(expression string) (name string) {
-	parts := strings.Split(expression, ".")
-	name = strings.TrimRight(parts[len(parts)-1], "}")
+	name = expression[strings.LastIndex(expression, ".")+1:]
+	name = strings.TrimRight(name, "}")
 	name = strings.TrimSpace(name)
 	return strings.ToUpper(name)
 }