Merge pull request #1769 from ceh/vet-fixes

Address go vet reports

Makefile

 TEST?=./...
+VETARGS?=-asmdecl -atomic -bool -buildtags -copylocks -methods \
+         -nilfunc -printf -rangeloops -shift -structtags -unsafeptr
 
 default: test
 
@@ -10,6 +12,7 @@ dev:
 
 test:
 	go test $(TEST) $(TESTARGS) -timeout=10s
+	@$(MAKE) vet
 
 testrace:
 	go test -race $(TEST) $(TESTARGS)
@@ -17,4 +20,14 @@ testrace:
 updatedeps:
 	go get -d -v -p 2 ./...
 
-.PHONY: bin default test updatedeps
+vet:
+	@go tool vet 2>/dev/null ; if [ $$? -eq 3 ]; then \
+		go get golang.org/x/tools/cmd/vet; \
+	fi
+	@go tool vet $(VETARGS) . ; if [ $$? -eq 1 ]; then \
+		echo ""; \
+		echo "Vet found suspicious constructs. Please check the reported constructs"; \
+		echo "and fix them if necessary before submitting the code for reviewal."; \
+	fi
+
+.PHONY: bin default test updatedeps vet

builder/amazon/common/step_ami_region_copy.go

@@ -79,7 +79,7 @@ func amiRegionCopy(state multistep.StateBag, auth aws.Auth, imageId string,
 
 	if err != nil {
 		return "", fmt.Errorf("Error Copying AMI (%s) to region (%s): %s",
-			imageId, target, err)
+			imageId, target.Name, err)
 	}
 
 	stateChange := StateChangeConf{
@@ -91,7 +91,7 @@ func amiRegionCopy(state multistep.StateBag, auth aws.Auth, imageId string,
 
 	if _, err := WaitForState(&stateChange); err != nil {
 		return "", fmt.Errorf("Error waiting for AMI (%s) in region (%s): %s",
-			resp.ImageId, target, err)
+			resp.ImageId, target.Name, err)
 	}
 
 	return resp.ImageId, nil

builder/amazon/ebs/step_create_ami.go

@@ -87,7 +87,7 @@ func (s *stepCreateAMI) Cleanup(state multistep.StateBag) {
 		ui.Error(fmt.Sprintf("Error deregistering AMI, may still be around: %s", err))
 		return
 	} else if resp.Return == false {
-		ui.Error(fmt.Sprintf("Error deregistering AMI, may still be around: %s", resp.Return))
+		ui.Error(fmt.Sprintf("Error deregistering AMI, may still be around: %t", resp.Return))
 		return
 	}
 }

builder/digitalocean/builder_test.go

@@ -264,7 +264,7 @@ func TestBuilderPrepare_SSHUsername(t *testing.T) {
 	}
 
 	if b.config.SSHUsername != "root" {
-		t.Errorf("invalid: %d", b.config.SSHUsername)
+		t.Errorf("invalid: %s", b.config.SSHUsername)
 	}
 
 	// Test set
@@ -297,7 +297,7 @@ func TestBuilderPrepare_SSHTimeout(t *testing.T) {
 	}
 
 	if b.config.RawSSHTimeout != "1m" {
-		t.Errorf("invalid: %d", b.config.RawSSHTimeout)
+		t.Errorf("invalid: %s", b.config.RawSSHTimeout)
 	}
 
 	// Test set
@@ -338,7 +338,7 @@ func TestBuilderPrepare_StateTimeout(t *testing.T) {
 	}
 
 	if b.config.RawStateTimeout != "6m" {
-		t.Errorf("invalid: %d", b.config.RawStateTimeout)
+		t.Errorf("invalid: %s", b.config.RawStateTimeout)
 	}
 
 	// Test set
@@ -379,7 +379,7 @@ func TestBuilderPrepare_PrivateNetworking(t *testing.T) {
 	}
 
 	if b.config.PrivateNetworking != false {
-		t.Errorf("invalid: %s", b.config.PrivateNetworking)
+		t.Errorf("invalid: %t", b.config.PrivateNetworking)
 	}
 
 	// Test set
@@ -394,7 +394,7 @@ func TestBuilderPrepare_PrivateNetworking(t *testing.T) {
 	}
 
 	if b.config.PrivateNetworking != true {
-		t.Errorf("invalid: %s", b.config.PrivateNetworking)
+		t.Errorf("invalid: %t", b.config.PrivateNetworking)
 	}
 }
 

builder/googlecompute/step_create_instance_test.go

@@ -39,7 +39,7 @@ func TestStepCreateInstance(t *testing.T) {
 		t.Fatal("should've deleted instance")
 	}
 	if driver.DeleteInstanceZone != config.Zone {
-		t.Fatal("bad zone: %#v", driver.DeleteInstanceZone)
+		t.Fatalf("bad zone: %#v", driver.DeleteInstanceZone)
 	}
 }
 

builder/googlecompute/step_teardown_instance_test.go

@@ -26,7 +26,7 @@ func TestStepTeardownInstance(t *testing.T) {
 		t.Fatal("should've deleted instance")
 	}
 	if driver.DeleteInstanceZone != config.Zone {
-		t.Fatal("bad zone: %#v", driver.DeleteInstanceZone)
+		t.Fatalf("bad zone: %#v", driver.DeleteInstanceZone)
 	}
 
 	// cleanup
@@ -36,6 +36,6 @@ func TestStepTeardownInstance(t *testing.T) {
 		t.Fatal("should've deleted disk")
 	}
 	if driver.DeleteDiskZone != config.Zone {
-		t.Fatal("bad zone: %#v", driver.DeleteDiskZone)
+		t.Fatalf("bad zone: %#v", driver.DeleteDiskZone)
 	}
 }

builder/openstack/artifact.go

@@ -41,6 +41,6 @@ func (a *Artifact) State(name string) interface{} {
 }
 
 func (a *Artifact) Destroy() error {
-	log.Printf("Destroying image: %d", a.ImageId)
+	log.Printf("Destroying image: %s", a.ImageId)
 	return a.Conn.DeleteImageById(a.ImageId)
 }

builder/openstack/step_wait_for_rackconnect.go

@@ -21,7 +21,6 @@ func (s *StepWaitForRackConnect) Run(state multistep.StateBag) multistep.StepAct
 	csp := state.Get("csp").(gophercloud.CloudServersProvider)
 	server := state.Get("server").(*gophercloud.Server)
 	ui := state.Get("ui").(packer.Ui)
-	fmt.Printf("%s", server)
 
 	ui.Say(fmt.Sprintf("Waiting for server (%s) to become RackConnect ready...", server.Id))
 

builder/parallels/iso/builder_test.go

@@ -75,7 +75,7 @@ func TestBuilderPrepare_DiskSize(t *testing.T) {
 	}
 
 	if b.config.DiskSize != 60000 {
-		t.Fatalf("bad size: %s", b.config.DiskSize)
+		t.Fatalf("bad size: %d", b.config.DiskSize)
 	}
 }
 

builder/qemu/builder_test.go

@@ -160,7 +160,7 @@ func TestBuilderPrepare_DiskSize(t *testing.T) {
 	}
 
 	if b.config.DiskSize != 60000 {
-		t.Fatalf("bad size: %s", b.config.DiskSize)
+		t.Fatalf("bad size: %d", b.config.DiskSize)
 	}
 }
 

builder/virtualbox/iso/builder_test.go

@@ -83,7 +83,7 @@ func TestBuilderPrepare_DiskSize(t *testing.T) {
 	}
 
 	if b.config.DiskSize != 60000 {
-		t.Fatalf("bad size: %s", b.config.DiskSize)
+		t.Fatalf("bad size: %d", b.config.DiskSize)
 	}
 }
 

builder/vmware/common/step_shutdown_test.go

@@ -127,7 +127,7 @@ func TestStepShutdown_locks(t *testing.T) {
 	lockPath := filepath.Join(dir.dir, "nope.lck")
 	err := ioutil.WriteFile(lockPath, []byte("foo"), 0644)
 	if err != nil {
-		t.Fatalf("err: %s")
+		t.Fatalf("err: %s", err)
 	}
 
 	// Remove the lock file after a certain time

builder/vmware/iso/builder_test.go

@@ -175,7 +175,7 @@ func TestBuilderPrepare_DiskSize(t *testing.T) {
 	}
 
 	if b.config.DiskSize != 60000 {
-		t.Fatalf("bad size: %s", b.config.DiskSize)
+		t.Fatalf("bad size: %d", b.config.DiskSize)
 	}
 }
 

common/step_create_floppy_test.go

@@ -76,7 +76,7 @@ func TestStepCreateFloppy(t *testing.T) {
 		floppy_path := state.Get("floppy_path").(string)
 
 		if _, err := os.Stat(floppy_path); err != nil {
-			t.Fatal("file not found: %s for %v", floppy_path, step.Files)
+			t.Fatalf("file not found: %s for %v", floppy_path, step.Files)
 		}
 
 		if len(step.FilesAdded) != expected {
@@ -86,7 +86,7 @@ func TestStepCreateFloppy(t *testing.T) {
 		step.Cleanup(state)
 
 		if _, err := os.Stat(floppy_path); err == nil {
-			t.Fatal("file found: %s for %v", floppy_path, step.Files)
+			t.Fatalf("file found: %s for %v", floppy_path, step.Files)
 		}
 	}
 }
@@ -177,7 +177,7 @@ func xxxTestStepCreateFloppy_notfound(t *testing.T) {
 		floppy_path := state.Get("floppy_path").(string)
 
 		if _, err := os.Stat(floppy_path); err != nil {
-			t.Fatal("file not found: %s for %v", floppy_path, step.Files)
+			t.Fatalf("file not found: %s for %v", floppy_path, step.Files)
 		}
 
 		if len(step.FilesAdded) != expected {
@@ -187,7 +187,7 @@ func xxxTestStepCreateFloppy_notfound(t *testing.T) {
 		step.Cleanup(state)
 
 		if _, err := os.Stat(floppy_path); err == nil {
-			t.Fatal("file found: %s for %v", floppy_path, step.Files)
+			t.Fatalf("file found: %s for %v", floppy_path, step.Files)
 		}
 	}
 }

communicator/ssh/communicator_test.go

@@ -83,10 +83,10 @@ func newMockLineServer(t *testing.T) string {
 			}
 			t.Log("Accepted channel")
 
-			go func() {
+			go func(channelType string) {
 				defer channel.Close()
-				conn.OpenChannel(newChannel.ChannelType(), nil)
-			}()
+				conn.OpenChannel(channelType, nil)
+			}(newChannel.ChannelType())
 		}
 		conn.Close()
 	}()

packer/template_test.go

@@ -497,7 +497,7 @@ func TestParseTemplate_Provisioners(t *testing.T) {
 
 	result, err := ParseTemplate([]byte(data), nil)
 	if err != nil {
-		t.Fatal("err: %s", err)
+		t.Fatalf("err: %s", err)
 	}
 	if result == nil {
 		t.Fatal("should have result")
@@ -529,7 +529,7 @@ func TestParseTemplate_ProvisionerPauseBefore(t *testing.T) {
 
 	result, err := ParseTemplate([]byte(data), nil)
 	if err != nil {
-		t.Fatal("err: %s", err)
+		t.Fatalf("err: %s", err)
 	}
 	if result == nil {
 		t.Fatal("should have result")

post-processor/vagrant-cloud/client.go

@@ -78,7 +78,7 @@ func (v VagrantCloudClient) Get(path string) (*http.Response, error) {
 	req.Header.Add("Content-Type", "application/json")
 	resp, err := v.client.Do(req)
 
-	log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%s", resp)
+	log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%+v", resp)
 
 	return resp, err
 }
@@ -96,7 +96,7 @@ func (v VagrantCloudClient) Delete(path string) (*http.Response, error) {
 	req.Header.Add("Content-Type", "application/json")
 	resp, err := v.client.Do(req)
 
-	log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%s", resp)
+	log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%+v", resp)
 
 	return resp, err
 }
@@ -128,7 +128,7 @@ func (v VagrantCloudClient) Upload(path string, url string) (*http.Response, err
 
 	resp, err := v.client.Do(request)
 
-	log.Printf("Post-Processor Vagrant Cloud Upload Response: \n\n%s", resp)
+	log.Printf("Post-Processor Vagrant Cloud Upload Response: \n\n%+v", resp)
 
 	return resp, err
 }
@@ -153,7 +153,7 @@ func (v VagrantCloudClient) Post(path string, body interface{}) (*http.Response,
 
 	resp, err := v.client.Do(req)
 
-	log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%s", resp)
+	log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%+v", resp)
 
 	return resp, err
 }
@@ -172,7 +172,7 @@ func (v VagrantCloudClient) Put(path string) (*http.Response, error) {
 
 	resp, err := v.client.Do(req)
 
-	log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%s", resp)
+	log.Printf("Post-Processor Vagrant Cloud API Response: \n\n%+v", resp)
 
 	return resp, err
 }

post-processor/vagrant/post-processor_test.go

@@ -132,7 +132,7 @@ func TestPostProcessorPrepare_vagrantfileTemplateExists(t *testing.T) {
 	c["vagrantfile_template"] = name
 
 	if err := f.Close(); err != nil {
-		t.Fatal("err: %s", err)
+		t.Fatalf("err: %s", err)
 	}
 
 	if err := os.Remove(name); err != nil {

provisioner/puppet-masterless/provisioner.go

@@ -190,7 +190,7 @@ func (p *Provisioner) Prepare(raws ...interface{}) error {
 				fmt.Errorf("module_path[%d] is invalid: %s", i, err))
 		} else if !info.IsDir() {
 			errs = packer.MultiErrorAppend(errs,
-				fmt.Errorf("module_path[%d] must point to a directory"))
+				fmt.Errorf("module_path[%d] must point to a directory", i))
 		}
 	}
 

provisioner/salt-masterless/provisioner.go

@@ -118,14 +118,14 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 		}
 		ui.Message(fmt.Sprintf("Downloading saltstack bootstrap to /tmp/install_salt.sh"))
 		if err = cmd.StartWithUi(comm, ui); err != nil {
-			return fmt.Errorf("Unable to download Salt: %d", err)
+			return fmt.Errorf("Unable to download Salt: %s", err)
 		}
 		cmd = &packer.RemoteCmd{
 			Command: fmt.Sprintf("sudo sh /tmp/install_salt.sh %s", p.config.BootstrapArgs),
 		}
-		ui.Message(fmt.Sprintf("Installing Salt with command %s", cmd))
+		ui.Message(fmt.Sprintf("Installing Salt with command %s", cmd.Command))
 		if err = cmd.StartWithUi(comm, ui); err != nil {
-			return fmt.Errorf("Unable to install Salt: %d", err)
+			return fmt.Errorf("Unable to install Salt: %s", err)
 		}
 	}
 
@@ -146,7 +146,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 		src = filepath.ToSlash(filepath.Join(p.config.TempConfigDir, "minion"))
 		dst = "/etc/salt/minion"
 		if err = p.moveFile(ui, comm, dst, src); err != nil {
-			return fmt.Errorf("Unable to move %s/minion to /etc/salt/minion: %d", p.config.TempConfigDir, err)
+			return fmt.Errorf("Unable to move %s/minion to /etc/salt/minion: %s", p.config.TempConfigDir, err)
 		}
 	}
 
@@ -161,7 +161,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 	src = filepath.ToSlash(filepath.Join(p.config.TempConfigDir, "states"))
 	dst = "/srv/salt"
 	if err = p.moveFile(ui, comm, dst, src); err != nil {
-		return fmt.Errorf("Unable to move %s/states to /srv/salt: %d", p.config.TempConfigDir, err)
+		return fmt.Errorf("Unable to move %s/states to /srv/salt: %s", p.config.TempConfigDir, err)
 	}
 
 	if p.config.LocalPillarRoots != "" {
@@ -176,7 +176,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 		src = filepath.ToSlash(filepath.Join(p.config.TempConfigDir, "pillar"))
 		dst = "/srv/pillar"
 		if err = p.moveFile(ui, comm, dst, src); err != nil {
-			return fmt.Errorf("Unable to move %s/pillar to /srv/pillar: %d", p.config.TempConfigDir, err)
+			return fmt.Errorf("Unable to move %s/pillar to /srv/pillar: %s", p.config.TempConfigDir, err)
 		}
 	}
 
@@ -220,7 +220,7 @@ func (p *Provisioner) moveFile(ui packer.Ui, comm packer.Communicator, dst, src
 			err = fmt.Errorf("Bad exit status: %d", cmd.ExitStatus)
 		}
 
-		return fmt.Errorf("Unable to move %s/minion to /etc/salt/minion: %d", p.config.TempConfigDir, err)
+		return fmt.Errorf("Unable to move %s/minion to /etc/salt/minion: %s", p.config.TempConfigDir, err)
 	}
 	return nil
 }