Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shfmt がチェック漏れしている #62

Open
KEINOS opened this issue Jun 13, 2021 · 1 comment
Open

shfmt がチェック漏れしている #62

KEINOS opened this issue Jun 13, 2021 · 1 comment
Labels
bug Something isn't working

Comments

@KEINOS
Copy link
Member

KEINOS commented Jun 13, 2021

TL; DR (今北産業)

  1. 6d0cf1e 現在の ./.github/run-lint.shshfmt の lint チェック時に検知漏れがある。
  2. どうも shfmt のバージョンにも関係していそう
    • shfmt v3.3.0 -> 検知しない
    • shfmt v3.2.1-0 -> 検知しない
    • shfmt v3.1.1-0 -> 検知する
  3. shfmt のバージョンによって .editorconfig が反映されないケースがあるっぽい

TS; DR

以下の 40 行目のインデントにスペースが 1 つ余計に入っています。

runShellSpec() {
printf "%s" '- ShellSpec '
result=$(shellspec 2>&1) || {
printf >&2 ": NG\n%s" "$result"
return $FAILURE
}
echo "$(echo "$result" | head -n 2 | tail -n 1)" 'OK'
}

しかし、テスト結果はパスしています。

===============================================================================
 Requirement Check for Linting and Static Analysis
===============================================================================
- ShellCheck version: 0.7.0
- shfmt version: v3.3.0
-------------------------------------------------------------------------------
 Running linters
-------------------------------------------------------------------------------
- Shellformat ... OK
- ShellCheck ... OK

これは少し古いローカルの shfmt v3.2.1-0 でも同じ現象でした。

shfmt -i=4 ./.github/run-tests.shインデントをオプションで指定すると検知することから、.editorconfig が反映されないパターンがあるようです。

$ shfmt --version
3.2.1-0
$ shfmt -d ./.github/run-test.sh
$ echo $?
0

$ shfmt -i=4 -d ./.github/run-test.sh
--- ./.github/run-test.sh.orig
+++ ./.github/run-test.sh
@@ -37,7 +37,7 @@
         return $FAILURE
     }
 
-     echo "$(echo "$result" | head -n 2 | tail -n 1)" 'OK'
+    echo "$(echo "$result" | head -n 2 | tail -n 1)" 'OK'
 }
 
 # -----------------------------------------------------------------------------

問題は、さらに古い shfmt のバージョンだと検知することです。shellspec の Alpine Docker イメージshfmt を入れると shfmt v3.1.1-0 が入るのですが、その場合は検知します。

#14 0.278 ===============================================================================                                                                                                                                         
#14 0.278  Requirement Check for Linting and Static Analysis
#14 0.278 ===============================================================================
#14 0.283 - ShellCheck version: 0.7.1
#14 0.289 - shfmt version: 3.1.1-0
#14 0.290 -------------------------------------------------------------------------------
#14 0.290  Running linters
#14 0.290 -------------------------------------------------------------------------------
#14 0.290 - Shellformat ... --- ./.github/run-test.sh.orig
#14 0.305 +++ ./.github/run-test.sh
#14 0.305 @@ -1,71 +1,71 @@
#14 0.305  #!/bin/sh
#14 0.305  # =============================================================================
#14 0.305  #  ShellSpec による動的/単体テストの実行スクリプト
#14 0.305  # =============================================================================
#14 0.305  
...(略)...
#14 0.306  # -----------------------------------------------------------------------------
#14 0.306  #  Functions
#14 0.306  # -----------------------------------------------------------------------------
#14 0.306  
#14 0.306  runShellSpec() {
#14 0.307      printf "%s" '- ShellSpec '
#14 0.307  
#14 0.307      result=$(shellspec 2>&1) || {
#14 0.307          printf >&2 ": NG\n%s" "$result"
#14 0.307  
#14 0.307          return $FAILURE
#14 0.307      }
#14 0.307  
#14 0.307 -     echo "$(echo "$result" | head -n 2 | tail -n 1)" 'OK'
#14 0.307 +    echo  "$(echo "$result" | head -n 2 | tail -n 1)" 'OK'
#14 0.307  }
...(略)...

ただ、このバージョン(shfmt v3.1.1-0)は shellspec のテスト構文を正しく解釈できないため、とんでもない数のエラーを吐き出します。そのため、shfmt v3.2 以降での対策が必要です。

  • 再現用 Dockerfile
FROM shellspec/shellspec:latest AS testbuild

# Install miminum requirements for QiiCipher
RUN apk add --no-cache \
    openssl \
    openssh \
    ca-certificates && update-ca-certificates

# Install requirements for testing
RUN apk add --no-cache \
    git \
    shellcheck \
    shfmt

# Copy the hole repo
COPY . /app
WORKDIR /app

# Run tests
RUN \
    /app/.github/run-lint.sh \
    && /app/.github/run-test.sh
@KEINOS KEINOS added the bug Something isn't working label Jun 14, 2021
@KEINOS
Copy link
Member Author

KEINOS commented Jul 12, 2021

どうも、shfmt 本家の issue #393 やソースを見ると、shfmt の呼び出し時にオプションがある場合は .editorconfig は無視されるっぽい。

以下で -d オプションを付けてるのが原因っぽい(未検証)

# 拡張子 .sh ありのスクリプトの shfmt
runShfmtForShExt() {
find . -name '*.sh' -type f -print0 | xargs -0 shfmt -d
}
# 拡張子 .sh なしのスクリプトの shfmt
runShfmtForNoExt() {
result=$SUCCESS
# shellcheck disable=SC2086
set -- $LIST_SCRIPT_NO_EXT
while [ "${1:+none}" ]; do
path_file_target="${PATH_DIR_BIN}/${1}"
shfmt -d "$path_file_target" || {
echo >&2 "shfmt fail: ${path_file_target}"
result=$FAILURE
}
shift
done
return $result
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant