Task: Update static code analysis

Dockerfile.dev

@@ -7,12 +7,10 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
 ENV GLIDE_VERSION v0.12.3
 
 RUN go get -u github.com/jteeuwen/go-bindata/... \
-    && go get github.com/golang/lint/golint \
+    && go get github.com/alecthomas/gometalinter \
+    && gometalinter --install --vendor \
     && go get golang.org/x/tools/cmd/goimports \
-    && go get github.com/kisielk/errcheck \
     && go get github.com/axw/gocov/gocov \
-    && go get github.com/t-yuki/gocov-xml \
-    && go get github.com/tebeka/go2xunit \
     && curl -Lo /tmp/glide.tgz https://github.com/Masterminds/glide/releases/download/$GLIDE_VERSION/glide-$GLIDE_VERSION-linux-amd64.tar.gz \
     && tar -C /usr/bin -xzf /tmp/glide.tgz --strip=1 linux-amd64/glide \
     && rm /tmp/glide.tgz

Makefile

@@ -51,10 +51,7 @@ add-dep: build/image_build
 format: vendor
 	${DOCKERNOVENDOR} bash ./scripts/fmt.sh
 
-lint: format
-	${DOCKERRUN} bash ./scripts/lint.sh
-
-check: lint
+check: format
 	${DOCKERRUN} bash ./scripts/check.sh
 
 # default task

parser.go

@@ -137,7 +137,7 @@ func (t *ServiceTemplateDefinition) parse(data []byte, resolver Resolver, hooks
 		data := MustAsset(normType)
 
 		var tt ServiceTemplateDefinition
-		err := yaml.Unmarshal(data, &tt)
+		err = yaml.Unmarshal(data, &tt)
 		if err != nil {
 			return err
 		}

resolver.go

@@ -22,7 +22,8 @@ func defaultResolver(location string) ([]byte, error) {
 
 	switch u.Scheme {
 	case "http", "https":
-		res, err := http.Get(u.String())
+		var res *http.Response
+		res, err = http.Get(u.String())
 		if err != nil {
 			return r, err
 		}

scripts/check.sh

@@ -1,22 +1,19 @@
 #!/bin/bash
 
-rm -f err.txt*
-rm -f vet.txt
+set -o errexit
+set -o nounset
+set -o pipefail
 
-for pkg in $(glide nv); do
-    errcheck $pkg >> err.txt 2>&1
-    go vet $pkg >> vet.txt 2>&1
-done
-
-# ignore generated files
-sed -i.prev '/defer/d' err.txt
-
-# remove when it exists
-rm -f err.txt.prev
-
-if [[ -s err.txt ]] || [[ -s vet.txt ]]
-then
-    cat err.txt
-    cat vet.txt
-    exit 1
-fi
+# Excludes:
+#   - normative_definitions.go is generated code so static analysis will always have some issue
+#   - when using defer there is no way to check to returned value so ignore
+gometalinter \
+    --exclude='normative_definitions\.go:.*$' \
+    --exclude='error return value not checked.*(Close|Log|Print).*\(errcheck\)$' \
+    --disable=aligncheck \
+    --disable=dupl \
+    --disable=gotype \
+    --enable=unused \
+    --cyclo-over=20 \
+    --tests \
+    --deadline=20s

service_template_test.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -303,6 +304,60 @@ func toBytes(s string) []byte {
 	return b
 }
 
+func checkVersion(value string, expected map[string]string, t *testing.T) {
+	var v Version
+	var data = toBytes(value)
+	if err := yaml.Unmarshal(data, &v); err != nil {
+		t.Log(err)
+		t.Fail()
+	}
+
+	major, err := strconv.Atoi(expected["major"])
+	if err != nil {
+		t.Log(expected["major"], "must be convertible to an int")
+		t.Fatal(err)
+	}
+	if v.GetMajor() != major {
+		t.Log(v.String(), "not parsed correctly", v.GetMajor())
+		t.Fail()
+	}
+
+	minor, err := strconv.Atoi(expected["minor"])
+	if err != nil {
+		t.Log(expected["minor"], "must be convertible to an int")
+		t.Fatal(err)
+	}
+	if v.GetMinor() != minor {
+		t.Log(v.String(), "not parsed correctly", v.GetMinor())
+		t.Fail()
+	}
+
+	fix, err := strconv.Atoi(expected["fix"])
+	if err != nil {
+		t.Log(expected["fix"], "must be convertible to an int")
+		t.Fatal(err)
+	}
+	if v.GetFixVersion() != fix {
+		t.Log(v.String(), "not parsed correctly", v.GetFixVersion())
+		t.Fail()
+	}
+
+	if v.GetQualifier() != expected["rel"] {
+		t.Log(v.String(), "not parsed correctly", v.GetQualifier())
+		t.Fail()
+	}
+
+	build, err := strconv.Atoi(expected["build"])
+	if err != nil {
+		t.Log(expected["build"], "must be convertible to an int")
+		t.Fatal(err)
+	}
+	if v.GetBuildVersion() != build {
+		t.Log(v.String(), "not parsed correctly", v.GetBuildVersion())
+		t.Fail()
+	}
+}
+
 func TestVersion(t *testing.T) {
 	fname := "./tests/custom_types/custom_policy_types.yaml"
 	var s ServiceTemplateDefinition
@@ -348,117 +403,45 @@ func TestVersion(t *testing.T) {
 		}
 	}
 
-	var v Version
-	var str = "1.0.0.alpha-10"
-	var data = toBytes(str)
-	if err = yaml.Unmarshal(data, &v); err != nil {
-		t.Log(err)
-		t.Fail()
-	}
-	if v.GetMajor() != 1 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetMinor() != 0 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetFixVersion() != 0 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetQualifier() != "alpha" {
-		t.Log(v.String(), "not parsed correctly", v.GetQualifier())
-		t.Fail()
-	}
-	if v.GetBuildVersion() != 10 {
-		t.Log(v.String(), "not parsed correctly", v.GetBuildVersion())
-		t.Fail()
+	expected := map[string]string{
+		"major": "1",
+		"minor": "0",
+		"fix":   "0",
+		"rel":   "alpha",
+		"build": "10",
 	}
+	checkVersion("1.0.0.alpha-10", expected, t)
 
-	str = "1.0.alpha-9"
-	data = toBytes(str)
-	if err = yaml.Unmarshal(data, &v); err != nil {
-		t.Log(err)
-		t.Fail()
-	}
-	if v.GetMajor() != 1 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetMinor() != 0 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetFixVersion() != 0 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetQualifier() != "alpha" {
-		t.Log(v.String(), "not parsed correctly", v.GetQualifier())
-		t.Fail()
-	}
-	if v.GetBuildVersion() != 9 {
-		t.Log(v.String(), "not parsed correctly", v.GetBuildVersion())
-		t.Fail()
+	expected = map[string]string{
+		"major": "1",
+		"minor": "0",
+		"fix":   "0",
+		"rel":   "alpha",
+		"build": "9",
 	}
+	checkVersion("1.0.alpha-9", expected, t)
 
-	str = "1.0"
-	data = toBytes(str)
-	if err = yaml.Unmarshal(data, &v); err != nil {
-		t.Log(err)
-		t.Fail()
-	}
-	if v.GetMajor() != 1 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetMinor() != 0 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetFixVersion() != 0 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetQualifier() != "" {
-		t.Log(v.String(), "not parsed correctly", v.GetQualifier())
-		t.Fail()
-	}
-	if v.GetBuildVersion() != 0 {
-		t.Log(v.String(), "not parsed correctly", v.GetBuildVersion())
-		t.Fail()
+	expected = map[string]string{
+		"major": "1",
+		"minor": "0",
+		"fix":   "0",
+		"rel":   "",
+		"build": "0",
 	}
+	checkVersion("1.0", expected, t)
 
-	str = "1"
-	data = toBytes(str)
-	if err = yaml.Unmarshal(data, &v); err != nil {
-		t.Log(err)
-		t.Fail()
-	}
-	if v.GetMajor() != 1 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetMinor() != 0 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetFixVersion() != 0 {
-		t.Log(v.String(), "not parsed correctly")
-		t.Fail()
-	}
-	if v.GetQualifier() != "" {
-		t.Log(v.String(), "not parsed correctly", v.GetQualifier())
-		t.Fail()
-	}
-	if v.GetBuildVersion() != 0 {
-		t.Log(v.String(), "not parsed correctly", v.GetBuildVersion())
-		t.Fail()
+	expected = map[string]string{
+		"major": "1",
+		"minor": "0",
+		"fix":   "0",
+		"rel":   "",
+		"build": "0",
 	}
+	checkVersion("1", expected, t)
 
-	str = "test"
-	data = toBytes(str)
+	var v Version
+	str := "test"
+	data := toBytes(str)
 	if err = yaml.Unmarshal(data, &v); err == nil {
 		t.Log(str, "is not a valid version but parsed successfully")
 		t.Fail()
@@ -686,7 +669,7 @@ func TestEvaluateRelationship(t *testing.T) {
 	}
 
 	v := pa.Evaluate(&s, "tosca.relationships.HostedOn")
-	if vstr, ok := v.(string); ok {
+	if vstr, isString := v.(string); isString {
 		if vstr != "1" {
 			t.Log(fname, "property evaluation failed to get value for `test`", vstr)
 			t.Fail()
@@ -760,7 +743,7 @@ func TestEvaluatePropertyHostGetAttributeFunc(t *testing.T) {
 	}
 
 	v := pa.Evaluate(&s, "dbms")
-	if vstr, ok := v.(string); ok {
+	if vstr, isString := v.(string); isString {
 		if vstr != "127.0.0.1" {
 			t.Log(fname, "property evaluation failed to get value for `ip_address`", vstr)
 			t.Fail()

tosca_namespace_alias.go

@@ -93,16 +93,16 @@ func parseToscaVersion(s string) (semver.Version, error) {
 	}
 	if len(parts) == 3 {
 		if strings.ContainsAny(parts[len(parts)-1], "-") {
-			x, parts := parts[len(parts)-1], parts[:len(parts)-1]
-			parts = append(parts, "0")
-			s = strings.Join(parts, ".")
+			x, tparts := parts[len(parts)-1], parts[:len(parts)-1]
+			tparts = append(tparts, "0")
+			s = strings.Join(tparts, ".")
 			s = s + "-" + strings.Join(strings.SplitN(x, "-", 2), ".")
 		}
 	}
 	if len(parts) == 4 {
 		if strings.ContainsAny(parts[len(parts)-1], "-") {
-			x, parts := parts[len(parts)-1], parts[:len(parts)-1]
-			s = strings.Join(parts, ".")
+			x, tparts := parts[len(parts)-1], parts[:len(parts)-1]
+			s = strings.Join(tparts, ".")
 			s = s + "-" + strings.Join(strings.SplitN(x, "-", 2), ".")
 		}
 	}