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

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

Open
pnovotnak opened this issue Nov 14, 2023 · 2 comments
Labels
cli Related to the command-line interface

Comments

@pnovotnak
Copy link

pnovotnak commented Nov 14, 2023

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?

@pnovotnak pnovotnak changed the title Possible bug: Diff flag suppresses errors, with no clear way to enable errors Possible bug: Diff flag suppresses non-fixable errors, with no clear way to enable Nov 14, 2023
@zanieb
Copy link
Member

zanieb commented Nov 14, 2023

Hi! Thanks for the well written issue.

This is a duplicate of #8584

Similarly, it may be best resolved by #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

@charliermarsh charliermarsh added the cli Related to the command-line interface label Nov 15, 2023
@pjonsson
Copy link
Contributor

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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command-line interface
Projects
None yet
Development

No branches or pull requests

4 participants