astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.4k stars 1.08k forks source link

Possible bug: Diff flag suppresses non-fixable errors, with no clear way to enable #8677

Open pnovotnak opened 11 months ago

pnovotnak commented 11 months ago

Hello! I discovered what I think is a possible bug with the tool that I wanted to bring to your attention.

I'm using this config:

[tool.ruff]
extend-select = ["I", "F"]
target-version = 'py310'

My test case is this file (call it x.py):


def f(x: "NotImported") -> None:
    pass

In CI, it's useful to have the --diff flag enabled. However, with the following script:

#!/bin/bash
set -ex

ruff check --diff .
ruff format --check --diff .

No errors are reported when fixes are not available. I think that makes sense, but it's still not reporting errors when --no-fix-only is passed.

$ ruff check --no-fix-only --diff x.py
$ echo $?
0
$ ruff check x.py
x.py:3:11: F821 Undefined name `NotImported`
Found 1 error.

I'm not sure exactly what output should be in the --no-fix-only --diff case, but a non-zero exit would be nice, perhaps errors can be printed to stderr?

zanieb commented 11 months ago

Hi! Thanks for the well written issue.

This is a duplicate of https://github.com/astral-sh/ruff/issues/8584

Similarly, it may be best resolved by https://github.com/astral-sh/ruff/issues/7352

Perhaps it's possible for us to allow opt-in to this behavior with --no-fix-only but I don't think that's a very clear user experience and may prefer #7352

pjonsson commented 7 months ago

I ran into exactly the same problem, and just like the reporter, I tried adding --diff --exit-non-zero-on-fix --no-fix-only to get my exit error code to be non-zero.

@zanieb Isn't it fairly standard behavior that options to the right have precedence over options to the left on the command line, so one can append more arguments ($cmd --no-x --x --no-x --x --no-x) to get the desired ($cmd --no-x) behavior?

If anyone wants more context, here is an excerpt from the Makefile I'm trying to incorporate Ruff into:

Q ?= @

lint.black:
    $(Q)poetry run black --check --diff $(PYTHON_SRC_DIRS)

lint.isort:
    $(Q)poetry run isort --check-only --filter-files --diff $(PYTHON_SRC_DIRS)

ifdef CI_BUILDS_DIR
lint.mypy: MYPY_CI_PARAMS ?= --junit-xml mypy-report.xml --junit-format=per_file
endif
lint.mypy:
    $(Q)poetry run mypy --warn-unused-ignores $(MYPY_CI_PARAMS) $(PYTHON_SRC_DIRS)

<more lint rules that execute various tools>