Merge pull request #47377 from spxtr/golint

hack/verify-golint.sh

@@ -44,13 +44,13 @@ array_contains () {
 }
 
 # Check that the file is in alphabetical order
-linted_file="${KUBE_ROOT}/hack/.linted_packages"
-if ! diff -u "${linted_file}" <(LC_ALL=C sort "${linted_file}"); then
+failure_file="${KUBE_ROOT}/hack/.golint_failures"
+if ! diff -u "${failure_file}" <(LC_ALL=C sort "${failure_file}"); then
 	{
 		echo
-		echo "hack/.linted_packages is not in alphabetical order. Please sort it:"
+		echo "hack/.golint_failures is not in alphabetical order. Please sort it:"
 		echo
-		echo "  LC_ALL=C sort -o hack/.linted_packages hack/.linted_packages"
+		echo "  LC_ALL=C sort -o hack/.golint_failures hack/.golint_failures"
 		echo
 	} >&2
 	false
@@ -63,28 +63,30 @@ export IFS=$'\n'
 all_packages=(
 	$(go list -e ./... | egrep -v "/(third_party|vendor|staging/src/k8s.io/client-go/pkg|generated|clientset_generated)" | sed -e 's|^k8s.io/kubernetes/||' -e "s|^_${KUBE_ROOT}/\?||")
 )
-linted_packages=(
-	$(cat $linted_file)
+failing_packages=(
+	$(cat $failure_file)
 )
 unset IFS
-linted=()
 errors=()
+not_failing=()
 for p in "${all_packages[@]}"; do
 	# Run golint on package/*.go file explicitly to validate all go files
 	# and not just the ones for the current platform.
-	failedLint=$(golint "$p"/*.go)
-	if [ "$failedLint" ]; then
-		if array_contains "$p" "${linted_packages[@]}"; then
-			errors+=( "$failedLint" )
-		fi
-	else
-		array_contains "$p" "${linted_packages[@]}" || linted+=( "$p" )
+	# Packages with a corresponding foo_test package will make golint fail
+	# with a useless error. Just ignore that, see golang/lint#68.
+	failedLint=$(golint "$p"/*.go 2>/dev/null)
+	array_contains "$p" "${failing_packages[@]}" && in_failing=$? || in_failing=$?
+	if [[ -n "${failedLint}" ]] && [[ "${in_failing}" -ne "0" ]]; then
+		errors+=( "${failedLint}" )
+	fi
+	if [[ -z "${failedLint}" ]] && [[ "${in_failing}" -eq "0" ]]; then
+		not_failing+=( $p )
 	fi
 done
 
-# Check that all linted_packages actually still exist
+# Check that all failing_packages actually still exist
 gone=()
-for p in "${linted_packages[@]}"; do
+for p in "${failing_packages[@]}"; do
 	array_contains "$p" "${all_packages[@]}" || gone+=( "$p" )
 done
 
@@ -98,38 +100,33 @@ else
 			echo "$err"
 		done
 		echo
-		echo 'Please fix the above errors. You can test via "golint" and commit the result.'
+		echo 'Please review the above warnings. You can test via "golint" and commit the result.'
+		echo 'If the above warnings do not make sense, you can exempt this package from golint'
+		echo 'checking by adding it to hack/.golint_failures (if your reviewer is okay with it).'
 		echo
 	} >&2
 	false
 fi
 
-# check to make sure all packages that pass lint are in the linted file.
-echo
-if [ ${#linted[@]} -eq 0 -a ${#gone[@]} -eq 0 ]; then
-	echo 'Success! All packages that should pass lint are listed in the linted file.'
-else
+if [[ ${#not_failing[@]} -gt 0 ]]; then
+	{
+		echo "Some packages in hack/.golint_failures are passing golint. Please remove them."
+		echo
+		for p in "${not_failing[@]}"; do
+			echo "  $p"
+		done
+		echo
+	} >&2
+	false
+fi
+
+if [[ ${#gone[@]} -gt 0 ]]; then
 	{
-		if [ ${#gone[@]} -gt 0 ]; then
-			echo "Some packages in hack/.linted_packages do not exist anymore. Please remove them"
-			echo "from hack/.linted_packages:"
-			echo
-			for p in "${gone[@]}"; do
-				echo "  $p"
-			done
-			echo
-		fi
-		if [ ${#linted[@]} -gt 0 ]; then
-			echo "Some packages passed golint but are not listed in hack/.linted_packages."
-			echo "Please add them in alphabetical order:"
-			echo
-			for p in "${linted[@]}"; do
-				echo "  echo $p >> hack/.linted_packages"
-			done
-			echo "  LC_ALL=C sort -o hack/.linted_packages hack/.linted_packages"
-			echo
-		fi
-		echo 'You can test via this script and commit the result.'
+		echo "Some packages in hack/.golint_failures do not exist anymore. Please remove them."
+		echo
+		for p in "${gone[@]}"; do
+			echo "  $p"
+		done
 		echo
 	} >&2
 	false