Skip to content
This repository has been archived by the owner on Nov 21, 2022. It is now read-only.

Commit

Permalink
Fix inplace option being always overwritten
Browse files Browse the repository at this point in the history
Observation
-----------

I observed that the following action **always** passes - even after I
added bad indentation on purpose.

```
name: lint
on: push
jobs:
  clang-format:
    runs-on: ubuntu-latest
    steps:
    - name: Checkout
      uses: actions/checkout@v2

    - name: clang format
      uses: DoozyX/[email protected]
      with:
        extensions: 'h,cpp'
        clangFormatVersion: 10
```

For example, the following call always returns an exit code of 0:

```
docker run -it --rm --workdir /src -v $(pwd):/src clang-format-lint \
  "--clang-format-executable" "/clang-format/clang-format10" "-r" \
  "--inplace" "false" "--extensions" "h,cpp" "--exclude" "none" "."
```

The bug
-------

Turns out that `--inplace` was parsed wrong and as soon as `--inplace`
is passed (and it is always passed) it essentially actually meant
`--inplace True`. And when `--inplace True` is passed, then the exit
code will be 0 and thus the action always is successful.

See
https://stackoverflow.com/questions/15008758/parsing-boolean-values-with-argparse
for more information on parsing booleans.

Running the above Docker command with my fix actually returns an exit
code of 1 and thus the actions also fails as expected.
  • Loading branch information
schra committed Oct 17, 2020
1 parent 2a69814 commit 86f2d10
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion run-clang-format.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import subprocess
import sys
import traceback
from distutils.util import strtobool

from functools import partial

Expand Down Expand Up @@ -300,7 +301,7 @@ def main():
parser.add_argument(
'-i',
'--inplace',
type=bool,
type=lambda x: bool(strtobool(x)),
default=False,
help='Just fix files (`clang-format -i`) instead of returning a diff')

Expand Down

0 comments on commit 86f2d10

Please sign in to comment.