## Summary
Improve `blanket-noqa` error message in cases where codes are provided
but not detected due to formatting issues. Namely `# noqa X100` (missing
colon) or `noqa : X100` (space before colon). The behavior is similar to
`NQA002` and `NQA003` from `flake8-noqa` mentioned in #850. The idea to
merge the rules into `PGH004` was suggested by @MichaReiser
https://github.com/astral-sh/ruff/pull/10325#issuecomment-2025535444.
## Test Plan
Test cases added to fixture.
Fixes#3172
## Summary
Allow prefixing [extend-]per-file-ignores patterns with `!` to negate
the pattern; listed rules / prefixes will be ignored in all files that
don't match the pattern.
## Test Plan
Added tests for the feature.
Rendered docs and checked rendered output.
Fixes#5499
## Summary
Add support for `FORCE_COLOR` env var, as specified at
https://force-color.org/
## Test Plan
I wrote an integration test for this, and then realized that can't work,
since we use a dev-dependency on `colored` with the `no-color` feature
to avoid ANSI color codes in test snapshots.
So this is just tested manually.
`cargo run --features test-rules -- check --no-cache --isolated -
--select RUF901 --diff < /dev/null` shows a colored diff.
`cargo run --features test-rules -- check --no-cache --isolated -
--select RUF901 --diff < /dev/null | less` does not have color, since we
pipe it to `less`.
`FORCE_COLOR=1 cargo run --features test-rules -- check --no-cache
--isolated - --select RUF901 --diff < /dev/null | less` does have color
(after this diff), even though we pipe it to `less`.
## Summary
Came across this code while digging into the semantic model with
@AlexWaygood, and found it confusing because of how it splits
`push_scope` from the paired `pop_scope` (took me a few minutes to even
figure out if/where we were popping the pushed scope). Since this
"cleanup" is already totally split by node type, there doesn't seem to
be any gain in having it as a separate "step" rather than just
incorporating it into the traversal clauses for those node types.
I left the equivalent cleanup step alone for the expression case,
because in that case it is actually generic across several different
node types, and due to the use of the common `visit_generators` utility
there isn't a clear way to keep the pushes and corresponding pops
localized.
Feel free to just reject this if I've missed a good reason for it to
stay this way!
## Test Plan
Tests and clippy.
## Summary
This is a follow-up to https://github.com/astral-sh/ruff/pull/10764.
Support for diagnostics, quick fixes, and source actions can now be
disabled via client settings.
## Test Plan
### Manual Testing
Set up your workspace as described in the test plan in
https://github.com/astral-sh/ruff/pull/10764, up to step 2. You don't
need to add a debug statement.
The configuration for `folder_a` and `folder_b` should be as follows:
`folder_a`:
```json
{
"ruff.codeAction.fixViolation": {
"enable": true
}
}
```
`folder_b`
```json
{
"ruff.codeAction.fixViolation": {
"enable": false
}
}
```
Finally, open up your VS Code User Settings and un-check the `Ruff > Fix
All` setting.
1. Open a Python file in `folder_a` that has existing problems. The
problems should be highlighted, and quick fix should be available.
`source.fixAll` should not be available as a source action.
2. Open a Python file in `folder_b` that has existing problems. The
problems should be highlighted, but quick fixes should not be available
for any of them. `source.fixAll` should not be available as a source
action.
3. Open up your VS Code Workspace Settings (second tab under the search
bar) and un-check `Ruff > Lint: Enable`
4. Both files you tested in steps 1 and 2 should now lack any visible
diagnostics. `source.organizeImports` should still be available as a
source action.
## Summary
Fixes#3011.
Type checkers currently allow forward references in all contexts in stub
files, and stubs frequently make use of this capability (although it
doesn't actually seem to be specc'd anywhere --neither in PEP 484, nor
https://typing.readthedocs.io/en/latest/source/stubs.html#id6, nor the
CPython typing docs). Implementing it so that Ruff allows forward
references in _all contexts_ in stub files seems non-trivial, however
(or at least, I couldn't figure out how to do it easily), so this PR
does not do that. Perhaps it _should_; if we think this apporach isn't
principled enough, I'm happy to close it and postpone changing anything
here.
However, this does reduce the number of F821 errors Ruff emits on
typeshed down from 76 to 2, which would mean that we could enable the
rule at typeshed. The remaining 2 F821 errors can be trivially fixed at
typeshed by moving definitions around; forward references in class bases
were really the only remaining places where there was a real _use case_
for forward references in stub files that Ruff wasn't yet allowing.
## Test plan
`cargo test`. I also ran this PR branch on typeshed to check to see if
there were any new false positives caused by the changes here; there
were none.
## Summary
`Path.read_bytes()` does not support any keyword arguments, so `FURB101`
should not be triggered if the file is opened in `rb` mode with any
keyword arguments.
## Test Plan
Move erroneous test to "Non-error" section of fixture.
## Summary
Historically, given:
```python
__all__ = [ # noqa: F822
"Bernoulli",
"Beta",
"Binomial",
]
```
The F822 violations would be attached to the `__all__`, so this `# noqa`
would be enforced for _all_ definitions in the list. This changed in
https://github.com/astral-sh/ruff/pull/10525 for the better, in that we
now use the range of each string. But these `# noqa` directives stopped
working.
This PR sets the `__all__` as a parent range in the diagnostic, so that
these directives are respected once again.
Closes https://github.com/astral-sh/ruff/issues/10795.
## Test Plan
`cargo test`
## Summary
Add new rule `pyupgrade - UP042` (I picked next available number).
Closes https://github.com/astral-sh/ruff/discussions/3867
Closes https://github.com/astral-sh/ruff/issues/9569
It should warn + provide a fix `class A(str, Enum)` -> `class
A(StrEnum)` for py311+.
## Test Plan
Added UP042.py test.
## Notes
I did not find a way to call `remove_argument` 2 times consecutively, so
the automatic fixing works only for classes that inherit exactly `str,
Enum` (regardless of the order).
I also plan to extend this rule to support IntEnum in next PR.
## Summary
This builds off of the work in
https://github.com/astral-sh/ruff/pull/10652 to implement a command
executor, backwards compatible with the commands from the previous LSP
(`ruff.applyAutofix`, `ruff.applyFormat` and
`ruff.applyOrganizeImports`).
This involved a lot of refactoring and tweaks to the code action
resolution code - the most notable change is that workspace edits are
specified in a slightly different way, using the more general `changes`
field instead of the `document_changes` field (which isn't supported on
all LSP clients). Additionally, the API for synchronous request handlers
has been updated to include access to the `Requester`, which we use to
send a `workspace/applyEdit` request to the client.
## Test Plan
https://github.com/astral-sh/ruff/assets/19577865/7932e30f-d944-4e35-b828-1d81aa56c087
## Summary
When a language server initializes, it is passed a serialized JSON
object, which is known as its "initialization options". Until now, `ruff
server` has ignored those initialization options, meaning that
user-provided settings haven't worked. This PR is the first step for
supporting settings from the LSP client. It implements procedures to
deserialize initialization options into a settings object, and then
resolve those settings objects into concrete settings for each
workspace.
One of the goals for user settings implementation in `ruff server` is
backwards compatibility with `ruff-lsp`'s settings. We won't support all
settings that `ruff-lsp` had, but the ones that we do support should
work the same and use the same schema as `ruff-lsp`.
These are the existing settings from `ruff-lsp` that we will continue to
support, and which are part of the settings schema in this PR:
| Setting | Default Value | Description |
|----------------------------------------|---------------|----------------------------------------------------------------------------------------|
| `codeAction.disableRuleComment.enable` | `true` | Whether to display
Quick Fix actions to disable rules via `noqa` suppression comments. |
| `codeAction.fixViolation.enable` | `true` | Whether to display Quick
Fix actions to autofix violations. |
| `fixAll` | `true` | Whether to register Ruff as capable of handling
`source.fixAll` actions. |
| `lint.enable` | `true` | Whether to enable linting. Set to `false` to
use Ruff exclusively as a formatter. |
| `organizeImports` | `true` | Whether to register Ruff as capable of
handling `source.organizeImports` actions. |
To be clear: this PR does not implement 'support' for these settings,
individually. Rather, it constructs a framework for these settings to be
used by the server in the future.
Notably, we are choosing *not* to support `lint.args` and `format.args`
as settings for `ruff server`. This is because we're now interfacing
with Ruff at a lower level than its CLI, and converting CLI arguments
back into configuration is too involved.
We will have support for linter and formatter specific settings in
follow-up PRs. We will also 'hook up' user settings to work with the
server in follow up PRs.
## Test Plan
### Snapshot Tests
Tests have been created in
`crates/ruff_server/src/session/settings/tests.rs` to ensure that
deserialization and settings resolution works as expected.
### Manual Testing
Since we aren't using the resolved settings anywhere yet, we'll have to
add a few printing statements.
We want to capture what the resolved settings look like when sent as
part of a snapshot, so modify `Session::take_snapshot` to be the
following:
```rust
pub(crate) fn take_snapshot(&self, url: &Url) -> Option<DocumentSnapshot> {
let resolved_settings = self.workspaces.client_settings(url, &self.global_settings);
tracing::info!("Resolved settings for document {url}: {resolved_settings:?}");
Some(DocumentSnapshot {
configuration: self.workspaces.configuration(url)?.clone(),
resolved_client_capabilities: self.resolved_client_capabilities.clone(),
client_settings: resolved_settings,
document_ref: self.workspaces.snapshot(url)?,
position_encoding: self.position_encoding,
url: url.clone(),
})
}
```
Once you've done that, build the server and start up your extension
testing environment.
1. Set up a workspace in VS Code with two workspace folders, each one
having some variant of Ruff file-based configuration (`pyproject.toml`,
`ruff.toml`, etc.). We'll call these folders `folder_a` and `folder_b`.
2. In each folder, open up `.vscode/settings.json`.
3. In folder A, use these settings:
```json
{
"ruff.codeAction.disableRuleComment": {
"enable": true
}
}
```
4. In folder B, use these settings:
```json
{
"ruff.codeAction.disableRuleComment": {
"enable": false
}
}
```
5. Finally, open up your VS Code User Settings and un-check the `Ruff >
Code Action: Disable Rule Comment` setting.
6. When opening files in `folder_a`, you should see logs that look like
this:
```
Resolved settings for document <file>: ResolvedClientSettings { fix_all: true, organize_imports: true, lint_enable: true, disable_rule_comment_enable: true, fix_violation_enable: true }
```
7. When opening files in `folder_b`, you should see logs that look like
this:
```
Resolved settings for document <file>: ResolvedClientSettings { fix_all: true, organize_imports: true, lint_enable: true, disable_rule_comment_enable: false, fix_violation_enable: true }
```
8. To test invalid configuration, change `.vscode/settings.json` in
either folder to be this:
```json
{
"ruff.codeAction.disableRuleComment": {
"enable": "invalid"
},
}
```
10. You should now see these error logs:
```
<time> [info] <duration> ERROR ruff_server::session::settings Failed to deserialize initialization options: data did not match any variant of untagged enum InitializationOptions. Falling back to default client settings...
<time> [info] <duration> WARN ruff_server::server No workspace settings found for file:///Users/jane/testbed/pandas
<duration> WARN ruff_server::server No workspace settings found for file:///Users/jane/foss/scipy
```
11. Opening files in either folder should now print the following
configuration:
```
Resolved settings for document <file>: ResolvedClientSettings { fix_all: true, organize_imports: true, lint_enable: true, disable_rule_comment_enable: true, fix_violation_enable: true }
```
## Summary
This PR adds a new semantic model flag to indicate that the checker is
inside an f-string replacement field. This will be used to ignore
certain checks if the target version doesn't support a specific feature
like PEP 701.
fixes: #10761
## Test Plan
Add a test case from the raised issue.
Fixes#3259
## Summary
Renames `UnnecessaryComprehensionAnyAll` to
`UnnecessaryComprehensionInCall` and extends the check to `sum`, `min`,
and `max`, in addition to `any` and `all`.
## Test Plan
Updated snapshot test.
Built docs locally and verified the docs for this rule still render
correctly.
## Summary
Needed for https://github.com/astral-sh/ruff/pull/10686.
We no longer support `root_uri` as an initialization parameter, relying
solely on `workspace_folders` to find the working directories. This
means that the minimum supported LSP version is now `0.3.6`.
## Test Plan
When opening a folder in VS Code, you shouldn't see any errors in the
log which say `No workspace(s) were provided(...)`.
## Summary
We may not have had access to this in the past, but in short, if the
diagnostic is related to a specific section of a docstring, it seems
better to highlight the section (via the header) than the _entire_
docstring.
This should be completely compatible with existing `# noqa` since it's
always inside of a multi-line string anyway, and in such cases the `#
noqa` is always placed at the end of the multiline string.
Closes https://github.com/astral-sh/ruff/issues/10736.
## Summary
This PR adds the `as_str` implementation for all the operator methods.
It already exists for `CmpOp` which is being [used in the
linter](ffcd77860c/crates/ruff_linter/src/rules/flake8_simplify/rules/key_in_dict.rs (L117))
and it makes sense to implement it for the rest as well. This will also
be utilized in error messages for the new parser.
## Summary
This PR removes unused operator methods and impl traits. There is
already the `is_macro::Is` implementation for all the operators and this
seems unnecessary.
## Summary
We lost the per-rule ignores when these were migrated to the AST, so if
_any_ `Q` rule is enabled, they're now all enabled.
Closes https://github.com/astral-sh/ruff/issues/10724.
## Test Plan
Ran:
```shell
ruff check . --isolated --select Q --ignore Q000
ruff check . --isolated --select Q --ignore Q001
ruff check . --isolated --select Q --ignore Q002
ruff check . --isolated --select Q --ignore Q000,Q001
ruff check . --isolated --select Q --ignore Q000,Q002
ruff check . --isolated --select Q --ignore Q001,Q002
```
...against:
```python
'''
bad docsting
'''
a = 'single'
b = '''
bad multi line
'''
```
## Summary
An annotated lambda assignment within a class scope is often
intentional. For example, within a dataclass or Pydantic model, these
are treated as fields rather than methods (and so can be passed values
in constructors).
I originally wrote this to special-case dataclasses and Pydantic
models... But was left feeling like we'd see more false positives here
for little gain (an annotated lambda within a `class` is likely
intentional?). Open to opinions, though.
Closes https://github.com/astral-sh/ruff/issues/10718.
## Summary
Currently, [this
line](716688d44e/crates/ruff_linter/src/fix/edits.rs (L101))
assumes that the `noqa` comment begins with an octothorpe followed by a
space. (`# `) With anyone's random code, this of course is not always
true.
When there's a multi-byte character after the leading octothorpe, such
as
[`\u0085`](https://www.fileformat.info/info/unicode/char/85/index.htm),
we try slicing from within the character, causing a panic.
To fix this, the logic has been changed to remove unused `noqa`
directives and keep any trailing comments, or removing the whole comment
if the comment is just the unused `noqa`
Fixes#10097.
## Test Plan
`cargo test`
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Implement FURB164 in the issue #1348.
Relevant Refurb docs is here:
https://github.com/dosisod/refurb/blob/v2.0.0/docs/checks.md#furb164-no-from-float
I've changed the name from `no-from-float` to
`verbose-decimal-fraction-construction`.
## Test Plan
<!-- How was it tested? -->
I've written it in the `FURB164.py`.
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
When `relative-imports-order = "closest-to-furthest"` is set, we should
_still_ put non-relative imports after relative imports. It's rare for
them to be in the same section, but _possible_ if you use
`known-local-folder`.
Closes https://github.com/astral-sh/ruff/issues/10655.
## Test Plan
New tests.
Also sorted this file:
```python
from ..models import ABC
from .models import Question
from .utils import create_question
from django_polls.apps.polls.models import Choice
```
With both:
- `isort view.py`
- `ruff check view.py --select I --fix`
And the following `pyproject.toml`:
```toml
[tool.ruff.lint.isort]
order-by-type = false
relative-imports-order = "closest-to-furthest"
known-local-folder = ["django_polls"]
[tool.isort]
profile = "black"
reverse_relative = true
known_local_folder = ["django_polls"]
```
I verified that Ruff and isort gave the same result, and that they
_still_ gave the same result when removing the relevant setting:
```toml
[tool.ruff.lint.isort]
order-by-type = false
known-local-folder = ["django_polls"]
[tool.isort]
profile = "black"
known_local_folder = ["django_polls"]
```
## Summary
Add a setting `extend-allowed-calls` to allow users to define their own
list of calls which allow boolean traps.
Resolves#10485.
Resolves#10356.
## Test Plan
Extended text fixture and added setting test.
## Summary
This PR fixes the bug for `DTZ007` rule where it didn't consider to
check for the presence of `%z` in f-strings. It also considers the
string parts of an implicitly concatenated f-strings for which I want to
find a better solution (#10308).
fixes: #10601
## Test Plan
Add test cases and update the snapshots.
## Summary
Fixes#10589.
Code that violates `F401` or `F841` (in other words, unused variables or
imports) should now appear greyed out or 'unused' in an editor.
## Test Plan
Put the following test code in a new file within the extension
development host window:
```python
import math
def func():
if False:
unused = "<- this should be greyed out"
```
The following test code should have greyed out/unused import and
variable names, like so:
<img width="294" alt="Screenshot 2024-03-28 at 4 23 18 AM"
src="https://github.com/astral-sh/ruff/assets/19577865/e84a6e7a-49e2-4fed-9624-f8f9559e0837">
## Summary
Fixes#10588.
Most diagnostics from `ruff server` now appear as a much less alarming
warning instead of an error. Three diagnostics still appear as errors:
`F821` (`undefined name <name>`), `E902` (`IOError`) and `E999`
(`SyntaxError`).
## Test Plan
With an extension using the path to a locally-built executable, open a
file with multiple highlighted problems. Toggle the `Experimental
Server` setting on and off. The highlights should stay as warnings.
Then, modify the file to have a syntactically incorrect element. The
start of the invalid syntax should now have a red highlight.
<!--
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 updates the warning message for rule S305 to accurately reflect
the security concern over using ECB mode in block ciphers, which is
considered insecure compared to other modes like CBC or CTR. The
previous message incorrectly mentioned AES as a [block cipher
mode](https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation),
which has been corrected to avoid confusion.
Ref:
c85576d903/bandit/blacklists/calls.py (L99-L102)825fd7c990/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs (L187-L216)
## Test Plan
No testing required as the change is limited to a minor change of
warning message update.
## Summary
The example for tab-after-comma (E242):
```python
a = 4,\t5
```
Use instead:
```python
a = 4, 3
```
is confusing since both the whitespace and the numbers are changed.
Change so the examples use the same numbers before/after.
## Test Plan
Untested.
- Clearly state in the documentation that passing `tz=None` is just as bad as not passing a `tz=` argument, from the perspective of these rules.
- Clearly state in the error messages exactly what the user is doing wrong, if the user is passing `tz=None` rather than failing to pass a `tz=` argument at all.
- Make error messages more concise, and separate out the suggested remedy from the thing that the user is identified as doing wrong.
Co-authored-by: Christian Clauss <cclauss@me.com>
## Summary
Fixes#10618.
This PR introduces a proper API for sending requests to the client and
handling any response sent back. Dynamic capability registration now
uses this new API, fixing an issue where a much more simplistic response
handler silently flushes a code action request that needed a response.
## Test Plan
#10618 can no longer be reproduced. No errors about unhandled responses
should appear in the extension output, and you should see this new log
when the server starts:
```
<DATE> <TIME> [info] <DURATION> INFO ruff_server::server Configuration file watcher successfully registered
```
## Summary
This PR adds an overview and roadmap to the `README.md` for the
`ruff_server` crate along with a rudimentary `CONTRIBUTING.md` that
explains some of the technical decisions behind the project and basic
information about local testing.
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Similar to #10419, there was a case where there is a collision of C401
and C416 (as discussed in #10101).
Fixed this by implementing short-circuit for the comprehension of the
form `{x for x in foo}`.
## Test Plan
<!-- How was it tested? -->
Extended `C401.py` with the case where `set` is not builtin function,
and divided the case where the short-circuit should occur.
Removed the last testcase of `print(f"{ {set(a for a in 'abc')} }")`
test as this is invalid as a python code, but should I keep this?
## Summary
This is just a nitpicky improvement, but I thought it'd be a good
opportunity to look at the ruff source.
> The rules list in the documentation is generated using the registry
order. Currently, flake8-logging is separated from the rest of the
flake8 plugins. This patch puts it next to them.
https://docs.astral.sh/ruff/rules/
If it makes sense, we could alternatively just sort the linters in
https://github.com/astral-sh/ruff/blob/main/crates/ruff_dev/src/generate_rules_table.rs.
Signed-off-by: Filipe Laíns <lains@riseup.net>
## Summary
This is not the holistic solution but just to fix that issue.
fixes: #10546
## Test Plan
Add a regression test for it and check the snapshots.
## Summary
Fixed false-positive on the rule `PLW1641`, where the explicit
assignment on the `__hash__` method is not counted as an definition of
`__hash__`. (Discussed in #10557).
Also, added one new testcase.
## Test Plan
Checked on `cargo test` in `eq_without_hash.py`.
Before the change, for the assignment into `__hash__`, only `__hash__ =
None` was counted as an explicit definition of `__hash__` method.
Probably any assignment into `__hash__` property could be counted as an
explicit definition of hash, so I removed `value.is_none_literal_expr()`
check.
## Summary
Closes#10228
The PR makes the blank lines rules keep track of the cell status when
running on a notebook, and makes the rules not trigger when the line is
the first of the cell.
## Test Plan
The example given in #10228 is added as a fixture, along with a few
tests from the main blank lines fixtures.
## Summary
Continuing with #7595, this PR moves the `Q004` rule to the AST checker.
## Test Plan
- [x] Existing test cases should pass
- [x] No ecosystem updates
## Summary
PEP 420 says [nested namespace
packages](https://peps.python.org/pep-0420/#nested-namespace-packages)
are allowed, i.e. marking a directory as a namespace package marks all
subdirectories in the subtree as namespace packages.
`is_package` is modified to use `Path::starts_with` and the order of
checks is reversed to do in-memory checks first before hitting the disk.
## Test Plan
Added unit tests. Previously all tests were run with `namespace_packages
== &[]`. Verified that one of the tests was failing before changing the
implementation.
## Future Improvements
The `is_package_with_cache` can probably be rewritten to avoid repeated
calls to `Path::starts_with`, by caching all directories up to the
`namespace_root`:
```ruff
let namespace_root = namespace_packages
.iter()
.filter(|namespace_package| path.starts_with(namespace_package))
.min();
```
## Summary
Closes#10337.
I've fixed the code to count usage of variable.
Usage count inside the block is reset when there is a following
statement.
- continue
- break
- return
## Test Plan
Add test case.
## Summary
The fix for PYI025 is currently marked as unsafe in non-global scopes
for both `.py` and `.pyi` files, on the grounds that all global-scope
symbols in Python are implicitly exported from the module, so changing
the name of something in the global scope could break other modules that
import the module we're fixing. Unlike in `.py` files, however, imported
symbols are never implicitly re-exported from stub files. Symbols are
only understood by static analysis tools as being re-exported from stubs
if they are marked as explicit re-exports, which take three forms:
```py
from foo import * # all symbols from foo are re-exported from the stub
# the "redundant" alias marks it as an explicit re-export
# (note that the alias needs to be identical to the symbol's "actual" name
# in order for it to be a re-export)
from bar import barrr as barrr
# inclusion in __all__ also marks it as an explicit re-export,
# just like in `.py` files
from baz import bazzz
__all__ = ["bazzz"]
```
This is [specc'd in PEP
484](https://peps.python.org/pep-0484/#stub-files), and means that we
can mark the fix for PYI025 as safe in more cases for `.pyi` files.
## Test Plan
`cargo test`. An existing test case goes from being an unsafe fix to a
safe fix in a `.pyi` fixture. I also added a new fixture so we have
coverage of global-scope imports that are marked as re-exports using
"redundant" `from collections.abc import Set as Set` aliases.
## Summary
Continuing with https://github.com/astral-sh/ruff/issues/7595, this PR
moves the `Q001`, `Q002`, `Q003` rules to the AST based checker.
## Test Plan
Make sure all of the existing test cases pass and verify there are no
ecosystem changes.
## Summary
This error was found browsing
https://github.com/qarmin/Automated-Fuzzer/actions/runs/8396966850.
Which failed when trying to autofix the PT014 violation in the following
code:
```python
@pytest.mark.parametrize('data, spec', [(1.0, 1.0), (1.0, 1.0)])
def test_numbers(data, spec):
...
```
Investigation revealed that the implementation was not properly tested,
when the duplicate value was also the last in the list. In particular
the following function, which is in charge of finding the comma
following an element to create the suggested fix,
0a99bd84ce/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs (L647-L651)
would find the next comma even if it was outside the list itself leading
to a lot of code being deleted.
This PR fixes that.
## Test Plan
Added misbehaving code to the test fixture.
## Summary
Ensures that we use the raw identifier as provided in the source code,
rather than the normalized Unicode identifier.
This _does_ mean that we treat these as two separate identifiers, and
_don't_ merge them, even though Python will treat them as the same
symbol:
```python
import numpy as ℂℇℊℋℌℍℎℐℑℒℓℕℤΩℨKÅℬℭℯℰℱℹℴ
import numpy as CƐgHHHhIILlNZΩZKÅBCeEFio
```
I think that's fine, this is super rare anyway and would likely be
confusing for users.
Closes https://github.com/astral-sh/ruff/issues/10528.
## Test Plan
`cargo test`
## Summary
Adds commas as an accepted separator between copyright years by default,
which is actually documented in one spot, but not currently accurate.
Fixes#9477.
## Summary
Fixes#10366.
`ruff server` now registers a file watcher on the client side using the
LSP protocol, and listen for events on configuration files. On such an
event, it reloads the configuration in the 'nearest' workspace to the
file that was changed.
## Test Plan
N/A
## Summary
Some contributors have referenced settings in their documentation
without adding the settings to an options section, this has lead to some
rendering issues (#10427). This PR addresses this looking for potential
inline links to settings, cross-checking them with the options sections,
and then linking them anyway if they are not found.
Resolves#10427.
## Test Plan
Manually verified that the correct modifications were made and no docs
were broken.
## Summary
In https://github.com/astral-sh/ruff/pull/10341, we fixed some false
positives in `.pyi` files, but introduced others. This PR effectively
reverts the change in #10341 and fixes it in a slightly different way.
Instead of changing the _bindings_ we generate in the semantic model in
`.pyi` files, we instead change how we _resolve_ them.
Closes https://github.com/astral-sh/ruff/issues/10509.
- Improve clarity over the motivation for some rules
- Improve links to external references. In particular, reduce links to PEPs, as PEPs are generally historical documents rather than pieces of living documentation. Where possible, it's better to link to the official typing spec, the other docs at typing.readthedocs.io/en/latest, or the docs at docs.python.org/3/library/typing.html.
- Use more concise language in a few places
<!--
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
Fix `E231` bug: Inconsistent catch compared to pycodestyle, such as when
dict nested in list. Resolves#10113.
## Test Plan
Example from #10113 added to test fixture.
## Summary
This is a follow up on https://github.com/astral-sh/ruff/pull/10492
I incorrectly assumed that `subscript.value.end()` always points past
the value. However, this isn't the case for parenthesized values where
the end "ends" before the parentheses.
## Test Plan
I added new tests for the parenthesized case.
## Summary
This PR fixes an instability where formatting a subscribt
where the `slice` is not an `ExprSlice` and it has a trailing
end-of-line comment after its opening `[` required two formatting passes
to be stable.
The fix is to associate the trailing end-of-line comment as dangling
comment on `[` to preserve its position, similar to how Ruff does it for
other parenthesized expressions.
This also matches how trailing end-of-line subscript comments are
handled when the `slice` is an `ExprSlice`.
Fixes https://github.com/astral-sh/ruff/issues/10355
## Versioning
Shipping this as part of a patch release is fine because:
* It fixes a stability issue
* It doesn't impact already formatted code because Ruff would already
have moved to the comment to the end of the line (instead of preserving
it)
## Test Plan
Added tests
Fix a typos in the error message of rule C400
With the latest version of Ruff (0.3.3) if I have a `scratch.py` script
like that:
```python
from typing import Dict, List, Tuple
def generate_samples(test_cases: Dict) -> List[Tuple]:
return list(
(input, expected)
for input, expected in zip(test_cases["input_value"], test_cases["expected_value"])
)
```
and I run ruff
```shell
>>> ruff check scratch.py --select C400
>>> scratch.py:5:12: C400 Unnecessary generator (rewrite using `list()`
```
This PR fixes the error message from _"(rewrite using `list()`"_ to
_"(rewrite using `list()`)"_, and it fixes also the doc.
Related question: why I have this error message? The rule is not correct
in this case. Should I open an issue for that?
## Summary
This PR fixes a panic in the linter for `W605`.
Consider the following f-string:
```python
f"{{}}ab"
```
The `FStringMiddle` token would contain `{}ab`. Notice that the escaped
braces have _reduced_ the string. This means we cannot use the text
value from the token to determine the location of the escape sequence
but need to extract it from the source code.
fixes: #10434
## Test Plan
Add new test cases and update the snapshots.
## Summary
We're seeing failures in https://github.com/astral-sh/ruff/issues/10470
because `resolve_qualified_import_name` isn't guaranteed to return a
specific import if a symbol is accessible in two ways (e.g., you have
both `import logging` and `from logging import error` in scope, and you
want `logging.error`). This PR breaks up the failing tests such that the
imports aren't in the same scope.
Closes https://github.com/astral-sh/ruff/issues/10470.
## Test Plan
I added a `bindings.reverse()` to `resolve_qualified_import_name` to
ensure that the tests pass regardless of the binding order.
## Summary
I used `cargo-shear` (see
[tweet](https://twitter.com/boshen_c/status/1770106165923586395)) to
remove some unused dependencies that `cargo udeps` wasn't reporting.
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
`cargo test`
<!--
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
Clarify `extend-select` documentation to avoid confusion regarding the
default `select`. Also match the `select` documentation. Resolves
#10389.
## Test Plan
<!-- How was it tested? -->
## Summary
This adds automatic fixes for the `PT007` rule.
I am currently reviewing and adding Ruff rules to Home Assistant. One
rule is PT007, which has multiple hundred occurrences in the codebase,
but no automatic fix, and this is not fun to do manually, especially
because using Regexes are not really possible with this.
My knowledge of the Ruff codebase and Rust in general is not good and
this is my first PR here, so I hope it is not too bad.
One thing where I need help is: How can I have the transformed code to
be formatted automatically, instead of it being minimized as it does it
now?
## Test Plan
Using the existing fixtures and updated snapshots.
## Summary
This PR removes the `Iterator::chain(...)` sequence in
`RuleCodePrefix::iter()` with `Vec::expand` to avoid an
overlong-recursive types.
The existing `RuleCodePrefix::iter` method chains all rule group
iterators together. This leads to very long recursive types
`Chain<Map<Chain<Map<Chain<Map.....>>>>` (proportional to the number of
rule groups).
This PR rewrites the macro to use `Vec::extend` instead, which removes
the long recursive type (at the cost of introducing a potential
allocation).
## Alternatives
An alternative would be to use a stack allocated array by unrolling the
`Linter::iter` methods (generated by `EnumIter`).
I don't think it's worth the extra complexity, considering that
`RuleCodePrefix::iter` isn't a hot function.
## Test Plan
`cargo test`
## Summary
Fixes#10324
This removes an overeager failure case where we would exit early if no
root directory or workspace folders were provided on server
initialization. We now fall-back to the current working directory as a
workspace for that file.
## Test Plan
N/A
## Summary
#10151 documented the deviations between Ruff and Black with the new
2024 style guide in the `ruff-python-formatter/README.md`. However,
that's not the documentation shown
on the website when navigating to [intentional
deviations](https://docs.astral.sh/ruff/formatter/black/).
This PR streamlines the `ruff-python-formatter/README.md` and links to
the documentation on the website instead of repeating the same content.
The PR also makes the 2024 style guide deviations available on the
website documentation.
## Test Plan
I built the documentation locally and verified that the 2024 style guide
known deviations are now shown on the website.
## Summary
In issue https://github.com/astral-sh/ruff/issues/6785 it is reported
that a docstring in the form of `''"assert" ' SAM macro definitions '''`
is autocorrected to `"""assert" ' SAM macro definitions '''` (note the
triple quotes one only one side), which breaks the python program due
`undetermined string lateral`.
* `Q002`: Not only would docstrings in the form of `''"assert" ' SAM
macro definitions '''` (single quotes) be autofixed wrongly, but also
e.g. `""'assert' ' SAM macro definitions '''` (double quotes). The bug
is present for docstrings in all scopes (e.g. module docstrings, class
docstrings, function docstrings)
* `Q000`: The autofix error is not only present for `Q002` (docstrings),
but also for inline strings (`Q000`). Therefore `s = ''"assert" ' SAM
macro definitions '''` will also be wrongly autofixed.
Note that situation in which the first string is non-empty can be fixed,
e.g. `'123'"assert" ' SAM macro definitions '''` -> `"123""assert" ' SAM
macro definitions '''` is valid.
## What
* Change FixAvailability of `Q000` `Q002` to `Sometimes`
* Changed both rules such that docstrings/inline strings that cannot be
fixed are still reported as bad quotes via diagnostics, but no fix is
provided
## Test Plan
* For `Q000`: Add docstrings in different scopes that (partially) would
have been autofixed wrongly
* For `Q002`: Add inline strings that (partially) would have been
autofixed wrongly
Closes https://github.com/astral-sh/ruff/issues/6785
## Summary
The upstream category check here
fd26b29986/crates/ruff_linter/src/upstream_categories.rs (L54-L65)
was not working because the code is actually "E0001" not "PLE0001", I
changed it so it will detect the upstream category correctly.
I also sorted the upstream categories alphabetically, so that the
document generation will be deterministic.
## Test Plan
I compared the diff before and after the change.
Fixes#10426
## Summary
Fix rule B030 giving a false positive with Tuple operations like `+`.
[Playground](https://play.ruff.rs/17b086bc-cc43-40a7-b5bf-76d7d5fce78a)
```python
try:
...
except (ValueError,TypeError) + (EOFError,ArithmeticError):
...
```
## Reviewer notes
This is a little more convoluted than I was expecting -- because we can
have valid nested Tuples with operations done on them, the flattening
logic has become a bit more complex.
Shall I guard this behind --preview?
## Test Plan
Unit tested.
## Summary
Implement `singledispatchmethod-function` from pylint, part of #970.
This is essentially a copy paste of #8934 for `@singledispatchmethod`
decorator.
## Test Plan
Text fixture added.
## Summary
Short-circuit implementation mentioned in #10403.
I implemented this by extending C400:
- Made `UnnecessaryGeneratorList` have information of whether the the
short-circuiting occurred (to put diagnostic)
- Add additional check for whether in `unnecessary_generator_list`
function.
Please give me suggestions if you think this isn't the best way to
handle this :)
## Test Plan
Extended `C400.py` a little, and written the cases where:
- Code could be converted to one single conversion to `list` e.g.
`list(x for x in range(3))` -> `list(range(3))`
- Code couldn't be converted to one single conversion to `list` e.g.
`list(2 * x for x in range(3))` -> `[2 * x for x in range(3)]`
- `list` function is not built-in, and should not modify the code in any
way.
## Summary
Trailing ellipses in objects defined in `typing.TYPE_CHECKING` might be
meaningful (it might be declaring a stub). Thus, we should skip the
`unnecessary-placeholder` (`PIE970`) rule in such contexts.
Closes#10358.
## Test Plan
`cargo nextest run`
## Summary
Given `del X`, we'll typically add a `BindingKind::Deletion` to `X` to
shadow the current binding. However, if the deletion is inside of a
conditional operation, we _won't_, as in:
```python
def f():
global X
if X > 0:
del X
```
We will, however, track it as a reference to the binding. This PR adds
the expression context to those resolved references, so that we can
detect that the `X` in `global X` was "assigned to".
Closes https://github.com/astral-sh/ruff/issues/10397.
## Summary
The previous documentation sounded as if typing a mutable default as a
`ClassVar` were optional. However, it is not, as not doing so causes a
`ValueError`. The snippet below was tested in Python's interactive
shell:
```
>>> from dataclasses import dataclass
>>> @dataclass
... class A:
... mutable_default: list[int] = []
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python3.11/dataclasses.py", line 1230, in dataclass
return wrap(cls)
^^^^^^^^^
File "/usr/lib/python3.11/dataclasses.py", line 1220, in wrap
return _process_class(cls, init, repr, eq, order, unsafe_hash,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/dataclasses.py", line 958, in _process_class
cls_fields.append(_get_field(cls, name, type, kw_only))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/dataclasses.py", line 815, in _get_field
raise ValueError(f'mutable default {type(f.default)} for field '
ValueError: mutable default <class 'list'> for field mutable_default is not allowed: use default_factory
>>>
```
This behavior is also documented in Python's docs, see
[here](https://docs.python.org/3/library/dataclasses.html#mutable-default-values):
> [...] the
[dataclass()](https://docs.python.org/3/library/dataclasses.html#dataclasses.dataclass)
decorator will raise a
[ValueError](https://docs.python.org/3/library/exceptions.html#ValueError)
if it detects an unhashable default parameter. The assumption is that if
a value is unhashable, it is mutable. This is a partial solution, but it
does protect against many common errors.
And
[here](https://docs.python.org/3/library/dataclasses.html#class-variables)
it is documented why it works if it is typed as a `ClassVar`:
> One of the few places where
[dataclass()](https://docs.python.org/3/library/dataclasses.html#dataclasses.dataclass)
actually inspects the type of a field is to determine if a field is a
class variable as defined in [PEP
526](https://peps.python.org/pep-0526/). It does this by checking if the
type of the field is typing.ClassVar. If a field is a ClassVar, it is
excluded from consideration as a field and is ignored by the dataclass
mechanisms. Such ClassVar pseudo-fields are not returned by the
module-level
[fields()](https://docs.python.org/3/library/dataclasses.html#dataclasses.fields)
function.
In this PR I have changed the documentation to make it a little bit
clearer that not using `ClassVar` makes the code invalid.
## Summary
Ignoring all lines until the first logical line does not match the
behavior from pycodestyle. This PR therefore removes the `if
state.is_not_first_logical_line` skipping the line check before the
first logical line, and applies it only to `E302`.
For example, in the snippet below a rule violation should be detected on
the second comment and on the import.
```python
# first comment
# second comment
import foo
```
Fixes#10374
## Test Plan
Add test cases, update the snapshots and verify the ecosystem check output
## Summary
This PR updates the `StringLike::FString` variant to use `ExprFString`
instead of `FStringLiteralElement`.
For context, the reason it used `FStringLiteralElement` is that the node
is actually the string part of an f-string ("foo" in `f"foo{x}"`). But,
this is inconsistent with other variants where the captured value is the
_entire_ string.
This is also problematic w.r.t. implicitly concatenated strings. Any
rules which work with `StringLike::FString` doesn't account for the
string part in an implicitly concatenated f-strings. For example, we
don't flag confusable character in the first part of `"𝐁ad" f"𝐁ad
string"`, but only the second part
(https://play.ruff.rs/16071c4c-a1dd-4920-b56f-e2ce2f69c843).
### Update `PYI053`
_This is included in this PR because otherwise it requires a temporary
workaround to be compatible with the old logic._
This PR also updates the `PYI053` (`string-or-bytes-too-long`) rule for
f-string to consider _all_ the visible characters in a f-string,
including the ones which are implicitly concatenated. This is consistent
with implicitly concatenated strings and bytes.
For example,
```python
def foo(
# We count all the characters here
arg1: str = '51 character ' 'stringgggggggggggggggggggggggggggggggg',
# But not here because of the `{x}` replacement field which _breaks_ them up into two chunks
arg2: str = f'51 character {x} stringgggggggggggggggggggggggggggggggggggggggggggg',
) -> None: ...
```
This PR fixes it to consider all _visible_ characters inside an f-string
which includes expressions as well.
fixes: #10310fixes: #10307
## Test Plan
Add new test cases and update the snapshots.
## Review
To facilitate the review process, the change have been split into two
commits: one which has the code change while the other has the test
cases and updated snapshots.
## Summary
Fixes#10367.
While the server is still in an unstable state, requiring a `--preview`
flag would be a good way to indicate this to end users.
<!--
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
Adds a successful check message after no errors were found
Implements #8553
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
Ran a check on a test file with `cargo run -p ruff_cli -- check test.py
--no-cache` and outputted as expected.
Ran the same check with `cargo run -p ruff_cli -- check test.py
--no-cache --silent` and the command was gone as expected.
<!-- How was it tested? -->
---------
Co-authored-by: Zanie Blue <contact@zanie.dev>
Re-implementation of https://github.com/astral-sh/ruff/pull/5845 but
instead of deprecating the option I toggle the default. Now users can
_opt-in_ via the setting which will give them an unsafe fix to remove
the import. Otherwise, we raise violations but do not offer a fix. The
setting is a bit of a misnomer in either case, maybe we'll want to
remove it still someday.
As discussed there, I think the safe fix should be to import it as an
alias. I'm not sure. We need support for offering multiple fixes for
ideal behavior though? I think we should remove the fix entirely and
consider it separately.
Closes https://github.com/astral-sh/ruff/issues/5697
Supersedes https://github.com/astral-sh/ruff/pull/5845
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
This PR adds methods on `FString` to iterate over the two different kind
of elements it can have - literals and expressions. This is similar to
the methods we have on `ExprFString`.
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
Fix "TRIO115 false positive with with sleep(var) where var starts as 0"
#9935 based on the discussion in the issue.
## Test Plan
Issue code added to fixture
## Summary
I used `codespell` and `gramma` to identify mispellings and grammar
errors throughout the codebase and fixed them. I tried not to make any
controversial changes, but feel free to revert as you see fit.
## Summary
We had a report of a test failure on a specific architecture, and
looking into it, I think the test assumes that the hash keys are
iterated in a specific order. This PR thus adds a variant to our
settings display macro specifically for maps and sets. Like `CacheKey`,
it sorts the keys when printing.
Closes https://github.com/astral-sh/ruff/issues/10359.
## Summary
Fixes#10351
It seems the bug was caused by this section of code
b669306c87/crates/ruff_python_index/src/indexer.rs (L55-L58)
It's true that newline tokens cannot be immediately followed by line
continuations, but only outside parentheses. e.g. the exception
```
(
1
\
+ 2)
```
But why was this check put there in the first place? Is it guarding
against something else?
## Test Plan
New test was added to indexer
## Summary
When negating an expression like `a or b`, we need to wrap it in
parentheses, e.g., `not (a or b)` instead of `not a or b`, due to
operator precedence.
Closes https://github.com/astral-sh/ruff/issues/10335.
## Test Plan
`cargo test`
This PR fixes the following false positive in a `.pyi` stub file:
```py
x: int
y = x # F821 currently emitted here, but shouldn't be in a stub file
```
In a `.py` file, this is invalid regardless of whether `from __future__ import annotations` is enabled or not. In a `.pyi` stub file, however, it's always valid, as an annotation counts as a binding in a stub file even if no value is assigned to the variable.
I also added more test coverage for `.pyi` stub files in various edge cases where ruff's behaviour is currently correct, but where `.pyi` stub files do slightly different things to `.py` files.
## Summary
Fixes#10295.
`E225` (`Missing whitespace around operator`) and `E275` (`Missing
whitespace after keyword`) try to add a white space even when the next
character is a `)` (which is a syntax error in most cases, the
exceptions already being handled). This causes `E202` (`Whitespace
before close bracket`) to try to remove the added whitespace, resulting
in an infinite loop when `E225`/`E275` re-add it.
This PR adds an exception in `E225` and `E275` to not trigger in case
the next token is a `)`. It is a bit simplistic, but it solves the
example given in the issue without introducing a change in behavior
(according to the fixtures).
## Test Plan
`cargo test` and the `ruff-ecosystem` check were used to check that the
PR's changes do not have side-effects.
A new fixture was added to check that running the 3 rules on the example
given in the issue does not cause ruff to fail to converge.
<!--
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 introduces the `ruff_server` crate and a new `ruff server`
command. `ruff_server` is a re-implementation of
[`ruff-lsp`](https://github.com/astral-sh/ruff-lsp), written entirely in
Rust. It brings significant performance improvements, much tighter
integration with Ruff, a foundation for supporting entirely new language
server features, and more!
This PR is an early version of `ruff_lsp` that we're calling the
**pre-release** version. Anyone is more than welcome to use it and
submit bug reports for any issues they encounter - we'll have some
documentation on how to set it up with a few common editors, and we'll
also provide a pre-release VSCode extension for those interested.
This pre-release version supports:
- **Diagnostics for `.py` files**
- **Quick fixes**
- **Full-file formatting**
- **Range formatting**
- **Multiple workspace folders**
- **Automatic linter/formatter configuration** - taken from any
`pyproject.toml` files in the workspace.
Many thanks to @MichaReiser for his [proof-of-concept
work](https://github.com/astral-sh/ruff/pull/7262), which was important
groundwork for making this PR possible.
## Architectural Decisions
I've made an executive choice to go with `lsp-server` as a base
framework for the LSP, in favor of `tower-lsp`. There were several
reasons for this:
1. I would like to avoid `async` in our implementation. LSPs are mostly
computationally bound rather than I/O bound, and `async` adds a lot of
complexity to the API, while also making harder to reason about
execution order. This leads into the second reason, which is...
2. Any handlers that mutate state should be blocking and run in the
event loop, and the state should be lock-free. This is the approach that
`rust-analyzer` uses (also with the `lsp-server`/`lsp-types` crates as a
framework), and it gives us assurances about data mutation and execution
order. `tower-lsp` doesn't support this, which has caused some
[issues](https://github.com/ebkalderon/tower-lsp/issues/284) around data
races and out-of-order handler execution.
3. In general, I think it makes sense to have tight control over
scheduling and the specifics of our implementation, in exchange for a
slightly higher up-front cost of writing it ourselves. We'll be able to
fine-tune it to our needs and support future LSP features without
depending on an upstream maintainer.
## Test Plan
The pre-release of `ruff_server` will have snapshot tests for common
document editing scenarios. An expanded test suite is on the roadmap for
future version of `ruff_server`.
## Summary
Fix#10282
This PR updates the Python grammar to include the `*` character in
`*args` `**kwargs` in the range of the `Parameter`
```
def f(*args, **kwargs): pass
# ~~~~ ~~~~~~ <-- range before the PR
# ^^^^^ ^^^^^^^^ <-- range after
```
The invalid syntax `def f(*, **kwargs): ...` is also now correctly
reported.
## Test Plan
Test cases were added to `function.rs`.
## Summary
This PR changes how we format `with` statements with a single with item
for Python 3.8 or older. This change is not compatible with Black.
This is how we format a single-item with statement today
```python
def run(data_path, model_uri):
with pyspark.sql.SparkSession.builder.config(
key="spark.python.worker.reuse", value=True
).config(key="spark.ui.enabled", value=False).master(
"local-cluster[2, 1, 1024]"
).getOrCreate():
# ignore spark log output
spark.sparkContext.setLogLevel("OFF")
print(score_model(spark, data_path, model_uri))
```
This is different than how we would format the same expression if it is
inside any other clause header (`while`, `if`, ...):
```python
def run(data_path, model_uri):
while (
pyspark.sql.SparkSession.builder.config(
key="spark.python.worker.reuse", value=True
)
.config(key="spark.ui.enabled", value=False)
.master("local-cluster[2, 1, 1024]")
.getOrCreate()
):
# ignore spark log output
spark.sparkContext.setLogLevel("OFF")
print(score_model(spark, data_path, model_uri))
```
Which seems inconsistent to me.
This PR changes the formatting of the single-item with Python 3.8 or
older to match that of other clause headers.
```python
def run(data_path, model_uri):
with (
pyspark.sql.SparkSession.builder.config(
key="spark.python.worker.reuse", value=True
)
.config(key="spark.ui.enabled", value=False)
.master("local-cluster[2, 1, 1024]")
.getOrCreate()
):
# ignore spark log output
spark.sparkContext.setLogLevel("OFF")
print(score_model(spark, data_path, model_uri))
```
According to our versioning policy, this style change is gated behind a
preview flag.
## Test Plan
See added tests.
Added
## Summary
Fixes https://github.com/astral-sh/ruff/issues/10267
The issue with the current formatting is that the formatter flips
between the `SingleParenthesizedContextManager` and
`ParenthesizeIfExpands` or `SingleWithTarget` because the layouts use
incompatible formatting ( `SingleParenthesizedContextManager`:
`maybe_parenthesize_expression(context)` vs `ParenthesizeIfExpands`:
`parenthesize_if_expands(item)`, `SingleWithTarget`:
`optional_parentheses(item)`.
The fix is to ensure that the layouts between which the formatter flips
when adding or removing parentheses are the same. I do this by
introducing a new `SingleWithoutTarget` layout that uses the same
formatting as `SingleParenthesizedContextManager` if it has no target
and prefer `SingleWithoutTarget` over using `ParenthesizeIfExpands` or
`SingleWithTarget`.
## Formatting change
The downside is that we now use `maybe_parenthesize_expression` over
`parenthesize_if_expands` for expressions where
`can_omit_optional_parentheses` returns `false`. This can lead to stable
formatting changes. I only found one formatting change in our ecosystem
check and, unfortunately, this is necessary to fix the instability (and
instability fixes are okay to have as part of minor changes according to
our versioning policy)
The benefit of the change is that `with` items with a single context
manager and without a target are now formatted identically to how the
same expression would be formatted in other clause headers.
## Test Plan
I ran the ecosystem check locally
## Summary
This PR refactors the with item formatting to use more explicit layouts
to make it easier to understand the different formatting cases.
The benefit of the explicit layout is that it makes it easier to reasons
about layout transition between format runs. For example, today it's
possible that `SingleWithTarget` or `ParenthesizeIfExpands` add
parentheses around the with items for `with aaaaaaaaaa + bbbbbbbbbbbb:
pass`, resulting in `with (aaaaaaaaaa + bbbbbbbbbbbb): pass`. The
problem with this is that the next formatting pass uses the
`SingleParenthesizedContextExpression` layout that uses
`maybe_parenthesize_expression` which is different from
`parenthesize_if_expands(&expr)` or `optional_parentheses(&expr)`.
## Test Plan
`cargo test`
I ran the ecosystem checks locally and there are no changes.
This PR modifies our AST so that nodes for string literals, bytes literals and f-strings all retain the following information:
- The quoting style used (double or single quotes)
- Whether the string is triple-quoted or not
- Whether the string is raw or not
This PR is a followup to #10256. Like with that PR, this PR does not, in itself, fix any bugs. However, it means that we will have the necessary information to preserve quoting style and rawness of strings in the `ExprGenerator` in a followup PR, which will allow us to provide a fix for https://github.com/astral-sh/ruff/issues/7799.
The information is recorded on the AST nodes using a bitflag field on each node, similarly to how we recorded the information on `Tok::String`, `Tok::FStringStart` and `Tok::FStringMiddle` tokens in #10298. Rather than reusing the bitflag I used for the tokens, however, I decided to create a custom bitflag for each AST node.
Using different bitflags for each node allows us to make invalid states unrepresentable: it is valid to set a `u` prefix on a string literal, but not on a bytes literal or an f-string. It also allows us to have better debug representations for each AST node modified in this PR.
## Summary
Fixes the handling end of line comments that belong to `**kwargs` when
the `**kwargs` come after a slash.
The issue was that we missed to include the `**kwargs` start position
when determining the start of the next node coming after the `/`.
Fixes https://github.com/astral-sh/ruff/issues/10281
## Test Plan
Added test
## Summary
Changes the generic recommendation to replace
```python
if foo == True: ...
```
with `if cond:` to `if foo:`.
Still uses a generic message for compound comparisons as a specific
message starts to become confusing. For example,
```python
if foo == True != False: ...
```
produces two recommendations, one of which would recommend `if True:`,
which is confusing.
Resolves [recommendation in a previous
PR](https://github.com/astral-sh/ruff/pull/8613/files#r1514915070).
## Test Plan
`cargo nextest run`
## Summary
The code later in this file that checks for slices relies on the stack
of brackets to determine the position. I'm not sure why format strings
were being excluded from this, but the tests still pass with these match
guards removed.
Closes#10278
## Test Plan
~Still needs a test.~ Test case added for this example.
## Summary
This is a follow-up to https://github.com/astral-sh/ruff/pull/10238 to
offer fixes for the f-string rule regardless of the line length of the
resulting fix. To quote Alex in the linked PR:
> Yes, from the user's perspective I'd rather have a fix that may lead
to line length issues than have to fix them myself :-) Cleaning up line
lengths is easier than changing from `"".format()` to `f""`
I agree with this position, which is that if we're going to offer a
diagnostic, we should really be offering the user the ability to fix it
-- otherwise, we're just inconveniencing them.
## Summary
Given a format string like `"{x} {x}".format(x=foo())`, we should avoid
converting to an f-string, since doing so would require repeating the
function call (`f"{foo()} {foo()}"`), which could introduce side
effects.
Closes https://github.com/astral-sh/ruff/issues/10258.
## Summary
Fixes https://github.com/astral-sh/ruff/issues/10235
This PR changes `UP032` to flag all `"".format` calls that can
technically be rewritten to an f-string, even if rewritting it to an
fstring, at least automatically, exceeds the line length (or increases
the amount by which it goes over the line length).
I looked at the Git history to understand whether the check prevents
some false positives (reported by an issue), but i haven't found a
compelling reason to limit the rule to only flag format calls that stay
in the line length limit:
* https://github.com/astral-sh/ruff/pull/7818 Changed the heuristic to
determine if the fix fits to address
https://github.com/astral-sh/ruff/discussions/7810
* https://github.com/astral-sh/ruff/pull/1905 first version of the rule
I did take a look at pyupgrade and couldn't find a similar check, at
least not in the rule code (maybe it's checked somewhere else?)
https://github.com/asottile/pyupgrade/blob/main/pyupgrade/_plugins/fstrings.py
## Breaking Change?
This could be seen as a breaking change according to ruff's [versioning
policy](https://docs.astral.sh/ruff/versioning/):
> The behavior of a stable rule is changed
* The scope of a stable rule is significantly increased
* The intent of the rule changes
* Does not include bug fixes that follow the original intent of the rule
It does increase the scope of the rule, but it is in the original intent
of the rule (so it's not).
## Test Plan
See changed test output
## Summary
When you try to remove an internal representation leaking into another
type and end up rewriting a simple version of `smallvec`.
The goal of this PR is to replace the `Box<[&'a str]>` with
`Box<QualifiedName>` to avoid that the internal `QualifiedName`
representation leaks (and it gives us a nicer API too). However, doing
this when `QualifiedName` uses `SmallVec` internally gives us all sort
of funny lifetime errors. I was lost but @BurntSushi came to rescue me.
He figured out that `smallvec` has a variance problem which is already
tracked in https://github.com/servo/rust-smallvec/issues/146
To fix the variants problem, I could use the smallvec-2-alpha-4 or
implement our own smallvec. I went with implementing our own small vec
for this specific problem. It obviously isn't as sophisticated as
smallvec (only uses safe code), e.g. it doesn't perform any size
optimizations, but it does its job.
Other changes:
* Removed `Imported::qualified_name` (the version that returns a
`String`). This can be replaced by calling `ToString` on the qualified
name.
* Renamed `Imported::call_path` to `qualified_name` and changed its
return type to `&QualifiedName`.
* Renamed `QualifiedName::imported` to `user_defined` which is the more
common term when talking about builtins vs the rest/user defined
functions.
## Test plan
`cargo test`
## Summary
Fixes https://github.com/astral-sh/ruff/issues/10039
The [recommendation for typing stub
files](https://typing.readthedocs.io/en/latest/source/stubs.html#blank-lines)
is to use **one** blank line to group related definitions and
otherwise omit blank lines.
The newly added blank line rules (`E3*`) didn't account for typing stub
files and enforced two empty lines at the top level and one empty line
otherwise, making it impossible to group related definitions.
This PR implements the `E3*` rules to:
* Not enforce blank lines. The use of blank lines in typing definitions
is entirely up to the user.
* Allow at most one empty line, including between top level statements.
## Test Plan
Added unit tests (It may look odd that many snapshots are empty but the
point is that the rule should no longer emit diagnostics)
## Summary
This PR changes the `E3*` rules to respect the `isort`
`lines-after-imports` and `lines-between-types` settings. Specifically,
the following rules required changing
* `TooManyBlannkLines` : Respects both settings.
* `BlankLinesTopLevel`: Respects `lines-after-imports`. Doesn't need to
respect `lines-between-types` because it only applies to classes and
functions
The downside of this approach is that `isort` and the blank line rules
emit a diagnostic when there are too many blank lines. The fixes aren't
identical, the blank line is less opinionated, but blank lines accepts
the fix of `isort`.
<details>
<summary>Outdated approach</summary>
Fixes
https://github.com/astral-sh/ruff/issues/10077#issuecomment-1961266981
This PR changes the blank line rules to not enforce the number of blank
lines after imports (top-level) if isort is enabled and leave it to
isort to enforce the right number of lines (depends on the
`isort.lines-after-imports` and `isort.lines-between-types` settings).
The reason to give `isort` precedence over the blank line rules is that
they are configurable. Users that always want to blank lines after
imports can use `isort.lines-after-imports=2` to enforce that
(specifically for imports).
This PR does not fix the incompatibility with the formatter in pyi files
that only uses 0 to 1 blank lines. I'll address this separately.
</details>
## Review
The first commit is a small refactor that simplified implementing the
fix (and makes it easier to reason about what's mutable and what's not).
## Test Plan
I added a new test and verified that it fails with an error that the fix
never converges. I verified the snapshot output after implementing the
fix.
---------
Co-authored-by: Hoël Bagard <34478245+hoel-bagard@users.noreply.github.com>
## Summary
When users provide configurations via `--config`, we use `shellexpand`
to ensure that we expand signifiers like `~` and environment variables.
In https://github.com/astral-sh/ruff/pull/9599, we modified `--config`
to accept either a path or an arbitrary setting. However, the detection
(to determine whether the value is a path or a setting) was lacking the
`shellexpand` behavior -- it was downstream. So we were always treating
paths like `~/ruff.toml` as values, not paths.
Closes https://github.com/astral-sh/ruff-vscode/issues/413.
The expression types in our AST are called `ExprYield`, `ExprAwait`,
`ExprStringLiteral` etc, except `ExprNamedExpr`, `ExprIfExpr` and
`ExprGenratorExpr`. This seems to align with [Python AST's
naming](https://docs.python.org/3/library/ast.html) but feels
inconsistent and excessive.
This PR removes the `Expr` postfix from `ExprNamedExpr`, `ExprIfExpr`,
and `ExprGeneratorExpr`.
## Summary
Charlie can probably explain this better than I but it turns out,
`CallPath` is used for two different things:
* To represent unqualified names like `version` where `version` can be a
local variable or imported (e.g. `from sys import version` where the
full qualified name is `sys.version`)
* To represent resolved, full qualified names
This PR splits `CallPath` into two types to make this destinction clear.
> Note: I haven't renamed all `call_path` variables to `qualified_name`
or `unqualified_name`. I can do that if that's welcomed but I first want
to get feedback on the approach and naming overall.
## Test Plan
`cargo test`
## Summary
This PR fixes for `invalid-first-argument` rules.
The fixes rename the first argument of methods and class methods to the
valid one. References to this argument are also renamed.
Fixes are skipped when another argument is named as the valid first
argument.
The fix is marked as unsafe due
The functions for the `N804` and `N805` rules are now merged, as they
only differ by the name of the valid first argument.
The rules were moved from the AST iteration to the deferred scopes to be
in the function scope while creating the fix.
## Test Plan
`cargo test`
## Summary
Allows `required-version` to be set with a version specifier, like
`>=0.3.1`.
If a single version is provided, falls back to assuming `==0.3.1`, for
backwards compatibility.
Closes https://github.com/astral-sh/ruff/issues/10192.
## Summary
This PR removes the unneeded lifetime `'b` from many of our `Visitor`
implementations.
The lifetime is unneeded because it is only constraint by `'a`, so we
can use `'a` directly.
## Test Plan
`cargo build`
## Summary
This PR changes the `CallPath` type alias to a newtype wrapper.
A newtype wrapper allows us to limit the API and to experiment with
alternative ways to implement matching on `CallPath`s.
## Test Plan
`cargo test`
<!--
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
Removes the unnecessary `exc` variable in `TRY300`'s docs example.
## Test Plan
```
python scripts/generate_mkdocs.py; mkdocs serve -f mkdocs.public.yml
```
Auguste Lalande
Charlie Marsh
Carl Meyer
Carl Meyer
Jane Lewis
Aleksei Latyshev
Alex Waygood
renovate[bot]
Tibor Reiss
Steve C
Bohdan
Micha Reiser
Dhruv Manilawala
Boshen
hikaru-kajita
Mateusz Sokół
Mix
Peter A. Jonsson
Aurelio Jargas
Filipe Laíns
Hoël Bagard
Alexey Preobrazhenskiy
yt2b
Ryan May
veryyet
Sergey Chudov
Vasco Schiavo
Sid
Robin Caloudis
Ottavio Hartman
Zanie Blue
Tom Kuson
Guilherme Vasconcelos
Tri Ho
Anuraag (Rag) Agrawal
Mathieu Kniewallner
Gautier Moin
Samuel Cormier-Iijima
Eero Vaher
ooo oo
Dominik Spicher
Jeremy Hiatt
trag1c
Meheret
Greenstar