test: Catch naked returns and check for canonically named imports

This catches scenarios where we forgot to prefix the error with return.
One of our contributors occasionally made this typo, and since core go
vet didn't (surprisingly) catch it, we should add a test!

It also adds a simple check for import naming aliases. Expanding this
test to add other cases and check for differently named values might
make sense.

engine/resources/file_test.go

@@ -25,7 +25,7 @@ import (
 
 	"github.com/purpleidea/mgmt/engine"
 	"github.com/purpleidea/mgmt/engine/graph/autoedge"
-	"github.com/purpleidea/mgmt/engine/util"
+	engineUtil "github.com/purpleidea/mgmt/engine/util"
 	"github.com/purpleidea/mgmt/pgraph"
 )
 
@@ -118,13 +118,13 @@ func TestMiscEncodeDecode2(t *testing.T) {
 		return
 	}
 
-	b64, err := util.ResToB64(input)
+	b64, err := engineUtil.ResToB64(input)
 	if err != nil {
 		t.Errorf("Can't encode: %v", err)
 		return
 	}
 
-	output, err := util.B64ToRes(b64)
+	output, err := engineUtil.B64ToRes(b64)
 	if err != nil {
 		t.Errorf("Can't decode: %v", err)
 		return

engine/resources/test_test.go

@@ -21,7 +21,7 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/purpleidea/mgmt/engine/util"
+	engineUtil "github.com/purpleidea/mgmt/engine/util"
 )
 
 func TestStructTagToFieldName0(t *testing.T) {
@@ -33,7 +33,7 @@ func TestStructTagToFieldName0(t *testing.T) {
 		Delta   int `lang:"surprise"`
 	}
 
-	mapping, err := util.StructTagToFieldName(&TestStruct{})
+	mapping, err := engineUtil.StructTagToFieldName(&TestStruct{})
 	if err != nil {
 		t.Errorf("failed: %+v", err)
 		return
@@ -62,7 +62,7 @@ func TestLowerStructFieldNameToFieldName0(t *testing.T) {
 		Delta     int
 	}
 
-	mapping, err := util.LowerStructFieldNameToFieldName(&TestStruct{})
+	mapping, err := engineUtil.LowerStructFieldNameToFieldName(&TestStruct{})
 	if err != nil {
 		t.Errorf("failed: %+v", err)
 		return
@@ -100,7 +100,7 @@ func TestLowerStructFieldNameToFieldName1(t *testing.T) {
 		Delta      int
 	}
 
-	mapping, err := util.LowerStructFieldNameToFieldName(&TestStruct{})
+	mapping, err := engineUtil.LowerStructFieldNameToFieldName(&TestStruct{})
 	if err == nil {
 		t.Errorf("expected failure, but passed with: %+v", mapping)
 		return
@@ -108,7 +108,7 @@ func TestLowerStructFieldNameToFieldName1(t *testing.T) {
 }
 
 func TestLowerStructFieldNameToFieldName2(t *testing.T) {
-	mapping, err := util.LowerStructFieldNameToFieldName(&TestRes{})
+	mapping, err := engineUtil.LowerStructFieldNameToFieldName(&TestRes{})
 	if err != nil {
 		t.Errorf("failed: %+v", err)
 		return

test/test-govet.sh

@@ -31,6 +31,32 @@ function token-coloncheck() {
 	return 0
 }
 
+function naked-error() {
+	# the $ before the \t magically makes grep match the tab somehow...
+	if grep $'\terrors.New(' "$1"; then	# missing a leading return
+		return 1
+	fi
+	if grep $'\tfmt.Errorf(' "$1"; then	# missing a leading return
+		return 1
+	fi
+	if grep $'\terrwrap.Wrap' "$1"; then	# missing a leading return
+		return 1
+	fi
+	return 0
+}
+
+function consistent-imports() {
+	if grep $'\t"github.com/pkg/errors"' "$1"; then	# import as errwrap
+		return 1
+	fi
+	if grep $'\t"github.com/hashicorp/go-multierror"' "$1"; then	# import as multierr
+		return 1
+	fi
+	if grep $'\t"github.com/purpleidea/mgmt/engine/util"' "$1"; then	# import as engineUtil
+		return 1
+	fi
+}
+
 # loop through directories in an attempt to scan each go package
 for dir in `find . -maxdepth 5 -type d -not -path './old/*' -not -path './old' -not -path './tmp/*' -not -path './tmp' -not -path './.*' -not -path './vendor/*'`; do
 	match="$dir/*.go"
@@ -53,6 +79,8 @@ for file in `find . -maxdepth 3 -type f -name '*.go' -not -path './old/*' -not -
 	run-test grep 'log.' "$file" | grep '\\n"' && fail_test 'no newline needed in log.Printf()'	# no \n needed in log.Printf()
 	run-test simplify-gocase "$file"
 	run-test token-coloncheck "$file"
+	run-test naked-error "$file"
+	run-test consistent-imports "$file"
 done
 
 if [[ -n "$failures" ]]; then