As per Cargo.toml our minimal supported Rust version is 1.65.0, so we
should be using that version in our CI for cargo test and cargo build.
This was apparently accidentally changed in
79ca66ace5.
Previous output for `ruff --explain E711`:
E711 (pycodestyle): Comparison to `None` should be `cond is None`
New output:
none-comparison
Code: E711 (pycodestyle)
Autofix is always available.
Message formats:
* Comparison to `None` should be `cond is None`
* Comparison to `None` should be `cond is not None`
For now, we're just gonna avoid flagging this for `elif` blocks, following the same reasoning as for ternaries. We can handle all of these cases, but we'll knock out the TODOs as a pair, and this avoids broken code.
Closes#2007.
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
This PR adds a new check that turns expressions such as `[1, 2, 3] + foo` into `[1, 2, 3, *foo]`, since the latter is easier to read and faster:
```
~ $ python3.11 -m timeit -s 'b = [6, 5, 4]' '[1, 2, 3] + b'
5000000 loops, best of 5: 81.4 nsec per loop
~ $ python3.11 -m timeit -s 'b = [6, 5, 4]' '[1, 2, 3, *b]'
5000000 loops, best of 5: 66.2 nsec per loop
```
However there's a couple of gotchas:
* This felt like a `simplify` rule, so I borrowed an unused `SIM` code even if the upstream `flake8-simplify` doesn't do this transform. If it should be assigned some other code, let me know 😄
* **More importantly** this transform could be unsafe if the other operand of the `+` operation has overridden `__add__` to do something else. What's the `ruff` policy around potentially unsafe operations? (I think some of the suggestions other ported rules give could be semantically different from the original code, but I'm not sure.)
* I'm not a very established Rustacean, so there's no doubt my code isn't quite idiomatic. (For instance, is there a neater way to write that four-way `match` statement?)
Thanks for `ruff`, by the way! :)
This commit fixes a bug accidentally introduced in
6cf770a692,
which resulted every `ruff --explain <code>` invocation to fail with:
thread 'main' panicked at 'Mismatch between definition and access of `explain`.
Could not downcast to ruff::registry::Rule, need to downcast to &ruff::registry::Rule',
ruff_cli/src/cli.rs:184:18
We also add an integration test for --explain to prevent such bugs from
going by unnoticed in the future.
# This commit has been generated via the following Python script:
# (followed by `cargo +nightly fmt` and `cargo dev generate-all`)
# For the reasoning see the previous commit(s).
import re
import sys
for path in (
'src/violations.rs',
'src/rules/flake8_tidy_imports/banned_api.rs',
'src/rules/flake8_tidy_imports/relative_imports.rs',
):
with open(path) as f:
text = ''
while line := next(f, None):
if line.strip() != 'fn message(&self) -> String {':
text += line
continue
text += ' #[derive_message_formats]\n' + line
body = next(f)
while (line := next(f)) != ' }\n':
body += line
# body = re.sub(r'(?<!code\| |\.push\()format!', 'format!', body)
body = re.sub(
r'("[^"]+")\s*\.to_string\(\)', r'format!(\1)', body, re.DOTALL
)
body = re.sub(
r'(r#".+?"#)\s*\.to_string\(\)', r'format!(\1)', body, re.DOTALL
)
text += body + ' }\n'
while (line := next(f)).strip() != 'fn placeholder() -> Self {':
text += line
while (line := next(f)) != ' }\n':
pass
with open(path, 'w') as f:
f.write(text)
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
While ruff displays the string returned by Violation::message in its
output for detected violations the messages displayed in the README
and in the `--explain <code>` output previously used the
DiagnosticKind::summary() function which for some verbose messages
provided shorter descriptions.
This commit removes DiagnosticKind::summary, and moves the more
extensive documentation into doc comments ... these are not displayed
yet to the user but doing that is very much planned.
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.
Fixes: #1953
@charliermarsh thank you for the tips in the issue.
I'm not very familiar with Rust, so please excuse if my string formatting syntax is messy.
In terms of testing, I compared output of `flake8 --format=pylint ` and `cargo run --format=pylint` on the same code and the output syntax seems to check out.
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 was automatically generated by running the following
# script (followed by `cargo +nightly fmt`):
import glob
import re
from typing import NamedTuple
class Rule(NamedTuple):
code: str
name: str
path: str
def rules() -> list[Rule]:
"""Returns all the rules defined in `src/registry.rs`."""
file = open('src/registry.rs')
rules = []
while next(file) != 'ruff_macros::define_rule_mapping!(\n':
continue
while (line := next(file)) != ');\n':
line = line.strip().rstrip(',')
if line.startswith('//'):
continue
code, path = line.split(' => ')
name = path.rsplit('::')[-1]
rules.append(Rule(code, name, path))
return rules
code2name = {r.code: r.name for r in rules()}
for pattern in ('src/**/*.rs', 'ruff_cli/**/*.rs', 'ruff_dev/**/*.rs', 'scripts/add_*.py'):
for name in glob.glob(pattern, recursive=True):
with open(name) as f:
text = f.read()
text = re.sub('Rule(?:Code)?::([A-Z]\w+)', lambda m: 'Rule::' + code2name[m.group(1)], text)
text = re.sub(r'(?<!"<FilePattern>:<)RuleCode\b', 'Rule', text)
text = re.sub('(use crate::registry::{.*, Rule), Rule(.*)', r'\1\2', text) # fix duplicate import
with open(name, 'w') as f:
f.write(text)
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
