Instagram / Instagram/Fixit

`fixit fix` exits with `0` even if there were errors

Open
#409 2 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Python
Stars
714
Forks
72
PR merge metrics
No merged PRs in 30d

Description

`fixit fix` will exit with an exit code of `0` if it finds errors but does not apply fixes. This makes using it in CI (or pre-commit hooks) awkward because it hides errors that can't be auto-fixed. The solution is to run the tool *twice: once to fix fixable errors, then once again to re-report them in a way that can cause a Ci job to fail. This is a frustrating duplication of work.

Given the outcome of #258, I think this is a bug rather than intentional design, but I'm happy to be corrected if there's another change I've missed 🙂

There are two cases where I would expect a non-zero exit code from `fixit fix`.

## failure to run

When it fails to evaluate the rules at all because of an error:

```console
$ cat t.py
this is not valid python

$ fixit fix -a t.py
t.py: EXCEPTION: Syntax Error @ 1:1.
parser error: error at 1:24: expected one of !=, %, &, (, *, **, +, ,, -, ., /, //, ;, <, <<, <=, ==, >, >=, >>, @, NEWLINE, [, ^, and, if, in, is, not, or, |

this is not valid python
^
Traceback (most recent call last):
File "/private/tmp/tmpvenv-c329d/venv/lib/python3.11/site-packages/fixit/api.py", line 100, in fixit_bytes
runner = LintRunner(path, content)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/private/tmp/tmpvenv-c329d/venv/lib/python3.11/site-packages/fixit/engine.py", line 55, in __init__
self.module: Module = parse_module(source)
^^^^^^^^^^^^^^^^^^^^
File "/private/tmp/tmpvenv-c329d/venv/lib/python3.11/site-packages/libcst/_parser/entrypoints.py", line 109, in parse_module
result = _parse(
^^^^^^^
File "/private/tmp/tmpvenv-c329d/venv/lib/python3.11/site-packages/libcst/_parser/entrypoints.py", line 55, in _parse
return parse(source_str)
^^^^^^^^^^^^^^^^^
libcst._exceptions.ParserSyntaxError: Syntax Error @ 1:1.
parser error: error at 1:24: expected one of !=, %, &, (, *, **, +, ,, -, ., /, //, ;, <, <<, <=, ==, >, >=, >>, @, NEWLINE, [, ^, and, if, in, is, not, or, |

this is not valid python
^
🛠️ 1 file checked, 1 file with errors 🛠️

$ echo $?
0
```

I would expect `fixit` to exit with a non-zero code in the above case.

## errors found but not fixed

`fixit fix` also exits `0` if errors were found but no fixes were applied:

```console
$ cat t.py
try:
...
except ValueError or KeyError:
...

$ fixit fix -a t.py
t.py@1:0 AvoidOrInExcept: Avoid using 'or' in an except block. For example:'except ValueError or TypeError' only catches 'ValueError'. Instead, use parentheses, 'except (ValueError, TypeError)'
🛠️ 1 file checked, 1 file with errors 🛠️

$ echo $?
0
```

I would also expect it to exit non-zero in this case, although the suggestion in #258 for that to be opt-in seems reasonable if backwards-compatibility is a concern.

---

Fixit version 2.1.0
Python 3.11.5 (main, Aug 24 2023, 15:09:45) [Clang 14.0.3 (clang-1403.0.22.14.1)]

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.