## Summary
Given a statement like:
```python
result = (
f(111111111111111111111111111111111111111111111111111111111111111111111111111111111)
+ 1
)()
```
When we go to parenthesize the target of the assignment, we use
`maybe_parenthesize_expression` with `Parenthesize::IfBreaks`. This then
checks if the call on the right-hand side needs to be parenthesized, the
implementation of which looks like:
```rust
impl NeedsParentheses for ExprCall {
fn needs_parentheses(
&self,
_parent: AnyNodeRef,
context: &PyFormatContext,
) -> OptionalParentheses {
if CallChainLayout::from_expression(self.into(), context.source())
== CallChainLayout::Fluent
{
OptionalParentheses::Multiline
} else if context.comments().has_dangling(self) {
OptionalParentheses::Always
} else {
self.func.needs_parentheses(self.into(), context)
}
}
}
```
Checking for `self.func.needs_parentheses(self.into(), context)` is
problematic, since, as in the example above, `self.func` may _already_
be parenthesized -- in which case, we _don't_ want to parenthesize the
entire expression. If we do, we end up with this non-ideal formatting:
```python
result = (
(
f(
111111111111111111111111111111111111111111111111111111111111111111111111111111111
)
+ 1
)()
)
```
This PR modifies the `NeedsParentheses` implementations for call chain
expressions to return `Never` if the inner expression has its own
parentheses, in which case, the formatting implementations for those
expressions will preserve them anyway.
Closes https://github.com/astral-sh/ruff/issues/7370.
## Test Plan
Zulip improves a bit, everything else is unchanged.
Before:
| project | similarity index | total files | changed files |
|--------------|------------------:|------------------:|------------------:|
| cpython | 0.76083 | 1789 | 1632 |
| django | 0.99981 | 2760 | 40 |
| transformers | 0.99944 | 2587 | 413 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99983 | 3496 | 18 |
| warehouse | 0.99834 | 648 | 20 |
| zulip | 0.99956 | 1437 | 23 |
After:
| project | similarity index | total files | changed files |
|--------------|------------------:|------------------:|------------------:|
| cpython | 0.76083 | 1789 | 1632 |
| django | 0.99981 | 2760 | 40 |
| transformers | 0.99944 | 2587 | 413 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99983 | 3496 | 18 |
| warehouse | 0.99834 | 648 | 20 |
| **zulip** | **0.99962** | **1437** | **22** |
## Summary
When fixing `reversed(sorted(x, reverse=False))`, we rewrite as
`sorted(x, reverse=True)`. However, if the `reverse` argument isn't
`True` or `False`, we leave it as-is, which is incorrect.
Now, given `reversed(sorted(x, reverse=y))`, we rewrite as `sorted(x,
reverse=not y)`.
## Summary
Adds warnings for cases where:
- A selector does not include any rules because preview is disabled
- A nursery rule is selected without the preview flag
## Test plan
Add integration tests
Moves the new rule from nursery to preview for the upcoming release.
Adds new test coverage for selection of a single preview rule and fixes
a bug where preview rules were incorrectly selectable with exact codes.
## Summary
This PR adds a benchmarking script for the formatter, which benchmarks
the Ruff formatter against Black, yapf, and autopep8.
Three benchmarks are included:
1. Format everything.
2. Format everything, but use a single thread.
3. Format everything, but `--check` (don't write to disk).
There's some nuance in figuring out the right combination of arguments
to each command, but the _main_ nuance is to ensure that we always run
the given formatter (and modify the target repo in-place) prior to
benchmarking it, so that the formatters aren't disadvantaged by the
existing formatting of the target repo. (E.g.: prior to benchmarking
Black's preview style, we need to make sure we format the target repo
with Black's preview style; otherwise, preview style appears much
slower.)
Part of https://github.com/astral-sh/ruff/issues/7309.
## Summary
This PR upgrades `strum` from 0.24.x to 0.25.x.
The breaking changes are:
* strum macros now uses syn2
* The `to_string` behavior changed when using `default`. I did a quick search, we aren't using `strum(default)`
`strum` now has a `#[derive(EnumIs)]` macro that generates `is_` methods.
## Test Plan
cargo test
## Summary
The only commit is [api: add follow_root_links() option to WalkDir](dcc527d832) whicih addsa new option wheter `walkdir` should follow a root symlink or not.
The new option defaults to `true` which is the same as before.
## Test Plan
`cargo test`
## Summary
This PR bumps the pyproject-toml crate to 0.7.0. The only difference is that it now depends on indexmap 2. I reviewed the indexmap 2 changes and they don't seem relevant to us.
I used this opportunity to remove the default features from `serde_with` which removes our indexmap 1 dependency (and some other unused dependencies)
## Test Plan
`cargo test`
## Motivation
The `ast::Arguments` for call argument are split into positional
arguments (args) and keywords arguments (keywords). We currently assume
that call consists of first args and then keywords, which is generally
the case, but not always:
```python
f(*args, a=2, *args2, **kwargs)
class A(*args, a=2, *args2, **kwargs):
pass
```
The consequence is accidentally reordering arguments
(https://github.com/astral-sh/ruff/pull/7268).
## Summary
`Arguments::args_and_keywords` returns an iterator of an `ArgOrKeyword`
enum that yields args and keywords in the correct order. I've fixed the
obvious `args` and `keywords` usages, but there might be some cases with
wrong assumptions remaining.
## Test Plan
The generator got new test cases, otherwise the stacked PR
(https://github.com/astral-sh/ruff/pull/7268) which uncovered this.
## Summary
This PR updates the `FileCache` to include an optional `NotebookIndex`
to support caching for Jupyter Notebooks.
We only require the index to compute the diagnostics and thus we don't
really need to store the entire `Notebook` on the `Diagnostics` struct.
This means we only need the index to be stored in the cache to
reconstruct the `Diagnostics`.
## Test Plan
Update an existing test case to run over the fixtures under
`ruff_notebook` crate where there are multiple Jupyter Notebook.
Locally, the following commands were run in order:
1. Remove the cache: `rm -rf .ruff_cache`
2. Run without cache: `cargo run --bin ruff -- check --isolated
crates/ruff_notebook/resources/test/fixtures/jupyter/unused_variable.ipynb
--no-cache`
3. Run with cache: `cargo run --bin ruff -- check --isolated
crates/ruff_notebook/resources/test/fixtures/jupyter/unused_variable.ipynb`
4. Check whether the `.ruff_cache` directory was created or not
5. Run with cache again and verify: `cargo run --bin ruff -- check
--isolated
crates/ruff_notebook/resources/test/fixtures/jupyter/unused_variable.ipynb`
## Benchmarks
https://github.com/astral-sh/ruff/pull/6863#issuecomment-1715675186fixes: #6671
## Summary
Closes#6958.
If a method has the `override` decorator, there is nothing you can do
about incorrect dunder methods, so they should be ignored.
## Test Plan
Overridden incorrect dunder method was added to the tests to verify ruff
doesn't catch it when evaluating the file. Snapshot changes are all just
line number changes
## Summary
This PR updates the lexer test snapshots to include the range value as
well. This is mainly a mechanical refactor.
### Motivation
The main motivation is so that we can verify that the ranges are valid
and do not overlap.
## Test Plan
`cargo test`
Related https://github.com/astral-sh/ruff/issues/7276
Our docs publishing action does not allow targetting a specific commit
when run manually which means we cannot update the documentation to
anything but the latest commit on `main`. This change allows a ref to be
provided.
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
## Summary
This PR updates the remaining lexer test cases to use the snapshots.
This is mainly a mechanical refactor.
## Motivation
The main motivation is so that when we add the token range values to the
test case output, it's easier to update the test cases.
The reason they were not using the snapshots before was because of the usage of
`test_case` macro. The macros is mainly used for different EOL test cases. If we
just generate the snapshots directly, then the snapshot name would be suffixed
with `-1`, `-2`, etc. as the test function is still the same. So, we'll create
the snapshot ourselves with the platform name for the respective EOL
test cases.
## Test Plan
`cargo test`
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Extends work in #7046 (some relevant discussion there)
Changes:
- All nursery rules are now referred to as preview rules
- Documentation for the nursery is updated to describe preview
- Adds a "PREVIEW" selector for preview rules
- This is primarily to allow `--preview --ignore PREVIEW --extend-select
FOO001,BAR200`
- Using `--preview` enables preview rules that match selectors
Notable decisions:
- Preview rules are not selectable by their rule code without enabling
preview
- Retains the "NURSERY" selector for backwards compatibility
- Nursery rules are selectable by their rule code for backwards
compatiblity
Additional work:
- Selection of preview rules without the "--preview" flag should display
a warning
- Use of deprecated nursery selection behavior should display a warning
- Nursery selection should be removed after some time
## Test Plan
<!-- How was it tested? -->
Manual confirmation (i.e. we don't have an preview rules yet just
nursery rules so I added a preview rule for manual testing)
New unit tests
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
