Update common github actions (#2875)

Signed-off-by: Matt Moore (via Sockpuppet) <mattmoor@vmware.com>
knative · Oct 1, 2020 · ab5a284 · ab5a284
1 parent 812c402
commit ab5a284
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 30 deletions.
diff --git a/.github/workflows/knative-style.yaml b/.github/workflows/knative-style.yaml
@@ -26,9 +26,20 @@ jobs:
   autoformat:
     name: Auto-format and Check
     runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false # Keep running if one leg fails.
+      matrix:
+        tool:
+          - goimports
+          - gofmt
+
+        include:
+        - tool: gofmt
+          options: -s
+        - tool: goimports
+          importpath: golang.org/x/tools/cmd/goimports
 
     steps:
-
       - name: Set up Go 1.15.x
         uses: actions/setup-go@v2
         with:
@@ -39,41 +50,30 @@ jobs:
         uses: actions/checkout@v2
 
       - name: Install Dependencies
+        if: ${{ matrix.importpath != '' }}
         run: |
           cd $(mktemp -d)
-          GO111MODULE=on go get golang.org/x/tools/cmd/goimports
+          GO111MODULE=on go get ${{ matrix.importpath }}
 
-      # Run this last because it alters the workspace.
-      # TODO: add prettier step
-      - name: Go Imports
-        shell: bash
-        run: |
-          goimports -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
-          echo "::set-env name=FMT_TOOL::goimports"
-      - name: Verify goimport
+      - name: ${{ matrix.tool }} ${{ matrix.options }}
         shell: bash
         run: |
-          if [[ $(git diff-index --name-only HEAD --) ]]; then
-              echo "Found diffs in:"
-              git diff-index --name-only HEAD --
-              echo "${{ github.repository }} is out of style. Please run $FMT_TOOL."
-              exit 1
-          fi
-          echo "${{ github.repository }} is formatted correctly."
+          ${{ matrix.tool }} ${{ matrix.options }} -w $(find . -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
 
-      - name: Go Format
+      - name: Verify ${{ matrix.tool }}
         shell: bash
         run: |
-          gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
-          echo "::set-env name=FMT_TOOL::gofmt"
-      - name: Verify gofmt
-        shell: bash
-        # TODO(mattmoor): combine with above via anchors when actions support it.
-        run: |
+          # From: https://backreference.org/2009/12/23/how-to-match-newlines-in-sed/
+          # This is to leverage this workaround:
+          # https://github.com/actions/toolkit/issues/193#issuecomment-605394935
+          function urlencode() {
+            sed ':begin;$!N;s/\n/%0A/;tbegin'
+          }
           if [[ $(git diff-index --name-only HEAD --) ]]; then
-              echo "Found diffs in:"
-              git diff-index --name-only HEAD --
-              echo "${{ github.repository }} is out of style. Please run $FMT_TOOL."
+              for x in $(git diff-index --name-only HEAD --); do
+                echo "::error file=$x::Please run ${{ matrix.tool }} ${{ matrix.options }}.%0A$(git diff $x | urlencode)"
+              done
+              echo "${{ github.repository }} is out of style. Please run ${{ matrix.tool }} ${{ matrix.options }}."
               exit 1
           fi
           echo "${{ github.repository }} is formatted correctly."
@@ -120,7 +120,6 @@ jobs:
         uses: golangci/golangci-lint-action@v2
         with:
           version: v1.30
-          only-new-issues: true # for initial defensiveness
 
       - name: misspell
         shell: bash
@@ -197,6 +196,40 @@ jobs:
 
           echo '::endgroup::'
 
+      - name: Redundant Format
+        shell: bash
+        if: ${{ always() }}
+        env:
+          REVIEWDOG_GITHUB_API_TOKEN: ${{ github.token }}
+        run: |
+          set -e
+          cd "${GITHUB_WORKSPACE}" || exit 1
+
+          echo '::group:: Flagging possibly unnecessary format calls for trailing argument with reviewdog 🐶 ...'
+          # Don't fail because of misspell
+          set +o pipefail
+          # For all look for formatting calls and extract the ones that
+          # have only a single formatting argument and that argument is in the last position.
+          # But exclude the `fmt.Errorf` calls since there's no `fmt.Error` and `errors.New` is not
+          # as versatile.
+          # Also ignore `%T` — to print the type and `%q` to quote the final string.
+          fn=$(find . -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
+          grep -nE '[^.]+\.(Fatalf|Errorf|Warnf|Infof|Debugf|Logf|Sprintf|Fprintf|Printf)[^\%]+%[^Tqx]",[^,]+' $fn | grep -v "fmt.Errorf" |
+            while read -r ent ; do
+              file=$(echo $ent | cut -d':' -f 1);
+              line=$(echo $ent | cut -d':' -f 2);
+              ch=$(echo $ent | cut -d':' -f3-);
+              err=$(echo $ch | sed -E 's/([^.]+\.)(Fatal|Error|Warn|Info|Debug|Log|Sprint|Print|Fprint)f([^\%]+)(%[^wq]",)([^,]+)/\1\2\3",\5/');
+              echo "$file:$line: Please consider avoiding tail format like this:%0A$err"
+          done |
+          reviewdog -efm="%f:%l: %m" \
+                -name="Redundant Format" \
+                -reporter="github-pr-check" \
+                -filter-mode="added" \
+                -fail-on-error="true" \
+                -level="error"
+          echo '::endgroup::'
+
       # This is mostly copied from https://github.com/get-woke/woke-action-reviewdog/blob/main/entrypoint.sh
       # since their action is not yet released under a stable version.
       - name: Language

diff --git a/.github/workflows/knative-verify.yaml b/.github/workflows/knative-verify.yaml
@@ -63,12 +63,23 @@ jobs:
     - name: Verify
       shell: bash
       run: |
+        # From: https://backreference.org/2009/12/23/how-to-match-newlines-in-sed/
+        # This is to leverage this workaround:
+        # https://github.com/actions/toolkit/issues/193#issuecomment-605394935
+        function urlencode() {
+          sed ':begin;$!N;s/\n/%0A/;tbegin'
+        }
+
         pushd ./src/knative.dev/${{ github.event.repository.name }}
         if [[ -z "$(git status --porcelain)" ]]; then
             echo "${{ github.repository }} up to date."
         else
-            repoDiff=$(git diff-index --name-only HEAD --)
-            echo "Found diffs in: $repoDiff"
+            # Print it both ways because sometimes we haven't modified the file (e.g. zz_generated),
+            # and sometimes we have (e.g. configmap checksum).
+            echo "Found diffs in: $(git diff-index --name-only HEAD --)"
+            for x in $(git diff-index --name-only HEAD --); do
+                echo "::error file=$x::Please run ./hack/update-codegen.sh.%0A$(git diff $x | urlencode)"
+            done
             echo "${{ github.repository }} is out of date. Please run hack/update-codegen.sh"
             exit 1
         fi