From a407771eaf41f70f1556be9a695d70124806eb12 Mon Sep 17 00:00:00 2001
From: James Shubin <james@shubin.ca>
Date: Sat, 5 May 2018 13:34:52 -0400
Subject: [PATCH] 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 |  6 +++---
 engine/resources/test_test.go | 10 +++++-----
 test/test-govet.sh            | 28 ++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/engine/resources/file_test.go b/engine/resources/file_test.go
index 9a11d1bc..529e68f2 100644
--- a/engine/resources/file_test.go
+++ b/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
diff --git a/engine/resources/test_test.go b/engine/resources/test_test.go
index 2303c3c7..bb8f0183 100644
--- a/engine/resources/test_test.go
+++ b/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
diff --git a/test/test-govet.sh b/test/test-govet.sh
index 6d8e959f..7ebbaa67 100755
--- a/test/test-govet.sh
+++ b/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