Conform to all linter warnings/advice

Summary:
Fixed all linter warnings and advice except for vet's rule about unkeyed
composite literals which was disabled via a `-composites=false` flag in
`.arclint`. Most unkeyed literals (e.g. `build.Run{"command"}`) in this
project just seem too usefully succinct compared to their more verbose
keyed counterparts.

Depends on D841

Test Plan: Run `arc lint --everything` and verify there are no warnings or advice.

Reviewers: thcipriani, hashar, #release-engineering-team

Reviewed By: thcipriani, hashar, #release-engineering-team

Tags: #release-engineering-team

Differential Revision: https://phabricator.wikimedia.org/D845

8 files changed, 74 insertions, 73 deletions

diff --git a/.arclint b/.arclint
index a112053..c413c3c 100644
--- a/.arclint
+++ b/.arclint
@@ -7,6 +7,7 @@
     },
     "govet": {
       "type": "govet",
+      "flags": ["-composites=false"],
       "include": "(\\.go$)"
     },
     "gofmt": {
diff --git a/config/common.go b/config/common.go
index 11357d8..62fb483 100644
--- a/config/common.go
+++ b/config/common.go
@@ -18,18 +18,18 @@ type CommonConfig struct {
 
 // Merge takes another CommonConfig and merges its fields this one's.
 //
-func (cc1 *CommonConfig) Merge(cc2 CommonConfig) {
+func (cc *CommonConfig) Merge(cc2 CommonConfig) {
 	if cc2.Base != "" {
-		cc1.Base = cc2.Base
+		cc.Base = cc2.Base
 	}
 
-	cc1.Apt.Merge(cc2.Apt)
-	cc1.Node.Merge(cc2.Node)
-	cc1.Runs.Merge(cc2.Runs)
-	cc1.SharedVolume.Merge(cc2.SharedVolume)
+	cc.Apt.Merge(cc2.Apt)
+	cc.Node.Merge(cc2.Node)
+	cc.Runs.Merge(cc2.Runs)
+	cc.SharedVolume.Merge(cc2.SharedVolume)
 
-	if len(cc1.EntryPoint) < 1 {
-		cc1.EntryPoint = cc2.EntryPoint
+	if len(cc.EntryPoint) < 1 {
+		cc.EntryPoint = cc2.EntryPoint
 	}
 }
 
diff --git a/config/reader.go b/config/reader.go
index 7aa060d..7aaa22e 100644
--- a/config/reader.go
+++ b/config/reader.go
@@ -72,7 +72,7 @@ func ReadConfigFile(path string) (*Config, error) {
 
 	if err != nil {
 		return nil, err
-	} else {
-		return ReadConfig(data)
 	}
+
+	return ReadConfig(data)
 }
diff --git a/config/runs.go b/config/runs.go
index ef9f4ea..5986112 100644
--- a/config/runs.go
+++ b/config/runs.go
@@ -17,8 +17,8 @@ const LocalLibPrefix = "/opt/lib"
 type RunsConfig struct {
 	In          string            `yaml:"in"`          // working directory
 	As          string            `yaml:"as"`          // unprivileged user
-	Uid         int               `yaml:"uid"`         // unprivileged UID
-	Gid         int               `yaml:"gid"`         // unprivileged GID
+	UID         int               `yaml:"uid"`         // unprivileged user ID
+	GID         int               `yaml:"gid"`         // unprivileged group ID
 	Environment map[string]string `yaml:"environment"` // environment variables
 }
 
@@ -33,11 +33,11 @@ func (run *RunsConfig) Merge(run2 RunsConfig) {
 	if run2.As != "" {
 		run.As = run2.As
 	}
-	if run2.Uid != 0 {
-		run.Uid = run2.Uid
+	if run2.UID != 0 {
+		run.UID = run2.UID
 	}
-	if run2.Gid != 0 {
-		run.Gid = run2.Gid
+	if run2.GID != 0 {
+		run.GID = run2.GID
 	}
 
 	if run.Environment == nil {
@@ -55,9 +55,9 @@ func (run *RunsConfig) Merge(run2 RunsConfig) {
 func (run RunsConfig) Home() string {
 	if run.As == "" {
 		return "/root"
-	} else {
-		return "/home/" + run.As
 	}
+
+	return "/home/" + run.As
 }
 
 // InstructionsForPhase injects build instructions related to the runtime
@@ -92,9 +92,9 @@ func (run RunsConfig) InstructionsForPhase(phase build.Phase) []build.Instructio
 		if run.As != "" {
 			runAll.Runs = append(runAll.Runs,
 				build.Run{"groupadd -o -g %s -r",
-					[]string{strconv.Itoa(run.Gid), run.As}},
+					[]string{strconv.Itoa(run.GID), run.As}},
 				build.Run{"useradd -o -m -d %s -r -g %s -u %s",
-					[]string{run.Home(), run.As, strconv.Itoa(run.Uid), run.As}},
+					[]string{run.Home(), run.As, strconv.Itoa(run.UID), run.As}},
 				build.Run{"chown %s:%s",
 					[]string{run.As, run.As, LocalLibPrefix}},
 			)
diff --git a/config/runs_test.go b/config/runs_test.go
index c8bb2bc..3aa8825 100644
--- a/config/runs_test.go
+++ b/config/runs_test.go
@@ -28,8 +28,8 @@ func TestRunsConfig(t *testing.T) {
 
 	assert.Equal(t, "someuser", variant.Runs.As)
 	assert.Equal(t, "/some/directory", variant.Runs.In)
-	assert.Equal(t, 666, variant.Runs.Uid)
-	assert.Equal(t, 777, variant.Runs.Gid)
+	assert.Equal(t, 666, variant.Runs.UID)
+	assert.Equal(t, 777, variant.Runs.GID)
 	assert.Equal(t, map[string]string{"FOO": "bar"}, variant.Runs.Environment)
 }
 
@@ -49,8 +49,8 @@ func TestRunsConfigInstructions(t *testing.T) {
 	cfg := config.RunsConfig{
 		As:  "someuser",
 		In:  "/some/directory",
-		Uid: 666,
-		Gid: 777,
+		UID: 666,
+		GID: 777,
 		Environment: map[string]string{
 			"fooname": "foovalue",
 			"barname": "barvalue",
diff --git a/docker/compiler.go b/docker/compiler.go
index 2823777..68f9384 100644
--- a/docker/compiler.go
+++ b/docker/compiler.go
@@ -87,7 +87,7 @@ func compileStage(buffer *bytes.Buffer, stage string, vcfg *config.VariantConfig
 
 func compileInstructions(buffer *bytes.Buffer, instructions ...build.Instruction) {
 	for _, instruction := range instructions {
-		dockerInstruction, _ := NewDockerInstruction(instruction)
+		dockerInstruction, _ := NewInstruction(instruction)
 		write(buffer, dockerInstruction.Compile())
 	}
 }
diff --git a/docker/instructions.go b/docker/instructions.go
index f362814..52c281a 100644
--- a/docker/instructions.go
+++ b/docker/instructions.go
@@ -8,126 +8,126 @@ import (
 	"phabricator.wikimedia.org/source/blubber/build"
 )
 
-// NewDockerInstruction takes a general internal build.Instruction and returns
+// NewInstruction takes a general internal build.Instruction and returns
 // a corresponding compilable Docker specific instruction. The given internal
 // instruction is partially compiled at this point by calling Compile() which
 // applies its own logic for escaping arguments, etc.
 //
-func NewDockerInstruction(instruction build.Instruction) (DockerInstruction, error) {
+func NewInstruction(instruction build.Instruction) (Instruction, error) {
 	switch instruction.(type) {
 	case build.Run, build.RunAll:
-		var dockerInstruction DockerRun
+		var dockerInstruction Run
 		dockerInstruction.arguments = instruction.Compile()
 		return dockerInstruction, nil
 	case build.Copy:
-		var dockerInstruction DockerCopy
+		var dockerInstruction Copy
 		dockerInstruction.arguments = instruction.Compile()
 		return dockerInstruction, nil
 	case build.CopyFrom:
-		var dockerInstruction DockerCopyFrom
+		var dockerInstruction CopyFrom
 		dockerInstruction.arguments = instruction.Compile()
 		return dockerInstruction, nil
 	case build.Env:
-		var dockerInstruction DockerEnv
+		var dockerInstruction Env
 		dockerInstruction.arguments = instruction.Compile()
 		return dockerInstruction, nil
 	case build.Label:
-		var dockerInstruction DockerLabel
+		var dockerInstruction Label
 		dockerInstruction.arguments = instruction.Compile()
 		return dockerInstruction, nil
 	case build.Volume:
-		var dockerInstruction DockerVolume
+		var dockerInstruction Volume
 		dockerInstruction.arguments = instruction.Compile()
 		return dockerInstruction, nil
 	}
 
-	return nil, errors.New("Unable to create DockerInstruction")
+	return nil, errors.New("Unable to create Instruction")
 }
 
-// DockerInstruction defines an interface for instruction compilation.
+// Instruction defines an interface for instruction compilation.
 //
-type DockerInstruction interface {
+type Instruction interface {
 	Compile() string
 	Arguments() []string
 }
 
-type abstractDockerInstruction struct {
+type abstractInstruction struct {
 	arguments []string
 }
 
-func (di abstractDockerInstruction) Arguments() []string {
+func (di abstractInstruction) Arguments() []string {
 	return di.arguments
 }
 
-// DockerRun compiles into a RUN instruction.
+// Run compiles into a RUN instruction.
 //
-type DockerRun struct{ abstractDockerInstruction }
+type Run struct{ abstractInstruction }
 
 // Compile compiles RUN instructions.
 //
-func (dr DockerRun) Compile() string {
+func (dr Run) Compile() string {
 	return fmt.Sprintf(
 		"RUN %s\n",
 		join(dr.arguments, ""))
 }
 
-// DockerCopy compiles into a COPY instruction.
+// Copy compiles into a COPY instruction.
 //
-type DockerCopy struct{ abstractDockerInstruction }
+type Copy struct{ abstractInstruction }
 
 // Compile compiles COPY instructions.
 //
-func (dc DockerCopy) Compile() string {
+func (dc Copy) Compile() string {
 	return fmt.Sprintf(
 		"COPY [%s]\n",
 		join(dc.arguments, ", "))
 }
 
-// DockerCopyFrom compiles into a COPY --from instruction.
+// CopyFrom compiles into a COPY --from instruction.
 //
-type DockerCopyFrom struct{ abstractDockerInstruction }
+type CopyFrom struct{ abstractInstruction }
 
 // Compile compiles COPY --from instructions.
 //
-func (dcf DockerCopyFrom) Compile() string {
+func (dcf CopyFrom) Compile() string {
 	return fmt.Sprintf(
 		"COPY --from=%s [%s]\n",
 		dcf.arguments[0],
 		join(dcf.arguments[1:], ", "))
 }
 
-// DockerEnv compiles into a ENV instruction.
+// Env compiles into a ENV instruction.
 //
-type DockerEnv struct{ abstractDockerInstruction }
+type Env struct{ abstractInstruction }
 
 // Compile compiles ENV instructions.
 //
-func (de DockerEnv) Compile() string {
+func (de Env) Compile() string {
 	return fmt.Sprintf(
 		"ENV %s\n",
 		join(de.arguments, " "))
 }
 
-// DockerLabel compiles into a LABEL instruction.
+// Label compiles into a LABEL instruction.
 //
-type DockerLabel struct{ abstractDockerInstruction }
+type Label struct{ abstractInstruction }
 
 // Compile returns multiple key="value" arguments as a single LABEL
 // instruction.
 //
-func (dl DockerLabel) Compile() string {
+func (dl Label) Compile() string {
 	return fmt.Sprintf(
 		"LABEL %s\n",
 		join(dl.arguments, " "))
 }
 
-// DockerVolume compiles into a VOLUME instruction.
+// Volume compiles into a VOLUME instruction.
 //
-type DockerVolume struct{ abstractDockerInstruction }
+type Volume struct{ abstractInstruction }
 
 // Compile compiles VOLUME instructions.
 //
-func (dv DockerVolume) Compile() string {
+func (dv Volume) Compile() string {
 	return fmt.Sprintf(
 		"VOLUME [%s]\n",
 		join(dv.arguments, ", "))
diff --git a/docker/instructions_test.go b/docker/instructions_test.go
index a4f648b..181ab71 100644
--- a/docker/instructions_test.go
+++ b/docker/instructions_test.go
@@ -11,9 +11,9 @@ import (
 
 func TestRun(t *testing.T) {
 	i := build.Run{"echo", []string{"hello"}}
-	di, err := docker.NewDockerInstruction(i)
+	di, err := docker.NewInstruction(i)
 
-	var dockerRun docker.DockerRun
+	var dockerRun docker.Run
 
 	assert.Nil(t, err)
 	assert.IsType(t, dockerRun, di)
@@ -26,9 +26,9 @@ func TestRunAll(t *testing.T) {
 		{"echo", []string{"yo"}},
 	}}
 
-	di, err := docker.NewDockerInstruction(i)
+	di, err := docker.NewInstruction(i)
 
-	var dockerRun docker.DockerRun
+	var dockerRun docker.Run
 
 	assert.Nil(t, err)
 	assert.IsType(t, dockerRun, di)
@@ -38,9 +38,9 @@ func TestRunAll(t *testing.T) {
 func TestCopy(t *testing.T) {
 	i := build.Copy{[]string{"foo1", "foo2"}, "bar"}
 
-	di, err := docker.NewDockerInstruction(i)
+	di, err := docker.NewInstruction(i)
 
-	var dockerCopy docker.DockerCopy
+	var dockerCopy docker.Copy
 
 	assert.Nil(t, err)
 	assert.IsType(t, dockerCopy, di)
@@ -50,9 +50,9 @@ func TestCopy(t *testing.T) {
 func TestCopyFrom(t *testing.T) {
 	i := build.CopyFrom{"foo", build.Copy{[]string{"foo1", "foo2"}, "bar"}}
 
-	di, err := docker.NewDockerInstruction(i)
+	di, err := docker.NewInstruction(i)
 
-	var dockerCopyFrom docker.DockerCopyFrom
+	var dockerCopyFrom docker.CopyFrom
 
 	assert.Nil(t, err)
 	assert.IsType(t, dockerCopyFrom, di)
@@ -62,9 +62,9 @@ func TestCopyFrom(t *testing.T) {
 func TestEnv(t *testing.T) {
 	i := build.Env{map[string]string{"foo": "bar", "bar": "foo"}}
 
-	di, err := docker.NewDockerInstruction(i)
+	di, err := docker.NewInstruction(i)
 
-	var dockerEnv docker.DockerEnv
+	var dockerEnv docker.Env
 
 	assert.Nil(t, err)
 	assert.IsType(t, dockerEnv, di)
@@ -74,9 +74,9 @@ func TestEnv(t *testing.T) {
 func TestLabel(t *testing.T) {
 	i := build.Label{map[string]string{"foo": "bar", "bar": "foo"}}
 
-	di, err := docker.NewDockerInstruction(i)
+	di, err := docker.NewInstruction(i)
 
-	var dockerLabel docker.DockerLabel
+	var dockerLabel docker.Label
 
 	assert.Nil(t, err)
 	assert.IsType(t, dockerLabel, di)
@@ -86,9 +86,9 @@ func TestLabel(t *testing.T) {
 func TestVolume(t *testing.T) {
 	i := build.Volume{"/foo/dir"}
 
-	di, err := docker.NewDockerInstruction(i)
+	di, err := docker.NewInstruction(i)
 
-	var dockerVolume docker.DockerVolume
+	var dockerVolume docker.Volume
 
 	assert.Nil(t, err)
 	assert.IsType(t, dockerVolume, di)
@@ -97,21 +97,21 @@ func TestVolume(t *testing.T) {
 
 func TestEscapeRun(t *testing.T) {
 	i := build.Run{"/bin/true\nRUN echo HACKED!", []string{}}
-	dr, _ := docker.NewDockerInstruction(i)
+	dr, _ := docker.NewInstruction(i)
 
 	assert.Equal(t, "RUN /bin/true\\nRUN echo HACKED!\n", dr.Compile())
 }
 
 func TestEscapeCopy(t *testing.T) {
 	i := build.Copy{[]string{"file.a", "file.b"}, "dest"}
-	dr, _ := docker.NewDockerInstruction(i)
+	dr, _ := docker.NewInstruction(i)
 
 	assert.Equal(t, "COPY [\"file.a\", \"file.b\", \"dest\"]\n", dr.Compile())
 }
 
 func TestEscapeEnv(t *testing.T) {
 	i := build.Env{map[string]string{"a": "b\nRUN echo HACKED!"}}
-	dr, _ := docker.NewDockerInstruction(i)
+	dr, _ := docker.NewInstruction(i)
 
 	assert.Equal(t, "ENV a=\"b\\nRUN echo HACKED!\"\n", dr.Compile())
 }