## Summary
The motivation here is that it will make this rule easier to rewrite as
a deferred check. Right now, we can't run this rule in the deferred
phase, because it depends on the `except_handler` to power its autofix.
Instead of lexing the `except_handler`, we can use the `SimpleTokenizer`
from the formatter, and just lex forwards and backwards.
For context, this rule detects the unused `e` in:
```python
try:
pass
except ValueError as e:
pass
```
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Resolve#5854
## Test Plan
<!-- How was it tested? -->
New test cases
---------
Co-authored-by: konsti <konstin@mailbox.org>
## Summary
This PR just naively unrolls `collect_call_path` to handle attribute
resolutions of up to eight segments. In profiling via Instruments, it
seems to be about 4x faster for a very hot code path (4% of total
execution time on `main`, 1% here).
Profiling by running `RAYON_NUM_THREADS=1 cargo instruments -t time
--profile release-debug --time-limit 10000 -p ruff_cli -o
FromSlice.trace -- check crates/ruff/resources/test/cpython --silent -e
--no-cache --select ALL`, and modifying the linter to loop infinitely up
to the specified time (10 seconds) to increase sample size.
Before:
<img width="1792" alt="Screen Shot 2023-07-15 at 5 13 34 PM"
src="https://github.com/astral-sh/ruff/assets/1309177/4a8b0b45-8b67-43e9-af5e-65b326928a8e">
After:
<img width="1792" alt="Screen Shot 2023-07-15 at 8 38 51 PM"
src="https://github.com/astral-sh/ruff/assets/1309177/d8829159-2c79-4a49-ab3c-9e4e86f5b2b1">
## Summary
Before:
```
» ruff litestar tests --fix
warning: Invalid `# noqa` directive on line 19: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on line 65: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on line 74: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on line 22: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on line 66: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on line 75: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
```
After:
```
» cargo run --bin ruff ../litestar/litestar ../litestar/tests
Finished dev [unoptimized + debuginfo] target(s) in 0.15s
Running `target/debug/ruff ../litestar/litestar ../litestar/tests`
warning: Detected debug build without --no-cache.
warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_bigint.py:19: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_bigint.py:65: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_bigint.py:74: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_uuid.py:22: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_uuid.py:66: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_uuid.py:75: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
```
## Test Plan
I didn't find any existing tests with this warning.
Closes https://github.com/astral-sh/ruff/issues/5855
## Summary
Previously, `StmtIf` was defined recursively as
```rust
pub struct StmtIf {
pub range: TextRange,
pub test: Box<Expr>,
pub body: Vec<Stmt>,
pub orelse: Vec<Stmt>,
}
```
Every `elif` was represented as an `orelse` with a single `StmtIf`. This
means that this representation couldn't differentiate between
```python
if cond1:
x = 1
else:
if cond2:
x = 2
```
and
```python
if cond1:
x = 1
elif cond2:
x = 2
```
It also makes many checks harder than they need to be because we have to
recurse just to iterate over an entire if-elif-else and because we're
lacking nodes and ranges on the `elif` and `else` branches.
We change the representation to a flat
```rust
pub struct StmtIf {
pub range: TextRange,
pub test: Box<Expr>,
pub body: Vec<Stmt>,
pub elif_else_clauses: Vec<ElifElseClause>,
}
pub struct ElifElseClause {
pub range: TextRange,
pub test: Option<Expr>,
pub body: Vec<Stmt>,
}
```
where `test: Some(_)` represents an `elif` and `test: None` an else.
This representation is different tradeoff, e.g. we need to allocate the
`Vec<ElifElseClause>`, the `elif`s are now different than the `if`s
(which matters in rules where want to check both `if`s and `elif`s) and
the type system doesn't guarantee that the `test: None` else is actually
last. We're also now a bit more inconsistent since all other `else`,
those from `for`, `while` and `try`, still don't have nodes. With the
new representation some things became easier, e.g. finding the `elif`
token (we can use the start of the `ElifElseClause`) and formatting
comments for if-elif-else (no more dangling comments splitting, we only
have to insert the dangling comment after the colon manually and set
`leading_alternate_branch_comments`, everything else is taken of by
having nodes for each branch and the usual placement.rs fixups).
## Merge Plan
This PR requires coordination between the parser repo and the main ruff
repo. I've split the ruff part, into two stacked PRs which have to be
merged together (only the second one fixes all tests), the first for the
formatter to be reviewed by @michareiser and the second for the linter
to be reviewed by @charliermarsh.
* MH: Review and merge
https://github.com/astral-sh/RustPython-Parser/pull/20
* MH: Review and merge or move later in stack
https://github.com/astral-sh/RustPython-Parser/pull/21
* MH: Review and approve
https://github.com/astral-sh/RustPython-Parser/pull/22
* MH: Review and approve formatter PR
https://github.com/astral-sh/ruff/pull/5459
* CM: Review and approve linter PR
https://github.com/astral-sh/ruff/pull/5460
* Merge linter PR in formatter PR, fix ecosystem checks (ecosystem
checks can't run on the formatter PR and won't run on the linter PR, so
we need to merge them first)
* Merge https://github.com/astral-sh/RustPython-Parser/pull/22
* Create tag in the parser, update linter+formatter PR
* Merge linter+formatter PR https://github.com/astral-sh/ruff/pull/5459
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
## Summary
Originally `join_with` was used in the formatters README.md. Now it uses
```rs
f.join_comma_separated(item.end())
.nodes(elts.iter())
.finish()
```
## Test Plan
None
## Summary
For formatter instabilities, the message we get look something like
this:
```text
Unstable formatting /home/konsti/ruff/target/checkouts/deepmodeling:dpdispatcher/dpdispatcher/slurm.py
@@ -47,9 +47,9 @@
- script_header_dict["slurm_partition_line"] = (
- NOT_YET_IMPLEMENTED_ExprJoinedStr
- )
+ script_header_dict[
+ "slurm_partition_line"
+ ] = NOT_YET_IMPLEMENTED_ExprJoinedStr
Unstable formatting /home/konsti/ruff/target/checkouts/deepmodeling:dpdispatcher/dpdispatcher/pbs.py
@@ -26,9 +26,9 @@
- pbs_script_header_dict["select_node_line"] += (
- NOT_YET_IMPLEMENTED_ExprJoinedStr
- )
+ pbs_script_header_dict[
+ "select_node_line"
+ ] += NOT_YET_IMPLEMENTED_ExprJoinedStr
```
For ruff crashes. you don't even get that but just the file that crashed
it. To extract the actual bug, you'd need to manually remove parts of
the file, rerun to see if the bug still occurs (and revert if it
doesn't) until you have a minimal example.
With this script, you run
```shell
cargo run --bin ruff_shrinking -- target/checkouts/deepmodeling:dpdispatcher/dpdispatcher/slurm.py target/minirepo/code.py "Unstable formatting" "target/debug/ruff_dev format-dev --stability-check target/minirepo"
```
and get
```python
class Slurm():
def gen_script_header(self, job):
if resources.queue_name != "":
script_header_dict["slurm_partition_line"] = f"#SBATCH --partition {resources.queue_name}"
```
which is an nice minimal example.
I've been using this script and it would be easier for me if this were
part of main. The main disadvantage to merging is that it adds
additional dependencies.
## Test Plan
I've been using this for a number of minimization. This is an internal
helper script you only run manually. I could add a test that minimizes a
rule violation if required.
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Fixes#5739
## Test Plan
<!-- How was it tested? -->
Manually tested:
```sh
$ tree dir
dir
├── dir.py
│ └── file.py
└── file.py
1 directory, 2 files
$ cargo run -p ruff_cli -- check dir --no-cache
Finished dev [unoptimized + debuginfo] target(s) in 0.08s
Running `target/debug/ruff check dir --no-cache`
dir/dir.py/file.py:1:7: F821 Undefined name `a`
dir/file.py:1:7: F821 Undefined name `a`
Found 2 errors.
```
Is a unit test needed?
## Summary
This PR does some non-behavior-changing refactoring of the AST checker.
Specifically, it breaks the `Stmt`, `Expr`, and `ExceptHandler` visitors
into four distinct, consistent phases:
1. **Phase 1: Analysis**: Run any lint rules on the node.
2. **Phase 2: Binding**: Bind any symbols declared by the node.
3. **Phase 3: Recursion**: Visit all child nodes.
4. **Phase 4: Clean-up**: Pop scopes, etc.
There are some fuzzy boundaries in the last three phases, but the most
important divide is between the Phase 1 and all the others -- the goal
here is (as much as possible) to disentangle all of the vanilla
lint-rule calls from any other semantic analysis or model building.
Part of the motivation here is that I'm considering re-ordering some of
these phases, and it was just impossible to reason about that change as
long as we had miscellaneous binding-creation and scope-modification
code intermingled with lint rules. However, this could also enable us to
(e.g.) move the entire analysis phase elsewhere, and even with a more
limited API that has read-only access to `Checker` (but can push to a
diagnostics vector).
## Summary
Comparing repos with black requires that we use the settings as black,
notably line length and magic trailing comma behaviour. Excludes and
preserving quotes (vs. a preference for either quote style) is not yet
implemented because they weren't needed for the test projects.
In the other two commits i fixed the output when the progress bar is
hidden (this way is recommonded in the indicatif docs), added a
`scratch.pyi` file to gitignore because black formats stub files
differently and also updated the ecosystem readme with the projects json
without forks.
## Test Plan
I added a `line-length` vs `line_length` test. Otherwise only my
personal usage atm, a PR to integrate the script into the CI to check
some projects will follow.
## Summary
Closes#5628 by only emitting if `sep=","`. Includes documentation
(completes the `pandas-vet` ruleset).
Related to #2646.
## Test Plan
`cargo test`
Moves the computation of the `start_offset` for overlong lines to just
before the result is returned. There is a slight overhead for overlong
lines (double the work for the first `limit` characters).
In practice this results in a speedup on the CPython codebase. Most
lines are not overlong, or are not enforced because the line ends with a
URL, or does not contain whitespace. Nonetheless, the 0.3% of overlong
lines are a lot compared to other violations.
### Before
_Selected W505 and E501_
_All rules_
### After
_Selected W505 and E501_
_All rules_
CPython line statistics:
- Number of Python lines: 867.696
- Number of overlong lines: 2.963 (0.3%)
<details>
Benchmark selected:
```shell
cargo build --release && hyperfine --warmup 10 --min-runs 50 \
"./target/release/ruff ./crates/ruff/resources/test/cpython/ --no-cache -e --select W505,E501"
```
Benchmark all:
```shell
cargo build --release && hyperfine --warmup 10 --min-runs 50 \
"./target/release/ruff ./crates/ruff/resources/test/cpython/ --no-cache -e --select ALL"
```
Overlong lines in CPython
```shell
cargo run -p ruff_cli -- check crates/ruff/resources/test/cpython/Lib --no-cache --select=E501,W505 --statistics
```
Total Python lines:
```shell
find crates/ruff/resources/test/cpython/ -name '*.py' | xargs wc -l
```
</details>
(Performance tested on Mac M1)
## Summary
The motivating change here is to remove `let range =
except_handler.try_identifier().unwrap();` and instead just do
`name.range()`, since exception names now have ranges attached to them
by the parse. This also required some refactors (which are improvements)
to the built-in attribute shadowing rules, since at least one invocation
relied on passing in the exception handler and calling
`.try_identifier()`. Now that we have easy access to identifiers, we can
remove the whole `AnyShadowing` abstraction.
## Summary
This is more similar to how these flags work in other contexts (e.g.,
`visit_annotation`), and also ensures that we unset it prior to visit
the `orelse` and `finalbody` (a subtle bug).
## Summary
The intent of this rule is to always flag the `global` declaration, not
the usage. The current implementation does the wrong thing if a global
is assigned multiple times. Using `semantic().global()` is also more
efficient.
## Summary
Adds autofix for `hasattr` case of B004. I don't think it's safe (or
simple) to implement it for the `getattr` case because, inter alia,
calling `getattr` may have side effects.
Fixes#3545
## Test Plan
Existing tests were sufficient. Updated snapshots
## Summary
I'm doing some unrelated profiling, and I noticed that this method is
actually measurable on the CPython benchmark -- it's > 1% of execution
time. We don't need to lex here, we already know the ranges of all
comments, so we can just do a simple binary search for overlap, which
brings the method down to 0%.
## Test Plan
`cargo test`
## Summary
When required-imports is set with the syntax from ... import ... as ...,
autofix I002 is failing
## Test Plan
Reuse the same python files as
`crates/ruff/src/rules/isort/mod.rs:required_import` test.
## Summary
Previously, the `quoted-annotation` rule only removed quotes when `from
__future__ import annotations` was present. However, there are some
other cases in which this is also safe -- for example:
```python
def foo():
x: "MyClass"
```
We already model these in the semantic model, so this PR just expands
the scope of the rule to handle those.
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
This PR uses the `join_comma_separated` builder for formatting set
expressions
to ensure the formatting preserves magic commas, if the setting is
enabled.
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
See the fixed black tests
<!-- How was it tested? -->
## Summary
Format `DictComp` like `ListComp` from #5600. It's not 100%, but I
figured maybe it's worth starting to explore.
## Test Plan
Added ruff fixture based on `ListComp`'s.
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
This PR improves the parentheses handling for with items to get closer
to black's formatting.
### Case 1:
```python
# Black / Input
with (
[
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"bbbbbbbbbb",
"cccccccccccccccccccccccccccccccccccccccccc",
dddddddddddddddddddddddddddddddd,
] as example1,
aaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+ cccccccccccccccccccccccccccc
+ ddddddddddddddddd as example2,
CtxManager2() as example2,
CtxManager2() as example2,
CtxManager2() as example2,
):
...
# Before
with (
[
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"bbbbbbbbbb",
"cccccccccccccccccccccccccccccccccccccccccc",
dddddddddddddddddddddddddddddddd,
] as example1,
(
aaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+ cccccccccccccccccccccccccccc
+ ddddddddddddddddd
) as example2,
CtxManager2() as example2,
CtxManager2() as example2,
CtxManager2() as example2,
):
...
```
Notice how Ruff wraps the binary expression in an extra set of
parentheses
### Case 2:
Black does not expand the with-items if the with has no parentheses:
```python
# Black / Input
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c:
...
# Before
with (
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c
):
...
```
Or
```python
# Black / Input
with [
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"bbbbbbbbbb",
"cccccccccccccccccccccccccccccccccccccccccc",
dddddddddddddddddddddddddddddddd,
] as example1, aaaaaaaaaaaaaaaaaaaaaaaaaa * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb * cccccccccccccccccccccccccccc + ddddddddddddddddd as example2, CtxManager222222222222222() as example2:
...
# Before (Same as Case 1)
with (
[
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"bbbbbbbbbb",
"cccccccccccccccccccccccccccccccccccccccccc",
dddddddddddddddddddddddddddddddd,
] as example1,
(
aaaaaaaaaaaaaaaaaaaaaaaaaa
* bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
* cccccccccccccccccccccccccccc
+ ddddddddddddddddd
) as example2,
CtxManager222222222222222() as example2,
):
...
```
## Test Plan
I added new snapshot tests
Improves the django similarity index from 0.973 to 0.977
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
Format `SetComp` like `ListComp`.
## Test Plan
Derived from `ListComp`'s fixture.
## Summary
`B006` should allow using `bytes(...)` as an argument defaule value.
## Test Plan
A new test case
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
## Summary
Non-behavioral change, but this is the same in each branch. Visiting the
`func` first also means we've visited the `func` by the time we try to
resolve it (via `resolve_call_path`), which should be helpful in a
future refactor.
## Summary
The AST pass is broken up into three phases: pre-visit (which includes
analysis), recurse (visit all members), and post-visit (clean-up). We're
not supposed to edit semantic model flags in the pre-visit phase, but it
looks like we were for literal detection. This didn't matter in
practice, but I'm looking into some AST refactors for which this _does_
cause issues.
No behavior changes expected.
## Test Plan
Good test coverage on these.
## Summary
`PERF102` looks for unused keys or values in `dict.items()` calls, and
suggests instead using `dict.keys()` or `dict.values()`. Previously,
this check determined usage by looking for underscore-prefixed
variables. However, we can use the semantic model to actually detect
whether a variable is used. This has two nice effects:
1. We avoid odd false-positives whereby underscore-prefixed variables
are actually used.
2. We can catch more cases (fewer false-negatives) by detecting unused
loop variables that _aren't_ underscore-prefixed.
Closes#5692.
## Summary
Nested calls to `sorted` can only be collapsed if the calls are
identical (i.e., they have the exact same keyword arguments).
Update C414 to only flag such cases.
Fixes#5712
## Test Plan
Updated snapshots.
Tested against flake8-comprehensions. It incorrectly flags these cases.
## Summary
This is really bad PR hygiene, but a mix of: using `Locator`-based fixes
in a few places (in lieu of `Generator`-based fixes), using match syntax
to avoid `.len() == 1` checks, using common helpers in more places, etc.
## Test Plan
`cargo test`
## Summary
Consider single element subscript expr for implicit optional.
On `main`, the cases where there is only a single element in the
subscript
list was giving false positives such as for the following:
```python
typing.Union[None]
typing.Literal[None]
```
## Test Plan
`cargo test`
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
Check for `Any` in other types for `ANN401`. This reuses the logic from
`implicit-optional` rule to resolve the type to `Any`.
Following types are supported:
* `Union[Any, ...]`
* `Any | ...`
* `Optional[Any]`
* `Annotated[<any of the above variant>, ...]`
* Forward references i.e., `"Any | ..."`
## Test Plan
Added test cases for various combinations.
fixes: #5458
## Summary
Do not raise `EXE001` and `EXE002` if WSL is detected. Uses the
[`wsl`](https://crates.io/crates/wsl) crate.
Closes#5445.
## Test Plan
`cargo test`
I don't use Windows, so was unable to test on a WSL environment. It
would be good if someone who runs Windows could check the functionality.
## Summary
Python doesn't allow `"Foo" | None` if the annotation will be evaluated
at runtime (see the comments in the PR, or the semantic model
documentation for more on what this means and when it is true), but it
_does_ allow it if the annotation is typing-only.
This, for example, is invalid, as Python will evaluate `"Foo" | None` at
runtime in order to
populate the function's `__annotations__`:
```python
def f(x: "Foo" | None): ...
```
This, however, is valid:
```python
def f():
x: "Foo" | None
```
As is this:
```python
from __future__ import annotations
def f(x: "Foo" | None): ...
```
Closes#5706.
## Summary
`StmtAnnAssign` would not insert parentheses when breaking the same way
`StmtAssign` does, causing unstable formatting and likely some syntax
errors.
## Test Plan
I added a regression test.
## Summary
The previous dummy was causing instabilities since it turned a string
into a variable.
E.g.
```python
script_header_dict[
"slurm_partition_line"
] = f"#SBATCH --partition {resources.queue_name}"
```
has an instability as
```python
- script_header_dict["slurm_partition_line"] = (
- NOT_YET_IMPLEMENTED_ExprJoinedStr
- )
+ script_header_dict[
+ "slurm_partition_line"
+ ] = NOT_YET_IMPLEMENTED_ExprJoinedStr
```
## Test Plan
The instability is gone, otherwise it's still a dummy
## Summary
Implement Pylint rule [`consider-using-in`
(`R1714`)](https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/consider-using-in.html)
as `repeated-equality-comparison-target` (`PLR1714`). This rule checks
for expressions that can be re-written as a membership test for better
readability and performance.
For example,
```python
foo == "bar" or foo == "baz" or foo == "qux"
```
should be rewritten as
```python
foo in {"bar", "baz", "qux"}
```
Related to #970. Includes documentation.
### Implementation quirks
The implementation does not work with Yoda conditions (e.g., `"a" ==
foo` instead of `foo == "a"`). The Pylint version does. I couldn't find
a way of supporting Yoda-style conditions without it being inefficient,
so didn't (I don't think people write Yoda conditions any way).
## Test Plan
Added fixture.
`cargo test`
## Summary
We have two `Cursor` implementations. This PR moves the implementation
from the formatter into `ruff_python_whitespace` (kind of a poorly-named
crate now) and uses it for both use-cases.
## Summary
Document all `ruff_dev` subcommands and document the `format_dev` flags
in the formatter readme.
CC @zanieb please flag everything that isn't clear or missing
## Test Plan
n/a
Detects invalid types for tuple, list, bytes, string indices.
For example, the following will raise a `TypeError` at runtime and when
imported Python will display a `SyntaxWarning`
```python
var = [1, 2, 3]["x"]
```
```
example.py:1: SyntaxWarning: list indices must be integers or slices, not str; perhaps you missed a comma?
var = [1, 2, 3]["x"]
Traceback (most recent call last):
File "example.py", line 1, in <module>
var = [1, 2, 3]["x"]
~~~~~~~~~^^^^^
TypeError: list indices must be integers or slices, not str
```
Previously, Ruff would not report the invalid syntax but now a violation
will be reported. This does not apply to cases where a variable, call,
or complex expression is used in the index — detection is roughly
limited to static definitions, which matches Python's warnings.
```
❯ ./target/debug/ruff example.py --select RUF015 --show-source --no-cache
example.py:1:17: RUF015 Indexed access to type `list` uses type `str` instead of an integer or slice.
|
1 | var = [1, 2, 3]["x"]
| ^^^ RUF015
|
```
Closes https://github.com/astral-sh/ruff/issues/5082
xref
ffff1440d1
## Summary
Replaces `DictionaryKey` enum with the more general `ComparableExpr`
when checking for duplicate keys
## Test Plan
Added test fixture from issue. Can potentially be expanded further
depending on what exactly we want to flag (e.g. do we also want to check
for unhashable types?) and which `ComparableExpr::XYZ` types we consider
literals.
## Issue link
Closes: https://github.com/astral-sh/ruff/issues/5691
## Summary
Similar to #5567, we can remove the use of regex, plus simplify the
representation (use `Option`), add snapshot tests, etc.
This is about 100x faster than using a regex for cases that match (2.5ns
vs. 250ns). It's obviously not a hot path, but I prefer the consistency
with other similar comment-parsing. I may DRY these up into some common
functionality later on.
<!--
Thank you for contributing to Ruff! To help us out with reviewing, please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
This PR matches Black' behavior where it only omits the optional parentheses if the expression starts or ends with a parenthesized expression:
```python
a + [aaa, bbb, cccc] * c # Don't omit
[aaa, bbb, cccc] + a * c # Split
a + c * [aaa, bbb, ccc] # Split
```
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
This improves the Jaccard index from 0.945 to 0.946
<!--
Thank you for contributing to Ruff! To help us out with reviewing, please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
This PR improves the Black compatibility when it comes to breaking comprehensions.
We want to avoid line breaks before the target and `in` whenever possible. Furthermore, `if X is not None` should be grouped together, similar to other binary like expressions
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
`cargo test`
<!-- How was it tested? -->
## Summary
We don't use `ModExpression` anywhere but it's part of the AST, removes
one `not_implemented_yet` and is a trivial 2-liner, so i implemented
formatting for `ModExpression`.
## Test Plan
None, this kind of node does not occur in file input. Otherwise all the
tests for expressions
<!--
Thank you for contributing to Ruff! To help us out with reviewing, please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
I started working on this because I assumed that I would need access to options inside of `NeedsParantheses` but it then turned out that I won't.
Anyway, it kind of felt nice to pass fewer arguments. So I'm gonna put this out here to get your feedback if you prefer this over passing individual fiels.
Oh, I sneeked in another change. I renamed `context.contents` to `source`. `contents` is too generic and doesn't tell you anything.
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
It compiles
<!--
Thank you for contributing to Ruff! To help us out with reviewing, please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
This PR removes the `mode` field from `BestFitting` because it is no longer used (we now use `conditional_group` and `fits_expanded).
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
`cargo test`
<!-- How was it tested? -->
Charlie Marsh
Zanie Blue
Nikita Sobolev
Harutaka Kawamura
konsti
Micha Reiser
Chris Pryer
Simon Brugman
David Szotten
Luc Khai Hai
Tom Kuson
Justin Prieto
guillaumeLepape
skykasko
eggplants
Dhruv Manilawala
qdegraaf