Update style checks

.golangci.yml

@@ -1,7 +1,10 @@
 # Originally copied from github.com/stackrox/stackrox, and regularly kept in-sync manually.
 
 run:
-  timeout: 5m
+  timeout: 16m
+
+output:
+  format: "junit-xml:report.xml,colored-line-number"
 
 issues:
   exclude-use-default: false
@@ -73,21 +76,22 @@ linters:
     # - dupl
     # - errcheck
     # - funlen
+    # - forbidigo
     # - gochecknoglobals
     # - gochecknoinits
     # - gocognit
     # - goconst
     - exportloopref
-    # - gocritic TODO: add in follow-up
+    - gocritic
     # - gocyclo
     # - godot
     # - godox
     # - goerr113
     - gofmt
     - goimports
+    - revive # replaces golint
     # - gomnd
     # - goprintffuncname
-    # - gosec
     - gosec
     - gosimple
     - govet
@@ -100,10 +104,9 @@ linters:
     # - nestif
     - nolintlint
     # - prealloc
-    - revive # replaces golint
     - rowserrcheck
     # - scopelint
-    # - staticcheck
+    - staticcheck
     # - structcheck
     # - stylecheck
     # - testpackage

Makefile

@@ -4,6 +4,9 @@ export GOBIN
 PATH := $(GOBIN):$(PATH)
 export PATH
 
+# Set to empty string to echo some command lines which are hidden by default.
+SILENT ?= @
+
 SHELL = env GOBIN=$(GOBIN) PATH=$(PATH) /bin/bash
 BASE_DIR=$(CURDIR)
 
@@ -92,29 +95,28 @@ build-updater: deps
 ## Style ##
 ###########
 .PHONY: style
-style: blanks golangci-lint staticcheck no-large-files
+style: golangci-lint blanks newlines check-service-protos no-large-files
 
-.PHONY: staticcheck
-staticcheck: $(STATICCHECK_BIN)
+.PHONY: check-service-protos
+check-service-protos:
 	@echo "+ $@"
-	@$(BASE_DIR)/tools/staticcheck-wrap.sh ./...
+	$(SILENT)$(BASE_DIR)/tools/check-service-protos/run.sh
 
 .PHONY: no-large-files
 no-large-files:
-	@echo "+ $@"
-	@$(BASE_DIR)/tools/large-git-files/find.sh
+	$(BASE_DIR)/tools/detect-large-files.sh "$(BASE_DIR)/tools/allowed-large-files"
 
 .PHONY: golangci-lint
-golangci-lint: $(GOLANGCILINT_BIN) proto-generated-srcs
+golangci-lint: $(GOLANGCILINT_BIN)
 ifdef CI
 	@echo '+ $@'
 	@echo 'The environment indicates we are in CI; running linters in check mode.'
 	@echo 'If this fails, run `make style`.'
 	@echo "Running with no tags..."
 	golangci-lint run
 	@echo "Running with release tags..."
-	@# We use --tests=false because some unit tests don\'t compile with release tags,
-	@# since they use functions that we don\'t define in the release build. That\'s okay.
+	@# We use --tests=false because some unit tests don't compile with release tags,
+	@# since they use functions that we don't define in the release build. That's okay.
 	golangci-lint run --build-tags "$(subst $(comma),$(space),$(RELEASE_GOTAGS))" --tests=false
 else
 	golangci-lint run --fix
@@ -125,9 +127,18 @@ endif
 blanks:
 	@echo "+ $@"
 ifdef CI
-	@echo $(FORMATTING_FILES) | xargs $(BASE_DIR)/tools/import_validate.py
+	$(SILENT)git grep -L '^// Code generated by .* DO NOT EDIT\.' -- '*.go' | xargs -n 1000 $(BASE_DIR)/tools/import_validate.py
+else
+	$(SILENT)git grep -L '^// Code generated by .* DO NOT EDIT\.' -- '*.go' | xargs -n 1000 $(BASE_DIR)/tools/fix-blanks.sh
+endif
+
+.PHONY: newlines
+newlines:
+	@echo "+ $@"
+ifdef CI
+	$(SILENT)git grep --cached -Il '' | xargs $(BASE_DIR)/tools/check-newlines.sh
 else
-	@echo $(FORMATTING_FILES) | xargs $(BASE_DIR)/tools/fix-blanks.sh
+	$(SILENT)git grep --cached -Il '' | xargs $(BASE_DIR)/tools/check-newlines.sh --fix
 endif
 
 .PHONY: dev

scripts/ci/jobs/style-checks.sh

@@ -1,11 +1,21 @@
 #!/usr/bin/env bash
 
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")"/../../.. && pwd)"
+source "$ROOT/scripts/ci/lib.sh"
+
 set -euo pipefail
 
 style_checks() {
     info "Starting style checks"
 
-    make style
+    make style || touch FAIL
+
+    info "Saving junit XML report"
+    mkdir -p junit-reports
+    cp -a report.xml "junit-reports/" || true
+    store_test_results junit-reports reports
+
+    [[ ! -f FAIL ]] || die "Style checks failed"
 }
 
 style_checks "$*"

tools/check-newlines.sh

@@ -0,0 +1,47 @@
+#!/usr/bin/env bash
+
+# Checks if all files given as arguments are either empty or end with newline characters.
+
+join_by() { local IFS="$1"; shift; echo "$*"; }
+
+# File extensions to apply the "file must end with a newline character" rule to.
+EXTENSIONS=(go sh yml yaml proto ts tsx)
+
+EXT_REGEX="\\.($(join_by '|' "${EXTENSIONS[@]}"))\$"
+
+fix=0
+
+check_or_fix_newlines() {
+  file="$1"
+  [[ -s "$file" ]] || return 0  # skip check for empty files
+  if [[ -n "$(tail -c 1 "$file")" ]]; then
+    if (( fix )); then
+      echo >&2 "Added missing newline at end of file: $file"
+      echo >>"$file"
+      return 0
+    fi
+    echo >&2 "Missing newline at end of file: $file"
+    return 1
+  fi
+}
+
+# Do nothing if no arguments given
+[[ "$#" -gt 0 ]] || exit 0
+
+if [[ "$1" == "--fix" ]]; then
+  fix=1
+  shift
+fi
+
+all_files=("$@")
+
+IFS=$'\n' read -d '' -r -a files < <(
+  printf '%s\n' "${all_files[@]}" | grep -E "$EXT_REGEX"
+)
+
+status=0
+for file in "${files[@]}"; do
+  check_or_fix_newlines "$file" || status=1
+done
+
+exit "$status"

tools/check-service-protos/run.sh

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+# Flag service protos files which include "google/api/annotations.proto" but do not have their file names end with _service.proto
+all_protos=$(git ls-files *.proto)
+IFS=$'\n' read -d '' -r -a all_service_protos_without_service_in_name < <(
+  git grep -l 'import .*"google/api/annotations.proto";' -- '*.proto' | grep -vE '_service\.proto$'
+)
+
+[[ "${#all_service_protos_without_service_in_name[@]}" == 0 ]] || {
+  echo "Found service proto files that do not end with _service.proto"
+  echo "Files were: "
+  printf "  %s\n" ${all_service_protos_without_service_in_name[@]}
+  exit 1
+} >&2

tools/detect-large-files.sh

@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+# Finds large files that have been checked in to Git.
+set -euo pipefail
+
+GIT_REPO_TOP="$(git rev-parse --show-toplevel)"
+
+allowlist_path=""
+if [[ $# -eq 1 ]]
+then
+  allowlist_path="$1"
+fi
+
+allowed_files=""
+if [[ -n "$allowlist_path" ]]
+then
+  LC_ALL=C LC_COLLATE=C sort --check "${allowlist_path}"
+  allowed_files=$(egrep -v '^\s*(#.*)$' "${allowlist_path}" | xargs git -C "$GIT_REPO_TOP" ls-files --)
+fi
+
+large_files=$(git ls-tree --full-tree -l -r HEAD "$GIT_REPO_TOP" | awk '$4 > 50*1024 {print$5}')
+
+
+IFS=$'\n' read -d '' -r -a non_allowed_files < <(
+  {
+    echo "${large_files}"
+    echo "${allowed_files}"
+    echo "${allowed_files}"
+  } | sort | uniq -u
+) || true
+
+
+[[ "${#non_allowed_files[@]}" == 0 ]] || {
+  echo "Found large files in the working tree. Please remove them!"
+  echo "If you must add them, you need to explicitly add them to the allow list file ${allowlist_path:-and provide it as an argument to this script.}"
+  echo "Files were: "
+  printf '  %s\n' "${non_allowed_files[@]}"
+  exit 1
+} >&2