As we surface rule names more to users we want
them to be easier to type than PascalCase.
Prior art:
Pylint and ESLint also use kebab-case for their rule names.
Clippy uses snake_case but only for syntactical reasons
(so that the argument to e.g. #![allow(clippy::some_lint)]
can be parsed as a path[1]).
[1]: https://doc.rust-lang.org/reference/paths.html
The idea is nice and simple we replace:
fn placeholder() -> Self;
with
fn message_formats() -> &'static [&'static str];
So e.g. if a Violation implementation defines:
fn message(&self) -> String {
format!("Local variable `{name}` is assigned to but never used")
}
it would also have to define:
fn message_formats() -> &'static [&'static str] {
&["Local variable `{name}` is assigned to but never used"]
}
Since we however obviously do not want to duplicate all of our format
strings we simply introduce a new procedural macro attribute
#[derive_message_formats] that can be added to the message method
declaration in order to automatically derive the message_formats
implementation.
This commit implements the macro. The following and final commit
updates violations.rs to use the macro. (The changes have been separated
because the next commit is autogenerated via a Python script.)
ruff_dev::generate_rules_table previously documented which rules are
autofixable via DiagnosticKind::fixable ... since the DiagnosticKind was
obtained via Rule::kind (and Violation::placeholder) which we both want
to get rid of we have to obtain the autofixability via another way.
This commit implements such another way by adding an AUTOFIX
associated constant to the Violation trait. The constant is of the type
Option<AutoFixkind>, AutofixKind is a new struct containing an
Availability enum { Sometimes, Always}, letting us additionally document
that some autofixes are only available sometimes (which previously
wasn't documented). We intentionally introduce this information in a
struct so that we can easily introduce further autofix metadata in the
future such as autofix applicability[1].
[1]: https://doc.rust-lang.org/stable/nightly-rustc/rustc_errors/enum.Applicability.html
This commit series removes the following associated
function from the Violation trait:
fn placeholder() -> Self;
ruff previously used this placeholder approach for the messages it
listed in the README and displayed when invoked with --explain <code>.
This approach is suboptimal for three reasons:
1. The placeholder implementations are completely boring code since they
just initialize the struct with some dummy values.
2. Displaying concrete error messages with arbitrary interpolated values
can be confusing for the user since they might not recognize that the
values are interpolated.
3. Some violations have varying format strings depending on the
violation which could not be documented with the previous approach
(while we could have changed the signature to return Vec<Self> this
would still very much suffer from the previous two points).
We therefore drop Violation::placeholder in favor of a new macro-based
approach, explained in commit 4/5.
Violation::placeholder is only invoked via Rule::kind, so we firstly
have to get rid of all Rule::kind invocations ... this commit starts
removing the trivial cases.
Since the UI still relies on the rule codes this improves the developer
experience by letting developers view the code of a Rule enum variant by
hovering over it.
This commit series refactors ruff to decouple "rules" from "rule codes",
in order to:
1. Make our code more readable by changing e.g.
RuleCode::UP004 to Rule::UselessObjectInheritance.
2. Let us cleanly map multiple codes to one rule, for example:
[UP004] in pyupgrade, [R0205] in pylint and [PIE792] in flake8-pie
all refer to the rule UselessObjectInheritance but ruff currently
only associates that rule with the UP004 code (since the
implementation was initially modeled after pyupgrade).
3. Let us cleanly map one code to multiple rules, for example:
[C0103] from pylint encompasses N801, N802 and N803 from pep8-naming.
The latter two steps are not yet implemented by this commit series
but this refactoring enables us to introduce such a mapping. Such a
mapping would also let us expand flake8_to_ruff to support e.g. pylint.
After the next commit which just does some renaming the following four
commits remove all trait derivations from the Rule (previously RuleCode)
enum that depend on the variant names to guarantee that they are not
used anywhere anymore so that we can rename all of these variants in the
eigth and final commit without breaking anything.
While the plan very much is to also surface these human-friendly names
more in the user interface this is not yet done in this commit series,
which does not change anything about the UI: it's purely a refactor.
[UP004]: pyupgrade doesn't actually assign codes to its messages.
[R0205]: https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/useless-object-inheritance.html
[PIE792]: https://github.com/sbdchd/flake8-pie#pie792-no-inherit-object
[C0103]: https://pylint.pycqa.org/en/latest/user_guide/messages/convention/invalid-name.html
Implements [flake8-commas](https://github.com/PyCQA/flake8-commas). Fixes#1058.
The plugin is mostly redundant with Black (and also deprecated upstream), but very useful for projects which can't/won't use an auto-formatter.
This linter works on tokens. Before porting to Rust, I cleaned up the Python code ([link](https://gist.github.com/bluetech/7c5dcbdec4a73dd5a74d4bc09c72b8b9)) and made sure the tests pass. In the Rust version I tried to add explanatory comments, to the best of my understanding of the original logic.
Some changes I did make:
- Got rid of rule C814 - "missing trailing comma in Python 2". Ruff doesn't support Python 2.
- Merged rules C815 - "missing trailing comma in Python 3.5+" and C816 - "missing trailing comma in Python 3.6+" into C812 - "missing trailing comma". These Python versions are outdated, didn't think it was worth the complication.
- Added autofixes for C812 and C819.
Autofix is missing for C818 - "trailing comma on bare tuple prohibited". It needs to turn e.g. `x = 1,` into `x = (1, )`, it's a bit difficult to do with tokens only, so I skipped it for now.
I ran the rules on cpython/Lib and on a big internal code base and it works as intended (though I only sampled the diffs).
define_rule_mapping! was previously implemented as a declarative macro,
which was however partially relying on an origin_by_code! proc macro
because declarative macros cannot match on substrings of identifiers.
Currently all define_rule_mapping! lines look like the following:
TID251 => violations::BannedApi,
TID252 => violations::BannedRelativeImport,
We want to break up violations.rs, moving the violation definitions to
the respective rule modules. To do this we want to change the previous
lines to:
TID251 => rules::flake8_tidy_imports::banned_api::BannedApi,
TID252 => rules::flake8_tidy_imports::relative_imports::RelativeImport,
This however doesn't work because the define_rule_mapping! macro is
currently defined as:
($($code:ident => $mod:ident::$name:ident,)+) => { ... }
That is it only supported $module::$name but not longer paths with
multiple modules. While we could define `=> $path:path`[1] then we
could no longer access the last path segment, which we need because
we use it for the DiagnosticKind variant names. And
`$path:path::$last:ident` doesn't work either because it would be
ambiguous (Rust wouldn't know where the path ends ... so path fragments
have to be followed by some punctuation/keyword that may not be part of
paths). And we also cannot just introduce a procedural macro like
path_basename!(...) because the following is not valid Rust code:
enum Foo { foo!(...), }
(macros cannot be called in the place where you define variants.)
So we have to convert define_rule_mapping! into a proc macro in order to
support paths of arbitrary length and this commit implements that.
[1]: https://doc.rust-lang.org/reference/macros-by-example.html#metavariables
We don't have any doctests, but `cargo test --all` spends more than half
the time on doctests? A little confusing, but this brings the test time
from > 4s to < 2s on my machine.
We can reverse this later if it really becomes necessary, but I expect
safe Rust to be sufficient for all our needs.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
