Open adrinjalali opened 2 months ago
Thanks for the detailed write up.
However, preview mode enables many other rules which don't work as intended on our repo, give false positives, ...
This is somewhat intentional because we want to encourage users to give feedback on preview rules. Whether we promote a rule to stable depends on the feedback we receive from users. Without feedback, we'll end up promoting a rule that then creates a lot of churn downstream.
This is related to https://github.com/astral-sh/ruff/issues/7434, and I think explicit-preview-rules introduced in https://github.com/astral-sh/ruff/pull/7390 somewhat intends to fix the issue, but in this case it doesn't.
Could you explain which part isn't working for you?
I tried to reproduce your setup by enabling the same rules as in your example and using explcit-preview-rules
(playground). To me it is working as expected because Ruff doesn't report an error for the preview rule E201
that flags the whitespace after [
.
I can set explicit-preview-rules
to false
and the E201
violation then gets reported (playground)
So on scikit-learn, adding
preview = true
# This enables us to use the explicit preview rules that we want only
explicit-preview-rules = true
results in:
Found 364 errors.
No fixes available (308 hidden fixes can be enabled with the `--unsafe-fixes` option).
while on main
, we get:
Found 14 errors.
$ ruff --version
ruff 0.5.1
Here are the outputs of ruff check .
on the repo with and without the preview config added:
Thanks, I now see what's happening. What you're experience isn't that it enables additional preview-only rules, but that you see new violations because of some preview-only changes to stable rules.
We don't support enabling preview functionality for specific rules only. Enabling preview mode enables the preview functionality for all rules (if they have any). explicit-preview-rules
only controls if preview-only rules should be enabled when using a selector like E
.
Oh now I understand. However, that puts us in a weird position, because:
CPY001
I feel like there should be a middle ground there? I hope this makes sense.
If those rule changes are giving you false positives in preview mode, I'd highly recommend opening issues because the behavior is likely to become stable soon.
Unfortunately we don't have enough bandwidth to manage different preview modes, that's why we have a single global flag. explicit-preview-rules
is already the compromise.
An option here is to allow explicit selection of preview rules without enabling preview mode. We'd probably need to display a warning that the rule is in preview, though.
Not sure if this is the same issue when dealing with formatting.
Related to this issue, the "single flag to rule them all" has caused confusion and consternation. My projects adopted the setting in order to make "quote-style=preserve" work for compatibility when migrating from black, but that caused "hugged parenthesis" to be adopted implicitly. Later, when quote-style=preserve graduated to stable, we tried to remove the setting but that would roll back other behaviors.
In my projects, I'm opting to have "preview=true" by default (and the uncertainty that will bring), but at least that will allow the projects to continue to shift left (work at head).
I realize it's difficult to manage feature-specific flags, but just be aware that the user experience is muddled by the shifting meaning of this flag over time and releases.
Our usecase https://github.com/scikit-learn/scikit-learn/pull/29477 is that we'd like to enable
CPY001
. Since it's apreview
rule, one needs to enable preview mode.However, preview mode enables many other rules which don't work as intended on our repo, give false positives, ...
That's why I looked into enabling preview rules only explicitly. However, when one does something like
select = ["E", "F", "W", "I", "CPY001"]
in the configuration, everything under those categories are enabled, regardless of them being in preview mode or not.I'd argue that the above
select
doesn't really explicitly select those preview rules underE
,F
, ..., however, they're selected.This is related to https://github.com/astral-sh/ruff/issues/7434, and I think
explicit-preview-rules
introduced in https://github.com/astral-sh/ruff/pull/7390 somewhat intends to fix the issue, but in this case it doesn't.What should one do in this case?
In effect, I think I'd expect this config:
to only include stable rules under those categories, and enable
CPY001
on top of them, and no other preview rule should be enabled.