Support sqlfluff fix (#9)

* Support `sqlfluff fix` * Support version 0.9.4 and 0.10.1 * Update * Not return exit code of `sqlfluff fix` * exit 0 when fix
yu-iskw · Mar 18, 2022 · 9018bbf · 9018bbf
1 parent 362d575
commit 9018bbf
Show file tree

Hide file tree

Showing 8 changed files with 149 additions and 99 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -3,12 +3,13 @@ on:
   pull_request:
 
 jobs:
-  test-check:
+  test-lint:
     name: runner / sqlfluff (github-check)
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
-        sqlfluff: ["0.8.2", "0.9.0", "0.9.1"]
+        sqlfluff: ["0.9.4", "0.10.1"]
     steps:
       - uses: actions/checkout@v2
       - uses: ./
@@ -17,9 +18,13 @@ jobs:
           github_token: ${{ secrets.github_token }}
           reporter: github-check
           sqlfluff_version: ${{ matrix.sqlfluff }}
+          sqlfluff_command: lint
           templater: jinja
           config: "${{ github.workspace }}/testdata/test_failed_dbt/.sqlfluff"
           paths: '${{ github.workspace }}/testdata/test_failed_dbt/models'
+      - name: check the exit code
+        if: ${{ !success() }}
+        run: echo 'The previous step should fail' && exit 1
       - name: "Test outputs"
         if: always()
         shell: bash
@@ -29,59 +34,23 @@ jobs:
           echo '${{ steps.lint-sql.outputs.sqlfluff-exit-code }}'
           echo '${{ steps.lint-sql.outputs.reviewdog-exit-code }}'
 
-  test-pr-check:
-    if: github.event_name == 'pull_request'
-    name: runner / sqlfluff (github-pr-check)
-    runs-on: ubuntu-latest
-    strategy:
-      matrix:
-        sqlfluff: ["0.8.2", "0.9.0", "0.9.1"]
-    steps:
-      - uses: actions/checkout@v2
-      - uses: ./
-        id: lint-sql
-        with:
-          github_token: ${{ secrets.github_token }}
-          reporter: github-pr-check
-          sqlfluff_version: ${{ matrix.sqlfluff }}
-          templater: jinja
-          config: "${{ github.workspace }}/testdata/test_failed_dbt/.sqlfluff"
-          paths: '${{ github.workspace }}/testdata/test_failed_dbt/models'
-      - name: "Test outputs"
-        if: always()
-        shell: bash
-        run: |
-          echo '${{ steps.lint-sql.outputs.sqlfluff-results }}' | jq -r '.'
-          echo '${{ steps.lint-sql.outputs.sqlfluff-results-rdjson }}' | jq -r '.'
-          echo '${{ steps.lint-sql.outputs.sqlfluff-exit-code }}'
-          echo '${{ steps.lint-sql.outputs.reviewdog-exit-code }}'
-
-  test-pr-review:
-    if: github.event_name == 'pull_request'
-    name: runner / sqlfluff (github-pr-review)
+  test-fix:
+    needs: ["test-lint"]
+    name: runner / sqlfluff (test-fix)
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
-        sqlfluff: ["0.8.2", "0.9.0", "0.9.1"]
+        sqlfluff: ["0.9.4", "0.10.1"]
     steps:
       - uses: actions/checkout@v2
       - uses: ./
         id: lint-sql
         with:
           github_token: ${{ secrets.github_token }}
-          reporter: github-pr-review
+          reporter: github-check
           sqlfluff_version: ${{ matrix.sqlfluff }}
+          sqlfluff_command: fix
           templater: jinja
           config: "${{ github.workspace }}/testdata/test_failed_dbt/.sqlfluff"
           paths: '${{ github.workspace }}/testdata/test_failed_dbt/models'
-      - name: "Test outputs"
-        if: always()
-        shell: bash
-        run: |
-          echo '${{ steps.lint-sql.outputs.sqlfluff-results }}' | jq -r '.'
-          echo '${{ steps.lint-sql.outputs.sqlfluff-results-rdjson }}' | jq -r '.'
-          echo '${{ steps.lint-sql.outputs.sqlfluff-exit-code }}'
-          echo '${{ steps.lint-sql.outputs.reviewdog-exit-code }}'
-      - name: check the exit code
-        if: ${{ !success() }}
-        run: echo 'The previous step should fail' && exit 1
diff --git a/Makefile b/Makefile
@@ -0,0 +1,2 @@
+setup-dev:
+	SQLFLUFF_VERSION=v0.10.1 pip install -r requirements/requirements.txt
diff --git a/README.md b/README.md
@@ -8,14 +8,27 @@
 [![GitHub release (latest SemVer)](https://img.shields.io/github/v/release/yu-iskw/action-sqlfluff?logo=github&sort=semver)](https://github.com/yu-iskw/action-sqlfluff/releases)
 [![action-bumpr supported](https://img.shields.io/badge/bumpr-supported-ff69b4?logo=github&link=https://github.com/haya14busa/action-bumpr)](https://github.com/haya14busa/action-bumpr)
 
-![github-pr-review demo](./docs/images/github-pr-review-demo.png)
 
-This is a github action to lint SQL with [sqlfluff](https://github.com/sqlfluff/sqlfluff).
-The action automatically leaves comments about SQL violation using [reviewdog](https://github.com/reviewdog/reviewdog)
+This is a github action to lint and fix SQL with [sqlfluff](https://github.com/sqlfluff/sqlfluff).
+The action has the two modes corresponding to `sqlfluff lint` and `sqlfluff fix`.
+One is to automatically leaves comments about SQL violation using [reviewdog](https://github.com/reviewdog/reviewdog)
+The other is to automatically suggests code formatting on github pull request with reviewdog too.
+
+## Lint mode
+The lint mode leaves comments on github pull requests.
+Comments are pointed out by sqlfluff.
+![github-pr-review demo (lint)](./docs/images/github-pr-review-demo-lint.png)
+
+## Fix mode
+The fix mode suggests code formatting based on `sqlfluff fix`.
+![github-pr-review demo (fix)](./docs/images/github-pr-review-demo-fix.png)
 
 ## NOTE
-If you use dbt v0, `sqlfluff_version` has to be `0.8.2` because `sqlfluff==0.8.2` doesn't support dbt v1.
-If you use dbt v1, `sqlfluff_version` can be `0.9.0` or later.
+The `sqlfluff_version` input must be `0.9.0` or later, because `sqlfluff fix` at `0.8.2` or earlier doesn't support the `--config` option.
+
+The tested sqlfluff versions in the repositories are:
+- 0.9.4
+- 0.10.1
 
 ## Input
 
@@ -58,9 +71,13 @@ inputs:
   sqlfluff_version:
     description: |
       sqlfluff version. Use the latest version if not set.
-      It must be 0.8.2 or later.
+      It must be 0.9.0 or later, because `sqlfluff<=0.8.2` doesn't support the `--config` option in the `fix` sub command.
+    required: false
+    default: '0.9.4'
+  sqlfluff_command:
+    description: 'The sub command of sqlfluff. One of lint and fix'
     required: false
-    default: '0.9.1'
+    default: 'lint'
   paths:
     description: |
       PATH is the path to a sql file or directory to lint.
@@ -129,6 +146,7 @@ inputs:
 ```
 
 ## Outputs
+The outputs are available only when the `sqlfluff_command` input is `lint`.
 ```yaml
 outputs:
   sqlfluff-results:
@@ -162,7 +180,8 @@ jobs:
         with:
           github_token: ${{ secrets.github_token }}
           reporter: github-pr-review
-          sqlfluff_version: "0.9.1"
+          sqlfluff_version: "0.10.1"
+          sqlfluff_command: "lint" # Or fix
           config: "${{ github.workspace }}/.sqlfluff"
           paths: '${{ github.workspace }}/models'
       - name: 'Show outputs (Optional)'

diff --git a/action.yml b/action.yml
@@ -39,9 +39,13 @@ inputs:
   sqlfluff_version:
     description: |
       sqlfluff version. Use the latest version if not set.
-      It must be 0.8.2 or later.
+      It must be 0.9.0 or later, because `sqlfluff<=0.8.2` doesn't support the `--config` option in the `fix` sub command.
     required: false
-    default: '0.9.1'
+    default: '0.10.1'
+  sqlfluff_command:
+    description: 'The sub command of sqlfluff. One of lint and fix'
+    required: false
+    default: 'lint'
   paths:
     description: |
       PATH is the path to a sql file or directory to lint.
@@ -138,6 +142,7 @@ runs:
     REVIEWDOG_ESLINT_FLAGS: ${{ inputs.eslint_flags }}
     REVIEWDOG_TOOL_NAME: ${{ inputs.tool_name }}
     SQLFLUFF_VERSION: ${{ inputs.sqlfluff_version }}
+    SQLFLUFF_COMMAND: ${{ inputs.sqlfluff_command }}
     SQLFLUFF_CONFIG: ${{ inputs.config }}
     SQLFLUFF_PATHS: ${{ inputs.paths }}
     SQLFLUFF_PROCESSES: ${{ inputs.processes }}

diff --git a/docs/images/github-pr-review-demo-fix.png b/docs/images/github-pr-review-demo-fix.png
diff --git a/docs/images/github-pr-review-demo.png → docs/images/github-pr-review-demo-lint.png b/docs/images/github-pr-review-demo.png → docs/images/github-pr-review-demo-lint.png
diff --git a/entrypoint.sh b/entrypoint.sh
@@ -11,60 +11,109 @@ if [[ "${SQLFLUFF_VERSION:?}" =~ 0\.8.* ]]; then
 else
   pip install --no-cache-dir -r "${SCRIPT_DIR}/requirements/requirements.txt"
 fi
+# Make sure the version of sqlfluff
 sqlfluff --version
 echo '::endgroup::'
 
-echo '::group:: Running sqlfluff 🐶 ...'
-# Allow failures now, as reviewdog handles them
-set +Eeuo pipefail
+if [[ "${SQLFLUFF_COMMAND:?}" == "lint" ]]; then
+  echo '::group:: Running sqlfluff 🐶 ...'
+  # Allow failures now, as reviewdog handles them
+  set +Eeuo pipefail
+  lint_results="sqlfluff-lint.json"
+  # shellcheck disable=SC2086,SC2046
+  sqlfluff lint \
+    --format json \
+    $(if [[ "x${SQLFLUFF_CONFIG}" != "x" ]]; then echo "--config ${SQLFLUFF_CONFIG}"; fi) \
+    $(if [[ "x${SQLFLUFF_DIALECT}" != "x" ]]; then echo "--dialect ${SQLFLUFF_DIALECT}"; fi) \
+    $(if [[ "x${SQLFLUFF_PROCESSES}" != "x" ]]; then echo "--processes ${SQLFLUFF_PROCESSES}"; fi) \
+    $(if [[ "x${SQLFLUFF_RULES}" != "x" ]]; then echo "--rules ${SQLFLUFF_RULES}"; fi) \
+    $(if [[ "x${SQLFLUFF_EXCLUDE_RULES}" != "x" ]]; then echo "--exclude-rules ${SQLFLUFF_EXCLUDE_RULES}"; fi) \
+    $(if [[ "x${SQLFLUFF_TEMPLATER}" != "x" ]]; then echo "--templater ${SQLFLUFF_TEMPLATER}"; fi) \
+    $(if [[ "x${SQLFLUFF_DISABLE_NOQA}" != "x" ]]; then echo "--disable-noqa ${SQLFLUFF_DISABLE_NOQA}"; fi) \
+    $(if [[ "x${SQLFLUFF_DIALECT}" != "x" ]]; then echo "--dialect ${SQLFLUFF_DIALECT}"; fi) \
+    "${SQLFLUFF_PATHS:?}" |
+    tee "$lint_results"
+  sqlfluff_exit_code=$?
 
-# Make sure the version of sqlfluff
-sqlfluff --version
+  echo "::set-output name=sqlfluff-results::$(cat <"$lint_results" | jq -r -c '.')" # Convert to a single line
+  echo "::set-output name=sqlfluff-exit-code::${sqlfluff_exit_code}"
 
-lint_results="sqlfluff-lint.json"
-# shellcheck disable=SC2086,SC2046
-sqlfluff lint \
-  --format json \
-  $(if [[ "x${SQLFLUFF_CONFIG}" != "x" ]]; then echo "--config ${SQLFLUFF_CONFIG}"; fi) \
-  $(if [[ "x${SQLFLUFF_DIALECT}" != "x" ]]; then echo "--dialect ${SQLFLUFF_DIALECT}"; fi) \
-  $(if [[ "x${SQLFLUFF_PROCESSES}" != "x" ]]; then echo "--processes ${SQLFLUFF_PROCESSES}"; fi) \
-  $(if [[ "x${SQLFLUFF_RULES}" != "x" ]]; then echo "--rules ${SQLFLUFF_RULES}"; fi) \
-  $(if [[ "x${SQLFLUFF_EXCLUDE_RULES}" != "x" ]]; then echo "--exclude-rules ${SQLFLUFF_EXCLUDE_RULES}"; fi) \
-  $(if [[ "x${SQLFLUFF_TEMPLATER}" != "x" ]]; then echo "--templater ${SQLFLUFF_TEMPLATER}"; fi) \
-  $(if [[ "x${SQLFLUFF_DISABLE_NOQA}" != "x" ]]; then echo "--disable-noqa ${SQLFLUFF_DISABLE_NOQA}"; fi) \
-  $(if [[ "x${SQLFLUFF_DIALECT}" != "x" ]]; then echo "--dialect ${SQLFLUFF_DIALECT}"; fi) \
-  "${SQLFLUFF_PATHS:?}" |
-  tee "$lint_results"
-sqlfluff_exit_code=$?
-
-echo "::set-output name=sqlfluff-results::$(cat <"$lint_results" | jq -r -c '.')" # Convert to a single line
-echo "::set-output name=sqlfluff-exit-code::${sqlfluff_exit_code}"
+  set -Eeuo pipefail
+  echo '::endgroup::'
 
-set -Eeuo pipefail
-echo '::endgroup::'
+  echo '::group:: Running reviewdog 🐶 ...'
+  # Allow failures now, as reviewdog handles them
+  set +Eeuo pipefail
+
+  lint_results_rdjson="sqlfluff-lint.rdjson"
+  cat <"$lint_results" |
+    jq -r -f "${SCRIPT_DIR}/to-rdjson.jq" |
+    tee >"$lint_results_rdjson"
+
+  cat <"$lint_results_rdjson" |
+    reviewdog -f=rdjson \
+      -name="sqlfluff-lint" \
+      -reporter="${REVIEWDOG_REPORTER}" \
+      -filter-mode="${REVIEWDOG_FILTER_MODE}" \
+      -fail-on-error="${REVIEWDOG_FAIL_ON_ERROR}" \
+      -level="${REVIEWDOG_LEVEL}"
+  reviewdog_return_code="${PIPESTATUS[1]}"
+
+  echo "::set-output name=sqlfluff-results-rdjson::$(cat <"$lint_results_rdjson" | jq -r -c '.')" # Convert to a single line
+  echo "::set-output name=reviewdog-return-code::${reviewdog_return_code}"
 
-echo '::group:: Running reviewdog 🐶 ...'
-# Allow failures now, as reviewdog handles them
-set +Eeuo pipefail
+  set -Eeuo pipefail
+  echo '::endgroup::'
 
-lint_results_rdjson="sqlfluff-lint.rdjson"
-cat <"$lint_results" |
-  jq -r -f "${SCRIPT_DIR}/to-rdjson.jq" |
-  tee >"$lint_results_rdjson"
+  exit $sqlfluff_exit_code
+# END OF lint
+elif [[ "${SQLFLUFF_COMMAND}" == "fix" ]]; then
+  echo '::group:: Running sqlfluff 🐶 ...'
+  # Allow failures now, as reviewdog handles them
+  set +Eeuo pipefail
+  # shellcheck disable=SC2086,SC2046
+  sqlfluff fix --force \
+    $(if [[ "x${SQLFLUFF_CONFIG}" != "x" ]]; then echo "--config ${SQLFLUFF_CONFIG}"; fi) \
+    $(if [[ "x${SQLFLUFF_DIALECT}" != "x" ]]; then echo "--dialect ${SQLFLUFF_DIALECT}"; fi) \
+    $(if [[ "x${SQLFLUFF_PROCESSES}" != "x" ]]; then echo "--processes ${SQLFLUFF_PROCESSES}"; fi) \
+    $(if [[ "x${SQLFLUFF_RULES}" != "x" ]]; then echo "--rules ${SQLFLUFF_RULES}"; fi) \
+    $(if [[ "x${SQLFLUFF_EXCLUDE_RULES}" != "x" ]]; then echo "--exclude-rules ${SQLFLUFF_EXCLUDE_RULES}"; fi) \
+    $(if [[ "x${SQLFLUFF_TEMPLATER}" != "x" ]]; then echo "--templater ${SQLFLUFF_TEMPLATER}"; fi) \
+    $(if [[ "x${SQLFLUFF_DISABLE_NOQA}" != "x" ]]; then echo "--disable-noqa ${SQLFLUFF_DISABLE_NOQA}"; fi) \
+    $(if [[ "x${SQLFLUFF_DIALECT}" != "x" ]]; then echo "--dialect ${SQLFLUFF_DIALECT}"; fi) \
+    "${SQLFLUFF_PATHS:?}"
+  set -Eeuo pipefail
+  echo '::endgroup::'
 
-cat <"$lint_results_rdjson" |
-  reviewdog -f=rdjson \
-    -name="sqlfluff" \
+  # SEE https://github.com/reviewdog/action-suggester/blob/master/script.sh
+  echo '::group:: Running reviewdog 🐶 ...'
+  # Allow failures now, as reviewdog handles them
+  set +Eeuo pipefail
+
+  # Suggest the differences
+  temp_file=$(mktemp)
+  git diff | tee "${temp_file}"
+  git stash -u
+
+  # shellcheck disable=SC2034
+  reviewdog \
+    -name="sqlfluff-fix" \
+    -f=diff \
+    -f.diff.strip=1 \
     -reporter="${REVIEWDOG_REPORTER}" \
-    -level="${REVIEWDOG_LEVEL}" \
+    -filter-mode="${REVIEWDOG_FILTER_MODE}" \
     -fail-on-error="${REVIEWDOG_FAIL_ON_ERROR}" \
-    -filter-mode="${REVIEWDOG_FILTER_MODE}"
-reviewdog_return_code="${PIPESTATUS[1]}"
+    -level="${REVIEWDOG_LEVEL}" <"${temp_file}" || exit_code=$?
 
-echo "::set-output name=sqlfluff-results-rdjson::$(cat <"$lint_results_rdjson" | jq -r -c '.')" # Convert to a single line
-echo "::set-output name=reviewdog-return-code::${reviewdog_return_code}"
+  # Clean up
+  git stash drop || true
+  set -Eeuo pipefail
+  echo '::endgroup::'
 
-set -Eeuo pipefail
-echo '::endgroup::'
-
-exit $sqlfluff_exit_code
+  exit 0
+  # exit $exit_code
+# END OF fix
+else
+  echo 'ERROR: SQLFLUFF_COMMAND must be one of lint and fix'
+  exit 1
+fi
diff --git a/testdata/test_failed_dbt/models/staging/staging_test02.sql b/testdata/test_failed_dbt/models/staging/staging_test02.sql
@@ -0,0 +1,6 @@
+{{ config(enabled=true) }}
+
+SELECT
+  1 AS x,
+    2 AS y,
+  3 AS z
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		setup-dev:
		SQLFLUFF_VERSION=v0.10.1 pip install -r requirements/requirements.txt