## Summary
Right now, in the isort comment model, there's nowhere for trailing
comments on the _statement_ to go, as in:
```python
from mylib import (
MyClient,
MyMgmtClient,
) # some comment
```
If the comment is on the _alias_, we do preserve it, because we attach
it to the alias, as in:
```python
from mylib import (
MyClient,
MyMgmtClient, # some comment
)
```
Similarly, if the comment is trailing on an import statement
(non-`from`), we again attach it to the alias, because it can't be
parenthesized, as in:
```python
import foo # some comment
```
This PR adds logic to track and preserve those trailing comments.
We also no longer drop several other comments, like:
```python
from mylib import (
# some comment
MyClient
)
```
Closes https://github.com/astral-sh/ruff/issues/12487.
## Summary
This PR fixes a bug to raise a syntax error when an unparenthesized
generator expression is used as an argument to a call when there are
more than one argument.
For reference, the grammar is:
```
primary:
| ...
| primary genexp
| primary '(' [arguments] ')'
| ...
genexp:
| '(' ( assignment_expression | expression !':=') for_if_clauses ')'
```
The `genexp` requires the parenthesis as mentioned in the grammar. So,
the grammar for a call expression is either a name followed by a
generator expression or a name followed by a list of argument. In the
former case, the parenthesis are excluded because the generator
expression provides them while in the later case, the parenthesis are
explicitly provided for a list of arguments which means that the
generator expression requires it's own parenthesis.
This was discovered in https://github.com/astral-sh/ruff/issues/12420.
## Test Plan
Add test cases for valid and invalid syntax.
Make sure that the parser from CPython also raises this at the parsing
step:
```console
$ python3.13 -m ast parser/_.py
File "parser/_.py", line 1
total(1, 2, x for x in range(5), 6)
^^^^^^^^^^^^^^^^^^^
SyntaxError: Generator expression must be parenthesized
$ python3.13 -m ast parser/_.py
File "parser/_.py", line 1
sum(x for x in range(10), 10)
^^^^^^^^^^^^^^^^^^^^
SyntaxError: Generator expression must be parenthesized
```
## Summary
Fix panic reported in #12428. Where a string would sometimes get split
within a character boundary. This bypasses the need to split the string.
This does not guarantee the correct formatting of the docstring, but
neither did the previous implementation.
Resolves#12428
## Test Plan
Test case added to fixture
## Summary
These are the first rules implemented as part of #458, but I plan to
implement more.
Specifically, this implements `docstring-missing-exception` which checks
for raised exceptions not documented in the docstring, and
`docstring-extraneous-exception` which checks for exceptions in the
docstring not present in the body.
## Test Plan
Test fixtures added for both google and numpy style.
<!--
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? -->
This PR updates D301 rule to allow inclduing escaped docstring, e.g.
`\"""Foo.\"""` or `\"\"\"Bar.\"\"\"`, within a docstring.
Related issue: #12152
## Test Plan
Add more test cases to D301.py and update the snapshot file.
<!-- How was it tested? -->
## Summary
This PR allows us to fix both expressions in `foo == "a" or foo == "b"
or ("c" != bar and "d" != bar)`, but limits the rule to consecutive
comparisons, following https://github.com/astral-sh/ruff/issues/7797.
I think this logic was _probably_ added because of
https://github.com/astral-sh/ruff/pull/12368 -- the intent being that
we'd replace the _entire_ expression.
## Summary
Add new rule and implement for `unnecessary default type arguments`
under the `UP` category (`UP043`).
```py
// < py313
Generator[int, None, None]
// >= py313
Generator[int]
```
I think that as Python 3.13 develops, there might be more default type
arguments added besides `Generator` and `AsyncGenerator`. So, I made
this more flexible to accommodate future changes.
related issue: #12286
## Test Plan
snapshot included..!
## Summary
Pretty sure this should still be an error, but also, I think I added
this because of ecosystem CI? So want to see what pops up.
Closes https://github.com/astral-sh/ruff/issues/12164.
## Summary
This is the _intended_ default that PEP 597 _wants_, but it's not
backwards compatible. The fix is already unsafe, so it's better for us
to recommend the desired and expected behavior.
Closes https://github.com/astral-sh/ruff/issues/12069.
## Summary
I believe these should always bind more tightly -- e.g., in:
```python
for _ in bar(baz for foo in [1]):
pass
```
The inner `baz` and `foo` should be considered comprehension variables,
not for loop bindings.
We need to revisit this more holistically. In some of these cases,
`BindingKind` should probably be a flag, not an enum, since the values
aren't mutually exclusive. Separately, we should probably be more
precise in how we set it (e.g., by passing down from the parent rather
than sniffing in `handle_node_store`).
Closes https://github.com/astral-sh/ruff/issues/12339
## Summary
I don't know that there's more to do here. We could consider not raising
the violation at all for arguments, but that would have some false
negatives and could also be surprising to users.
Closes https://github.com/astral-sh/ruff/issues/12267.
## Summary
Ensures that, e.g., the following is not considered a
redefinition-without-use:
```python
import contextlib
foo = None
with contextlib.suppress(ImportError):
from some_module import foo
```
Closes https://github.com/astral-sh/ruff/issues/12309.
## Summary
Closes https://github.com/astral-sh/ruff/issues/12291.
## Test Plan
```shell
❯ cargo run check ../uv/foo --select INP
/Users/crmarsh/workspace/uv/foo/bar/baz.py:1:1: INP001 File `/Users/crmarsh/workspace/uv/foo/bar/baz.py` is part of an implicit namespace package. Add an `__init__.py`.
Found 1 error.
```
## Summary
I don't fully understand the purpose of this. In #7905, it was just
copied over from the previous non-preview implementation. But it means
that (e.g.) we don't treat `type(self.foo)` as a type -- which is wrong.
Closes https://github.com/astral-sh/ruff/issues/12290.
## Summary
Update the name of `ASYNC109` to match
[upstream](https://flake8-async.readthedocs.io/en/latest/rules.html).
Also update to the functionality to match upstream by supporting
additional context managers from `asyncio` and `anyio`. This doesn't
change any of the detection functionality, but recommends additional
context managers from `asyncio` and `anyio` depending on context.
Part of https://github.com/astral-sh/ruff/issues/12039.
## Test Plan
Added fixture for asyncio recommendation
## Summary
S113 exists because `requests` doesn't have a default timeout, so
request without timeout may hang indefinitely
> B113: Test for missing requests timeout
This plugin test checks for requests or httpx calls without a timeout
specified.
>
> Nearly all production code should use this parameter in nearly all
requests, **Failure to do so can cause your program to hang
indefinitely.**
But httpx has default timeout 5s, so S113 for httpx request without
`timeout` argument is a false positive, only valid case would be
`timeout=None`.
https://www.python-httpx.org/advanced/timeouts/
> HTTPX is careful to enforce timeouts everywhere by default.
>
> The default behavior is to raise a TimeoutException after 5 seconds of
network inactivity.
## Test Plan
snap updated
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
This is the implementation for the new rule of `pycodestyle (E204)`. It
follows the guidlines described in the contributing site, and as such it
has a new file named `whitespace_after_decorator.rs`, a new test file
called `E204.py`, and as such invokes the `function` in the `AST
statement checker` for functions and functions in classes. Linking #2402
because it has all the pycodestyle rules.
## Test Plan
<!-- How was it tested? -->
The file E204.py, has a `decorator` defined called wrapper, and this
decorator is used for 2 cases. The first one is when a `function` which
has a `decorator` is called in the file, and the second one is when
there is a `class` and 2 `methods` are defined for the `class` with a
`decorator` attached it.
Test file:
``` python
def foo(fun):
def wrapper():
print('before')
fun()
print('after')
return wrapper
# No error
@foo
def bar():
print('bar')
# E204
@ foo
def baz():
print('baz')
class Test:
# No error
@foo
def bar(self):
print('bar')
# E204
@ foo
def baz(self):
print('baz')
```
I am still new to rust and any suggestion is appreciated. Specially with
the way im using native ruff utilities.
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
Bandit now also reports `B113` on `httpx`
(https://github.com/PyCQA/bandit/pull/1060). This PR implements the same
logic, to detect missing or `None` timeouts for `httpx` alongside
`requests`.
## Test Plan
Snapshot tests.
## Summary
This PR updates the linter, specifically the token-based rules, to work
on the tokens that come after a syntax error.
For context, the token-based rules only diagnose the tokens up to the
first lexical error. This PR builds up an error resilience by
introducing a `TokenIterWithContext` which updates the `nesting` level
and tries to reflect it with what the lexer is seeing. This isn't 100%
accurate because if the parser recovered from an unclosed parenthesis in
the middle of the line, the context won't reduce the nesting level until
it sees the newline token at the end of the line.
resolves: #11915
## Test Plan
* Add test cases for a bunch of rules that are affected by this change.
* Run the fuzzer for a long time, making sure to fix any other bugs.
## Summary
This PR updates Ruff to **not** generate auto-fixes if the source code
contains syntax errors as determined by the parser.
The main motivation behind this is to avoid infinite autofix loop when
the token-based rules are run over any source with syntax errors in
#11950.
Although even after this, it's not certain that there won't be an
infinite autofix loop because the logic might be incorrect. For example,
https://github.com/astral-sh/ruff/issues/12094 and
https://github.com/astral-sh/ruff/pull/12136.
This requires updating the test infrastructure to not validate for fix
availability status when the source contained syntax errors. This is
required because otherwise the fuzzer might fail as it uses the test
function to run the linter and validate the source code.
resolves: #11455
## Test Plan
`cargo insta test`
## Summary
This PR updates various references in the linter to compute the
line-width for summing the width of each `char` in a `str` instead of
computing the width of the `str` itself.
Refer to #12133 for more details.
fixes: #12130
## Test Plan
Add a file with null (`\0`) character which is zero-width. Run this test
case on `main` to make sure it panics and switch over to this branch to
make sure it doesn't panic now.
## Summary
Use the following to reproduce this:
```console
$ cargo run -- check --select=E275,E203 --preview --no-cache ~/playground/ruff/src/play.py --fix
debug error: Failed to converge after 100 iterations in `/Users/dhruv/playground/ruff/src/play.py` with rule codes E275:---
yield,x
---
/Users/dhruv/playground/ruff/src/play.py:1:1: E275 Missing whitespace after keyword
|
1 | yield,x
| ^^^^^ E275
|
= help: Added missing whitespace after keyword
Found 101 errors (100 fixed, 1 remaining).
[*] 1 fixable with the `--fix` option.
```
## Test Plan
Add a test case and run `cargo insta test`.
## Summary
This patch inverts the defaults for
[pytest-fixture-incorrect-parentheses-style
(PT001)](https://docs.astral.sh/ruff/rules/pytest-fixture-incorrect-parentheses-style/)
and [pytest-incorrect-mark-parentheses-style
(PT003)](https://docs.astral.sh/ruff/rules/pytest-incorrect-mark-parentheses-style/)
to prefer dropping superfluous parentheses.
Presently, Ruff defaults to adding superfluous parentheses on pytest
mark and fixture decorators for documented purpose of consistency; for
example,
```diff
import pytest
-@pytest.mark.foo
+@pytest.mark.foo()
def test_bar(): ...
```
This behaviour is counter to the official pytest recommendation and
diverges from the flake8-pytest-style plugin as of version 2.0.0 (see
https://github.com/m-burst/flake8-pytest-style/issues/272). Seeing as
either default satisfies the documented benefit of consistency across a
codebase, it makes sense to change the behaviour to be consistent with
pytest and the flake8 plugin as well.
This change is breaking, so is gated behind preview (at least under my
understanding of Ruff versioning). The implementation of this gating
feature is a bit hacky, but seemed to be the least disruptive solution
without performing invasive surgery on the `#[option()]` macro.
Related to #8796.
### Caveat
Whilst updating the documentation, I sought to reference the pytest
recommendation to drop superfluous parentheses, but couldn't find any
official instruction beyond it being a revealed preference within the
pytest documentation code examples (as well as the linked issues from a
core pytest developer). Thus, the wording of the preference is
deliberately timid; it's to cohere with pytest rather than follow an
explicit guidance.
## Test Plan
`cargo nextest run`
I also ran
```sh
cargo run -p ruff -- check crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT001.py --no-cache --diff --select PT001
```
and compared against it with `--preview` to verify that the default does
change under preview (I also repeated this with `echo
'[tool.ruff]\npreview = true' > pyproject.toml` to verify that it works
with a configuration file).
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
Implement mutable-contextvar-default (B039) which was added to
flake8-bugbear in https://github.com/PyCQA/flake8-bugbear/pull/476.
This rule is similar to [mutable-argument-default
(B006)](https://docs.astral.sh/ruff/rules/mutable-argument-default) and
[function-call-in-default-argument
(B008)](https://docs.astral.sh/ruff/rules/function-call-in-default-argument),
except that it checks the `default` keyword argument to
`contextvars.ContextVar`.
```
B039.py:19:26: B039 Do not use mutable data structures for ContextVar defaults
|
18 | # Bad
19 | ContextVar("cv", default=[])
| ^^ B039
20 | ContextVar("cv", default={})
21 | ContextVar("cv", default=list())
|
= help: Replace with `None`; initialize with `.set()` after checking for `None`
```
In the upstream flake8-plugin, this rule is written expressly as a
corollary to B008 and shares much of its logic. Likewise, this
implementation reuses the logic of the Ruff implementation of B008,
namely
f765d19402/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs (L104-L106)
and
f765d19402/crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs (L106)
Thus, this rule deliberately replicates B006's and B008's heuristics.
For example, this rule assumes that all functions are mutable unless
otherwise qualified. If improvements are to be made to B039 heuristics,
they should probably be made to B006 and B008 as well (whilst trying to
match the upstream implementation).
This rule does not have an autofix as it is unknown where the ContextVar
next used (and it might not be within the same file).
Closes#12054
## Test Plan
`cargo nextest run`
## Summary
This adds a fix for the `duplicate-bases` rule that removes the
duplicate base from the class definition.
## Test Plan
`cargo nextest run duplicate_bases`, `cargo insta review`.
## Summary
Follow-up to #11902
This PR simplifies the `LinterResult` struct by avoiding the generic and
not store the `ParseError`.
This is possible because the callers already have access to the
`ParseError` via the `Parsed` output. This also means that we can
simplify the return type of `check_path` and avoid the generic `T` on
`LinterResult`.
## Test Plan
`cargo insta test`
## Summary
Follow-up to #11901
This PR avoids displaying the syntax errors as log message now that the
`E999` diagnostic cannot be disabled.
For context on why this was added, refer to
https://github.com/astral-sh/ruff/pull/2505. Basically, we would allow
ignoring the syntax error diagnostic because certain syntax feature
weren't supported back then like `match` statement. And, if a user
ignored `E999`, Ruff would give no feedback if the source code contained
any syntax error. So, this log message was a way to indicate to the user
even if `E999` was disabled.
The current state of the parser is such that (a) it matches with the
latest grammar and (b) it's easy to add support for any new syntax.
**Note:** This PR doesn't remove the `DisplayParseError` struct because
it's still being used by the formatter.
## Test Plan
Update existing snapshots from the integration tests.
## Summary
This PR updates the way syntax errors are handled throughout the linter.
The main change is that it's now not considered as a rule which involves
the following changes:
* Update `Message` to be an enum with two variants - one for diagnostic
message and the other for syntax error message
* Provide methods on the new message enum to query information required
by downstream usages
This means that the syntax errors cannot be hidden / disabled via any
disablement methods. These are:
1. Configuration via `select`, `ignore`, `per-file-ignores`, and their
`extend-*` variants
```console
$ cargo run -- check ~/playground/ruff/src/lsp.py --extend-select=E999
--no-preview --no-cache
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.10s
Running `target/debug/ruff check /Users/dhruv/playground/ruff/src/lsp.py
--extend-select=E999 --no-preview --no-cache`
warning: Rule `E999` is deprecated and will be removed in a future
release. Syntax errors will always be shown regardless of whether this
rule is selected or not.
/Users/dhruv/playground/ruff/src/lsp.py:1:8: F401 [*] `abc` imported but
unused
|
1 | import abc
| ^^^ F401
2 | from pathlib import Path
3 | import os
|
= help: Remove unused import: `abc`
```
3. Command-line flags via `--select`, `--ignore`, `--per-file-ignores`,
and their `--extend-*` variants
```console
$ cargo run -- check ~/playground/ruff/src/lsp.py --no-cache
--config=~/playground/ruff/pyproject.toml
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s
Running `target/debug/ruff check /Users/dhruv/playground/ruff/src/lsp.py
--no-cache --config=/Users/dhruv/playground/ruff/pyproject.toml`
warning: Rule `E999` is deprecated and will be removed in a future
release. Syntax errors will always be shown regardless of whether this
rule is selected or not.
/Users/dhruv/playground/ruff/src/lsp.py:1:8: F401 [*] `abc` imported but
unused
|
1 | import abc
| ^^^ F401
2 | from pathlib import Path
3 | import os
|
= help: Remove unused import: `abc`
```
This also means that the **output format** needs to be updated:
1. The `code`, `noqa_row`, `url` fields in the JSON output is optional
(`null` for syntax errors)
2. Other formats are changed accordingly
For each format, a new test case specific to syntax errors have been
added. Please refer to the snapshot output for the exact format for
syntax error message.
The output of the `--statistics` flag will have a blank entry for syntax
errors:
```
315 F821 [ ] undefined-name
119 [ ] syntax-error
103 F811 [ ] redefined-while-unused
```
The **language server** is updated to consider the syntax errors by
convert them into LSP diagnostic format separately.
### Preview
There are no quick fixes provided to disable syntax errors. This will
automatically work for `ruff-lsp` because the `noqa_row` field will be
`null` in that case.
<img width="772" alt="Screenshot 2024-06-26 at 14 57 08"
src="https://github.com/astral-sh/ruff/assets/67177269/aaac827e-4777-4ac8-8c68-eaf9f2c36774">
Even with `noqa` comment, the syntax error is displayed:
<img width="763" alt="Screenshot 2024-06-26 at 14 59 51"
src="https://github.com/astral-sh/ruff/assets/67177269/ba1afb68-7eaf-4b44-91af-6d93246475e2">
Rule documentation page:
<img width="1371" alt="Screenshot 2024-06-26 at 16 48 07"
src="https://github.com/astral-sh/ruff/assets/67177269/524f01df-d91f-4ac0-86cc-40e76b318b24">
## Test Plan
- [x] Disablement methods via config shows a warning
- [x] `select`, `extend-select`
- [ ] ~`ignore`~ _doesn't show any message_
- [ ] ~`per-file-ignores`, `extend-per-file-ignores`~ _doesn't show any
message_
- [x] Disablement methods via command-line flag shows a warning
- [x] `--select`, `--extend-select`
- [ ] ~`--ignore`~ _doesn't show any message_
- [ ] ~`--per-file-ignores`, `--extend-per-file-ignores`~ _doesn't show
any message_
- [x] File with syntax errors should exit with code 1
- [x] Language server
- [x] Should show diagnostics for syntax errors
- [x] Should not recommend a quick fix edit for adding `noqa` comment
- [x] Same for `ruff-lsp`
resolves: #8447
The motivation for this rule is solid; it's been in preview for a long
time; the implementation and tests seem sound; there are no open issues
regarding it, and as far as I can tell there never have been any.
The only issue I see is that the docs don't really describe the rule
accurately right now; I fix that in this PR.
## Summary
This rule removes `PLR1701` and redirects it to `SIM101`.
In addition to that, the `SIM101` autofix has been fixed to add padding
if required.
### `PLR1701` has bugs
It also seems that the implementation of `PLR1701` is incorrect in
multiple scenarios. For example, the following code snippet:
```py
# There are two _different_ variables `a` and `b`
if isinstance(a, int) or isinstance(b, bool) or isinstance(a, float):
pass
# There's another condition `or 1`
if isinstance(self.k, int) or isinstance(self.k, float) or 1:
pass
```
is fixed to:
```py
# Fixed to only considering variable `a`
if isinstance(a, (float, int)):
pass
# The additional condition is not present in the fix
if isinstance(self.k, (float, int)):
pass
```
Playground: https://play.ruff.rs/6cfbdfb7-f183-43b0-b59e-31e728b34190
## Documentation Preview
### `PLR1701`
<img width="1397" alt="Screenshot 2024-06-25 at 11 14 40"
src="https://github.com/astral-sh/ruff/assets/67177269/779ee84d-7c4d-4bb8-a3a4-c2b23a313eba">
## Test Plan
Remove the test cases for `PLR1701`, port the padding test case to
`SIM101` and update the snapshot.
## Summary
Right now, it's inconsistent... We sometimes match against the name, and
sometimes against the alias (`asname`). I could see a case for always
matching against the name, but matching against both seems fine too,
since the rule is really about the combination of the two?
Closes https://github.com/astral-sh/ruff/issues/12031.
## Summary
This PR fixes a bug where Ruff would raise `E203` for f-string debug
expression. This isn't valid because whitespaces are important for debug
expressions.
fixes: #12023
## Test Plan
Add test case and make sure there are no snapshot changes.
## Summary
This PR updates `F811` rule to include assignment as possible shadowed
binding. This will fix issue: #11828 .
## Test Plan
Add a test file, F811_30.py, which includes a redefinition after an
assignment and a verified snapshot file.
## Summary
Addresses #11974 to add a `RUF` rule to replace `print` expressions in
`assert` statements with the inner message.
An autofix is available, but is considered unsafe as it changes
behaviour of the execution, notably:
- removal of the printout in `stdout`, and
- `AssertionError` instance containing a different message.
While the detection of the condition is a straightforward matter,
deciding how to resolve the print arguments into a string literal can be
a relatively subjective matter. The implementation of this PR chooses to
be as tolerant as possible, and will attempt to reformat any number of
`print` arguments containing single or concatenated strings or variables
into either a string literal, or a f-string if any variables or
placeholders are detected.
## Test Plan
`cargo test`.
## Examples
For ease of discussion, this is the diff for the tests:
```diff
# Standard Case
# Expects:
# - single StringLiteral
-assert True, print("This print is not intentional.")
+assert True, "This print is not intentional."
# Concatenated string literals
# Expects:
# - single StringLiteral
-assert True, print("This print" " is not intentional.")
+assert True, "This print is not intentional."
# Positional arguments, string literals
# Expects:
# - single StringLiteral concatenated with " "
-assert True, print("This print", "is not intentional")
+assert True, "This print is not intentional"
# Concatenated string literals combined with Positional arguments
# Expects:
# - single stringliteral concatenated with " " only between `print` and `is`
-assert True, print("This " "print", "is not intentional.")
+assert True, "This print is not intentional."
# Positional arguments, string literals with a variable
# Expects:
# - single FString concatenated with " "
-assert True, print("This", print.__name__, "is not intentional.")
+assert True, f"This {print.__name__} is not intentional."
# Mixed brackets string literals
# Expects:
# - single StringLiteral concatenated with " "
-assert True, print("This print", 'is not intentional', """and should be removed""")
+assert True, "This print is not intentional and should be removed"
# Mixed brackets with other brackets inside
# Expects:
# - single StringLiteral concatenated with " " and escaped brackets
-assert True, print("This print", 'is not "intentional"', """and "should" be 'removed'""")
+assert True, "This print is not \"intentional\" and \"should\" be 'removed'"
# Positional arguments, string literals with a separator
# Expects:
# - single StringLiteral concatenated with "|"
-assert True, print("This print", "is not intentional", sep="|")
+assert True, "This print|is not intentional"
# Positional arguments, string literals with None as separator
# Expects:
# - single StringLiteral concatenated with " "
-assert True, print("This print", "is not intentional", sep=None)
+assert True, "This print is not intentional"
# Positional arguments, string literals with variable as separator, needs f-string
# Expects:
# - single FString concatenated with "{U00A0}"
-assert True, print("This print", "is not intentional", sep=U00A0)
+assert True, f"This print{U00A0}is not intentional"
# Unnecessary f-string
# Expects:
# - single StringLiteral
-assert True, print(f"This f-string is just a literal.")
+assert True, "This f-string is just a literal."
# Positional arguments, string literals and f-strings
# Expects:
# - single FString concatenated with " "
-assert True, print("This print", f"is not {'intentional':s}")
+assert True, f"This print is not {'intentional':s}"
# Positional arguments, string literals and f-strings with a separator
# Expects:
# - single FString concatenated with "|"
-assert True, print("This print", f"is not {'intentional':s}", sep="|")
+assert True, f"This print|is not {'intentional':s}"
# A single f-string
# Expects:
# - single FString
-assert True, print(f"This print is not {'intentional':s}")
+assert True, f"This print is not {'intentional':s}"
# A single f-string with a redundant separator
# Expects:
# - single FString
-assert True, print(f"This print is not {'intentional':s}", sep="|")
+assert True, f"This print is not {'intentional':s}"
# Complex f-string with variable as separator
# Expects:
# - single FString concatenated with "{U00A0}", all placeholders preserved
condition = "True is True"
maintainer = "John Doe"
-assert True, print("Unreachable due to", condition, f", ask {maintainer} for advice", sep=U00A0)
+assert True, f"Unreachable due to{U00A0}{condition}{U00A0}, ask {maintainer} for advice"
# Empty print
# Expects:
# - `msg` entirely removed from assertion
-assert True, print()
+assert True
# Empty print with separator
# Expects:
# - `msg` entirely removed from assertion
-assert True, print(sep=" ")
+assert True
# Custom print function that actually returns a string
# Expects:
@@ -100,4 +100,4 @@
# Use of `builtins.print`
# Expects:
# - single StringLiteral
-assert True, builtins.print("This print should be removed.")
+assert True, "This print should be removed."
```
## Known Issues
The current implementation resolves all arguments and separators of the
`print` expression into a single string, be it
`StringLiteralValue::single` or a `FStringValue::single`. This:
- potentially joins together strings well beyond the ideal character
limit for each line, and
- does not preserve multi-line strings in their original format, in
favour of a single line `"...\n...\n..."` format.
These are purely formatting issues only occurring in unusual scenarios.
Additionally, the autofix will tolerate `print` calls that were
previously invalid:
```python
assert True, print("this", "should not be allowed", sep=42)
```
This will be transformed into
```python
assert True, f"this{42}should not be allowed"
```
which some could argue is an alteration of behaviour.
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
<!--
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
Documentation mentions:
> PEP 563 enabled the use of a number of convenient type annotations,
such as `list[str]` instead of `List[str]`
but it meant [PEP 585](https://peps.python.org/pep-0585/) instead.
[PEP 563](https://peps.python.org/pep-0563/) is the one defining `from
__future__ import annotations`.
## Test Plan
No automated test required, just verify that
https://peps.python.org/pep-0585/ is the correct reference.
## Summary
This PR removes the duplication around `is_trivia` functions.
There are two of them in the codebase:
1. In `pycodestyle`, it's for newline, indent, dedent, non-logical
newline and comment
2. In the parser, it's for non-logical newline and comment
The `TokenKind::is_trivia` method used (1) but that's not correct in
that context. So, this PR introduces a new `is_non_logical_token` helper
method for the `pycodestyle` crate and updates the
`TokenKind::is_trivia` implementation with (2).
This also means we can remove `Token::is_trivia` method and the
standalone `token_source::is_trivia` function and use the one on
`TokenKind`.
## Test Plan
`cargo insta test`
## Summary
This PR updates the logical line rules entry-point function to only run
the logic if any of the rules within that group is enabled.
Although this shouldn't really give any performance improvements, it's
better not to do additional work if we can. This is also consistent with
how other rules are run.
## Test Plan
`cargo insta test`
## Summary
This PR updates the linter to show all the parse errors as diagnostics
instead of just the first one.
Note that this doesn't affect the parse error displayed as error log
message. This will be removed in a follow-up PR.
### Breaking?
I don't think this is a breaking change even though this might give more
diagnostics. The main reason is that this shouldn't affect any users
because it'll only give additional diagnostics in the case of multiple
syntax errors.
## Test Plan
Add an integration test case which would raise more than one parse
error.
<!--
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
related to https://github.com/astral-sh/ruff/issues/5306
The check right now only checks in the first 1024 bytes, and that's
really not enough when there's a docstring at the beginning of a file.
A more proper fix might be needed, which might be more complex (and I
don't have the `rust` skills to implement that). But this temporary
"fix" might enable more users to use this.
Context: We want to use this rule in
https://github.com/scikit-learn/scikit-learn/ and we got blocked because
of this hardcoded rule (which TBH took us quite a while to figure out
why it was failing since it's not documented).
## Test Plan
This is already kinda tested, modified the test for the new byte number.
<!-- How was it tested? -->
## Summary
This PR removes most of the syntax errors from the test cases. This
would create noise when https://github.com/astral-sh/ruff/pull/11901 is
complete. These syntax errors are also just noise for the test itself.
## Test Plan
Update the snapshots and verify that they're still the same.
## Summary
This PR updates the parser to remove building the `CommentRanges` and
instead it'll be built by the linter and the formatter when it's
required.
For the linter, it'll be built and owned by the `Indexer` while for the
formatter it'll be built from the `Tokens` struct and passed as an
argument.
## Test Plan
`cargo insta test`
## Summary
The fix for E203 now produces the same result as ruff format in cases
where a slice ends on a colon and the closing square bracket is on the
following line.
Refers to https://github.com/astral-sh/ruff/issues/10973
## Test Plan
The minimal reproduction case in the ticket was added as test case
producing no error. Additional cases with multiple spaces or a tab
before the colon where added to make sure that the rule still finds
these.
## Summary
This PR removes the `result-like` dependency and instead implement the
required functionality. The motivation being that `noqa.is_enabled()` is
easier to read than `noqa.into()`.
For context, I was just trying to understand the syntax error workflow
and I saw these flags which were being converted via `into`. I always
find `into` confusing because you never know what's it being converted
into unless you know the type. Later realized that it's just a boolean
flag. After removing the usages from these two flags, it turns out that
the dependency is only being used in one rule so I thought to remove
that as well.
## Test Plan
`cargo insta test`
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
This PR implements the [consider dict
items](https://pylint.pycqa.org/en/latest/user_guide/messages/convention/consider-using-dict-items.html)
rule from Pylint. Enabling this rule flags:
```python
ORCHESTRA = {
"violin": "strings",
"oboe": "woodwind",
"tuba": "brass",
"gong": "percussion",
}
for instrument in ORCHESTRA:
print(f"{instrument}: {ORCHESTRA[instrument]}")
for instrument in ORCHESTRA.keys():
print(f"{instrument}: {ORCHESTRA[instrument]}")
for instrument in (inline_dict := {"foo": "bar"}):
print(f"{instrument}: {inline_dict[instrument]}")
```
For not using `items()` to extract the value out of the dict. We ignore
the case of an assignment, as you can't modify the underlying
representation with the value in the list of tuples returned.
## Test Plan
<!-- How was it tested? -->
`cargo test`.
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
This PR is a follow-up to #11740 to restrict access to the `Parsed`
output by replacing the `parsed` API function with a more specific one.
Currently, that is `comment_ranges` but the linked PR exposes a `tokens`
method.
The main motivation is so that there's no way to get an incorrect
information from the checker. And, it also encapsulates the source of
the comment ranges and the tokens itself. This way it would become
easier to just update the checker if the source for these information
changes in the future.
## Test Plan
`cargo insta test`
## Summary
This PR fixes a bug where the checker would require the tokens for an
invalid offset w.r.t. the source code.
Taking the source code from the linked issue as an example:
```py
relese_version :"0.0is 64"
```
Now, this isn't really a valid type annotation but that's what this PR
is fixing. Regardless of whether it's valid or not, Ruff shouldn't
panic.
The checker would visit the parsed type annotation (`0.0is 64`) and try
to detect any violations. Certain rule logic requests the tokens for the
same but it would fail because the lexer would only have the `String`
token considering original source code. This worked before because the
lexer was invoked again for each rule logic.
The solution is to store the parsed type annotation on the checker if
it's in a typing context and use the tokens from that instead if it's
available. This is enforced by creating a new API on the checker to get
the tokens.
But, this means that there are two ways to get the tokens via the
checker API. I want to restrict this in a follow-up PR (#11741) to only
expose `tokens` and `comment_ranges` as methods and restrict access to
the parsed source code.
fixes: #11736
## Test Plan
- [x] Add a test case for `F632` rule and update the snapshot
- [x] Check all affected rules
- [x] No ecosystem changes
## Summary
This PR updates the return type of `parse_type_annotation` from `Expr`
to `Parsed<ModExpression>`. This is to allow accessing the tokens for
the parsed sub-expression in the follow-up PR.
## Test Plan
`cargo insta test`
## Summary
Ensures that we respect per-file ignores and exemptions for these rules.
Specifically, we allow:
```python
# ruff: noqa: PGH004
```
...to ignore `PGH004`.
## Summary
Should resolve https://github.com/astral-sh/ruff/issues/11454.
This is my first PR to `ruff`, so I may have missed something.
If I understood the suggestion in the issue correctly, rule `PGH004`
should be set to `Preview` again.
## Test Plan
Created two fixtures derived from the issue.
## Summary
This PR updates the logic for parsing type annotation to accept a
`ExprStringLiteral` node instead of the string value and the range.
The main motivation of this change is to simplify the implementation of
`parse_type_annotation` function with:
* Use the `opener_len` and `closer_len` from the string flags to get the
raw contents range instead of extracting it via
* `str::leading_quote(expression).unwrap().text_len()`
* `str::trailing_quote(expression).unwrap().text_len()`
* Avoid comparing the string content if we already know that it's
implicitly concatenated
## Test Plan
`cargo insta test`
## Summary
This PR updates the entire parser stack in multiple ways:
### Make the lexer lazy
* https://github.com/astral-sh/ruff/pull/11244
* https://github.com/astral-sh/ruff/pull/11473
Previously, Ruff's lexer would act as an iterator. The parser would
collect all the tokens in a vector first and then process the tokens to
create the syntax tree.
The first task in this project is to update the entire parsing flow to
make the lexer lazy. This includes the `Lexer`, `TokenSource`, and
`Parser`. For context, the `TokenSource` is a wrapper around the `Lexer`
to filter out the trivia tokens[^1]. Now, the parser will ask the token
source to get the next token and only then the lexer will continue and
emit the token. This means that the lexer needs to be aware of the
"current" token. When the `next_token` is called, the current token will
be updated with the newly lexed token.
The main motivation to make the lexer lazy is to allow re-lexing a token
in a different context. This is going to be really useful to make the
parser error resilience. For example, currently the emitted tokens
remains the same even if the parser can recover from an unclosed
parenthesis. This is important because the lexer emits a
`NonLogicalNewline` in parenthesized context while a normal `Newline` in
non-parenthesized context. This different kinds of newline is also used
to emit the indentation tokens which is important for the parser as it's
used to determine the start and end of a block.
Additionally, this allows us to implement the following functionalities:
1. Checkpoint - rewind infrastructure: The idea here is to create a
checkpoint and continue lexing. At a later point, this checkpoint can be
used to rewind the lexer back to the provided checkpoint.
2. Remove the `SoftKeywordTransformer` and instead use lookahead or
speculative parsing to determine whether a soft keyword is a keyword or
an identifier
3. Remove the `Tok` enum. The `Tok` enum represents the tokens emitted
by the lexer but it contains owned data which makes it expensive to
clone. The new `TokenKind` enum just represents the type of token which
is very cheap.
This brings up a question as to how will the parser get the owned value
which was stored on `Tok`. This will be solved by introducing a new
`TokenValue` enum which only contains a subset of token kinds which has
the owned value. This is stored on the lexer and is requested by the
parser when it wants to process the data. For example:
8196720f80/crates/ruff_python_parser/src/parser/expression.rs (L1260-L1262)
[^1]: Trivia tokens are `NonLogicalNewline` and `Comment`
### Remove `SoftKeywordTransformer`
* https://github.com/astral-sh/ruff/pull/11441
* https://github.com/astral-sh/ruff/pull/11459
* https://github.com/astral-sh/ruff/pull/11442
* https://github.com/astral-sh/ruff/pull/11443
* https://github.com/astral-sh/ruff/pull/11474
For context,
https://github.com/RustPython/RustPython/pull/4519/files#diff-5de40045e78e794aa5ab0b8aacf531aa477daf826d31ca129467703855408220
added support for soft keywords in the parser which uses infinite
lookahead to classify a soft keyword as a keyword or an identifier. This
is a brilliant idea as it basically wraps the existing Lexer and works
on top of it which means that the logic for lexing and re-lexing a soft
keyword remains separate. The change here is to remove
`SoftKeywordTransformer` and let the parser determine this based on
context, lookahead and speculative parsing.
* **Context:** The transformer needs to know the position of the lexer
between it being at a statement position or a simple statement position.
This is because a `match` token starts a compound statement while a
`type` token starts a simple statement. **The parser already knows
this.**
* **Lookahead:** Now that the parser knows the context it can perform
lookahead of up to two tokens to classify the soft keyword. The logic
for this is mentioned in the PR implementing it for `type` and `match
soft keyword.
* **Speculative parsing:** This is where the checkpoint - rewind
infrastructure helps. For `match` soft keyword, there are certain cases
for which we can't classify based on lookahead. The idea here is to
create a checkpoint and keep parsing. Based on whether the parsing was
successful and what tokens are ahead we can classify the remaining
cases. Refer to #11443 for more details.
If the soft keyword is being parsed in an identifier context, it'll be
converted to an identifier and the emitted token will be updated as
well. Refer
8196720f80/crates/ruff_python_parser/src/parser/expression.rs (L487-L491).
The `case` soft keyword doesn't require any special handling because
it'll be a keyword only in the context of a match statement.
### Update the parser API
* https://github.com/astral-sh/ruff/pull/11494
* https://github.com/astral-sh/ruff/pull/11505
Now that the lexer is in sync with the parser, and the parser helps to
determine whether a soft keyword is a keyword or an identifier, the
lexer cannot be used on its own. The reason being that it's not
sensitive to the context (which is correct). This means that the parser
API needs to be updated to not allow any access to the lexer.
Previously, there were multiple ways to parse the source code:
1. Passing the source code itself
2. Or, passing the tokens
Now that the lexer and parser are working together, the API
corresponding to (2) cannot exists. The final API is mentioned in this
PR description: https://github.com/astral-sh/ruff/pull/11494.
### Refactor the downstream tools (linter and formatter)
* https://github.com/astral-sh/ruff/pull/11511
* https://github.com/astral-sh/ruff/pull/11515
* https://github.com/astral-sh/ruff/pull/11529
* https://github.com/astral-sh/ruff/pull/11562
* https://github.com/astral-sh/ruff/pull/11592
And, the final set of changes involves updating all references of the
lexer and `Tok` enum. This was done in two-parts:
1. Update all the references in a way that doesn't require any changes
from this PR i.e., it can be done independently
* https://github.com/astral-sh/ruff/pull/11402
* https://github.com/astral-sh/ruff/pull/11406
* https://github.com/astral-sh/ruff/pull/11418
* https://github.com/astral-sh/ruff/pull/11419
* https://github.com/astral-sh/ruff/pull/11420
* https://github.com/astral-sh/ruff/pull/11424
2. Update all the remaining references to use the changes made in this
PR
For (2), there were various strategies used:
1. Introduce a new `Tokens` struct which wraps the token vector and add
methods to query a certain subset of tokens. These includes:
1. `up_to_first_unknown` which replaces the `tokenize` function
2. `in_range` and `after` which replaces the `lex_starts_at` function
where the former returns the tokens within the given range while the
latter returns all the tokens after the given offset
2. Introduce a new `TokenFlags` which is a set of flags to query certain
information from a token. Currently, this information is only limited to
any string type token but can be expanded to include other information
in the future as needed. https://github.com/astral-sh/ruff/pull/11578
3. Move the `CommentRanges` to the parsed output because this
information is common to both the linter and the formatter. This removes
the need for `tokens_and_ranges` function.
## Test Plan
- [x] Update and verify the test snapshots
- [x] Make sure the entire test suite is passing
- [x] Make sure there are no changes in the ecosystem checks
- [x] Run the fuzzer on the parser
- [x] Run this change on dozens of open-source projects
### Running this change on dozens of open-source projects
Refer to the PR description to get the list of open source projects used
for testing.
Now, the following tests were done between `main` and this branch:
1. Compare the output of `--select=E999` (syntax errors)
2. Compare the output of default rule selection
3. Compare the output of `--select=ALL`
**Conclusion: all output were same**
## What's next?
The next step is to introduce re-lexing logic and update the parser to
feed the recovery information to the lexer so that it can emit the
correct token. This moves us one step closer to having error resilience
in the parser and provides Ruff the possibility to lint even if the
source code contains syntax errors.
## Summary
Implement support for RDJson output for `ruff check`, as requested in
#8655.
## Test Plan
Tested using a snapshot test. Same approach as for e.g. the JSON output
formatter.
## Additional info
I tried to keep the implementation close to the JSON implementation.
I had to deviate a bit to make the `suggestions` key work: If there are
no suggestions, then setting `suggestions` to `null` is invalid
according to the JSONSchema. Therefore, I opted for a slightly more
complex implementation, that skips the `suggestions` key entirely if
there are no fixes available for the given diagnostic. Maybe it would
have been easier to set `"suggestions": []`, but I ended up doing it
this way.
I didn't consider notebooks, as I _think_ that RDJson doesn't work with
notebooks. This should be confirmed, and if so, there should be some
form of warning or error emitted when trying to output diagnostics for a
notebook.
I also didn't consider `ruff format`, as this comment:
https://github.com/astral-sh/ruff/issues/8655#issuecomment-1811446160
suggests that that wouldn't be compatible.
I'm new to Rust, any feedback is appreciated. 🙂 I
implemented this in order to have a productive rainy saturday afternoon,
I'm not knowledgeable about RDJson beyond the sources linked in the
issue.
## Summary
This PR implements the rule B901, which is part of the opinionated rules
of `flake8-bugbear`.
This rule seems to be desired in `ruff` as per
https://github.com/astral-sh/ruff/issues/3758 and
https://github.com/astral-sh/ruff/issues/2954#issuecomment-1441162976.
## Test Plan
As this PR was made closely following the
[CONTRIBUTING.md](8a25531a71/CONTRIBUTING.md),
it tests using the snapshot approach, that is described there.
## Sources
The implementation is inspired by [the original implementation in the
`flake8-bugbear`
repository](d1aec4cbef/bugbear.py (L1092)).
The error message and [test
file](d1aec4cbef/tests/b901.py)
where also copied from there.
The documentation I came up with on my own and needs improvement. Maybe
the example given in
https://github.com/astral-sh/ruff/issues/2954#issuecomment-1441162976
could be used, but maybe they are too complex, I'm not sure.
## Open Questions
- [ ] Documentation. (See above.)
- [x] Can I access the parent in a visitor?
The [original
implementation](d1aec4cbef/bugbear.py (L1100))
references the `yield` statement's parent to check if it is an
expression statement. I didn't find a way to do this in `ruff` and used
the `is_expresssion_statement` field on the visitor instead. What are
your thoughts on this? Is it possible and / or desired to access the
parent node here?
- [x] Is `Option::is_some(...)` -> `...unwrap()` the right thing to do?
Referring to [this piece of
code](9d5a280f71/crates/ruff_linter/src/rules/flake8_bugbear/rules/return_x_in_generator.rs?plain=1#L91-L96).
From my understanding, the `.unwrap()` is safe, because it is checked
that `return_` is not `None`. However, I feel like I missed a more
elegant solution that does both in one.
## Other
I don't know a lot about this rule, I just implemented it because I
found it in a
https://github.com/astral-sh/ruff/labels/good%20first%20issue.
I'm new to Rust, so any constructive critisism is appreciated.
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
* Potentially resolves#11619 (nondeterministic hashmap order across
different architectures) in F401 by replacing a hashmap with
nondeterministic traversal order with an ordered mapping.
I'm not sure how to test this with our CI/CD. I don't have an s390x
machine at home. Should I try it in Qemu?
## Summary
In an `__init__.py` file, it's not uncommon to lack a logical indent
(since it may just contain imports). In such cases, we were always
falling back to four-space indent. This PR adds detection for indents
within import groups.
Closes https://github.com/astral-sh/ruff/issues/11606.
## Summary
This PR aims to close#10095 by adding an option
`init-allow-undef-export` to the `pyflakes` settings. This option is
currently set to `true` such that behavior is kept identical.
But setting this option to `false` will lead to `F822` warnings to be
shown in all files, **including** `__init__.py` files.
As I've mentioned on #10095, I think `init-allow-undef-export=false`
would be the more user-friendly default option, as it creates fewer
surprises. @charliermarsh what do you think about making that the
default?
With this option in place, it's a single line fix for people that rely
on the old behavior.
And thinking longer term, for future major releases, one could probably
consider deprecating the option and eventually having people just `noqa`
these warnings if they are not wanted.
## Test Plan
I've added a `test_init_f822_enabled` test which repeats the test that
is done in the `init` test but this time with
`init-allow-undef-export=false` and the snap file correctly shows that
ruff will then trigger the otherwise suppressed F822 warning.
closes#10095
## Summary
Removed stray space in sample code snippet that is against ruff's own
default formatting rules.
This documentation appears on
https://docs.astral.sh/ruff/rules/unused-import/
## Test Plan
This is a trivially obvious change, verifiable with `ruff format
--check`
## Summary
- Implements `Y066` from `flake8-pyi` as `PYI066`
- Fixes `PYI006` not being raised for `elif` clauses. This would have
conflicted with PYI006's implementation, so decided to do it in the same
PR.
## Test Plan
`cargo test` / `cargo insta review`
## Summary
This PR ensures that if a variable is bound via `global`, and then the
`global` is read, the originating variable is also marked as read. It's
not perfect, in that it won't detect _rebindings_, like:
```python
from app import redis_connection
def func():
global redis_connection
redis_connection = 1
redis_connection()
```
So, above, `redis_connection` is still marked as unused.
But it does avoid flagging `redis_connection` as unused in:
```python
from app import redis_connection
def func():
global redis_connection
redis_connection()
```
Closes https://github.com/astral-sh/ruff/issues/11518.
## Summary
Follow up to https://github.com/astral-sh/ruff/pull/11521
Removes the extra added complexity for catch all match cases. This
matches the implementation of plain `else` statements.
## Test Plan
Added new test cases.
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
## Summary
This PR brings back the functionality to remove empty strings when
converting to an f-string in `UP032`.
For context, https://github.com/astral-sh/ruff/pull/8712 added this
functionality to remove _trailing_ empty strings but it got removed in
https://github.com/astral-sh/ruff/pull/8697 possibly unexpectedly so.
There's one difference which is that this PR will remove _any_ empty
strings and not just trailing ones. For example,
```diff
--- /Users/dhruv/playground/ruff/src/UP032.py
+++ /Users/dhruv/playground/ruff/src/UP032.py
@@ -1,7 +1,5 @@
(
- "{a}"
- ""
- "{b}"
- ""
-).format(a=1, b=1)
+ f"{1}"
+ f"{1}"
+)
```
## Test Plan
Run `cargo insta test` and update the snapshots.
## Summary
This PR updates the sequence sorting (`RUF022` and `RUF023`) to avoid
using the owned data from the string token. Instead, we will directly
use the reference to the data on the AST. This does introduce a lot of
lifetimes but that's required.
The main motivation for this is to allow removing the `lex_starts_at`
usage easily.
### Alternatives
1. Extract the raw string content (stripping the prefix and quotes)
using the `Locator` and use that for comparison
2. Build up an
[`IndexVec`](3e30962077/crates/ruff_index/src/vec.rs)
and use the newtype index in place of the string value itself. This also
does require lifetimes so we might as well just use the method in this
PR.
## Test Plan
`cargo insta test` and no ecosystem changes
## Summary
Concurrent GitLab runners clone projects into separate directories, e.g.
`{builds_dir}/$RUNNER_TOKEN_KEY/$CONCURRENT_ID/$NAMESPACE/$PROJECT_NAME`.
Since the fingerprint uses the full path to the file, the fingerprints
calculated by Ruff are different depending on which concurrent runner it
executes on, so often an MR will appear to remove all existing issues
and add them with new fingerprints.
I've adjusted the fingerprint function to use the project relative path,
which fixes this. Unfortunately this will have a breaking change for any
current users of this output - the fingerprints will change and appear
in GitLab as all linting messages having been fixed and then created.
## Test Plan
`cargo nextest run`
Running `ruff check --output-format gitlab` in a git repo, moving the
repo and running again, verifying no diffs between the outputs
Hi!
I left out some of the functions in the migration rule which became
removed in NumPy 2.0:
- `np.alltrue`
- `np.anytrue`
- `np.cumproduct`
- `np.product`
Addressing: https://github.com/numpy/numpy/issues/26493
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Current doc says `sys.version[0]` will select the first digit of a major
version number (correct) then as an example says
> e.g., `"3.10"` would evaluate to `"1"`
(would actually evaluate to `"3"`). Changed the example version to a
two-digit number to make the problem more clear.
## Test Plan
<!-- How was it tested? -->
ran the following:
- `cargo run -p ruff -- check
crates/ruff_linter/resources/test/fixtures/flake8_2020/YTT301.py
--no-cache`
- `cargo insta review`
- `cargo test`
which all passed.
## Summary
Rule `logging-warn` (`G010`) prescribes a change from `warn` to
`warning` and has a corresponding autofix, but the autofix is mistakenly
titled ```"Convert to `warn`"``` instead of ```"Convert to `warning`"```
(the latter is what the autofix actually does). Seems to be a plain
typo.
## Summary
It turns out that `singledispatch` does end up evaluating all arguments,
even though only the first is used to dispatch.
Closes https://github.com/astral-sh/ruff/issues/11520.
## Summary
Addresses #8451 by implementing rule 116 to add an unsafe fix when sleep
is used with a >24 hour interval to instead consider sleeping forever.
This rule is added as async instead as I my understanding was that these
trio rules would be moved to async anyway.
There are a couple of TODOs, which address further extending the rule by
adding support for lookups and evaluations, and also supporting `anyio`.
## Summary
This PR updates the `FA102` rule logic to use the `Importer` which is
available on the `Checker`.
The main motivation is that this would make updating the `Importer` to
use the `Tokens` struct which will be required to remove the
`lex_starts_at` usage in `Insertion::start_of_block` method.
## Test Plan
`cargo insta test`
## Summary
Similar to #11414, this PR extends `UP037` to flag quoted annotations
that are located in positions that won't be evaluated at runtime.
For example, the quotes on `Tuple` are unnecessary in:
```python
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from typing import Tuple
def foo():
x: "Tuple[int, int]" = (0, 0)
foo()
```
## Summary
This PR moves the `has_comments` function from `Indexer` to
`CommentRanges`. The main motivation is that the `CommentRanges` will
now be built by the parser which is shared between the linter and the
formatter. Thus, the `CommentRanges` will be removed from the `Indexer`.
## Test Plan
`cargo test`
## Summary
Matching Pylint, we now omit the `try` body itself from branch counting.
Each `except` counts as a branch, as does the `else` and the `finally`.
Closes https://github.com/astral-sh/ruff/issues/11205.
The wording 'negative comparison' is a rather vague description of the
'is not' operation and does not describe what the 'not in' operation
does (potentially copied from 'is not'). This was replaced with more
precise language to describe the operators taken from the official
python docs[1].
Both rules didn't have a strong reasoning besides 'it's bad, use the
other'. The origin of these rules seems to be PEP8[2] which prefers 'is
not' over 'not ... is' for readability. This is now reflected in the
description.
[1]:
https://docs.python.org/3/reference/expressions.html#membership-test-operations
[2]: https://peps.python.org/pep-0008/#programming-recommendations
## Summary
If an annotation won't be evaluated at runtime, we don't need to flag
`from __future__ import annotations` as required. This applies both to
quoted annotations and annotations outside of runtime-evaluated
positions, like:
```python
def main() -> None:
a_list: list[str] | None = []
a_list.append("hello")
```
Closes https://github.com/astral-sh/ruff/issues/11397.
## Summary
* Update documentation for F401 following recent PRs
* #11168
* #11314
* Deprecate `ignore_init_module_imports`
* Add a deprecation pragma to the option and a "warn user once" message
when the option is used.
* Restore the old behavior for stable (non-preview) mode:
* When `ignore_init_module_imports` is set to `true` (default) there are
no `__init_.py` fixes (but we get nice fix titles!).
* When `ignore_init_module_imports` is set to `false` there are unsafe
`__init__.py` fixes to remove unused imports.
* When preview mode is enabled, it overrides
`ignore_init_module_imports`.
* Fixed a bug in fix titles where `import foo as bar` would recommend
reexporting `bar as bar`. It now says to reexport `foo as foo`. (In this
case we don't issue a fix, fwiw; it was just a fix title bug.)
## Test plan
Added new fixture tests that reuse the existing fixtures for
`__init__.py` files. Each of the three situations listed above has
fixture tests. The F401 "stable" tests cover:
> * When `ignore_init_module_imports` is set to `true` (default) there
are no `__init_.py` fixes (but we get nice fix titles!).
The F401 "deprecated option" tests cover:
> * When `ignore_init_module_imports` is set to `false` there are unsafe
`__init__.py` fixes to remove unused imports.
These complement existing "preview" tests that show the new behavior
which recommends fixes in `__init__.py` according to whether the import
is 1st party and other circumstances (for more on that behavior see:
#11314).
## Summary
This is a follow-up PR to #11445 update the `E27` rules to consider soft
keywords as well.
## Test Plan
Add test cases consisting of soft keywords and update the snapshot.
## Summary
We weren't treating the escaped newline as a valid condition to trigger
the safer fix (add an extra backslash before each invalid escape
sequence).
Closes https://github.com/astral-sh/ruff/issues/11461.
## Summary
This PR updates the `TokenKind::is_keyword` check to include soft
keywords. To account for this change, it adds a new
`is_non_soft_keyword` method.
The usage in logical line rules were updated to use the
`is_non_soft_keyword` method but it'll be updated to use `is_keyword` in
a follow-up PR (#11446).
While, the parser usages were kept as is. And because of that, the
snapshots for two test cases were updated in a better direction.
## Test Plan
`cargo insta test`
## Summary
We already have handling for "references that get quoted within our
quoted references", but we were assuming a specific ordering in the way
edits were generated.
Closes https://github.com/astral-sh/ruff/issues/11449.
## Summary
As discussed in issue #11408, PLR0912 has a broader definition of
"branches" than I expected. This updates the documentation to include
this definition.
I also updated the example to include several different types of
branches, while still maintaining dictionary lookup as an alternative
solution. (Crafting a realistic example was quite a challenge 😅).
Closes https://github.com/astral-sh/ruff/issues/11408.
## Summary
This moves the string-prefix enumerations in `ruff_python_ast` to a
separate submodule. I think this helps clarify that these prefixes are
purely abstract: they only depend on each other, and do not depend on
any of the other code in `nodes.rs` in any way. Moreover, while various
AST nodes _use_ them, they're not really nodes themselves, so they feel
slightly out of place in `nodes.rs`.
I considered moving all of them to `str.rs`, but it felt like enough
code that it could be a separate submodule.
## Test Plan
`cargo test`
Followup on #11168 and resolve#10391
# User facing changes
* F401 now recommends a fix to add unused import bindings to to
`__all__` if a single `__all__` list or tuple is found in `__init__.py`.
* If there are no `__all__` found in the file, fall back to recommending
redundant-aliases.
* If there are multiple `__all__` or only one but of the wrong type (non
list or tuple) then diagnostics are generated without fixes.
* `fix_title` is updated to reflect what the fix/recommendation is.
Subtlety: For a renamed import such as `import foo as bees`, we can
generate a fix to add `bees` to `__all__` but cannot generate a fix to
produce a redundant import (because that would break uses of the binding
`bees`).
# Implementation changes
* Add `name` field to `ImportBinding` to contain the name of the
_binding_ we want to add to `__all__` (important for the `import foo as
bees` case). It previously only contained the `AnyImport` which can give
us information about the import but not the binding.
* Add `binding` field to `UnusedImport` to contain the same. (Naming
note: the field `name` field already existed on `UnusedImport` and
contains the qualified name of the imported symbol/module)
* Change `fix_by_reexporting` to branch on the size of `dunder_all:
Vec<&Expr>`
* For length 0 call the edit-producing function `make_redundant_alias`.
* For length 1 call edit-producing function `add_to_dunder_all`.
* Otherwise, produce no fix.
* Implement the edit-producing function `add_to_dunder_all` and add unit
tests.
* Implement several fixture tests: empty `__all__ = []`, nonempty
`__all__ = ["foo"]`, mis-typed `__all__ = None`, plus-eq `__all__ +=
["foo"]`
* `UnusedImportContext::Init` variant now has two fields: whether the
fix is in `__init__.py` and how many `__all__` were found.
# Other changes
* Remove a spurious pattern match and instead use field lookups b/c the
addition of a field would have required changing the unrelated pattern.
* Tweak input type of `make_redundant_alias`
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
## Summary
This PR follows up from #11420 to move `UP034` to use `TokenKind`
instead of `Tok`.
The main reason to have a separate PR is so that the reviewing is easy.
This required a lot more updates because the rule used an index (`i`) to
keep track of the current position in the token vector. Now, as it's
just an iterator, we just use `next` to move the iterator forward and
extract the relevant information.
This is part of https://github.com/astral-sh/ruff/issues/11401
## Test Plan
`cargo test`
## Summary
This PR moves the following rules to use `TokenKind` instead of `Tok`:
* `PLE2510`, `PLE2512`, `PLE2513`, `PLE2514`, `PLE2515`
* `E701`, `E702`, `E703`
* `ISC001`, `ISC002`
* `COM812`, `COM818`, `COM819`
* `W391`
I've paused here because the next set of rules
(`pyupgrade::rules::extraneous_parentheses`) indexes into the token
slice but we only have an iterator implementation. So, I want to isolate
that change to make sure the logic is still the same when I move to
using the iterator approach.
This is part of #11401
## Test Plan
`cargo test`
## Summary
Alternative to #11237
This PR adds a new `Tokens` struct which is a newtype wrapper around a
vector of lexer output. This allows us to add a `kinds` method which
returns an iterator over the corresponding `TokenKind`. This iterator is
implemented as a separate `TokenKindIter` struct to allow using the type
and provide additional methods like `peek` directly on the iterator.
This exposes the linter to access the stream of `TokenKind` instead of
`Tok`.
Edit: I've made the necessary downstream changes and plan to merge the
entire stack at once.
## Summary
This PR adds a newtype wrapper around `Vec<FStringElement>` that derefs
to a `&Vec<FStringElement>`.
Both f-string and format specifier are made up of `Vec<FStringElement>`.
By creating a newtype wrapper around it, we can share the methods for
both parent types.
## Summary
This PR adds support to iterate over each part of a string-like
expression.
This similar to the one in the formatter:
128414cd95/crates/ruff_python_formatter/src/string/any.rs (L121-L125)
Although I don't think it's a 1-1 replacement in the formatter because
the one implemented in the formatter has another information for certain
variants (as can be seen for `FString`).
The main motivation for this is to avoid duplication for rules which
work only on the parts of the string and doesn't require any information
from the parent node. Here, the parent node being the expression node
which could be an implicitly concatenated string.
This PR also updates certain rule implementation to make use of this and
avoids logic duplication.
## Summary
This PR renames `AnyStringKind` to `AnyStringFlags` and `AnyStringFlags`
to `AnyStringFlagsInner`.
The main motivation is to have consistent usage of "kind" and "flags".
For each string kind, it's "flags" like `StringLiteralFlags`,
`BytesLiteralFlags`, and `FStringFlags` but it was `AnyStringKind` for
the "any" variant.
## Summary
Changes `future-rewritable-type-annotation` (`FA100`) message to be less
confusing. Uses phrasing from the rule documentation to be consistent.
For example,
```
from_typing_import.py:5:13: FA100 Add `from __future__ import annotations` to rewrite `typing.List` more succinctly
```
Closes#10573.
## Test Plan
`cargo nextest run`
## Summary
Should this consider the decorator only if the name is actually a
property or is the logic in this PR correct?
fixes: #11358
## Test Plan
Add test case.
## Summary
This PR fixes a bug where the auto-fix for `TCH005` would delete the
entire `if` statement.
The fix in this PR is to not consider it a violation if there are any
`elif`/`else` blocks. This also matches the behavior of the original
plugin.
fixes: #11368
## Test plan
Add test cases.
## Summary
`--add-noqa` now runs in two stages: first, the linter finds all
diagnostics that need noqa comments and generate edits on a per-line
basis. Second, these edits are applied, in order, to the document.
A public-facing function, `generate_noqa_edits`, has also been
introduced, which returns noqa edits generated on a per-diagnostic
basis. This will be used by `ruff server` for noqa comment quick-fixes.
## Test Plan
Unit tests have been updated.
## Summary
This PR adds updates the semantic model to detect attribute docstring.
Refer to [PEP 258](https://peps.python.org/pep-0258/#attribute-docstrings)
for the definition of an attribute docstring.
This PR doesn't add full support for it but only considers string
literals as attribute docstring for the following cases:
1. A string literal following an assignment statement in the **global
scope**.
2. A global class attribute
For an assignment statement, it's considered an attribute docstring only
if the target expression is a name expression (`x = 1`). So, chained
assignment, multiple assignment or unpacking, and starred expression,
which are all valid in the target position, aren't considered here.
In `__init__` method, an assignment to the `self` variable like `self.x = 1`
is also a candidate for an attribute docstring. **This PR does not
support this position.**
## Test Plan
I used the following source code along with a print statement to verify
that the attribute docstring detection is correct.
Refer to the PR description for the code snippet.
I'll add this in the follow-up PR
(https://github.com/astral-sh/ruff/pull/11302) which uses this method.
<!--
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
Resolves#11263
Detect `pathlib.Path.open` calls which do not specify a file encoding.
## Test Plan
Test cases added to fixture.
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Resolves https://github.com/astral-sh/ruff/issues/11313
## Summary
PLR0912(too-many-branches) did not count branches inside with: blocks.
With this fix, the branches inside with statements are also counted.
## Test Plan
Added a new test case.
## Summary
While I was here, I also updated the rule to use
`function_type::classify` rather than hard-coding `staticmethod` and
friends.
Per Carl:
> Enum instances are already referred to by the class, forming a cycle
that won't get collected until the class itself does. At which point the
`lru_cache` itself would be collected, too.
Closes https://github.com/astral-sh/ruff/issues/9912.
## Summary
Historically, we only ignored `flake8-blind-except` if you re-raised or
logged the exception as a _direct_ child statement; but it could be
nested somewhere. This was just a known limitation at the time of adding
the previous logic.
Closes https://github.com/astral-sh/ruff/issues/11289.
## Summary
In #9218 `Rule::NeverUnion` was partially removed from a
`checker.any_enabled` call. This makes the change consistent.
## Test Plan
`cargo test`
Resolves#10390 and starts to address #10391
# Changes to behavior
* In `__init__.py` we now offer some fixes for unused imports.
* If the import binding is first-party this PR suggests a fix to turn it
into a redundant alias.
* If the import binding is not first-party, this PR suggests a fix to
remove it from the `__init__.py`.
* The fix-titles are specific to these new suggested fixes.
* `checker.settings.ignore_init_module_imports` setting is
deprecated/ignored. There is probably a documentation change to make
that complete which I haven't done.
---
<details><summary>Old description of implementation changes</summary>
# Changes to the implementation
* In the body of the loop over import statements that contain unused
bindings, the bindings are partitioned into `to_reexport` and
`to_remove` (according to how we want to resolve the fact they're
unused) with the following predicate:
```rust
in_init && is_first_party(checker, &import.qualified_name().to_string())
// true means make it a reexport
```
* Instead of generating a single fix per import statement, we now
generate up to two fixes per import statement:
```rust
(fix_by_removing_imports(checker, node_id, &to_remove, in_init).ok(),
fix_by_reexporting(checker, node_id, &to_reexport, dunder_all).ok())
```
* The `to_remove` fixes are unsafe when `in_init`.
* The `to_explicit` fixes are safe. Currently, until a future PR, we
make them redundant aliases (e.g. `import a` would become `import a as
a`).
## Other changes
* `checker.settings.ignore_init_module_imports` is deprecated/ignored.
Instead, all fixes are gated on `checker.settings.preview.is_enabled()`.
* Got rid of the pattern match on the import-binding bound by the inner
loop because it seemed less readable than referencing fields on the
binding.
* [x] `// FIXME: rename "imports" to "bindings"` if reviewer agrees (see
code)
* [x] `// FIXME: rename "node_id" to "import_statement"` if reviewer
agrees (see code)
<details>
<summary><h2>Scope cut until a future PR</h2></summary>
* (Not implemented) The `to_explicit` fixes will be added to `__all__`
unless it doesn't exist. When `__all__` doesn't exist they're resolved
by converting to redundant aliases (e.g. `import a` would become `import
a as a`).
---
</details>
# Test plan
* [x] `crates/ruff_linter/resources/test/fixtures/pyflakes/F401_24`
contains an `__init__.py` with*out* `__all__` that exercises the
features in this PR, but it doesn't pass.
* [x]
`crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25_dunder_all`
contains an `__init__.py` *with* `__all__` that exercises the features
in this PR, but it doesn't pass.
* [x] Write unit tests for the new edit functions in
`fix::edits::make_redundant_alias`.
</details>
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
## Summary
This PR removes the `ImportMap` implementation and all its routing
through ruff.
The import map was added in https://github.com/astral-sh/ruff/pull/3243
but we then never ended up using it to do cross file analysis.
We are now working on adding multifile analysis to ruff, and revisit
import resolution as part of it.
```
hyperfine --warmup 10 --runs 20 --setup "./target/release/ruff clean" \
"./target/release/ruff check crates/ruff_linter/resources/test/cpython -e -s --extend-select=I" \
"./target/release/ruff-import check crates/ruff_linter/resources/test/cpython -e -s --extend-select=I"
Benchmark 1: ./target/release/ruff check crates/ruff_linter/resources/test/cpython -e -s --extend-select=I
Time (mean ± σ): 37.6 ms ± 0.9 ms [User: 52.2 ms, System: 63.7 ms]
Range (min … max): 35.8 ms … 39.8 ms 20 runs
Benchmark 2: ./target/release/ruff-import check crates/ruff_linter/resources/test/cpython -e -s --extend-select=I
Time (mean ± σ): 36.0 ms ± 0.7 ms [User: 50.3 ms, System: 58.4 ms]
Range (min … max): 34.5 ms … 37.6 ms 20 runs
Summary
./target/release/ruff-import check crates/ruff_linter/resources/test/cpython -e -s --extend-select=I ran
1.04 ± 0.03 times faster than ./target/release/ruff check crates/ruff_linter/resources/test/cpython -e -s --extend-select=I
```
I suspect that the performance improvement should even be more
significant for users that otherwise don't have any diagnostics.
```
hyperfine --warmup 10 --runs 20 --setup "cd ../ecosystem/airflow && ../../ruff/target/release/ruff clean" \
"./target/release/ruff check ../ecosystem/airflow -e -s --extend-select=I" \
"./target/release/ruff-import check ../ecosystem/airflow -e -s --extend-select=I"
Benchmark 1: ./target/release/ruff check ../ecosystem/airflow -e -s --extend-select=I
Time (mean ± σ): 53.7 ms ± 1.8 ms [User: 68.4 ms, System: 63.0 ms]
Range (min … max): 51.1 ms … 58.7 ms 20 runs
Benchmark 2: ./target/release/ruff-import check ../ecosystem/airflow -e -s --extend-select=I
Time (mean ± σ): 50.8 ms ± 1.4 ms [User: 50.7 ms, System: 60.9 ms]
Range (min … max): 48.5 ms … 55.3 ms 20 runs
Summary
./target/release/ruff-import check ../ecosystem/airflow -e -s --extend-select=I ran
1.06 ± 0.05 times faster than ./target/release/ruff check ../ecosystem/airflow -e -s --extend-select=I
```
## Test Plan
`cargo test`
## Summary
I think the check included here does make sense, but I don't see why we
would allow it if a value is provided for the attribute -- since, in
that case, isn't it _not_ abstract?
Closes: https://github.com/astral-sh/ruff/issues/11208.
## Summary
This PR adds an override to the fixer to ensure that we apply any
`redefined-while-unused` fixes prior to `unused-import`.
Closes https://github.com/astral-sh/ruff/issues/10905.
## Summary
Implement duplicate code detection as part of `RUF100`, mirroring the
behavior of `flake8-noqa` (`NQA005`) mentioned in #850. The idea to
merge the rule into `RUF100` was suggested by @MichaReiser
https://github.com/astral-sh/ruff/pull/10325#issuecomment-2025535444.
## Test Plan
Test cases were added to the fixture.
This syntax wasn't "deprecated" in Python 3; it was removed.
I started looking at this rule because I was curious how Ruff could even
detect this without a Python 2 parser. Then I realized that
"print >> f, x" is actually valid Python 3 syntax: it creates a tuple
containing a right-shifted version of the print function.
## Summary
Based on discussion in #10850.
As it stands today `RUF100` will attempt to replace code redirects with
their target codes even though this is not the "goal" of `RUF100`. This
behavior is confusing and inconsistent, since code redirects which don't
otherwise violate `RUF100` will not be updated. The behavior is also
undocumented. Additionally, users who want to use `RUF100` but do not
want to update redirects have no way to opt out.
This PR explicitly detects redirects with a new rule `RUF101` and
patches `RUF100` to keep original codes in fixes and reporting.
## Test Plan
Added fixture.
## Summary
Resolves#11102
The error stems from these lines
f5c7a62aa6/crates/ruff_linter/src/noqa.rs (L697-L702)
I don't really understand the purpose of incrementing the last index,
but it makes the resulting range invalid for indexing into `contents`.
For now I just detect if the index is too high in `blanket_noqa` and
adjust it if necessary.
## Test Plan
Created fixture from issue example.
## Summary
This allows `raise from` in BLE001.
```python
try:
...
except Exception as e:
raise ValueError from e
```
Fixes#10806
## Test Plan
Test case added.
## Summary
This PR refactors unary expression parsing with the following changes:
* Ability to get `OperatorPrecedence` from a unary operator (`UnaryOp`)
* Implement methods on `TokenKind`
* Add `as_unary_operator` which returns an `Option<UnaryOp>`
* Add `as_unary_arithmetic_operator` which returns an `Option<UnaryOp>`
(used for pattern parsing)
* Rename `is_unary` to `is_unary_arithmetic_operator` (used in the
linter)
resolves: #10752
## Test Plan
Verify that the existing test cases pass, no ecosystem changes, run the
Python based fuzzer on 3000 random inputs and run it on dozens of
open-source repositories.
## Summary
Fixes#10463
Add `FURB192` which detects violations like this:
```python
# Bad
a = sorted(l)[0]
# Good
a = min(l)
```
There is a caveat that @Skylion007 has pointed out, which is that
violations with `reverse=True` technically aren't compatible with this
change, in the edge case where the unstable behavior is intended. For
example:
```python
from operator import itemgetter
data = [('red', 1), ('blue', 1), ('red', 2), ('blue', 2)]
min(data, key=itemgetter(0)) # ('blue', 1)
sorted(data, key=itemgetter(0))[0] # ('blue', 1)
sorted(data, key=itemgetter(0), reverse=True)[-1] # ('blue, 2')
```
This seems like a rare edge case, but I can make the `reverse=True`
fixes unsafe if that's best.
## Test Plan
This is unit tested.
## References
https://github.com/dosisod/refurb/pull/333/files
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
The `operator.itemgetter` behavior changes where there's more than one
argument, such that `operator.itemgetter(0)` yields `r[0]`, rather than
`(r[0],)`.
Closes https://github.com/astral-sh/ruff/issues/11075.
## Summary
There is no class `integer` in python, nor is there a type `integer`, so
I updated the docs to remove the backticks on these references, such
that it is the representation of an integer, and not a reference.
## Summary
Move `blanket-noqa` rule from the token checker to the noqa checker.
This allows us to make use of the line directives already computed in
the noqa checker.
## Test Plan
Verified test results are unchanged.
Resolves#10187
<details>
<summary>Old PR description; accurate through commit e86dd7d; probably
best to leave this fold closed</summary>
## Description of change
In the case of a printf-style format string with only one %-placeholder
and a variable at right (e.g. `"%s" % var`):
* The new behavior attempts to dereference the variable and then match
on the bound expression to distinguish between a 1-tuple (fix), n-tuple
(bug 🐛), or a non-tuple (fix). Dereferencing is via
`analyze::typing::find_binding_value`.
* If the variable cannot be dereferenced, then the type-analysis routine
is called to distinguish only tuple (no-fix) or non-tuple (fix). Type
analysis is via `analyze::typing::is_tuple`.
* If any of the above fails, the rule still fires, but no fix is
offered.
## Alternatives
* If the reviewers think that singling out the 1-tuple case is too
complicated, I will remove that.
* The ecosystem results show that no new fixes are detected. So I could
probably delete all the variable dereferencing code and code that tries
to generate fixes, tbh.
## Changes to existing behavior
**All the previous rule-firings and fixes are unchanged except for** the
"false negatives" in
`crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_1.py`. Those
previous "false negatives" are now true positives and so I moved them to
`crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py`.
<details>
<summary>Existing false negatives that are now true positives</summary>
```
crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py:134:1: UP031 Use format specifiers instead of percent format
|
133 | # UP031 (no longer false negatives)
134 | 'Hello %s' % bar
| ^^^^^^^^^^^^^^^^ UP031
135 |
136 | 'Hello %s' % bar.baz
|
= help: Replace with format specifiers
crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py:136:1: UP031 Use format specifiers instead of percent format
|
134 | 'Hello %s' % bar
135 |
136 | 'Hello %s' % bar.baz
| ^^^^^^^^^^^^^^^^^^^^ UP031
137 |
138 | 'Hello %s' % bar['bop']
|
= help: Replace with format specifiers
crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py:138:1: UP031 Use format specifiers instead of percent format
|
136 | 'Hello %s' % bar.baz
137 |
138 | 'Hello %s' % bar['bop']
| ^^^^^^^^^^^^^^^^^^^^^^^ UP031
|
= help: Replace with format specifiers
```
One of them newly offers a fix.
```
# UP031 (no longer false negatives)
-'Hello %s' % bar
+'Hello {}'.format(bar)
```
This fix occurs because the new code dereferences `bar` to where it was
defined earlier in the file as a non-tuple:
```python
bar = {"bar": y}
```
---
</details>
## Behavior requiring new tests
Additionally, we now handle a few cases that we didn't previously test.
These cases are when a string has a single %-placeholder and the
righthand operand to the modulo operator is a variable **which can be
dereferenced.** One of those was shown in the previous section (the
"dereference non-tuple" case).
<details>
<summary>New cases handled</summary>
```
crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py:126:1: UP031 [*] Use format specifiers instead of percent format
|
125 | t1 = (x,)
126 | "%s" % t1
| ^^^^^^^^^ UP031
127 | # UP031: deref t1 to 1-tuple, offer fix
|
= help: Replace with format specifiers
crates/ruff_linter/resources/test/fixtures/pyupgrade/UP031_0.py:130:1: UP031 Use format specifiers instead of percent format
|
129 | t2 = (x,y)
130 | "%s" % t2
| ^^^^^^^^^ UP031
131 | # UP031: deref t2 to n-tuple, this is a bug
|
= help: Replace with format specifiers
```
One of these offers a fix.
```
t1 = (x,)
-"%s" % t1
+"{}".format(t1[0])
# UP031: deref t1 to 1-tuple, offer fix
```
The other doesn't offer a fix because it's a bug.
---
</details>
---
</details>
## Changes to existing behavior
In the case of a string with a single %-placeholder and a single
ambiguous righthand argument to the modulo operator, (e.g. `"%s" % var`)
the rule now fires and offers a fix. We explain about this in the "fix
safety" section of the updated documentation.
## Documentation changes
I swapped the order of the "known problems" and the "examples" sections
so that the examples which describe the rule are first, before the
exceptions to the rule are described. I also tweaked the language to be
more explicit, as I had trouble understanding the documentation at
first. The "known problems" section is now "fix safety" but the content
is largely similar.
The diff of the documentation changes looks a little difficult unless
you look at the individual commits.
## Summary
I happened to notice that we box `TypeParams` on `StmtClassDef` but not
on `StmtFunctionDef` and wondered why, since `StmtFunctionDef` is bigger
and sets the size of `Stmt`.
@charliermarsh found that at the time we started boxing type params on
classes, classes were the largest statement type (see #6275), but that's
no longer true.
So boxing type-params also on functions reduces the overall size of
`Stmt`.
## Test Plan
The `<=` size tests are a bit irritating (since their failure doesn't
tell you the actual size), but I manually confirmed that the size is
actually 120 now.
Occasionally you intentionally have iterables of differing lengths. The
rule permits this by explicitly adding `strict=False`, but this was not
documented.
## Summary
The rule does not currently document how to avoid it when having
differing length iterables is intentional. This PR adds that to the rule
documentation.
Add pylint rule invalid-hash-returned (PLE0309)
See https://github.com/astral-sh/ruff/issues/970 for rules
Test Plan: `cargo test`
TBD: from the description: "Strictly speaking `bool` is a subclass of
`int`, thus returning `True`/`False` is valid. To be consistent with
other rules (e.g.
[PLE0305](https://github.com/astral-sh/ruff/pull/10962)
invalid-index-returned), ruff will raise, compared to pylint which will
not raise."
(Supersedes #9152, authored by @LaBatata101)
## Summary
This PR replaces the current parser generated from LALRPOP to a
hand-written recursive descent parser.
It also updates the grammar for [PEP
646](https://peps.python.org/pep-0646/) so that the parser outputs the
correct AST. For example, in `data[*x]`, the index expression is now a
tuple with a single starred expression instead of just a starred
expression.
Beyond the performance improvements, the parser is also error resilient
and can provide better error messages. The behavior as seen by any
downstream tools isn't changed. That is, the linter and formatter can
still assume that the parser will _stop_ at the first syntax error. This
will be updated in the following months.
For more details about the change here, refer to the PR corresponding to
the individual commits and the release blog post.
## Test Plan
Write _lots_ and _lots_ of tests for both valid and invalid syntax and
verify the output.
## Acknowledgements
- @MichaReiser for reviewing 100+ parser PRs and continuously providing
guidance throughout the project
- @LaBatata101 for initiating the transition to a hand-written parser in
#9152
- @addisoncrump for implementing the fuzzer which helped
[catch](https://github.com/astral-sh/ruff/pull/10903)
[a](https://github.com/astral-sh/ruff/pull/10910)
[lot](https://github.com/astral-sh/ruff/pull/10966)
[of](https://github.com/astral-sh/ruff/pull/10896)
[bugs](https://github.com/astral-sh/ruff/pull/10877)
---------
Co-authored-by: Victor Hugo Gomes <labatata101@linuxmail.org>
Co-authored-by: Micha Reiser <micha@reiser.io>
Add pylint rule invalid-length-returned (PLE0303)
See https://github.com/astral-sh/ruff/issues/970 for rules
Test Plan: `cargo test`
TBD: from the description: "Strictly speaking `bool` is a subclass of
`int`, thus returning `True`/`False` is valid. To be consistent with
other rules (e.g.
[PLE0305](https://github.com/astral-sh/ruff/pull/10962)
invalid-index-returned), ruff will raise, compared to pylint which will
not raise."
## Summary
If the user is analyzing a script (i.e., we have no module path), it
seems reasonable to use the script name when trying to identify paths to
objects defined _within_ the script.
Closes https://github.com/astral-sh/ruff/issues/10960.
## Test Plan
Ran:
```shell
check --isolated --select=B008 \
--config 'lint.flake8-bugbear.extend-immutable-calls=["test.A"]' \
test.py
```
On:
```python
class A: pass
def f(a=A()):
pass
```
## Summary
This PR switches more callsites of `SemanticModel::is_builtin` to move
over to the new methods I introduced in #10919, which are more concise
and more accurate. I missed these calls in the first PR.
## Summary
This change adds a rule to detect functions declared `async` but lacking
any of `await`, `async with`, or `async for`. This resolves#9951.
## Test Plan
This change was tested by following
https://docs.astral.sh/ruff/contributing/#rule-testing-fixtures-and-snapshots
and adding positive and negative cases for each of `await` vs nothing,
`async with` vs `with`, and `async for` vs `for`.
## Summary
This PR moves the `Q003` rule to AST checker.
This is the final rule that used the docstring detection state machine
and thus this PR removes it as well.
resolves: #7595resolves: #7808
## Test Plan
- [x] `cargo test`
- [x] Make sure there are no changes in the ecosystem
## Summary
Adds more aggressive logic to PLR1730, `if-stmt-min-max`
Closes#10907
## Test Plan
`cargo test`
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
<!--
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
Hi! 👋
Thanks for sharing ruff as software libre — it helps me keep Python code
quality up with pre-commit, both locally and CI 🙏
While studying the examples at
https://docs.astral.sh/ruff/rules/function-uses-loop-variable/#example I
noticed that the last of the examples had a bug: prior to this fix, `ì`
was passed to the lambda for `x` rather than for `i` — the two are
mixed-up. The reason it's easy to overlook is because addition is an
commutative operation and so `x + i` and `i + x` give the same result
(and least with integers), despite the mix-up. For proof, let me demo
the relevant part with before and after:
```python
In [1]: from functools import partial
In [2]: [partial(lambda x, i: (x, i), i)(123) for i in range(3)]
Out[2]: [(0, 123), (1, 123), (2, 123)]
In [3]: [partial(lambda x, i: (x, i), i=i)(123) for i in range(3)]
Out[3]: [(123, 0), (123, 1), (123, 2)]
```
Does that make sense?
## Test Plan
<!-- How was it tested? -->
Was manually tested using IPython.
CC @r4f @grandchild
## Summary
If `RUF100` was included in a per-file-ignore, we respected it on cases
like `# noqa: F401`, but not the blanket variant (`# noqa`).
Closes https://github.com/astral-sh/ruff/issues/10906.
## Summary
Implement new rule: Prefer augmented assignment (#8877). It checks for
the assignment statement with the form of `<expr> = <expr>
<binary-operator> …` with a unsafe fix to use augmented assignment
instead.
## Test Plan
1. Snapshot test is included in the PR.
2. Manually test with playground.
## Summary
This PR adds the implementation for the current
[flake8-bugbear](https://github.com/PyCQA/flake8-bugbear)'s B038 rule.
The B038 rule checks for mutation of loop iterators in the body of a for
loop and alerts when found.
Rational:
Editing the loop iterator can lead to undesired behavior and is probably
a bug in most cases.
Closes#9511.
Note there will be a second iteration of B038 implemented in
`flake8-bugbear` soon, and this PR currently only implements the weakest
form of the rule.
I'd be happy to also implement the further improvements to B038 here in
ruff 🙂
See https://github.com/PyCQA/flake8-bugbear/issues/454 for more
information on the planned improvements.
## Test Plan
Re-using the same test file that I've used for `flake8-bugbear`, which
is included in this PR (look for the `B038.py` file).
Note: this is my first time using `rust` (beside `rustlings`) - I'd be
very happy about thorough feedback on what I could've done better
🙂 - Bring it on 😀
## Summary
Code cleanup for per-file ignores; use a struct instead of a tuple.
Named the structs for individual ignores and the list of ignores
`CompiledPerFileIgnore` and `CompiledPerFileIgnoreList`. Name choice is
because we already have a `PerFileIgnore` struct for a
pre-compiled-matchers form of the config. Name bikeshedding welcome.
## Test Plan
Refactor, should not change behavior; existing tests pass.
---------
Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
## Summary
Implement `write-whole-file` (`FURB103`), part of #1348. This is largely
a copy and paste of `read-whole-file` #7682.
## Test Plan
Text fixture added.
---------
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
## 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.
## 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
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 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
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
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.
<!--
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
<!-- 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
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.
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`
## 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
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.
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
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.
## 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`.
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
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>
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
```
## Summary
Fixes#10174 by allowing match cases to be enclosing nodes for
suppression comments. `else/elif` clauses are now also allowed to be
enclosing nodes.
## Test Plan
I've added the offending code from the original issue to the `RUF028`
snapshot test, and I've also expanded it to test the allowed `else/elif`
clause.
## Summary
This fixes https://github.com/astral-sh/ruff/issues/7868.
Support isort's `default-section` feature which allows any imports that
match sections that are not in `section-order` to be mapped to a
specifically named section.
https://pycqa.github.io/isort/docs/configuration/options.html#default-section
This has a few implications:
- It is no longer required that all known sections are defined in
`section-order`.
- This is technically a bw-incompat change because currently if folks
define custom groups, and do not define a `section-order`, the code used
to add all known sections to `section-order` while emitting warnings.
**However, when this happened, users would be seeing warnings so I do
not think it should count as a bw-incompat change.**
## Test Plan
- Added a new test.
- Did not break any existing tests.
Finally, I ran the following config against Pyramid's complex codebase
that was previously using isort and this change worked there.
### pyramid's previous isort config
5f7e286b06/pyproject.toml (L22-L37)
```toml
[tool.isort]
profile = "black"
multi_line_output = 3
src_paths = ["src", "tests"]
skip_glob = ["docs/*"]
include_trailing_comma = true
force_grid_wrap = false
combine_as_imports = true
line_length = 79
force_sort_within_sections = true
no_lines_before = "THIRDPARTY"
sections = "FUTURE,THIRDPARTY,FIRSTPARTY,LOCALFOLDER"
default_section = "THIRDPARTY"
known_first_party = "pyramid"
```
### tested with ruff isort config
```toml
[tool.ruff.lint.isort]
case-sensitive = true
combine-as-imports = true
force-sort-within-sections = true
section-order = [
"future",
"third-party",
"first-party",
"local-folder",
]
default-section = "third-party"
known-first-party = [
"pyramid",
]
```
<!--
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?
-->
Fixes#6611
## Summary
This lint rule spots comments that are _intended_ to suppress or enable
the formatter, but will be ignored by the Ruff formatter.
We borrow some functions the formatter uses for determining comment
placement / putting them in context within an AST.
The analysis function uses an AST visitor to visit each comment and
attach it to the AST. It then uses that context to check:
1. Is this comment in an expression?
2. Does this comment have bad placement? (e.g. a `# fmt: skip` above a
function instead of at the end of a line)
3. Is this comment redundant?
4. Does this comment actually suppress any code?
5. Does this comment have ambiguous placement? (e.g. a `# fmt: off`
above an `else:` block)
If any of these are true, a violation is thrown. The reported reason
depends on the order of the above check-list: in other words, a `# fmt:
skip` comment on its own line within a list expression will be reported
as being in an expression, since that reason takes priority.
The lint suggests removing the comment as an unsafe fix, regardless of
the reason.
## Test Plan
A snapshot test has been created.
## Summary
Adapts the fix for rule B006 to no longer modify the body of function
stubs, while retaining the change in method signature.
## Test Plan
The existing tests for B006 were adapted to reflect this change in
behavior.
## Relevant issue
https://github.com/astral-sh/ruff/issues/10083
## Summary
The `lxml` library has been modified to address known vulnerabilities
and unsafe defaults. As such, the `defusedxml`
library is no longer necessary, `defusedxml` has deprecated its `lxml`
module.
Closes https://github.com/astral-sh/ruff/issues/10030.
## Summary
Currently, rule `RUF015` is not able to detect the usage of
`list(iterable).pop(0)` falling under the category of an _unnecessary
iterable allocation for accessing the first element_. This PR wants to
change that. See the underlying issue for more details.
* Provide extension to detect `list(iterable).pop(0)`, but not
`list(iterable).pop(i)` where i > 1
* Update corresponding doc
## Test Plan
* `RUF015.py` and the corresponding snap file were extended such that
their correspond to the new behaviour
Closes https://github.com/astral-sh/ruff/issues/9190
---
PS: I've only been working on this ticket as I haven't seen any activity
from issue assignee @rmad17, neither in this repo nor in a fork. I hope
I interpreted his inactivity correctly. Didn't mean to steal his chance.
Since I stumbled across the underlying problem myself, I wanted to offer
a solution as soon as possible.
## Summary
It is a convention to use the `_()` alias for `gettext()`. We want to
avoid
statement expressions and assignments related to aliases of the gettext
API.
See https://docs.python.org/3/library/gettext.html for details. When one
uses `_() to mark a string for translation, the tools look for these
markers
and replace the original string with its translated counterpart. If the
string contains variable placeholders or formatting, it can complicate
the
translation process, lead to errors or incorrect translations.
## Test Plan
* Test file `RUF027_1.py` was extended such that the test reproduces the
false-positive
Closes https://github.com/astral-sh/ruff/issues/10023.
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
The original implementation of this applied the runtime-required context
to definitions _within_ the function, but not the signature itself. (We
had test coverage; the snapshot was just correctly showing the wrong
outcome.)
Closes https://github.com/astral-sh/ruff/issues/10089.
## Summary
Update PLR1714 to ignore `sys.platform` and `sys.version` checks.
I'm not sure if these checks or if we need to add more. Please advise.
Fixes#10017
## Test Plan
Added a new test case and ran `cargo nextest run`
## Summary
Allows, e.g.:
```python
import os
os.environ["WORLD_SIZE"] = "1"
os.putenv("CUDA_VISIBLE_DEVICES", "4")
import torch
```
For now, this is only allowed in preview.
Closes https://github.com/astral-sh/ruff/issues/10059
## Summary
Closes#10031
- Detect commented out `case` statements. Playground repro:
https://play.ruff.rs/5a305aa9-6e5c-4fa4-999a-8fc427ab9a23
- Add more support for one-line commented out code.
## Test Plan
Unit tested and tested with
```sh
cargo run -p ruff -- check crates/ruff_linter/resources/test/fixtures/eradicate/ERA001.py --no-cache --preview --select ERA001
```
TODO:
- [x] `cargo insta test`
## Summary
Part of #7595
This PR moves the `RUF001` and `RUF002` rules to the AST checker. This
removes the use of docstring detection from these rules.
## Test Plan
As this is just a refactor, make sure existing test cases pass.
## Summary
Fixes#9895
The cause for this panic came from an offset error in the code. When
analyzing a hypothetical f-string, we attempt to re-parse it as an
f-string, and use the AST data to determine, among other things, whether
the format specifiers are correct. To determine the 'correctness' of a
format specifier, we actually have to re-parse the format specifier, and
this is where the issue lies. To get the source text for the specifier,
we were taking a slice from the original file source text... even though
the AST data for the specifier belongs to the standalone parsed f-string
expression, meaning that the ranges are going to be way off. In a file
with Unicode, this can cause panics if the slice is inside a char
boundary.
To fix this, we now slice from the temporary source we created earlier
to parse the literal as an f-string.
## Test Plan
The RUF027 snapshot test was amended to include a string with format
specifiers which we _should_ be calling out. This is to ensure we do
slice format specifiers from the source text correctly.
## Summary
Ignore `async for` loops when checking the SIM113 rule.
Closes#9995
## Test Plan
A new test case was added to SIM113.py with an async for loop.
## Summary
This PR introduces a new semantic model flag `DOCSTRING` which suggests
that the model is currently in a module / class / function docstring.
This is the first step in eliminating the docstring detection state
machine which is prone to bugs as stated in #7595.
## Test Plan
~TODO: Is there a way to add a test case for this?~
I tested this using the following code snippet and adding a print
statement in the `string_like` analyzer to print if we're currently in a
docstring or not.
<details><summary>Test code snippet:</summary>
<p>
```python
"Docstring" ", still a docstring"
"Not a docstring"
def foo():
"Docstring"
"Not a docstring"
if foo:
"Not a docstring"
pass
class Foo:
"Docstring"
"Not a docstring"
foo: int
"Unofficial variable docstring"
def method():
"Docstring"
"Not a docstring"
pass
def bar():
"Not a docstring".strip()
def baz():
_something_else = 1
"""Not a docstring"""
```
</p>
</details>
## Summary
Implement [implicit readlines
(FURB129)](https://github.com/dosisod/refurb/blob/master/refurb/checks/iterable/implicit_readlines.py)
lint.
## Notes
I need a help/an opinion about suggested implementations.
This implementation differs from the original one from `refurb` in the
following way. This implementation checks syntactically the call of the
method with the name `readlines()` inside `for` {loop|generator
expression}. The implementation from refurb also
[checks](https://github.com/dosisod/refurb/blob/master/refurb/checks/iterable/implicit_readlines.py#L43)
that callee is a variable with a type `io.TextIOWrapper` or
`io.BufferedReader`.
- I do not see a simple way to implement the same logic.
- The best I can have is something like
```rust
checker.semantic().binding(checker.semantic().resolve_name(attr_expr.value.as_name_expr()?)?).statement(checker.semantic())
```
and analyze cases. But this will be not about types, but about guessing
the type by assignment (or with) expression.
- Also this logic has several false negatives, when the callee is not a
variable, but the result of function call (e.g. `open(...)`).
- On the other side, maybe it is good to lint this on other things,
where this suggestion is not safe, and push the developers to change
their interfaces to be less surprising, comparing with the standard
library.
- Anyway while the current implementation has false-positives (I
mentioned some of them in the test) I marked the fixes to be unsafe.
## Summary
Accept 0.0 and 1.0 as common magic values. This is in line with the
pylint behaviour, and I think makes sense conceptually.
## Test Plan
Test cases were added to
`crates/ruff_linter/resources/test/fixtures/pylint/magic_value_comparison.py`
## Summary
I was surprised to learn that we treat `x` in `[_ for x in y]` as an
"assignment" binding kind, rather than a dedicated comprehension
variable.
The docs previously mentioned an irrelevant config option, but were
missing a link to the relevant `ignore-init-module-imports` config
option which _is_ actually used.
Additionally, this commit adds a link to the documentation to explain
the conventions around a module interface which includes using a
redundant import alias to preserve an unused import.
(noticed this while filing #9962)
## Summary
This PR renames the semantic model flag `MODULE_DOCSTRING` to
`MODULE_DOCSTRING_BOUNDARY`. The main reason is for readability and for
the new semantic model flag `DOCSTRING` which tracks that the model is
in a module / class / function docstring.
I got confused earlier with the name until I looked at the use case and
it seems that the `_BOUNDARY` prefix is more appropriate for the
use-case and is consistent with other flags.
## Summary
This PR ensures that if a list `x` is modified within a `for` loop, we
avoid flagging `list(x)` as unnecessary. Previously, we only detected
calls to exactly `.append`, and they couldn't be nested within other
statements.
Closes https://github.com/astral-sh/ruff/issues/9925.
## Summary
If these are defined within class scopes, they're actually attributes of
the class, and can be accessed through the class itself.
(We preserve our existing behavior for `.pyi` files.)
Closes https://github.com/astral-sh/ruff/issues/9948.
Fixes#8368
Fixes https://github.com/astral-sh/ruff/issues/9186
## Summary
Arbitrary TOML strings can be provided via the command-line to override
configuration options in `pyproject.toml` or `ruff.toml`. As an example:
to run over typeshed and respect typeshed's `pyproject.toml`, but
override a specific isort setting and enable an additional pep8-naming
setting:
```
cargo run -- check ../typeshed --no-cache --config ../typeshed/pyproject.toml --config "lint.isort.combine-as-imports=false" --config "lint.extend-select=['N801']"
```
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
Co-authored-by: Zanie Blue <contact@zanie.dev>
## Summary
Currently these rules apply the heuristic that if the original sequence
doesn't have a newline in between the final sequence item and the
closing parenthesis, the autofix won't add one for you. The feedback
from @ThiefMaster, however, was that this was producing slightly unusual
formatting -- things like this:
```py
__all__ = [
"b", "c",
"a", "d"]
```
were being autofixed to this:
```py
__all__ = [
"a",
"b",
"c",
"d"]
```
When, if it was _going_ to be exploded anyway, they'd prefer something
like this (with the closing parenthesis on its own line, and a trailing comma added):
```py
__all__ = [
"a",
"b",
"c",
"d",
]
```
I'm still pretty skeptical that we'll be able to please everybody here
with the formatting choices we make; _but_, on the other hand, this
_specific_ change is pretty easy to make.
## Test Plan
`cargo test`. I also ran the autofixes for RUF022 and RUF023 on CPython
to check how they looked; they looked fine to me.
## Summary
If a generic appears multiple times on the right-hand side, we should
only include it once on the left-hand side when rewriting.
Closes https://github.com/astral-sh/ruff/issues/9904.
## Summary
This review contains a fix for
[D405](https://docs.astral.sh/ruff/rules/capitalize-section-name/)
(capitalize-section-name)
The problem is that Ruff considers the sub-section header as a normal
section if it has the same name as some section name. For instance, a
function/method has an argument named "parameters". This only applies if
you use Numpy style docstring.
See: [ISSUE](https://github.com/astral-sh/ruff/issues/9806)
The following will not raise D405 after the fix:
```python
def some_function(parameters: list[str]):
"""A function with a parameters parameter
Parameters
----------
parameters:
A list of string parameters
"""
...
```
## Test Plan
```bash
cargo test
```
---------
Co-authored-by: Mikko Leppänen <mikko.leppanen@vaisala.com>
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
This PR reduces the size of `Expr` from 80 to 64 bytes, by reducing the
sizes of...
- `ExprCall` from 72 to 56 bytes, by using boxed slices for `Arguments`.
- `ExprCompare` from 64 to 48 bytes, by using boxed slices for its
various vectors.
In testing, the parser gets a bit faster, and the linter benchmarks
improve quite a bit.
## Summary
Corrects mentions of `Path.is_link` to `Path.is_symlink` (the former
doesn't exist).
## Test Plan
```sh
python scripts/generate_mkdocs.py && mkdocs serve -f mkdocs.public.yml
```
Fixes#9857.
## Summary
Statements like `logging.info("Today it is: {day}")` will no longer be
ignored by RUF027. As before, statements like `"Today it is:
{day}".format(day="Tuesday")` will continue to be ignored.
## Test Plan
The snapshot tests were expanded to include new cases. Additionally, the
snapshot tests have been split in two to separate positive cases from
negative cases.
## Summary
Django's `mark_safe` can also be used as a decorator, so we should
detect usages of `@mark_safe` for the purpose of the relevant Bandit
rule.
Closes https://github.com/astral-sh/ruff/issues/9780.
## Summary
Given:
```python
"""Make a summary line.
Note:
----
Per the code comment the next two lines are blank. "// The first blank line is the line containing the closing
triple quotes, so we need at least two."
"""
```
It turns out we excluded the line ending in `"""`, because it's empty
(unlike for functions, where it consists of the indent). This PR changes
the `following_lines` iterator to always include the trailing newline,
which gives us correct and consistent handling between function and
module-level docstrings.
Closes https://github.com/astral-sh/ruff/issues/9877.
#2977 added the `allow-dict-calls-with-keyword-arguments` configuration
option for the `unnecessary-collection-call (C408)` rule, but it did not
update the rule description.
## Summary
When we fall through to parsing, the comment-detection rule is a
significant portion of lint time. This PR adds an additional fast
heuristic whereby we abort if a comment contains two consecutive name
tokens (via the zero-allocation lexer). For the `ctypeslib.py`, which
has a few cases that are now caught by this, it's a 2.5x speedup for the
rule (and a 20% speedup for token-based rules).
These are for descriptors which affects the behavior of the object _as a
property_; I do not think they should be called directly but there is no
alternative when working with the object directly.
Closes https://github.com/astral-sh/ruff/issues/9789
## Summary
These run over nearly every identifier. It's rare to override them, so
when not provided, we can just use a match against the hardcoded default
set.
<!--
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
Fixes#8151
This PR implements a new rule, `RUF027`.
## What it does
Checks for strings that contain f-string syntax but are not f-strings.
### Why is this bad?
An f-string missing an `f` at the beginning won't format anything, and
instead treat the interpolation syntax as literal.
### Example
```python
name = "Sarah"
dayofweek = "Tuesday"
msg = "Hello {name}! It is {dayofweek} today!"
```
It should instead be:
```python
name = "Sarah"
dayofweek = "Tuesday"
msg = f"Hello {name}! It is {dayofweek} today!"
```
## Heuristics
Since there are many possible string literals which contain syntax
similar to f-strings yet are not intended to be,
this lint will disqualify any literal that satisfies any of the
following conditions:
1. The string literal is a standalone expression. For example, a
docstring.
2. The literal is part of a function call with keyword arguments that
match at least one variable (for example: `format("Message: {value}",
value = "Hello World")`)
3. The literal (or a parent expression of the literal) has a direct
method call on it (for example: `"{value}".format(...)`)
4. The string has no `{...}` expression sections, or uses invalid
f-string syntax.
5. The string references variables that are not in scope, or it doesn't
capture variables at all.
6. Any format specifiers in the potential f-string are invalid.
## Test Plan
I created a new test file, `RUF027.py`, which is both an example of what
the lint should catch and a way to test edge cases that may trigger
false positives.
## Summary
It turns out we saw a panic in cases when dedenting blocks like the `def
wrapper` here:
```python
def instrument_url(f: UrlFuncT) -> UrlFuncT:
# TODO: Type this with ParamSpec to preserve the function signature.
if not INSTRUMENTING: # nocoverage -- option is always enabled; should we remove?
return f
else:
def wrapper(
self: "ZulipTestCase", url: str, info: object = {}, **kwargs: Union[bool, str]
) -> HttpResponseBase:
```
Since we relied on the first line to determine the indentation, instead
of the first non-empty line.
## Test Plan
`cargo test`
## Summary
This is a simple idea to avoid unnecessary work in the linter,
especially for rules that run on all name and/or all attribute nodes.
Imagine a rule like the NumPy deprecation check. If the user never
imported `numpy`, we should be able to skip that rule entirely --
whereas today, we do a `resolve_call_path` check on _every_ name in the
file. It turns out that there's basically a finite set of modules that
we care about, so we now track imports on those modules as explicit
flags on the semantic model. In rules that can _only_ ever trigger if
those modules were imported, we add a dedicated and extremely cheap
check to the top of the rule.
We could consider generalizing this to all modules, but I would expect
that not to be much faster than `resolve_call_path`, which is just a
hash map lookup on `TextSize` anyway.
It would also be nice to make this declarative, such that rules could
declare the modules they care about, the analyzers could call the rules
as appropriate. But, I don't think such a design should block merging
this.
## Summary
Often, when fixing, we need to dedent a block of code (e.g., if we
remove an `if` and dedent its body). Today, we use LibCST to parse and
adjust the indentation, which is really expensive -- but this is only
really necessary if the block contains a multiline string, since naively
adjusting the indentation for such a string can change the whitespace
_within_ the string.
This PR uses a simple dedent implementation for cases in which the block
doesn't intersect with a multi-line string (or an f-string, since we
don't support tracking multi-line strings for f-strings right now).
We could improve this even further by using the ranges to guide the
dedent function, such that we don't apply the dedent if the line starts
within a multiline string. But that would also need to take f-strings
into account, which is a little tricky.
## 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
When I was looking at the v0.2.0 release, this method showed up in a
CodSpeed regression (we were calling it more), so I decided to quickly
look at speeding it up. @BurntSushi suggested using Aho-Corasick, and it
looks like it's about 7 or 8x faster:
```text
Parser/AhoCorasick time: [8.5646 ns 8.5914 ns 8.6191 ns]
Parser/Iterator time: [64.992 ns 65.124 ns 65.271 ns]
```
## Test Plan
`cargo test`
I noticed that the comment doesn't match the behavior:
- zip function is not used anymore
- parameters are not scanned in reverse
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
No need
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
## Summary
Adds an additional warning macro (we should consolidate these later)
that shows a warning once based on the content of the warning itself.
This is less efficient than `warn_user_once!` and `warn_user_by_id!`,
but this is so expensive that it doesn't matter at all.
Applies this macro to the various warnings for the v0.2.0 release, and
also includes the filename in said warnings, so the FastAPI case is now:
```text
warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in /Users/crmarsh/workspace/fastapi/pyproject.toml:
- 'ignore' -> 'lint.ignore'
- 'select' -> 'lint.select'
- 'isort' -> 'lint.isort'
- 'pyupgrade' -> 'lint.pyupgrade'
- 'per-file-ignores' -> 'lint.per-file-ignores'
```
---------
Co-authored-by: Zanie <contact@zanie.dev>
Follow-up to #9754 and #9689. Alternative to #9714.
Marks `TRY200` as removed and redirects to `B904` instead of marking as
deprecated and suggesting `B904` instead.
Extends https://github.com/astral-sh/ruff/pull/9752 adding internal test
rules for redirection
Fixes a bug where we did not see warnings for exact codes that are
redirected (just prefixes)
Cherry-picked from https://github.com/astral-sh/ruff/pull/9714 which is
being abandoned for now because we need to invest more into our
redirection infrastructure before it is feasible.
Fixes a bug in the implementation where we improperly included
deprecated rules in `RuleSelector.rules()` when preview is on. Includes
some clean-up of error messages and the implementation.
# Conflicts:
# crates/ruff/tests/integration_test.rs
## Summary
This rule was added to `flake8-type-checking` as `TC010`. We're about to
stabilize it, so we might as well use the correct code.
See: https://github.com/astral-sh/ruff/issues/9573.
## Summary
This PR stabilizes the preview rules from:
- `flake8-trio` (6 rules)
- `flake8-quotes` (1 rule)
- `pyupgrade` (1 rule)
- `flake8-pyi` (1 rule)
- `flake8-simplify` (2 rules)
- `flake8-bandit` (9 rules; 14 remain in preview)
- `flake8-type-checking` (1 rule)
- `numpy` (1 rule)
- `ruff` (4 rules, one elevated from nursery; 6 remain in preview as
they were added within the last 30 days)
- `flake8-logging` (4 rules)
I see these are largely uncontroversial.
Adds a new `Deprecated` rule group in addition to `Stable` and
`Preview`.
Deprecated rules:
- Warn on explicit selection without preview
- Error on explicit selection with preview
- Are excluded when selected by prefix with preview
Deprecates `TRY200`, `ANN101`, and `ANN102` as a proof of concept. We
can consider deprecating them separately.
Fixes#7350
## Summary
* `--show-source` and `--no-show-source` are now deprecated.
* `output-format` supports two new variants, `full` and `concise`.
`text` is now a deprecated variant, and any use of it is treated as the
default serialization format.
* `--output-format` now default to `concise`
* In preview mode, `--output-format` defaults to `full`
* `--show-source` will still set `--output-format` to `full` if the
output format is not otherwise specified.
* likewise, `--no-show-source` can override an output format that was
set in a file-based configuration, though it will also be overridden by
`--output-format`
## Test Plan
A lot of tests were updated to use `--output-format=full`. Additional
tests were added to ensure the correct deprecation warnings appeared,
and that deprecated options behaved as intended.
# Conflicts:
# crates/ruff/tests/integration_test.rs
## Summary
Un-gates the behavior to allow `sys.path` modifications between imports,
which removed a bunch of false positives in the ecosystem CI at the
time.
## Summary
At present, our versioning policy forbids the addition of safe fixes to
stable rules outside of a minor release, so we've accumulated a bunch of
new fixes that are behind `--preview`, and can be ungated in v0.2.0.
To find these, I just grepped for `preview.is_enabled()` and identified
all such cases. I then audited the `preview_rules` test fixtures and
removed any tests that existed only to test this autofix behavior.
# Conflicts:
# crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM114_SIM114.py.snap
# crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM114_SIM114.py.snap
## Summary
This rule was added to flake8-bugbear. In general, we tend to prefer
redirecting to prominent plugins when our own rules are reimplemented
(since more projects have `B` activated than `RUF`).
## Test Plan
`cargo test`
# Conflicts:
# crates/ruff_linter/src/rules/ruff/rules/mod.rs
Updated implementation of https://github.com/astral-sh/ruff/pull/7369
which was left out in the cold.
This was motivated again following changes in #9691 and #9689 where we
could not test the changes without actually deprecating or removing
rules.
---
Follow-up to discussion in https://github.com/astral-sh/ruff/pull/7210
Moves integration tests from using rules that are transitively in
nursery / preview groups to dedicated test rules that only exist during
development. These rules always raise violations (they do not require
specific file behavior). The rules are not available in production or in
the documentation.
Uses features instead of `cfg(test)` for cross-crate support per
https://github.com/rust-lang/cargo/issues/8379
## Summary
Just like #6537 and #6538 but for the `default` second parameter to
`getenv()`.
Also rename "BAD" to "BAR" in the tests, since those strings shouldn't
trigger the rule.
## Test Plan
Added passing and failing examples to `invalid_envvar_default.py`.
## Summary
This review contains a fix for
[ASYNC101](https://docs.astral.sh/ruff/rules/open-sleep-or-subprocess-in-async-function/)
(open-sleep-or-subprocess-in-async-function)
The problem is that ruff does not take open calls from pathlib.Path into
account in async functions. Path.open() call is still a blocking call.
In addition, PTH123 suggests to use pathlib.Path instead of os.open. So
this might create an additional confusion.
See: https://github.com/astral-sh/ruff/issues/6892
## Test Plan
```bash
cargo test
```
## Summary
Tweaks PLR2004 to show the literal source text, rather than the constant
value.
I noticed this when I had a hexadecimal constant, and the linter turned
it into base-10.
Now, if you have `0x300`, it will show `0x300` instead of `768`.
Also, added backticks around the constant in the output message.
## Test Plan
`cargo test`
## Summary
Given:
```python
from dataclasses import InitVar, dataclass
@dataclass
class C:
i: int
j: int = None
database: InitVar[DatabaseType] = None
def __post_init__(self, database):
if self.j is None and database is not None:
self.j = database.lookup('j')
c = C(10, database=my_database)
```
We should avoid marking `InitVar` as typing-only, since it _is_ required
by the dataclass at runtime.
Note that by default, we _already_ don't flag this, since the
`@dataclass` member is needed at runtime too -- so it's only a problem
with `strict` mode.
Closes https://github.com/astral-sh/ruff/issues/9666.
## Summary
Per https://github.com/astral-sh/ruff/issues/9570:
> `dtype` are a bit of a strange beast, but definitely best thought of
as instances, not classes, and they are meant to be comparable not just
to their own class, but also to the corresponding scalar types (e.g.,
`x.dtype == np.float32`) and strings (e.g., `x.dtype == ['i1,i4']`;
basically, `__eq__` always tries to do `dtype(other)`.
This PR thus allows comparisons to `dtype` in preview.
Closes https://github.com/astral-sh/ruff/issues/9570.
## Test Plan
`cargo test`
## Summary
This review contains a fix for
[RET504](https://docs.astral.sh/ruff/rules/unnecessary-assign/)
(unnecessary-assign)
The problem is that Ruff suggests combining a return statement inside
contextlib.suppress. Even though it is an unsafe fix it might lead to an
invalid code that is not equivalent to the original one.
See: https://github.com/astral-sh/ruff/issues/5909
## Test Plan
```bash
cargo test
```
## Summary
Given a statement like `colors = 6`, we currently treat the cell as an
automagic (since `colors` is an automagic) -- i.e., we assume it's
equivalent to `%colors = 6`. This PR adds some additional detection
whereby if the statement is an _assignment_, we avoid treating it as
such. I audited the list of automagics, and I believe this is safe for
all of them.
Closes https://github.com/astral-sh/ruff/issues/8526.
Closes https://github.com/astral-sh/ruff/issues/9648.
## Test Plan
`cargo test`
## Summary
This review contains a fix for
[PIE810](https://docs.astral.sh/ruff/rules/multiple-starts-ends-with/)
(multiple-starts-ends-with)
The problem is that ruff suggests combining multiple startswith/endswith
calls into a single call even though there might be a call with tuple of
strs. This leads to calling startswith/endswith with tuple of tuple of
strs which is incorrect and violates startswith/endswith conctract and
results in runtime failure.
However the following will be valid and fixed correctly =>
```python
x = ("hello", "world")
y = "h"
z = "w"
msg = "hello world"
if msg.startswith(x) or msg.startswith(y) or msg.startswith(z) :
sys.exit(1)
```
```
ruff --fix --select PIE810 --unsafe-fixes
```
=>
```python
if msg.startswith(x) or msg.startswith((y,z)):
sys.exit(1)
```
See: https://github.com/astral-sh/ruff/issues/8906
## Test Plan
```bash
cargo test
```
## Summary
This rule was just incorrect, it didn't match the examples in the docs.
(It's a very rarely-used rule since it's not included in any of the
conventions.)
Closes https://github.com/astral-sh/ruff/issues/9452.
## Summary
Add a rule for defaultdict(default_factory=callable). Instead suggest
using defaultdict(callable).
See: https://github.com/astral-sh/ruff/issues/9509
If a user tries to bind a "non-callable" to default_factory, the rule
ignores it. Another option would be to warn that it's probably not what
you want. Because Python allows the following:
```python
from collections import defaultdict
defaultdict(default_factory=1)
```
this raises after you actually try to use it:
```python
dd = defaultdict(default_factory=1)
dd[1]
```
=>
```bash
KeyError: 1
```
Instead using callable directly in the constructor it will raise (not
being a callable):
```python
from collections import defaultdict
defaultdict(1)
```
=>
```bash
TypeError: first argument must be callable or None
```
## Test Plan
```bash
cargo test
```
## Summary
When we are analyzing the implicit return rule this change add an
additional check to verify if the call expression has been annotated
with NoReturn type from typing module.
See: https://github.com/astral-sh/ruff/issues/5474
## Test Plan
```bash
cargo test
```
## Summary
Checks for unnecessary `dict` comprehension when creating a new
dictionary from iterable. Suggest to replace with
`dict.fromkeys(iterable)`
See: https://github.com/astral-sh/ruff/issues/9592
## Test Plan
```bash
cargo test
```
## Summary
This PR introduces a new rule to sort `__slots__` and `__match_args__`
according to a [natural sort](https://en.wikipedia.org/wiki/Natural_sort_order), as was
requested in https://github.com/astral-sh/ruff/issues/1198#issuecomment-1881418365.
The implementation here generalises some of the machinery introduced in
3aae16f1bd
so that different kinds of sorts can be applied to lists of string
literals. (We use an "isort-style" sort for `__all__`, but that isn't
really appropriate for `__slots__` and `__match_args__`, where nearly
all items will be snake_case.) Several sections of code have been moved
from `sort_dunder_all.rs` to a new module, `sorting_helpers.rs`, which
`sort_dunder_all.rs` and `sort_dunder_slots.rs` both make use of.
`__match_args__` is very similar to `__all__`, in that it can only be a
tuple or a list. `__slots__` differs from the other two, however, in
that it can be any iterable of strings. If slots is a dictionary, the
values are used by the builtin `help()` function as per-attribute
docstrings that show up in the output of `help()`. (There's no
particular use-case for making `__slots__` a set, but it's perfectly
legal at runtime, so there's no reason for us not to handle it in this
rule.)
Note that we don't do an autofix for multiline `__slots__` if `__slots__` is a dictionary: that's out of scope. Everything else, we can nearly always fix, however.
## Test Plan
`cargo test` / `cargo insta review`.
I also ran this rule on CPython, and the diff looked pretty good
---
Co-authored-by: Micha Reiser <micha@reiser.io>
## Summary
Implement rule `mutable-fromkeys-value` (`RUF023`).
Autofixes
```python
dict.fromkeys(foo, [])
```
to
```python
{key: [] for key in foo}
```
The fix is marked as unsafe as it changes runtime behaviour. It also
uses `key` as the comprehension variable, which may not always be
desired.
Closes#4613.
## Test Plan
`cargo test`
## Summary
This PR detects whether PLR0917 is being applied to a method or class
method, and if so, it ignores the first argument for the purposes of
counting the number of positional arguments.
## Test Plan
New tests have been added to the corresponding fixture.
Closes#9552.
## Summary
Long ago, we had a single `ruff` crate. We started to break that up, and
at some point, we wanted to separate the CLI from the core library. So
we created `ruff_cli`, which created a `ruff` binary. Later, the `ruff`
crate was renamed to `ruff_linter` and further broken up into additional
crates.
(This is all from memory -- I didn't bother to look through the history
to ensure that this is 100% correct :))
Now that `ruff` no longer exists, this PR renames `ruff_cli` to `ruff`.
The primary benefit is that the binary target and the crate name are now
the same, which helps with downstream tooling like `cargo-dist`, and
also removes some complexity from the crate and `Cargo.toml` itself.
## Test Plan
- Ran `rm -rf target/release`.
- Ran `cargo build --release`.
- Verified that `./target/release/ruff` was created.
## Summary
#5920 with a fix for the erroneous slice in `module_name`. Fixes#9547.
## Test Plan
Added `import bbb.ccc._ddd as eee` to the test fixture to ensure it no
longer panics.
`cargo test`
## Summary
This implements the rule proposed in #1198 (though it doesn't close the
issue, as there are some open questions about configuration that might
merit some further discussion).
## Test Plan
`cargo test` / `cargo insta review`. I also ran this PR branch on the CPython
codebase with `--fix --select=RUF022 --preview `, and the results looked
pretty good to me.
---------
Co-authored-by: Micha Reiser <micha@reiser.io>
Co-authored-by: Andrew Gallant <andrew@astral.sh>
## Summary
add autofix for `deprecated_log_warn` (`PGH002`)
## Test Plan
`cargo test`
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
This PR is a more holistic fix for
https://github.com/astral-sh/ruff/issues/9534 and
https://github.com/astral-sh/ruff/issues/9159.
When we visit the AST, we track nodes that we need to visit _later_
(deferred nodes). For example, when visiting a function, we defer the
function body, since we don't want to visit the body until we've visited
the rest of the statements in the containing scope.
However, deferred nodes can themselves contain deferred nodes... For
example, a function body can contain a lambda (which contains a deferred
body). And then there are rarer cases, like a lambda inside of a type
annotation.
The aforementioned issues were fixed by reordering the deferral visits
to catch common cases. But even with those fixes, we still fail on cases
like:
```python
from __future__ import annotations
import re
from typing import cast
cast(lambda: re.match, 1)
```
Since we don't expect lambdas to appear inside of type definitions.
This PR modifies the `Checker` to keep visiting until all the deferred
stacks are empty. We _already_ do this for any one kind of deferred
node; now, we do it for _all_ of them at a level above.
## Summary
This is effectively the same problem as
https://github.com/astral-sh/ruff/pull/9175. And this just papers over
it again, though I'm gonna try a more holistic fix in a follow-up PR.
The _real_ fix here is that we need to continue to visit deferred items
until they're exhausted since, e.g., we still get this case wrong
(flagging `re` as unused):
```python
import re
cast(lambda: re.match, 1)
```
Closes https://github.com/astral-sh/ruff/issues/9534.
## Summary
Closes#9508 .
Add `__prepare__` method to dunder method list in
`is_known_dunder_method`.
## Test Plan
1. add "__prepare__" method to `Apple` class in
crates/ruff_linter/resources/test/fixtures/pylint/bad_dunder_method_name.py.
2. run `cargo test`
Implements SIM113 from #998
Added tests
Limitations
- No fix yet
- Only flag cases where index variable immediately precede `for` loop
@charliermarsh please review and let me know any improvements
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
In the `logical_lines`'s `expand_indent` , respect the
`LinterSettings::tab_size` setting instead of hardcoding the size of
tabs to 8.
Also see [this
conversation](https://github.com/astral-sh/ruff/pull/9266#discussion_r1447102212)
## Test Plan
Tested by running `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
Fixes#8334.
`Display` has been implemented for `ruff_workspace::Settings`, which
gives a much nicer and more readable output to `--show-settings`.
Internally, a `display_settings` utility macro has been implemented to
reduce the boilerplate of the display code.
### Work to be done
- [x] A lot of formatting for `Vec<_>` and `HashSet<_>` types have been
stubbed out, using `Debug` as a fallback. There should be a way to add
generic formatting support for these types as a modifier in
`display_settings`.
- [x] Several complex types were also stubbed out and need proper
`Display` implementations rather than falling back on `Debug`.
- [x] An open question needs to be answered: how important is it that
the output be valid TOML? Some types in settings, such as a hash-map
from a glob pattern to a multi-variant enum, will be hard to rework into
valid _and_ readable TOML.
- [x] Tests need to be implemented.
## Test Plan
Tests consist of a snapshot test for the default `--show-settings`
output and a doctest for `display_settings!`.
<!--
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 the message for `__aenter__ ` in PLC2801 (introduced in
https://github.com/astral-sh/ruff/pull/9166)
There is no `aenter` builtin in Python, so the current message is
misleading.
I take the message from original lint
https://github.com/pylint-dev/pylint/blob/main/pylint/constants.py#L211
P.S. I think here should be more accurate synchronization with original
lint (e.g. the current implementation will not lint `__enter__` on my
first sight), but it is out-of-scope of this change.
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
<!-- How was it tested? -->
<!--
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? -->
I noticed that there should be a missing period added to some of the new
error messages for Unnecessary dunder call:
```
sandpit\test.py:6:16: PLC2801 Unnecessary dunder call to `__getattribute__`. Access attribute directly or use getattr built-in function..
```
## Test Plan
Static analysis of the implementation, as this has no existing test
cases.
## Summary
We haven't found time to flip this on, so feels like it's best to remove
it for now -- can always restore from source when we get back to it.
## Summary
Closes#9319, implements the [`SIM911` rule from
`flake8-simplify`](https://github.com/MartinThoma/flake8-simplify/pull/183).
#### Note
I wasn't sure whether or not to include
```rs
if checker.settings.preview.is_disabled() {
return;
}
```
at the beginning of the function with violation logic if the rule's
already declared as part of `RuleGroup::Preview`.
I've seen both variants, so I'd appreciate some feedback on that :)
## Summary
This PR attempts to improve `builtin-attribute-shadowing` (`A003`), a
rule which has been repeatedly criticized, but _does_ have value (just
not in the current form).
Historically, this rule would flag cases like:
```python
class Class:
id: int
```
This led to an increasing number of exceptions and special-cases to the
rule over time to try and improve it's specificity (e.g., ignore
`TypedDict`, ignore `@override`).
The crux of the issue is that given the above, referencing `id` will
never resolve to `Class.id`, so the shadowing is actually fine. There's
one exception, however:
```python
class Class:
id: int
def do_thing() -> id:
pass
```
Here, `id` actually resolves to the `id` attribute on the class, not the
`id` builtin.
So this PR completely reworks the rule around this _much_ more targeted
case, which will almost always be a mistake: when you reference a class
member from within the class, and that member shadows a builtin.
Closes https://github.com/astral-sh/ruff/issues/6524.
Closes https://github.com/astral-sh/ruff/issues/7806.
Fixes#8721
## Summary
This implements the rule proposed in #8721, as RUF021. `and` always
binds more tightly than `or` when chaining the two together.
(This should definitely be autofixable, but I'm leaving that to a
followup PR for now.)
## Test Plan
`cargo test` / `cargo insta review`
## Summary
This PR enables Ruff to remove redefined imports, as in:
```python
import os
import os
print(os)
```
Previously, Ruff would flag `F811` on the second `import os`, but
couldn't fix it.
For now, this fix is marked as safe, but only available in preview.
Closes https://github.com/astral-sh/ruff/issues/3477.
## Summary
On `main`, we flag redefinitions in cases like:
```python
import os
x = 1
if x > 0:
import os
```
That is, we consider these to be in the "same branch", since they're not
in disjoint branches. This matches Flake8's behavior, but it seems to
lead to false positives.
## Summary
Given a docstring like:
```python
def func(x: int, args: tuple[int]):
"""Toggle the gizmo.
Args:
x: Some argument.
args: Some other arguments.
"""
```
We were considering the `args:` descriptor to be an indented docstring
section header (since `Args:`) is a valid header name. This led to very
confusing diagnostics.
This PR makes the parsing a bit more lax in this case, such that if we
see a nested header that's more deeply indented than the preceding
header, and the preceding section allows sub-items (like `Args:`), we
avoid treating the nested item as a section header.
Closes https://github.com/astral-sh/ruff/issues/9426.
## Summary
After we apply fixes, the source code might be transformed. And yet,
we're using the _unmodified_ source code to compute locations in some
cases (e.g., for displaying parse errors, or Jupyter Notebook cells).
This can lead to subtle errors in reporting, or even panics. This PR
modifies the linter to use the _transformed_ source code for such
computations.
Closes https://github.com/astral-sh/ruff/issues/9407.
I just fixed this false negative in flake8-pyi
(https://github.com/PyCQA/flake8-pyi/pull/460), and then realised ruff
has the exact same bug! Luckily it's a very easy fix.
(The bug is that unused protocols go undetected if they're generic.)
## Summary
Ensures that any lint rules that include line locations render them as
relative to the cell (and include the cell number) when inside a Jupyter
notebook.
Closes https://github.com/astral-sh/ruff/issues/6672.
## Test Plan
`cargo test`
## Summary
This PR adds an autofix for the newly added PYI058 rule (added in
#9313). ~~The PR's current implementation is that the fix is only
available if the fully qualified name of `Generator` or `AsyncGenerator`
is being used:~~
- ~~`-> typing.Generator` is converted to `-> typing.Iterator`;~~
- ~~`-> collections.abc.AsyncGenerator[str, Any]` is converted to `->
collections.abc.AsyncIterator[str]`;~~
- ~~but `-> Generator` is _not_ converted to `-> Iterator`. (It would
require more work to figure out if `Iterator` was already imported or
not. And if it wasn't, where should we import it from? `typing`,
`typing_extensions`, or `collections.abc`? It seems much more
complicated.)~~
The fix is marked as always safe for `__iter__` or `__aiter__` methods
in `.pyi` files, but unsafe for all such methods in `.py` files that
have more than one statement in the method body.
This felt slightly fiddly to accomplish, but I couldn't _see_ any
utilities in
https://github.com/astral-sh/ruff/tree/main/crates/ruff_linter/src/fix
that would have made it simpler to implement. Lmk if I'm missing
something, though -- my first time implementing an autofix! :)
## Test Plan
`cargo test` / `cargo insta review`.
## Summary
We had an early `continue` in this loop, and we weren't setting
`comparator = next;` when continuing... This PR removes the early
continue altogether for clarity.
Closes https://github.com/astral-sh/ruff/issues/9374.
## Test Plan
`cargo test`
## Summary
Changes message from `"Relative imports are banned"` to `"Prefer
absolute imports over relative imports from parent modules"`.
Closes#9363.
## Test Plan
`cargo test`
## Summary
This PR modifies our `Cargo.toml` files to use workspace dependencies
for _all_ dependencies, rather than the status quo of sporadically
trying to use workspace dependencies for those dependencies that are
used across multiple crates. I find the current situation more confusing
and harder to manage, since we have a mix of workspace and crate-local
dependencies, whereas this setup consistently uses the same approach for
all dependencies.
The "wsl" crate was last touched in 2019, whereas the "is-wsl" crate was
last updated in 2023. Additionally, it is unclear whether the "wsl"
crate supports both WSL1 and WSL2 (which was announced in 2019), whereas
the "is-wsl" crate explicitly supports both WSL1 and WSL2.
The required code changes are minimal, since both crates provide only a
`is_wsl() -> bool` function.
paramiko `set_missing_host_key_policy` has mandatory positional arg.
The [current
documentation](https://docs.astral.sh/ruff/rules/ssh-no-host-key-verification/#example)
leads to non-running code
```
>>> from paramiko import client
>>> ssh_client = client.SSHClient()
>>> ssh_client.set_missing_host_key_policy()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: SSHClient.set_missing_host_key_policy() missing 1 required positional argument: 'policy'
```
This PR modifies the documentation to set the policy to the [default
`RejectPolicy`](https://docs.paramiko.org/en/latest/api/client.html#paramiko.client.SSHClient.set_missing_host_key_policy)
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
## Summary
Remove the period from a couple short messages, for consistency with all
other short messages.
All other short rule messages lack such a period, except for long
messages made of multiple sentences.
## Test Plan
Tests modified accordingly.
Not sure if this would qualify as a breaking change because user-visible
messages are modified.
## Summary
This PR implements Y058 from flake8-pyi -- this is a new flake8-pyi rule
that was released as part of `flake8-pyi 23.11.0`. I've followed the
Python implementation as closely as possible (see
858c0918a8),
except that for the Ruff version, the rule also applies to `.py` files
as well as for `.pyi` files. (For `.py` files, we only emit the
diagnostic in very specific situations, however, as there's a much
higher likelihood of emitting false positives when applying this rule to
a `.py` file.)
## Test Plan
`cargo test`/`cargo insta review`
## Summary
Historically, we encoded this list by extracting the `__all__`. I went
to update it, but... is there really any value in it? Seems easier to
just treat `typing_extensions` as an alias for `typing`.
Closes https://github.com/astral-sh/ruff/issues/9334.
## Summary
This is a non-behavior-changing refactor to follow-up
https://github.com/astral-sh/ruff/pull/9321 by modifying
`DisplayParseError` to use owned data and make it useable as a
standalone error type (rather than using references and implementing
`Display`). I don't feel very strongly either way. I thought it was
awkward that the `FormatCommandError` had two branches in the display
path, and wanted to represent the `Parse` vs. other cases as a separate
enum, so here we are.
## Summary
The logic that detects continuations assumed that tokens themselves
cannot span multiple lines. However, strings _can_ -- even single-quoted
strings.
Closes https://github.com/astral-sh/ruff/issues/9323.
## Summary
I always found it odd that we had to pass this in, since it's really
higher-level context for the error. The awkwardness is further evidenced
by the fact that we pass in fake values everywhere (even outside of
tests). The source path isn't actually used to display the error; it's
only accessed elsewhere to _re-display_ the error in certain cases. This
PR modifies to instead pass the path directly in those cases.
## Summary
This PR modifies the semantics of `runtime-evaluated-decorators` to
respect decorators on both classes _and_ functions. Historically, this
only respected classes, since the common use-case is (e.g.)
`pydantic.BaseModel` -- but functions are equally valid.
Closes https://github.com/astral-sh/ruff/issues/9312.
## Test Plan
`cargo test`
## Summary
Given:
```python
from somewhere import get_cfg
def lookup_cfg(cfg_description):
cfg = get_cfg(cfg_description)
if cfg is not None:
return cfg
raise AttributeError(f"No cfg found matching {cfg_description}")
```
We were analyzing the method from last-to-first statement. So we saw the
`raise`, then assumed the method _always_ raised. In reality, though, it
_might_ return. This PR improves the branch analysis to respect these
mixed cases.
Closes https://github.com/astral-sh/ruff/issues/9269.
Closes https://github.com/astral-sh/ruff/issues/9304.
## Summary
Remove `:` for PLR0917 to make all PLR09XX message look the same
```
PLR0904 Too many public methods (21 > 20)
PLR0911 Too many return statements (16 > 6)
PLR0912 Too many branches (13 > 12)
PLR0913 Too many arguments in function definition (10 > 5)
PLR0915 Too many statements (118 > 50)
PLR0917 Too many positional arguments: (15/5)
```
## Test Plan
<!-- How was it tested? -->
---------
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
## Summary
Part of #970.
This adds Pylint's [R0244
empty_comment](https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/empty-comment.html)
lint as well as an always-safe fix.
## Test Plan
The included snapshot verifies the following:
- A line containing only a non-empty comment is not changed
- A line containing leading whitespace before a non-empty comment is not
changed
- A line containing only an empty comment has its content deleted
- A line containing only leading whitespace before an empty comment has
its content deleted
- A line containing only leading and trailing whitespace on an empty
comment has its content deleted
- A line containing trailing whitespace after a non-empty comment is not
changed
- A line containing only a single newline character (i.e. a blank line)
is not changed
- A line containing code followed by a non-empty comment is not changed
- A line containing code followed by an empty comment has its content
deleted after the last non-whitespace character
- Lines containing code and no comments are not changed
- Empty comment lines within block comments are ignored
- Empty comments within triple-quoted sections are ignored
## Comparison to Pylint
Running Ruff and Pylint 3.0.3 with Python 3.12.0 against the
`empty_comment.py` file added in this PR, we see the following:
* Identical behavior:
* empty_comment.py:3:0: R2044: Line with empty comment (empty-comment)
* empty_comment.py:4:0: R2044: Line with empty comment (empty-comment)
* empty_comment.py:5:0: R2044: Line with empty comment (empty-comment)
* empty_comment.py:18:0: R2044: Line with empty comment (empty-comment)
* Differing behavior:
* Pylint doesn't ignore empty comments in block comments commonly used
for visual spacing; I decided these were fine in this implementation
since many projects use these and likely do not want them removed.
* empty_comment.py:28:0: R2044: Line with empty comment (empty-comment)
* Pylint detects "empty comments" within the triple-quoted section at
the bottom of the file, which is arguably a bug in the Pylint
implementation since these are not truly comments. These are ignored by
this implementation.
* empty_comment.py:37:0: R2044: Line with empty comment (empty-comment)
* empty_comment.py:38:0: R2044: Line with empty comment (empty-comment)
* empty_comment.py:39:0: R2044: Line with empty comment (empty-comment)
## Summary
Hey there 👋 thanks for this great project!
On python code looking like the following
```
import yaml
from yaml.loader import SafeLoader
with MY_FILE_PATH.open("r") as my_file:
my_data = yaml.load(my_file, Loader=SafeLoader)
```
ruff reports this error:
```
S506 Probable use of unsafe loader `SafeLoader` with `yaml.load`. Allows instantiation of arbitrary objects. Consider `yaml.safe_load`.
```
This PR is an attempt to support SafeLoader being imported for either
`yaml` or `yaml.loader`
Disclaimer:
I am not familiar with Rust so this is likely not the better way of
doing it. Interested in hearing how to adapt this PR to provide similar
behavior in a better way
## Test Plan
The S506.py file was updated accordingly to cover the use cases and test
were confirmed to pass with this change.
## Summary
If `RUF100` is ignored via `per-file-ignores`, we need to avoid raising
it. `RUF100` has special "self-ignore" logic, since the rule itself
deals with `# noqa` directives. This PR wires up `per-file-ignores` to
that "self-ignore" logic.
Closes https://github.com/astral-sh/ruff/issues/9297.
We should avoid adding `-> None` to stubs in `.pyi` files, along with a
few other cases. (We already ignore abstract methods.)
Closes https://github.com/astral-sh/ruff/issues/9270.
## Summary
This PR adds some helper structs to the linter paths to enable passing
in the pre-computed tokens and parsed source code during benchmarking,
to remove lexing and parsing from the overall linter benchmark
measurement. We already remove parsing for the formatter, and we have
separate benchmarks for the lexer and the parser, so this should make it
much easier to measure linter performance changes.
## Summary
Adds a rule to detect unions that include `typing.NoReturn` or
`typing.Never`. In such cases, the use of the bottom type is redundant.
Closes https://github.com/astral-sh/ruff/issues/9113.
## Test Plan
`cargo test`
## Summary
fixes#6956
details in issue
Following an advice in
https://github.com/astral-sh/ruff/issues/6956#issuecomment-1817672585,
this change separates expressions to 3 levels of "constant likelihood":
* literals, empty dict and tuples... (definitely constant, level 2)
* CONSTANT_CASE vars (probably constant, 1)
* all other expressions (0)
a comparison is marked yoda if the level is strictly higher on its left
hand side
following
https://github.com/astral-sh/ruff/issues/6956#issuecomment-1697107822
marking compound expressions of literals (e.g. `60 * 60` ) as constants
this change current behaviour on
`SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)` in the fixture
from error to ok
## Summary
Given a function like:
```python
def func(x: int):
if not x:
raise ValueError
else:
raise TypeError
```
We now correctly use `NoReturn` as the return type, rather than `None`.
Closes https://github.com/astral-sh/ruff/issues/9201.
## Summary
Part of #8771. flake8-pyi will emit a Y018 error for unused TypeVars,
ParamSpecs or TypeVarTuples; Ruff currently only emits PYI018 for unused
TypeVars.
This is my first "proper" Ruff PR -- let me know if there's a better way
of doing this! Not sure if the repeated calls to `match_typing_expr()`
are ideal.
## Test Plan
I manually updated the fixtures to add some unused ParamSpecs and
TypeVarTuples, and then updated the snapshots using `cargo insta
review`. All tests then passed when run using `cargo test`.
## Summary
Given `Callable[[Callable[_P, _R]], Callable[_P, _R]]` from the
originating issue, when quoting `Callable`, we quoted the inner
`[Callable[_P, _R]]`, and then created a separate edit for the outer
`Callable`. Since there's an extra level of nesting in the subscript,
the edit for `[Callable[_P, _R]]` correctly did _not_ expand to the
entire expression. However, in this case, we should discard the inner
edit, since the expression is getting quoted by the outer edit anyway.
Closes https://github.com/astral-sh/ruff/issues/9162.
## Summary
A common mistake is to add quotes around one member in an `X | Y`-style
type union, as in:
```python
contract_versions_list: list[ContractVersion] | 'QuerySet[ContractVersion]' | None = None
```
However, doing so will lead to a runtime error if the annotation is
runtime-evaluated. This PR lints against such patterns.
Closes https://github.com/astral-sh/ruff/issues/9139.
## Summary
Fix dropped union expressions for piped non-types in `PYI055` autofix
If you had `type[int] | type[str] | str`, it would have dropped the
`str`, which breaks the type!
Closes#9156
## Test Plan
`cargo test`
Fix#9080
Example, where `[]` is a 2 byte non-breaking space:
```
def f():
""" Docstring header
^^^^ Real indentation is 4 chars
docstring body, over-indented
^^^^^^ Over-indentation is 6 - 4 = 2 chars due to this line
[] [] docstring body 2, further indented
^^^^^ We take these 4 chars/5 bytes to match the docstring ...
^^^ ... and these 2 chars/3 bytes to remove the `over_indented_size` ...
^^ ... but preserve this real indent
```
Given:
```python
x: DataFrame[
int
] = 1
```
We currently wrap the annotation in single quotes, which leads to a
syntax error:
```python
x: "DataFrame[
int
]" = 1
```
There are a few options for what to suggest for users here... Use triple
quotes:
```python
x: """DataFrame[
int
]""" = 1
```
Or, use an implicit string concatenation (which may require
parentheses):
```python
x: ("DataFrame["
"int"
"]") = 1
```
The solution I settled on here is to use the `Generator`, which
effectively means we write it out on a single line, like:
```python
x: "DataFrame[int]" = 1
```
It's kind of the "least opinionated" solution, but it does mean we'll
expand to a very long line in some cases.
Closes https://github.com/astral-sh/ruff/issues/9136.
This PR adds a `as_slice` method to all the string nodes which returns
all the parts of the nodes as a slice. This will be useful in the next
PR to split the string formatting to use this method to extract the
_single node_ or _implicitly concanated nodes_.
## Summary
Adds support for sarif v2.1.0 output to cli, usable via the
output-format paramter.
`ruff . --output-format=sarif`
Includes a few changes I wasn't sure of, namely:
* Adds a few derives for Clone & Copy, which I think could be removed
with a little extra work as well.
## Test Plan
I built and ran this against several large open source projects and
verified that the output sarif was valid, using [Microsoft's SARIF
validator tool](https://sarifweb.azurewebsites.net/Validation)
I've also attached an output of the sarif generated by this version of
ruff on the main branch of django at commit: b287af5dc9
[django_main_b287af5dc9_sarif.json](https://github.com/astral-sh/ruff/files/13626222/django_main_b287af5dc9_sarif.json)
Note: this needs to be regenerated with the latest changes and
confirmed.
## Open Points
[ ] Convert to just using all Rules all the time
[ ] Fix the issue with getting the file URI when compiling for web
assembly
## Summary
This allows us to fix usages like:
```python
from pandas import DataFrame
def baz() -> DataFrame:
...
```
By quoting the `DataFrame` in `-> DataFrame`. Without quotes, moving
`from pandas import DataFrame` into an `if TYPE_CHECKING:` block will
fail at runtime, since Python tries to evaluate the annotation to add it
to the function's `__annotations__`.
Unfortunately, this does require us to split our "annotation kind" flags
into three categories, rather than two:
- `typing-only`: The annotation is only evaluated at type-checking-time.
- `runtime-evaluated`: Python will evaluate the annotation at runtime
(like above) -- but we're willing to quote it.
- `runtime-required`: Python will evaluate the annotation at runtime
(like above), and some library (like Pydantic) needs it to be available
at runtime, so we _can't_ quote it.
This functionality is gated behind a setting
(`flake8-type-checking.quote-annotations`).
Closes https://github.com/astral-sh/ruff/issues/5559.
## Summary
Adds `find_assigned_value` a function which gets the `&Expr` assigned to
a given `id` if one exists in the semantic model.
Open TODOs:
- [ ] Handle `binding.kind.is_unpacked_assignment()`: I am bit confused
by this one. The snippet from its documentation does not appear to be
counted as an unpacked assignment and the only ones I could find for
which that was true were invalid Python like:
```python
x, y = 1
```
- [ ] How to handle AugAssign. Can we combine statements like:
```python
(a, b) = [(1, 2, 3), (4,)]
a += (6, 7)
```
to get the full value for a? Code currently just returns `None` for
these assign types
- [ ] Multi target assigns
```python
m_c = (m_d, m_e) = (0, 0)
trio.sleep(m_c) # OK
trio.sleep(m_d) # TRIO115
trio.sleep(m_e) # TRIO115
```
## Test Plan
Used the function in two rules:
- `TRIO115`
- `PERF101`
Expanded both their fixtures for explicit multi target check
## Summary
A fairly common pattern which triggers F841 is unused variables from
tuple assignments, e.g.:
user, created = User.objects.get_or_create(...)
^ F841: Local variable `created` is assigned to but never used
This error is currently not auto-fixable.
This PR adds support for fixing the error automatically by renaming the
unused variable to have a leading underscore (i.e. `_created`) **iff**
the `dummy-variable-rgx` setting would match it.
I considered using `renamers::Renamer` here, but because by the nature
of the error there should be no references to it, that seemed like
overkill. Also note that the fix might break by shadowing the new name
if it is already used elsewhere in the scope. I left it as is because
1. the renamed variable matches the "unused" regex, so it should
hopefully not already be used,
2. the fix is marked as unsafe so it should be reviewed manually
anyways, and
3. I'm not actually sure how to check the scope for the new variable
name 😅
Hides hints about unsafe fixes when they are disabled e.g. with
`--no-unsafe-fixes` or `unsafe-fixes = false`. By default, unsafe fix
hints are still displayed. This seems like a nice way to remove the nag
for users who have chosen not to apply unsafe fixes.
Inspired by comment at
https://github.com/astral-sh/ruff/issues/9063#issuecomment-1850289675
<!--
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? -->
Check floating-point numbers similarly to integers in FURB163. For
example, both `math.log(x, 10)` and `math.log(x, 10.0)` should be
changed to `math.log10(x)`.
## Test Plan
<!-- How was it tested? -->
Added couple of test cases.
## Summary
E274 currently flags any keyword at the start of a line indented with
tabs. This turns out to be due to a bug in `Whitespace::trailing` that
never considers any whitespace containing a tab as indentation.
## Test Plan
Added a simple test case.
This PR allows `matplotlib.use` calls to intersperse imports without
triggering `E402`. This is a pragmatic choice as it's common to require
`matplotlib.use` calls prior to importing from within `matplotlib`
itself.
Closes https://github.com/astral-sh/ruff/issues/9091.
## Summary
Fixes#8863 : Detect asyncio-dangling-task (RUF006) when discarding
return value
## Test Plan
added new two testcases, changed result of an old one that was made more
specific
## Summary
Fix a couple typos:
- I'm certain about `It's is` → `It is`.
- Not sure about `is it's` → `if it's` because I don't understand the
sentence.
## Test Plan
No tests.
## Summary
This PR updates the logic for `is_magic_cell` to include certain cell
magics. These cell magics would contain Python code following the line
defining the command. The code could define a variable which can then be
referenced in other cells. Currently, we would ignore the cell
completely leading to undefined-name violation.
As discussed in
https://github.com/astral-sh/ruff/issues/8354#issuecomment-1832221009
## Test Plan
Add new test case to validate this scenario.
## Summary
It's common to interleave a `sys.path` modification between imports at
the top of a file. This is a frequent cause of `# noqa: E402` false
positives, as seen in the ecosystem checks. This PR modifies E402 to
omit such modifications when determining the "import boundary".
(We could consider linting against `sys.path` modifications, but that
should be a separate rule.)
Closes: https://github.com/astral-sh/ruff/issues/5557.
## Summary
This PR introduces a new `StringLike` enum which is a narrow type to
indicate string-like nodes. These includes the string literals, bytes
literals, and the literal parts of f-strings.
The main motivation behind this is to avoid repetition of rule calling
in the AST checker. We add a new `analyze::string_like` function which
takes in the enum and calls all the respective rule functions which
expects atleast 2 of the variants of this enum.
I'm open to discarding this if others think it's not that useful at this
stage as currently only 3 rules require these nodes.
As suggested
[here](https://github.com/astral-sh/ruff/pull/8835#discussion_r1414746934)
and
[here](https://github.com/astral-sh/ruff/pull/8835#discussion_r1414750204).
## Test Plan
`cargo test`
Rebase of #6365 authored by @davidszotten.
## Summary
This PR updates the AST structure for an f-string elements.
The main **motivation** behind this change is to have a dedicated node
for the string part of an f-string. Previously, the existing
`ExprStringLiteral` node was used for this purpose which isn't exactly
correct. The `ExprStringLiteral` node should include the quotes as well
in the range but the f-string literal element doesn't include the quote
as it's a specific part within an f-string. For example,
```python
f"foo {x}"
# ^^^^
# This is the literal part of an f-string
```
The introduction of `FStringElement` enum is helpful which represent
either the literal part or the expression part of an f-string.
### Rule Updates
This means that there'll be two nodes representing a string depending on
the context. One for a normal string literal while the other is a string
literal within an f-string. The AST checker is updated to accommodate
this change. The rules which work on string literal are updated to check
on the literal part of f-string as well.
#### Notes
1. The `Expr::is_literal_expr` method would check for
`ExprStringLiteral` and return true if so. But now that we don't
represent the literal part of an f-string using that node, this improves
the method's behavior and confines to the actual expression. We do have
the `FStringElement::is_literal` method.
2. We avoid checking if we're in a f-string context before adding to
`string_type_definitions` because the f-string literal is now a
dedicated node and not part of `Expr`.
3. Annotations cannot use f-string so we avoid changing any rules which
work on annotation and checks for `ExprStringLiteral`.
## Test Plan
- All references of `Expr::StringLiteral` were checked to see if any of
the rules require updating to account for the f-string literal element
node.
- New test cases are added for rules which check against the literal
part of an f-string.
- Check the ecosystem results and ensure it remains unchanged.
## Performance
There's a performance penalty in the parser. The reason for this remains
unknown as it seems that the generated assembly code is now different
for the `__reduce154` function. The reduce function body is just popping
the `ParenthesizedExpr` on top of the stack and pushing it with the new
location.
- The size of `FStringElement` enum is the same as `Expr` which is what
it replaces in `FString::format_spec`
- The size of `FStringExpressionElement` is the same as
`ExprFormattedValue` which is what it replaces
I tried reducing the `Expr` enum from 80 bytes to 72 bytes but it hardly
resulted in any performance gain. The difference can be seen here:
- Original profile: https://share.firefox.dev/3Taa7ES
- Profile after boxing some node fields:
https://share.firefox.dev/3GsNXpD
### Backtracking
I tried backtracking the changes to see if any of the isolated change
produced this regression. The problem here is that the overall change is
so small that there's only a single checkpoint where I can backtrack and
that checkpoint results in the same regression. This checkpoint is to
revert using `Expr` to the `FString::format_spec` field. After this
point, the change would revert back to the original implementation.
## Review process
The review process is similar to #7927. The first set of commits update
the node structure, parser, and related AST files. Then, further commits
update the linter and formatter part to account for the AST change.
---------
Co-authored-by: David Szotten <davidszotten@gmail.com>
## Summary
Occasionally, valid code needs to use `argparse._SubParsersAction` in a
type annotation. This isn't great, but it's indicative of the fact that
public interfaces can return private types. If you accessed that private
type via a private interface, then we should be flagging the call site,
rather than the annotation.
Closes https://github.com/astral-sh/ruff/issues/9013.
## Summary
This PR updates the `ANN201`, `ANN202`, `ANN205`, and `ANN206` rules to
not create a fix for the return type when it's an abstract method and
the function body is empty i.e., it only contains either a pass
statement, docstring or an ellipsis literal.
fixes: #9004
## Test Plan
Add the following test cases:
- Abstract method with pass statement
- Abstract method with docstring
- Abstract method with ellipsis literal
- Abstract method with possible return type
This PR renames the semantic model flag `LITERAL` to `TYPING_LITERAL` to
better reflect its purpose. The main motivation behind this change is to
avoid any confusion with the "literal" terminology used in the AST for
literal nodes like string, bytes, numbers, etc.
## Summary
If a string has a Unicode prefix, we can't add the `r` prefix on top of
that -- we need to remove and replace it. (The Unicode prefix is
redundant anyway in Python 3.)
Closes https://github.com/astral-sh/ruff/issues/8967.
## Summary
Check PEP 695 type alias definitions for `snake-case-type-alias`
(`PYI042`) and `t-suffixed-type-alias` (`PYI043`)
Related to #8771.
## Test Plan
`cargo test`
## Summary
For `t-suffixed-type-alias` to trigger, the type alias needs to be
marked as such using the `typing.TypeAlias` annotation and the name of
the alias must be marked as private using a leading underscore. The
documentation example was of an unannotated type alias that was not
marked as private, which was misleading.
## Test Plan
The current example doesn't trigger the rule; the example in this merge
request does.
## Summary
- Adds `add_argument` similar to existing `remove_argument` utility to
safely add arguments to functions.
- Adds autofix for `PLW1514` as per specs requested in
https://github.com/astral-sh/ruff/issues/8883 as a test
## Test Plan
Checks on existing fixtures as well as additional test and fixture for
Python 3.9 and lower fix
## Issue Link
Closes: https://github.com/astral-sh/ruff/issues/8883
## Summary
Adds detection for branches without a `return` or `raise`, so that we
can properly `Optional` the return types. I'd like to remove this and
replace it with our code graph analysis from the `unreachable.rs` rule,
but it at least fixes the worst offenders.
Closes#8942.
## Summary
Triggers `no-return-argument-annotation-in-stub` (`PYI050`) for vararg
and kwarg `NoReturn` type annotations.
Related to #8771.
## Test Plan
`cargo test`
## Summary
When a function uses `@functools.singledispatch`, we need to treat the
first argument of any implementations as runtime-required.
Closes https://github.com/astral-sh/ruff/issues/6849.
## Summary
This PR fixes the bug where the autofix for `TRIO115` was taking the
entire arguments range for the fix which included the parenthesis as
well. This means that the fix would remove the arguments and the
parenthesis. The fix is to use the correct range.
fixes: #8713
## Test Plan
Update existing snapshots :)
## Summary
Part 2 of implementing the reverted autofix for `PYI030`
Also handles `typing.Union` and `typing_extensions.Literal` etc, uses
the first subscript name it finds for each offensive line.
## Test Plan
<!-- How was it tested? -->
`cargo test` and manually
---------
Co-authored-by: Zanie Blue <contact@zanie.dev>
## Summary
These rewrites are only (potentially) unsafe on Python versions that
predate their introduction into the standard library and grammar, so it
seems correct to mark them as safe on those later versions.
## Summary
Allows assignments of the form, e.g., `Attachment =
apps.get_model("zerver", "Attachment")`, for better compatibility with
Django.
Closes https://github.com/astral-sh/ruff/issues/7675.
## Test Plan
`cargo test`
## Summary
This PR updates the `E402` rule to work at cell level for Jupyter
notebooks. This is enabled only in preview to gather feedback.
The implementation basically resets the import boundary flag on the
semantic model when we encounter the first statement in a cell.
Another potential solution is to introduce `E403` rule that is
specifically for notebooks that works at cell level while `E402` will be
disabled for notebooks.
## Test Plan
Add a notebook with imports in multiple cells and verify that the rule
works as expected.
resolves: #8669
## Summary
Given `Union[Dict, None]` (in our internal representation), we were
filtering out `Dict` since we treat it as un-representable (i.e., we
can't convert it to an expression), returning just `None` as the type
annotation. We should require that all members of the union are
representable.
Closes https://github.com/astral-sh/ruff/issues/8879.
## Summary
Closes#1567.
Add both `length-sort` and `length-sort-straight` settings for isort.
Here are a few notable points:
- The length is determined using the
[`unicode_width`](https://crates.io/crates/unicode-width) crate, i.e. we
are talking about displayed length (this is explicitly mentioned in the
description of the setting)
- The dots are taken into account in the length to be compatible with
the original isort
- I had to reorder a few fields of the module key struct for it all to
make sense (notably the `force_to_top` field is now the first one)
## Test Plan
I added tests for the following cases:
- Basic tests for length-sort with ASCII characters only
- Tests with non-ASCII characters
- Tests with relative imports
- Tests for length-sort-straight
## Summary
This PR is a follow-up to the AST refactor which does the following:
- Remove `Deref` implementation on `StringLiteralValue` and use explicit
`as_str` calls instead. The `Deref` implementation would implicitly
perform allocations in case of implicitly concatenated strings. This is
to make sure the allocation is explicit.
- Now, certain methods can be implemented to do zero allocations which
have been implemented in this PR. They are:
- `is_empty`
- `len`
- `chars`
- Custom `PartialEq` implementation to compare each character
## Test Plan
Run the linter test suite and make sure all tests pass.
## Summary
This PR updates the string nodes (`ExprStringLiteral`,
`ExprBytesLiteral`, and `ExprFString`) to account for implicit string
concatenation.
### Motivation
In Python, implicit string concatenation are joined while parsing
because the interpreter doesn't require the information for each part.
While that's feasible for an interpreter, it falls short for a static
analysis tool where having such information is more useful. Currently,
various parts of the code uses the lexer to get the individual string
parts.
One of the main challenge this solves is that of string formatting.
Currently, the formatter relies on the lexer to get the individual
string parts, and formats them including the comments accordingly. But,
with PEP 701, f-string can also contain comments. Without this change,
it becomes very difficult to add support for f-string formatting.
### Implementation
The initial proposal was made in this discussion:
https://github.com/astral-sh/ruff/discussions/6183#discussioncomment-6591993.
There were various AST designs which were explored for this task which
are available in the linked internal document[^1].
The selected variant was the one where the nodes were kept as it is
except that the `implicit_concatenated` field was removed and instead a
new struct was added to the `Expr*` struct. This would be a private
struct would contain the actual implementation of how the AST is
designed for both single and implicitly concatenated strings.
This implementation is achieved through an enum with two variants:
`Single` and `Concatenated` to avoid allocating a vector even for single
strings. There are various public methods available on the value struct
to query certain information regarding the node.
The nodes are structured in the following way:
```
ExprStringLiteral - "foo" "bar"
|- StringLiteral - "foo"
|- StringLiteral - "bar"
ExprBytesLiteral - b"foo" b"bar"
|- BytesLiteral - b"foo"
|- BytesLiteral - b"bar"
ExprFString - "foo" f"bar {x}"
|- FStringPart::Literal - "foo"
|- FStringPart::FString - f"bar {x}"
|- StringLiteral - "bar "
|- FormattedValue - "x"
```
[^1]: Internal document:
https://www.notion.so/astral-sh/Implicit-String-Concatenation-e036345dc48943f89e416c087bf6f6d9?pvs=4
#### Visitor
The way the nodes are structured is that the entire string, including
all the parts that are implicitly concatenation, is a single node
containing individual nodes for the parts. The previous section has a
representation of that tree for all the string nodes. This means that
new visitor methods are added to visit the individual parts of string,
bytes, and f-strings for `Visitor`, `PreorderVisitor`, and
`Transformer`.
## Test Plan
- `cargo insta test --workspace --all-features --unreferenced reject`
- Verify that the ecosystem results are unchanged
## Summary
This PR updates the `E703` rule to avoid flagging any semicolons if
they're present after the last expression in a notebook cell. These are
intended to hide the cell output.
Part of #8669
## Test Plan
Add test notebook and update the snapshots.
## Summary
This PR updates `B015` and `B018` to ignore last top-level expressions
in each cell of a Jupyter Notebook.
Part of #8669
## Test Plan
Add test cases for both rules and update the snapshots.
closes#8732
I noticed that the reference to the setting in the rule docs doesn't
work, but there seem to be something wrong with pylint settings in
general in the docs - the "For related settings, see ...." is also
missing there.
# Summary
This setting behaves similarly to the ``from_first`` setting in isort
upstream, and sorts "from X import Y" type imports before straight
imports.
Like the other PR I added, happy to refactor if this is better in
another form.
Fixes#8662
# Test plan
I've added a unit test, and ran this on a large codebase that relies on
this setting in isort to verify it doesn't have unexpected side effects.
Semantically it makes sense to put certain contextmanagers into separate
with statements. Currently asyncio.timeout and its relatives in anyio
and trio are exempt from SIM117.
Closes https://github.com/astral-sh/ruff/issues/8606
## Summary
Exempt asyncio.timeout and related functions from SIM117 (Collapse with
statements where possible).
See https://github.com/astral-sh/ruff/issues/8606 for more.
## Test Plan
Extended the insta tests.
## Summary
Fixes#8750. `import __main__` is now considered a first-party import,
and is grouped accordingly by the linter and formatter.
## Test Plan
Added a test based off code supplied in the linked issue.
## Summary
In 2.0, Pydantic has moved the `BaseSettings` class to a separate
package called `pydantic-settings`
(https://docs.pydantic.dev/2.4/migration/#basesettings-has-moved-to-pydantic-settings),
which results in a false positive on `RUF012` (`mutable-class-default`).
A simple fix for that would be adding `pydantic_settings.BaseSettings`
base to the `has_default_copy_semantics` helper, which I've done in this
PR.
Related issue: #5308
## Test Plan
`cargo test`
## Summary
It turns out that some type checkers rely on the presence of ellipses in
`Protocol` interfaces and abstract methods, in order to differentiate
between default implementations and stubs. This PR modifies the preview
behavior of `PIE790` to avoid flagging "unnecessary" ellipses in such
cases.
Closes https://github.com/astral-sh/ruff/issues/8756.
## Test Plan
`cargo test`
If you define a subclass of `pydantic.BaseModel`, and then a subclass of
_that_ class in the same file, we'll now correctly treat it as
runtime-evaluated.
Closes https://github.com/astral-sh/ruff/issues/7893.
<!--
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 the Pylint rule E1132 to check for repeated keyword arguments in a
function call.
## Test Plan
Tested via the included unit tests and manual spot checking.
## Summary
Ensures that we can catch cases like:
```python
ages = {"Tom": 23, "Maria": 23, "Dog": 11}
age = ages.get("Cat", None)
```
Previously, the rule was somewhat useless, as it only checked for
literal accesses.
Closes https://github.com/astral-sh/ruff/issues/8760.
Closes https://github.com/astral-sh/ruff/issues/8695
We track the smallest offset seen for overindented lines then only
reduce the indentation of the lines that far to preserve indentation in
other lines. This rule's behavior now matches our formatter, which is
nice.
We may want to gate this with preview.
We ended up with a syntax error here via `from trio import
lowlevel.checkpoint`. The new solution avoids that error, but does miss
cases like:
```py
from trio.lowlevel import Timer
```
Where it could insert `from trio.lowlevel import Timer, checkpoint`.
Instead, it'll add `from trio import lowlevel`.
See:
https://github.com/astral-sh/ruff/issues/8402#issuecomment-1810838129
Update to [Rust
1.74](https://blog.rust-lang.org/2023/11/16/Rust-1.74.0.html) and use
the new clippy lints table.
The update itself introduced a new clippy lint about superfluous hashes
in raw strings, which got removed.
I moved our lint config from `rustflags` to the newly stabilized
[workspace.lints](https://doc.rust-lang.org/stable/cargo/reference/workspaces.html#the-lints-table).
One consequence is that we have to `unsafe_code = "warn"` instead of
"forbid" because the latter now actually bans unsafe code:
```
error[E0453]: allow(unsafe_code) incompatible with previous forbid
--> crates/ruff_source_file/src/newlines.rs:62:17
|
62 | #[allow(unsafe_code)]
| ^^^^^^^^^^^ overruled by previous forbid
|
= note: `forbid` lint level was set on command line
```
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
I think it's reasonable to avoid raising `INP001` for scripts, and
shebangs are one sufficient way to detect scripts.
Closes https://github.com/astral-sh/ruff/issues/8690.
## Summary
This exists to power a test, but it ends up affecting the behavior of
all files in the directory. Namely, it means that these files _aren't_
excluded when you format or lint them directly, since in that case, Ruff
will fall back to looking at the `pyproject.toml` in
`crates/ruff_linter/resources/test/fixtures`, which _doesn't_ exclude
these files, unlike our top-level `pyproject.toml`.
## Summary
We already support inserting imports for `I002` -- this PR just adds the
same fix for `FA102`, which is explicitly about `from __future__ import
annotations`.
Closes https://github.com/astral-sh/ruff/issues/8682.
## Summary
It seems like the range of an `ExprStringLiteral` can be somewhat
unreliable when the string is part of an implicit concatenation with an
f-string. Using the tokens themselves is more reliable.
Closes#8680.
Closes https://github.com/astral-sh/ruff/issues/7784.
## Summary
`display` is a special-cased builtin in IPython. This PR adds it to the
builtin namespace when analyzing IPython notebooks.
Closes https://github.com/astral-sh/ruff/issues/8702.
## Summary
This adds an autofix for PIE800 (unnecessary spread) -- whenever we see
a `**{...}` inside another dictionary literal, just delete the `**{` and
`}` to inline the key-value pairs. So `{"a": "b", **{"c": "d"}}` becomes
just `{"a": "b", "c": "d"}`.
I have enabled this just for preview mode.
## Test Plan
Updated the preview snapshot test.
## Summary
Implements
[FURB136](https://github.com/dosisod/refurb/blob/master/docs/checks.md#furb136-use-min-max)
that checks for `if` expressions that can be replaced with `min()` or
`max()` calls. See issue #1348 for more information.
This implementation diverges from Refurb's original implementation by
retaining the order of equal values. For example, Refurb suggest that
the following expressions:
```python
highest_score1 = score1 if score1 > score2 else score2
highest_score2 = score1 if score1 >= score2 else score2
```
should be to rewritten as:
```python
highest_score1 = max(score1, score2)
highest_score2 = max(score1, score2)
```
whereas this implementation provides more correct alternatives:
```python
highest_score1 = max(score2, score1)
highest_score2 = max(score1, score2)
```
## Test Plan
Unit test checks all eight possibilities.
## Summary
This adds a ``no-sections`` option for isort in the linter, similar to
the ``no_sections`` option that exists in upstream isort
(https://pycqa.github.io/isort/docs/configuration/options.html#no-sections)
This option puts all imports except for ``__future__`` into the same
section, and is mostly used by monorepos.
I've taken a bit of a leap in assuming that ruff wants to support the
exact same option; more than happy to refactor if you'd prefer a
different way of setting this up.
Fixes#8653
## Test Plan
I've added a test and have run it on a large Python codebase that uses
isort with --no-sections. The option is disabled by default.
While fixing #8661 I noticed that the code structure for sorting imports
could be simplified.
## Summary
- Move the logic for `force_sort_within_sections` from `isort/mod.rs` to
`isort/ordering.rs` => now there is just one line in `isort/mod.rs`:
`let imports = order_imports(import_block, settings);` which yields the
sorted imports
- Change the function signature of `order_imports` to directly return a
`Vec<EitherImport<'a>>` => no need for `OrderedImportBlock`
I think this is a bit of an improvement because the code is simpler and
there should be a bit of a speedup when setting
`force-sort-within-sections` to true. Indeed, when it's set to true
we're now directly ordering all the imports, whereas before we would
first order the straight imports, then the from imports, combine them
and finally sort the combination a second time (this is probably not
noticeable in practice though).
## Test Plan
No tests added, this is a simple refactor.
For the `PLW0129` rule, the f-string case shouldn't match against bytes
literal as f-strings cannot contain them. F-strings are made up of
either string literals or formatted expressions.
## Summary
This PR adds (unsafe) fixes to the flake8-annotations rules that enforce
missing return types, offering to automatically insert type annotations
for functions with literal return values. The logic is smart enough to
generate simplified unions (e.g., `float` instead of `int | float`) and
deal with implicit returns (`return` without a value).
Closes https://github.com/astral-sh/ruff/issues/1640 (though we could
open a separate issue for referring parameter types).
Closes https://github.com/astral-sh/ruff/issues/8213.
## Test Plan
`cargo test`
Fixes#8661
## Summary
Imports like `from x import y` don't have an "asname" for the module, so
they were placed before imports like `import x as w` since `None` <
`Some(s)` for any string s.
The fix is to first sort by `first_alias`, since it's `None` for `import
x as w`, and then by `asname`.
## Test Plan
I included the example from the issue to avoid future regressions.
When using the autofixer for `Q000` it does not remove the backslashes
from quotes that no longer need escaping.
This new rule checks for such backslashes (regardless whether they come
from the autofixer or not) and can remove them.
fixes#8617
## Summary
This PR extends `unnecessary-pass` (`PIE790`) to flag unnecessary
ellipsis expressions in addition to `pass` statements. A `pass` is
equivalent to a standalone `...`, so it feels correct to me that a
single rule should cover both cases.
When we look to v0.2.0, we should also consider deprecating `PYI013`,
which flags ellipses only for classes.
Closes https://github.com/astral-sh/ruff/issues/8602.
## Summary
PIE807 will rewrite `lambda: []` to `list` -- AFAICT though, the same
rationale also applies to dicts, so I've modified the code to also
rewrite `lambda: {}` to `dict`.
Two things I'm not sure about:
* Should this go to a new rule? This no longer actually matches the
behavior of flake8-pie, and while I think thematically it makes sense to
be part of the same rule, we could make it a standalone rule (but if so,
where should I put it and what error code should I use)?
* If we want a single rule, are there backwards compatibility concerns
with the rule name change (from `reimplemented_list_builtin` to
`reimplemented_container_builtin`?
## Test Plan
Added snapshot tests of the functionality.
## Summary
This PR makes `whitespace-before-punctuation` (`E203`) compatible with
the formatter by relaxing the rule a bit, as compared to the pycodestyle
implementation. It's also more consistent with PEP 8, which says:
> However, in a slice the colon acts like a binary operator, and should
have equal amounts on either side (treating it as the operator with the
lowest priority).
Closes https://github.com/astral-sh/ruff/issues/7259.
Closes https://github.com/astral-sh/ruff/issues/8642.
## Test Plan
`cargo test`
Previously, this rule used the range of the `self` annotation, but it's
a lot more natural to use the range of the function name (since it also
means the `# noqa` is associated with the method rather than its first
argument).
Closes https://github.com/astral-sh/ruff/issues/8635.
## Summary
- Close#7487
In the spirit of `flake8-boolean-trap`, any positional argument that can
accept a boolean should raise `FBT001`.
Raise `FBT001` for all annotations that accept booleans (e.g.
`Optional[bool]`, `Union[int, bool]`).
## Test Plan
Add a fixture, with an annotation using `|`, `Optional`, and `Union`,
and containing a boolean.
## Summary
Removes unnecessary commentary from the PD901 message. This does make it
different from pandas-vet, but it improves consistency with the rest of
messages.
Current Message:
> `df` is a bad variable name. Be kinder to your future self.
New Message
> `df` is a bad variable name.
## Test Plan
The relevant snapshot has been updated with the new message.
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
I got an error from RUF001 and wanted to override it. How to do that was
not quite obvious. In the process I have tried to improve the
documentation for the rule and it's siblings.
## Summary
Adds an extra check to F632 to check for any `is` comparisons to a
mutable initialisers.
Implements #8589 .
Example:
```Python
named_var = {}
if named_var is {}: # F632 (fix)
pass
```
The if condition will always evaluate to False because it checks on
identity and it's impossible to take the same identity as a hard coded
list/set/dict initializer.
## Test Plan
Multiple test cases were added to ensure the rule works + doesn't flag
false positives + the fix works correctly.
Previously, this lint had its alias detection logic a little
backwards. That is, for Python 3.11+, it would *only* detect
asyncio.TimeoutError as an alias, but it should have also detected
socket.timeout as an alias. And in Python <3.11, it would falsely
detect asyncio.TimeoutError as an alias where it should have only
detected socket.timeout as an alias.
We fix it so that both asyncio.TimeoutError and socket.timeout are
detected as aliases in Python 3.11+, and only socket.timeout is
detected as an alias in Python 3.10.
Fixes#8565
## Test Plan
I tested this by updating the existing snapshot test which had
erroneously
asserted that socket.timeout should not be replaced with TimeoutError in
Python
3.11+. I also added a new regression test that targets Python 3.10 and
ensures
that the suggestion to replace asyncio.TimeoutError with TimeoutError
does not
occur.
## Summary
This fixes the bug where the `flake8-commas` rules weren't taking the
new f-string tokens into account.
## Test Plan
Add new test cases around f-strings for all of `flake8-commas`'s rules.
fixes: #8556
## Summary
An assignment can be _both_ (e.g.) a loop variable _and_ assigned via
unpacking. In other words, unpacking is a quality of an assignment, not
a _kind_.
## Summary
This brings ruff's behavior in line with what `pep8-naming` already does
and thus closes#8397.
I had initially implemented this to look at the last segment of a dotted
path only when the entry in the `*-decorators` setting started with a
`.`, but in the end I thought it's better to remain consistent w/
`pep8-naming` and doing a match against the last segment of the
decorator name in any case.
If you prefer to diverge from this in favor of less ambiguity in the
configuration let me know and I'll change it so you would need to put
e.g. `.expression` in the `classmethod-decorators` list.
## Test Plan
Tested against the file in the issue linked below, plus the new testcase
added in this PR.
~Improves detection of types imported from `typing_extensions`. Removes
the hard-coded list of supported types in `typing_extensions`; instead
assuming all types could be imported from `typing`, `_typeshed`, or
`typing_extensions`.~
~The typing extensions package appears to re-export types even if they
do not need modification.~
Adds detection of `if typing_extensions.TYPE_CHECKING` blocks. Avoids
inserting a new `if TYPE_CHECKING` block and `from typing import
TYPE_CHECKING` if `typing_extensions.TYPE_CHECKING` is used (closes
https://github.com/astral-sh/ruff/issues/8427)
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
This PR fixes the bug where the generated fix for `EM*` rules would
replace a
triple-quoted (f-)string with a single-quoted (f-)string. This changes
the
semantic of the string in case it contains a single-quoted string
literal. This
is especially evident with f-strings where the expression could contain
another
string within it. For example,
```python
f"""normal {"another"} normal"""
```
## Test Plan
Add test case for triple-quoted string and update the snapshots.
fixes: #6988fixes: #7736
This ensures the python label is used for all python code blocks for
consistency.
## Test Plan
Visual inspection of all changes via git client ensuring no other
changes were made in error.
## Summary
This PR addresses the incompatibility with `jupyterlab-lsp` +
`python-lsp-ruff` arising from the inference of source type from file
extension, raised in #6847.
In particular it follows the suggestion in
https://github.com/astral-sh/ruff/issues/6847#issuecomment-1765724679 to
specify a mapping from file extension to source type.
The source types are
- python
- pyi
- ipynb
Usage:
```sh
ruff check --no-cache --stdin-filename Untitled.ipynb --extension ipynb:python
```
Unlike the original suggestion, `:` instead of `=` is used to associate
file extensions to language since that is what is used with
`--per-file-ignores` which is an existing option that accepts a mapping.
## Test Plan
2 tests added to `integration_test.rs` to ensure the override works as
expected
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
Prior to this change `extend_unsafe_fixes` took precedence over
`extend_safe_fixes` selectors, so any conflicts were resolved in favour
of `extend_unsafe_fixes`. Thanks to that ruff were conservatively
assuming that if configs conlict the fix corresponding to selected rule
will be treated as unsafe.
After this change we take into account Specificity of the selectors. For
conflicts between selectors of the same Specificity we will treat the
corresponding fixes as unsafe. But if the conflicting selectors are of
different specificity the more specific one will win.
## Test Plan
Tests were added for the `FixSafetyTable` struct. The
`check_extend_unsafe_fixes_conflict_with_extend_safe_fixes_by_specificity`
integration test was added to test conflicting rules of different
specificity.
Fixes#8404
---------
Co-authored-by: Zanie <contact@zanie.dev>
These names are only ever displayed internally right now and we could be
clearer in our test snapshots.
The diff is kind of scary because all of the tests fixtures are updated.
## Summary
This commit adds some additional error checking to the parser such that
assignments that are invalid syntax are rejected. This covers the
obvious cases like `5 = 3` and some not so obvious cases like `x + y =
42`.
This does add an additional recursive call to the parser for the cases
handling assignments. I had initially been concerned about doing this,
but `set_context` is already doing recursion during assignments, so I
didn't feel as though this was changing any fundamental performance
characteristics of the parser. (Also, in practice, I would expect any
such recursion here to be quite shallow since the recursion is done on
the target of an assignment. Such things are rarely nested much in
practice.)
Fixes#6895
## Test Plan
I've added unit tests covering every case that is detected as invalid on
an `Expr`.
## Summary
Fixes bug in `TRIO115` where it would not `return` for values that were
not a `NumberLiteral` so
```python
x = "bla"
trio.sleep(x)
```
would set off a false positive
## Test Plan
Added test case to fixture
## Summary
Given `key in obj.keys()`, `obj` _could_ be a dictionary, or it could be
another type that defines
a `.keys()` method. In the latter case, removing the `.keys()` attribute
could lead to a runtime error.
Previously, we marked all `SIM118` fixes as unsafe for this reason;
however, in preview, we now mark them as safe if we can
infer that the expression is a dictionary.
## Test Plan
Added a preview fixture.
## Summary
We have this pattern in a bunch of places, where we find the _only_
binding to a name (and return `None`) if it's bound multiple times. This
PR DRYs it up into a method on `SemanticModel`.
## Summary
If you want to create an edit with dynamic applicability, you have to
branch and repeat the edit entirely between the two branches. If you
further need the edit itself to be dynamic (e.g., perhaps you have a
single edit in one case, vs. multiple in another), you suddenly have
four branches. This PR just adds an alternate constructor that takes
applicability as an argument, as an escape hatch.
## Summary
Implement
[`no-is-type-none`](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/no_is_type_none.py)
as `type-none-comparison` (`FURB169`).
Auto-fixes comparisons that use `type` to compare the type of an object
to `type(None)` to a `None` identity check. For example,
```python
type(foo) is type(None)
```
becomes
```python
foo is None
```
Related to #1348.
## Test Plan
`cargo test`
## Summary
Black and Ruff's preview styles now collapse statements like:
```python
from contextlib import nullcontext
ctx = nullcontext()
with ctx: ...
```
Historically, we made an exception here for classes
(https://github.com/astral-sh/ruff/pull/2837). This PR extends it to
other statement kinds for consistency with the formatter.
Closes https://github.com/astral-sh/ruff/issues/8496.
## Summary
Adds `TRIO105` from the [flake8-trio
plugin](https://github.com/Zac-HD/flake8-trio). The `MethodName` logic
mirrors that of `TRIO100` to stay consistent within the plugin.
It is at 95% parity with the exception of upstream also checking for a
slightly more complex scenario where a call to `start()` on a
`trio.Nursery` context should also be immediately awaited. Upstream
plugin appears to just check for anything named `nursery` judging from
[the relevant issue](https://github.com/Zac-HD/flake8-trio/issues/56).
Unsure if we want to do so something similar or, alternatively, if there
is some capability in ruff to check for calls made on this context some
other way
## Test Plan
Added a new fixture, based on [the one from upstream
plugin](https://github.com/Zac-HD/flake8-trio/blob/main/tests/eval_files/trio105.py)
## Issue link
Refers: https://github.com/astral-sh/ruff/issues/8451
## Summary
Adds `memoryview` to the list of typeclasses that `fn is_type()` uses
for type comparison checks so that it raises a violation if `is`, `is
not` or `isinstance()` are not used.
## Test Plan
Added examples to existing fixture
## Issue Link
Closes: https://github.com/astral-sh/ruff/issues/8483
This is the one refactor in the NumPy 2.0 upgrade rule that isn't
compatible with earlier versions of NumPy, so I'm marking it as unsafe
and adding a dedicated message.
## Summary
Currently, `UP032` applied to raw strings results in format strings with
the prefix 'fr'. This gets changed to 'rf' by Ruff format (or Black). In
order to avoid that, this PR uses the prefix 'rf' to begin with.
## Test Plan
Updated the expectation on an existing test.
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Hi! Currently NumPy Python API is undergoing a cleanup process that will
be delivered in NumPy 2.0 (release is planned for the end of the year).
Most changes are rather simple (renaming, removing or moving a member of
the main namespace to a new place), and they could be flagged/fixed by
an additional ruff rule for numpy (e.g. changing occurrences of
`np.float_` to `np.float64`).
Would you accept such rule?
I named it `NPY201` in the existing group, so people will receive a
heads-up for changes arriving in 2.0 before actually migrating to it.
~~This is still a draft PR.~~ I'm not an expert in rust so if any part
of code can be done better please share!
NumPy 2.0 migration guide:
https://numpy.org/devdocs/numpy_2_0_migration_guide.html
NEP 52: https://numpy.org/neps/nep-0052-python-api-cleanup.html
NumPy cleanup tracking issue:
https://github.com/numpy/numpy/issues/23999
## Test Plan
A unit test is provided that checks all rule's fix cases.
## Summary
By using `set`, we were setting the bracket flag to `false` if another
operator was visited.
Closes https://github.com/astral-sh/ruff/issues/8379.
## Test Plan
`cargo test`
## Summary
This PR adds a new `LiteralExpressionRef` which wraps all of the literal
expression nodes in a single enum. This allows for a narrow type when
working exclusively with a literal node. Additionally, it also
implements a `Expr::as_literal_expr` method to return the new enum if
the expression is indeed a literal one.
A few rules have been updated to account for the new enum:
1. `redundant_literal_union`
2. `if_else_block_instead_of_dict_lookup`
3. `magic_value_comparison`
To account for the change in (2), a new `ComparableLiteral` has been
added which can be constructed from the new enum
(`ComparableLiteral::from(<LiteralExpressionRef>)`).
### Open Questions
1. The new `ComparableLiteral` can be exclusively used via the
`LiteralExpressionRef` enum. Should we remove all of the literal
variants from `ComparableExpr` and instead have a single
`ComparableExpr::Literal(ComparableLiteral)` variant instead?
## Test Plan
`cargo test`
## Summary
We were considering the `{` within an f-string to be a left brace, which
caused the "space-after-colon" rule to trigger incorrectly.
Closes https://github.com/astral-sh/ruff/issues/8299.
## Summary
If the value of `shell` wasn't literally `True`, we now show a message
describing it as truthy, rather than the (misleading) `shell=True`
literal in the diagnostic.
Closes https://github.com/astral-sh/ruff/issues/8310.
## Summary
This PR adds `Default` for the following literal nodes:
* `StringLiteral`
* `BytesLiteral`
* `BooleanLiteral`
* `NoneLiteral`
* `EllipsisLiteral`
The implementation creates the zero value of the respective literal
nodes in terms of the Python language.
## Test Plan
`cargo test`
## Summary
This PR splits the `Constant` enum as individual literal nodes. It
introduces the following new nodes for each variant:
* `ExprStringLiteral`
* `ExprBytesLiteral`
* `ExprNumberLiteral`
* `ExprBooleanLiteral`
* `ExprNoneLiteral`
* `ExprEllipsisLiteral`
The main motivation behind this refactor is to introduce the new AST
node for implicit string concatenation in the coming PR. The elements of
that node will be either a string literal, bytes literal or a f-string
which can be implemented using an enum. This means that a string or
bytes literal cannot be represented by `Constant::Str` /
`Constant::Bytes` which creates an inconsistency.
This PR avoids that inconsistency by splitting the constant nodes into
it's own literal nodes, literal being the more appropriate naming
convention from a static analysis tool perspective.
This also makes working with literals in the linter and formatter much
more ergonomic like, for example, if one would want to check if this is
a string literal, it can be done easily using
`Expr::is_string_literal_expr` or matching against `Expr::StringLiteral`
as oppose to matching against the `ExprConstant` and enum `Constant`. A
few AST helper methods can be simplified as well which will be done in a
follow-up PR.
This introduces a new `Expr::is_literal_expr` method which is the same
as `Expr::is_constant_expr`. There are also intermediary changes related
to implicit string concatenation which are quiet less. This is done so
as to avoid having a huge PR which this already is.
## Test Plan
1. Verify and update all of the existing snapshots (parser, visitor)
2. Verify that the ecosystem check output remains **unchanged** for both
the linter and formatter
### Formatter ecosystem check
#### `main`
| project | similarity index | total files | changed files |
|----------------|------------------:|------------------:|------------------:|
| cpython | 0.75803 | 1799 | 1647 |
| django | 0.99983 | 2772 | 34 |
| home-assistant | 0.99953 | 10596 | 186 |
| poetry | 0.99891 | 317 | 17 |
| transformers | 0.99966 | 2657 | 330 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99978 | 3669 | 20 |
| warehouse | 0.99977 | 654 | 13 |
| zulip | 0.99970 | 1459 | 22 |
#### `dhruv/constant-to-literal`
| project | similarity index | total files | changed files |
|----------------|------------------:|------------------:|------------------:|
| cpython | 0.75803 | 1799 | 1647 |
| django | 0.99983 | 2772 | 34 |
| home-assistant | 0.99953 | 10596 | 186 |
| poetry | 0.99891 | 317 | 17 |
| transformers | 0.99966 | 2657 | 330 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99978 | 3669 | 20 |
| warehouse | 0.99977 | 654 | 13 |
| zulip | 0.99970 | 1459 | 22 |
## Summary
Refactor for isort implementation. Closes#7738.
I introduced a `NatOrdString` and a `NatOrdStr` type to have a naturally
ordered `String` and `&str`, and I pretty much went back to the original
implementation based on `module_key`, `member_key` and
`sorted_by_cached_key` from itertools. I tried my best to avoid
unnecessary allocations but it may have been clumsy in some places, so
feedback is appreciated! I also renamed the `Prefix` enum to
`MemberType` (and made some related adjustments) because I think this
fits more what it is, and it's closer to the wording found in the isort
documentation.
I think the result is nicer to work with, and it should make
implementing #1567 and the like easier :)
Of course, I am very much open to any and all remarks on what I did!
## Test Plan
I didn't add any test, I am relying on the existing tests since this is
just a refactor.
## Summary
Implements pylint C0415 (import-outside-toplevel) — imports should be at
the top level of a file.
The great debate I had on this implementation is whether "top-level" is
one word or two (`toplevel` or `top_level`). I opted for 2 because that
seemed to be how it is used in the codebase but the rule string itself
uses one-word "toplevel." 🤷 I'd be happy to change it as desired.
I suppose this could be auto-fixed by moving the import to the
top-level, but it seems likely that the author's intent was to actually
import this dynamically, so I view the main point of this rule is to
force some sort of explanation, and auto-fixing might be annoying.
For reference, this is what "pylint" reports:
```
> pylint crates/ruff/resources/test/fixtures/pylint/import_outside_top_level.py
************* Module import_outside_top_level
...
crates/ruff/resources/test/fixtures/pylint/import_outside_top_level.py:4:4: C0415: Import outside toplevel (string) (import-outside-toplevel)
```
ruff would now report:
```
import_outside_top_level.py:4:5: PLC0415 `import` should be used only at the top level of a file
|
3 | def import_outside_top_level():
4 | import string # [import-outside-toplevel]
| ^^^^^^^^^^^^^ PLC0415
|
```
Contributes to https://github.com/astral-sh/ruff/issues/970.
## Test Plan
Snapshot test.
## Summary
Implement
[`no-isinstance-type-none`](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/no_isinstance_type_none.py)
as `isinstance-type-none` (`FURB168`).
Auto-fixes calls to `isinstance` to check if an object is `None` to a
`None` identity check. For example,
```python
isinstance(foo, type(None))
```
becomes
```python
foo is None
```
Related to #1348.
## Test Plan
`cargo test`
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
This PR removes the `debug_assertion` in the `Indexer` to allow
unterminated f-strings. This is mainly a fix in the development build
which now matches the release build.
The fix is simple: remove the `debug_assertion` which means that the
there could be `FStringStart` and possibly `FStringMiddle` tokens
without a corresponding f-string range in the `Indexer`. This means that
the code requesting for the f-string index need to account for the
`None` case, making the code safer.
This also updates the code which queries the `FStringRanges` to account
for the `None` case. This will happen when the `FStringStart` /
`FStringMiddle` tokens are present but the `FStringEnd` token isn't
which means that the `Indexer` won't contain the range for that
f-string.
## Test Plan
`cargo test`
Taking the following code as an example:
```python
f"{123}
```
This only emits a `FStringStart` token, but no `FStringMiddle` or
`FStringEnd` tokens.
And,
```python
f"\.png${
```
This emits a `FStringStart` and `FStringMiddle` token, but no
`FStringEnd` token.
fixes: #8065
## Summary
This PR fixes the `W605` rule implementation to provide the quickfix
message as
per the fix provided.
## Test Plan
Update snapshots.
fixes: #8155
## Summary
Avoid warning about incompatible rules except if their configuration
directly conflicts with the formatter. This should reduce the noise and
potentially the need for https://github.com/astral-sh/ruff/issues/8175
and https://github.com/astral-sh/ruff/issues/8185
I also extended the rule and option documentation to mention any
potential formatter incompatibilities or whether they're redundant when
using the formatter.
* `LineTooLong`: This is a use case we explicitly want to support. Don't
warn about it
* `TabIndentation`, `IndentWithSpaces`: Only warn if
`indent-style="tab"`
* `IndentationWithInvalidMultiple`,
`IndentationWithInvalidMultipleComment`: Only warn if `indent-width !=
4`
* `OverIndented`: Don't warn, but mention that the rule is redundant
* `BadQuotesInlineString`: Warn if quote setting is different from
`format.quote-style`
* `BadQuotesMultilineString`, `BadQuotesDocstring`: Warn if `quote !=
"double"`
## Test Plan
I added a new integration test for the default configuration with `ALL`.
`ruff format` now only shows two incompatible rules, which feels more
reasonable.
## Summary
This rule is now unsafe if we can't verify that the `obj` in `raise
obj()` is a class or builtin. (If we verify that it's a function, we
don't raise at all, as before.)
See the documentation change for motivation behind the unsafe edit.
Closes https://github.com/astral-sh/ruff/issues/8228.
## Summary
This PR refactors the formatter diff code to reuse the
`SourceKind::diff` logic. This has the benefit that the Notebook diff
now includes the cell numbers which was not present before.
## Test Plan
Update the snapshots and verified the cell numbers.
## Summary
Python 3.12 added the `__buffer__()`/`__release_buffer_()` special
methods, which are incorrectly flagged as invalid dunder methods by
`PLW3201`.
## Test Plan
Added definitions to the test suite, and confirmed they failed without
the fix and are ignored after the fix was done.
## Summary
This PR renames the `tab-size` configuration option to `indent-width` to
express that the formatter uses the option to determine the indentation
width AND as tab width.
I first preferred naming the option `tab-width` but then decided to go
with `indent-width` because:
* It aligns with the `indent-style` option
* It would allow us to write a lint rule that asserts that each
indentation uses `indent-width` spaces.
Closes#7643
## Test Plan
Added integration test
## Summary
This PR introduces a new `pycodestyl.max-line-length` option that allows overriding the global `line-length` option for `E501` only.
This is useful when using the formatter and `E501` together, where the formatter uses a lower limit and `E501` is only used to catch extra-long lines.
Closes#7644
## Considerations
~~Our fix infrastructure asserts in some places that the fix doesn't exceed the configured `line-width`. With this change, the question is whether it should use the `pycodestyle.max-line-width` or `line-width` option to make that decision.
I opted for the global `line-width` for now, considering that it should be the lower limit. However, this constraint isn't enforced and users not using the formatter may only specify `pycodestyle.max-line-width` because they're unaware of the global option (and it solves their need).~~
~~I'm interested to hear your thoughts on whether we should use `pycodestyle.max-line-width` or `line-width` to decide on whether to emit a fix or not.~~
Edit: The linter users `pycodestyle.max-line-width`. The `line-width` option has been removed from the `LinterSettings`
## Test Plan
Added integration test. Built the documentation and verified that the links are correct.
## Summary
First time contribute to `ruff`, so If there are low-level errors,
please forgive me. 🙇
Introduce auto fix for `E275`, this partially address #8121.
## Test Plan
Already coverd.
<!--
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? -->
Close#8123
## Test Plan
<!-- How was it tested? -->
New test cases
---------
Signed-off-by: harupy <hkawamura0130@gmail.com>
## Summary
This PR updates our E721 implementation and semantics to match the
updated `pycodestyle` logic, which I think is an improvement.
Specifically, we now allow `type(obj) is int` for exact type
comparisons, which were previously impossible. So now, we're largely
just linting against code like `type(obj) == int`.
This change is gated to preview mode.
Closes https://github.com/astral-sh/ruff/issues/7904.
## Test Plan
Updated the test fixture and ensured parity with latest Flake8.
## Summary
This PR updates our documentation for the upcoming formatter release.
Broadly, the documentation is now structured as follows:
- Overview
- Tutorial
- Installing Ruff
- The Ruff Linter
- Overview
- `ruff check`
- Rule selection
- Error suppression
- Exit codes
- The Ruff Formatter
- Overview
- `ruff format`
- Philosophy
- Configuration
- Format suppression
- Exit codes
- Black compatibility
- Known deviations
- Configuring Ruff
- pyproject.toml
- File discovery
- Configuration discovery
- CLI
- Shell autocompletion
- Preview
- Rules
- Settings
- Integrations
- `pre-commit`
- VS Code
- LSP
- PyCharm
- GitHub Actions
- FAQ
- Contributing
The major changes include:
- Removing the "Usage" section from the docs, and instead folding that
information into "Integrations" and the new Linter and Formatter
sections.
- Breaking up "Configuration" into "Configuring Ruff" (for generic
configuration), and new Linter- and Formatter-specific sections.
- Updating all example configurations to use `[tool.ruff.lint]` and
`[tool.ruff.format]`.
My suggestion is to pull and build the docs locally, and review by
reading them in the browser rather than trying to parse all the code
changes.
Closes https://github.com/astral-sh/ruff/issues/7235.
Closes https://github.com/astral-sh/ruff/issues/7647.
This is my first PR and I'm new at rust, so feel free to ask me to
rewrite everything if needed ;)
The rule must be called after deferred lambas have been visited because
of the last check (whether the lambda parameters are used in the body of
the function that's being called). I didn't know where to do it, so I
did what I could to be able to work on the rule itself:
- added a `ruff_linter::checkers::ast::analyze::lambda` module
- build a vec of visited lambdas in `visit_deferred_lambdas`
- call `analyze::lambda` on the vec after they all have been visited
Building that vec of visited lambdas was necessary so that bindings
could be properly resolved in the case of nested lambdas.
Note that there is an open issue in pylint for some false positives, do
we need to fix that before merging the rule?
https://github.com/pylint-dev/pylint/issues/8192
Also, I did not provide any fixes (yet), maybe we want do avoid merging
new rules without fixes?
## Summary
Checks for lambdas whose body is a function call on the same arguments
as the lambda itself.
### Bad
```python
df.apply(lambda x: str(x))
```
### Good
```python
df.apply(str)
```
## Test Plan
Added unit test and snapshot.
Manually compared pylint and ruff output on pylint's test cases.
## References
- [pylint
documentation](https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/unnecessary-lambda.html)
- [pylint
implementation](https://github.com/pylint-dev/pylint/blob/main/pylint/checkers/base/basic_checker.py#L521-L587)
- https://github.com/astral-sh/ruff/issues/970
## Summary
The lint checks for number of arguments in a function *definition*, but
the message says “function *call*”
## Test Plan
See what breaks and change the tests
Given `print(*a_list_with_elements, sep="\n")`, we can't remove the
separator (unlike in `print(a, sep="\n")`), since we don't know how many
arguments were provided.
Closes https://github.com/astral-sh/ruff/issues/8078.
- Add changelog entry for 0.1.1
- Bump version to 0.1.1
- Require preview for fix added in #7967
- Allow duplicate headings in changelog (markdownlint setting)
## Summary
In #6157 a warning was introduced when users use `ruff: noqa`
suppression in-line instead of at the file-level. I had this trigger
today after forgetting about it, and the warning is an excellent
improvement.
I knew immediately what the issue was because I raised it previously,
but on reading the warning I'm not sure it would be so obvious to all
users. This PR extends the error with a short sentence explaining that
line-level suppression should omit the `ruff:` prefix.
## Test Plan
Not sure it's necessary for such a trivial change :)
<!--
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
In https://github.com/astral-sh/ruff/pull/7968, I introduced a
regression whereby we started to treat imports used _only_ in type
annotation bounds (with `__future__` annotations) as unused.
The root of the issue is that I started using `visit_annotation` for
these bounds. So we'd queue up the bound in the list of deferred type
parameters, then when visiting, we'd further queue it up in the list of
deferred type annotations... Which we'd then never visit, since deferred
type annotations are visited _before_ deferred type parameters.
Anyway, the better solution here is to use a dedicated flag for these,
since they have slightly different behavior than type annotations.
I've also fixed what I _think_ is a bug whereby we previously failed to
resolve `Callable` in:
```python
type RecordCallback[R: Record] = Callable[[R], None]
from collections.abc import Callable
```
IIUC, the values in type aliases should be evaluated lazily, like type
parameters.
Closes https://github.com/astral-sh/ruff/issues/8017.
## Test Plan
`cargo test`
## Summary
Rule B005 of flake8-bugbear docs has a typo in one of the examples that
leads to a confusion in the correctness of `.strip()` method
```python
# Wrong output (used in docs)
"text.txt".strip(".txt") # "ex"
# Correct output
"text.txt".strip(".txt") # "e"
```
## Summary
### What it does
This rule triggers an error when a bare raise statement is not in an
except or finally block.
### Why is this bad?
If raise statement is not in an except or finally block, there is no
active exception to
re-raise, so it will fail with a `RuntimeError` exception.
### Example
```python
def validate_positive(x):
if x <= 0:
raise
```
Use instead:
```python
def validate_positive(x):
if x <= 0:
raise ValueError(f"{x} is not positive")
```
## Test Plan
Added unit test and snapshot.
Manually compared ruff and pylint outputs on pylint's tests.
## References
- [pylint
documentation](https://pylint.pycqa.org/en/stable/user_guide/messages/error/misplaced-bare-raise.html)
- [pylint
implementation](https://github.com/pylint-dev/pylint/blob/main/pylint/checkers/exceptions.py#L339)
## Summary
Given `type RecordOrThings = Record | int | str`, the right-hand side
won't be evaluated at runtime. Same goes for `Record` in `type
RecordCallback[R: Record] = Callable[[R], None]`. This PR modifies the
visitation logic to treat them as typing-only.
Closes https://github.com/astral-sh/ruff/issues/7966.
## Summary
This PR adds a new `cell` field to the JSON output format which
indicates the Notebook cell this diagnostic (and fix) belongs to. It
also updates the location for the diagnostic and fixes as per the
`NotebookIndex`. It will be used in the VSCode extension to display the
diagnostic in the correct cell.
The diagnostic and edit start and end source locations are translated
for the notebook as per the `NotebookIndex`. The end source location for
an edit needs some special handling.
### Edit end location
To understand this, the following context is required:
1. Visible lines in Jupyter Notebook vs JSON array strings: The newline
is part of the string in the JSON format. This means that if there are 3
visible lines in a cell where the last line is empty then the JSON would
contain 2 strings in the source array, both ending with a newline:
**JSON format:**
```json
[
"# first line\n",
"# second line\n",
]
```
**Notebook view:**
```python
1 # first line
2 # second line
3
```
2. If an edit needs to remove an entire line including the newline, then
the end location would be the start of the next row.
To remove a statement in the following code:
```python
import os
```
The edit would be:
```
start: row 1, col 1
end: row 2, col 1
```
Now, here's where the problem lies. The notebook index doesn't have any
information for row 2 because it doesn't exists in the actual notebook.
The newline was added by Ruff to concatenate the source code and it's
removed before writing back. But, the edit is computed looking at that
newline.
This means that while translating the end location for an edit belong to
a Notebook, we need to check if both the start and end location belongs
to the same cell. If not, then the end location should be the first
character of the next row and if so, translate that back to the last
character of the previous row. Taking the above example, the translated
location for Notebook would be:
```
start: row 1, col 1
end: row 1, col 10
```
## Test Plan
Add test cases for notebook output in the JSON format and update
existing snapshots.
## Summary
This PR refactors the `NotebookIndex` struct to use `OneIndexed` to make
the
intent of the code clearer.
## Test Plan
Update the existing test case and run `cargo test` to verify the change.
- [x] Verify `--diff` output
- [x] Verify the diagnostics output
- [x] Verify `--show-source` output
## Summary
This PR fixes the bug where the rule `E251` was being triggered on a equal token
inside a f-string which was used in the context of debug expressions.
For example, the following was being flagged before the fix:
```python
print(f"{foo = }")
```
But, now it is not. This leads to false negatives such as:
```python
print(f"{foo(a = 1)}")
```
One solution would be to know if the opened parentheses was inside a f-string or
not. If it was then we can continue flagging until it's closed. If not, then we
should not flag it.
## Test Plan
Add new test cases and check that they don't raise any false positives.
fixes: #7882
## Summary
`foo(**{})` was an overlooked edge case for `PIE804` which introduced a
crash within the Fix, introduced in #7884.
I've made it so that `foo(**{})` turns into `foo()` when applied with
`--fix`, but is that desired/expected? 🤔 Should we just ignore instead?
## Test Plan
`cargo test`
Closes https://github.com/astral-sh/ruff/issues/7572
Drops formatting specific rules from the default rule set as they
conflict with formatters in general (and in particular, conflict with
our formatter). Most of these rules are in preview, but the removal of
`line-too-long` and `mixed-spaces-and-tabs` is a change to the stable
rule set.
## Example
The following no longer raises `E501`
```
echo "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = 1" | ruff check -
```
## Summary
Throughout the codebase, we have this pattern:
```rust
let mut diagnostic = ...
if checker.patch(Rule::UnusedVariable) {
// Do the fix.
}
diagnostics.push(diagnostic)
```
This was helpful when we computed fixes lazily; however, we now compute
fixes eagerly, and this is _only_ used to ensure that we don't generate
fixes for rules marked as unfixable.
We often forget to add this, and it leads to bugs in enforcing
`--unfixable`.
This PR instead removes all of these checks, moving the responsibility
of enforcing `--unfixable` up to `check_path`. This is similar to how
@zanieb handled the `--extend-unsafe` logic: we post-process the
diagnostics to remove any fixes that should be ignored.
## Summary
Add autofix for `PLR1714` using tuples.
If added complexity is desired, we can lean into the `set` part by doing
some kind of builtin check on all of the comparator elements for
starters, since we otherwise don't know if something's hashable.
## Test Plan
`cargo test`, and manually.
<!--
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? -->
closes https://github.com/astral-sh/ruff/issues/7912
## Test Plan
<!-- How was it tested? -->
Adds two configuration-file only settings `extend-safe-fixes` and
`extend-unsafe-fixes` which can be used to promote and demote the
applicability of fixes for rules.
Fixes with `Never` applicability cannot be promoted.
## Summary
Given:
```python
baz: Annotated[
str,
[qux for qux in foo],
]
```
We treat `baz` as `BindingKind::Annotation`, to ensure that references
to `baz` are marked as unbound. However, we were _also_ treating `qux`
as `BindingKind::Annotation`, which meant that the load in the
comprehension _also_ errored.
Closes https://github.com/astral-sh/ruff/issues/7879.
## Summary
This PR upgrades some rules from "sometimes" to "always" fixes, now that
we're getting ready to ship support in the CLI. The focus here was on
identifying rules for which the diagnostic itself is high-confidence,
and the fix itself is too (assuming that the diagnostic is correct).
This is _unlike_ rules that _may_ be a false positive, like those that
(e.g.) assume an object is a dictionary when you call `.values()` on it.
Specifically, I upgraded:
- A bunch of rules that only apply to `.pyi` files.
- Rules that rewrite deprecated imports or aliases.
- Some other misc. rules, like: `empty-print-string`, `unused-noqa`,
`getattr-with-constant`.
Open to feedback on any of these.
## Summary
Adds autofix to `PYI030`
Closes#7854.
Unsure if the cloning method I chose is the best solution here, feel
free to suggest alternatives!
## Test Plan
`cargo test` as well as manually
## Summary
Restores functionality of #7875 but in the correct place. Closes#7877.
~~I couldn't figure out how to get cargo fmt to work, so hopefully
that's run in CI.~~ Nevermind, figured it out.
## Test Plan
Can see output of json.
## Summary
Fixes#7853.
The old and new source files were reversed in the call to
`TextDiff::from_lines`, so the diff output of the CLI was also reversed.
## Test Plan
Two snapshots were updated in the process, so any reversal should be
caught :)
## Summary
Resolves https://github.com/astral-sh/ruff/issues/7618.
The list of builtin iterator is not exhaustive.
## Test Plan
`cargo test`
``` python
a = [1, 2]
examples = [
enumerate(a),
filter(lambda x: x, a),
map(int, a),
reversed(a),
zip(a),
iter(a),
]
for example in examples:
print(next(example))
```
## Summary
Implement
[`no-single-item-in`](https://github.com/dosisod/refurb/blob/master/refurb/checks/iterable/no_single_item_in.py)
as `single-item-membership-test` (`FURB171`).
Uses the helper function `generate_comparison` from the `pycodestyle`
implementations; this function should probably be moved, but I am not
sure where at the moment.
Update: moved it to `ruff_python_ast::helpers`.
Related to #1348.
## Test Plan
`cargo test`
## Summary
Document the performance effects of `itertools.starmap`, including that
it is actually slower than comprehensions in Python 3.12.
Closes#7771.
## Test Plan
`python scripts/check_docs_formatted.py`
After working with the previous change in
https://github.com/astral-sh/ruff/pull/7821 I found the names a bit
unclear and their relationship with the user-facing API muddied. Since
the applicability is exposed to the user directly in our JSON output, I
think it's important that these names align with our configuration
options. I've replaced `Manual` or `Never` with `Display` which captures
our intent for these fixes (only for display). Here, we create room for
future levels, such as `HasPlaceholders`, which wouldn't fit into the
`Always`/`Sometimes`/`Never` levels.
Unlike https://github.com/astral-sh/ruff/pull/7819, this retains the
flat enum structure which is easier to work with.
Rebase of https://github.com/astral-sh/ruff/pull/5119 authored by
@evanrittenhouse with additional refinements.
## Changes
- Adds `--unsafe-fixes` / `--no-unsafe-fixes` flags to `ruff check`
- Violations with unsafe fixes are not shown as fixable unless opted-in
- Fix applicability is respected now
- `Applicability::Never` fixes are no longer applied
- `Applicability::Sometimes` fixes require opt-in
- `Applicability::Always` fixes are unchanged
- Hints for availability of `--unsafe-fixes` added to `ruff check`
output
## Examples
Check hints at hidden unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```
We could add an indicator for which violations have hidden fixes in the
future.
Check treats unsafe fixes as applicable with opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --unsafe-fixes
example.py:1:14: F601 [*] Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 2 fixable with the --fix option.
```
Also can be enabled in the config file
```
❯ cat ruff.toml
unsafe-fixes = true
```
And opted-out per invocation
```
❯ ruff check example.py --no-cache --select F601,W292 --no-unsafe-fixes
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```
Diff does not include unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292 --diff
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
x = {'a': 1, 'a': 1}
-print(('foo'))
+print(('foo'))
\ No newline at end of file
Would fix 1 error.
```
Unless there is opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --diff --unsafe-fixes
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
-x = {'a': 1}
-print(('foo'))
+x = {'a': 1, 'a': 1}
+print(('foo'))
\ No newline at end of file
Would fix 2 errors.
```
https://github.com/astral-sh/ruff/pull/7790 will improve the diff
messages following this pull request
Similarly, `--fix` and `--fix-only` require the `--unsafe-fixes` flag to
apply unsafe fixes.
## Related
Replaces #5119
Closes https://github.com/astral-sh/ruff/issues/4185
Closes https://github.com/astral-sh/ruff/issues/7214
Closes https://github.com/astral-sh/ruff/issues/4845
Closes https://github.com/astral-sh/ruff/issues/3863
Addresses https://github.com/astral-sh/ruff/issues/6835
Addresses https://github.com/astral-sh/ruff/issues/7019
Needs follow-up https://github.com/astral-sh/ruff/issues/6962
Needs follow-up https://github.com/astral-sh/ruff/issues/4845
Needs follow-up https://github.com/astral-sh/ruff/issues/7436
Needs follow-up https://github.com/astral-sh/ruff/issues/7025
Needs follow-up https://github.com/astral-sh/ruff/issues/6434
Follow-up #7790
Follow-up https://github.com/astral-sh/ruff/pull/7792
---------
Co-authored-by: Evan Rittenhouse <evanrittenhouse@gmail.com>
## Summary
This PR resolves an issue raised in
https://github.com/astral-sh/ruff/discussions/7810, whereby we don't fix
an f-string that exceeds the line length _even if_ the resultant code is
_shorter_ than the current code.
As part of this change, I've also refactored and extracted some common
logic we use around "ensuring a fix isn't breaking the line length
rules".
## Test Plan
`cargo test`
- Only trigger for immediately adjacent isinstance() calls with the same
target
- Preserve order of or conditions
Two existing tests changed:
- One was incorrectly reordering the or conditions, and is now correct.
- Another was combining two non-adjacent isinstance() calls. It's safe
enough in that example,
but this isn't safe to do in general, and it feels low-value to come up
with a heuristic for
when it is safe, so it seems better to not combine the calls in that
case.
Fixes https://github.com/astral-sh/ruff/issues/7797
## Summary
Check that the sequence type is a list, set, dict, or tuple before
recommending replacing the `enumerate(...)` call with `range(len(...))`.
Document behaviour so users are aware of the type inference limitation
leading to false negatives.
Closes#7656.
## Summary
Two of the three listed examples were wrong: one was semantically
incorrect, another was _correct_ but not actually within the scope of
the rule.
Good motivation for us to start linting documentation examples :)
Closes https://github.com/astral-sh/ruff/issues/7773.
## Summary
We'll revert back to the crates.io release once it's up-to-date, but
better to get this out now that Python 3.12 is released.
## Test Plan
`cargo test`
## Summary
This PR enables `ruff format` to format Jupyter notebooks.
Most of the work is contained in a new `format_source` method that
formats a generic `SourceKind`, then returns `Some(transformed)` if the
source required formatting, or `None` otherwise.
Closes https://github.com/astral-sh/ruff/issues/7598.
## Test Plan
Ran `cat foo.py | cargo run -p ruff_cli -- format --stdin-filename
Untitled.ipynb`; verified that the console showed a reasonable error:
```console
warning: Failed to read notebook Untitled.ipynb: Expected a Jupyter Notebook, which must be internally stored as JSON, but this file isn't valid JSON: EOF while parsing a value at line 1 column 0
```
Ran `cat Untitled.ipynb | cargo run -p ruff_cli -- format
--stdin-filename Untitled.ipynb`; verified that the JSON output
contained formatted source code.
## Summary
When writing back notebooks via `stdout`, we need to write back the
entire JSON content, not _just_ the fixed source code. Otherwise,
writing the output _back_ to the file will yield an invalid notebook.
Closes https://github.com/astral-sh/ruff/issues/7747
## Test Plan
`cargo test`
## Summary
There's no way for users to fix this warning if they're intentionally
using an "invalid" PEP 593 annotation, as is the case in CPython. This
is a symptom of having warnings that aren't themselves diagnostics. If
we want this to be user-facing, we should add a diagnostic for it!
## Test Plan
Ran `cargo run -p ruff_cli -- check foo.py -n` on:
```python
from typing import Annotated
Annotated[int]
```
## Summary
If we have, e.g.:
```python
sum((
factor.dims for factor in bases
), [])
```
We generate three edits: two insertions (for the `operator` and
`functools` imports), and then one replacement (for the `sum` call
itself). We need to ensure that the insertions come before the
replacement; otherwise, the edits will appear overlapping and
out-of-order.
Closes https://github.com/astral-sh/ruff/issues/7718.
## Summary
This PR modifies the `line-too-long` and `doc-line-too-long` rules to
ignore lines that are too long due to the presence of a pragma comment
(e.g., `# type: ignore` or `# noqa`). That is, if a line only exceeds
the limit due to the pragma comment, it will no longer be flagged as
"too long". This behavior mirrors that of the formatter, thus ensuring
that we don't flag lines under E501 that the formatter would otherwise
avoid wrapping.
As a concrete example, given a line length of 88, the following would
_no longer_ be considered an E501 violation:
```python
# The string literal is 88 characters, including quotes.
"shape:shape:shape:shape:shape:shape:shape:shape:shape:shape:shape:shape:shape:shape:sh" # type: ignore
```
This, however, would:
```python
# The string literal is 89 characters, including quotes.
"shape:shape:shape:shape:shape:shape:shape:shape:shape:shape:shape:shape:shape:shape:sha" # type: ignore
```
In addition to mirroring the formatter, this also means that adding a
pragma comment (like `# noqa`) won't _cause_ additional violations to
appear (namely, E501). It's very common for users to add a `# type:
ignore` or similar to a line, only to find that they then have to add a
suppression comment _after_ it that was required before, as in `# type:
ignore # noqa: E501`.
Closes https://github.com/astral-sh/ruff/issues/7471.
## Test Plan
`cargo test`
## Summary
This PR implements a variety of optimizations to improve performance of
the Eradicate rule, which always shows up in all-rules benchmarks and
bothers me. (These improvements are not hugely important, but it was
kind of a fun Friday thing to spent a bit of time on.)
The improvements include:
- Doing cheaper work first (checking for some explicit substrings
upfront).
- Using `aho-corasick` to speed an exact substring search.
- Merging multiple regular expressions using a `RegexSet`.
- Removing some unnecessary `\s*` and other pieces from the regular
expressions (since we already trim strings before matching on them).
## Test Plan
I benchmarked this function in a standalone crate using a variety of
cases. Criterion reports that this version is up to 80% faster, and
almost every case is at least 50% faster:
```
Eradicate/Detection/# Warn if we are installing over top of an existing installation. This can
time: [101.84 ns 102.32 ns 102.82 ns]
change: [-77.166% -77.062% -76.943%] (p = 0.00 < 0.05)
Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
3 (3.00%) high mild
Eradicate/Detection/#from foo import eradicate
time: [74.872 ns 75.096 ns 75.314 ns]
change: [-84.180% -84.131% -84.079%] (p = 0.00 < 0.05)
Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
Eradicate/Detection/# encoding: utf8
time: [46.522 ns 46.862 ns 47.237 ns]
change: [-29.408% -28.918% -28.471%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
6 (6.00%) high mild
1 (1.00%) high severe
Eradicate/Detection/# Issue #999
time: [16.942 ns 16.994 ns 17.058 ns]
change: [-57.243% -57.064% -56.815%] (p = 0.00 < 0.05)
Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe
Eradicate/Detection/# type: ignore
time: [43.074 ns 43.163 ns 43.262 ns]
change: [-17.614% -17.390% -17.152%] (p = 0.00 < 0.05)
Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
3 (3.00%) high mild
2 (2.00%) high severe
Eradicate/Detection/# user_content_type, _ = TimelineEvent.objects.using(db_alias).get_or_create(
time: [209.40 ns 209.81 ns 210.23 ns]
change: [-32.806% -32.630% -32.470%] (p = 0.00 < 0.05)
Performance has improved.
Eradicate/Detection/# this is = to that :(
time: [72.659 ns 73.068 ns 73.473 ns]
change: [-68.884% -68.775% -68.655%] (p = 0.00 < 0.05)
Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
7 (7.00%) high mild
2 (2.00%) high severe
Eradicate/Detection/#except Exception:
time: [92.063 ns 92.366 ns 92.691 ns]
change: [-64.204% -64.052% -63.909%] (p = 0.00 < 0.05)
Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
2 (2.00%) high mild
2 (2.00%) high severe
Eradicate/Detection/#print(1)
time: [68.359 ns 68.537 ns 68.725 ns]
change: [-72.424% -72.356% -72.278%] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) low mild
1 (1.00%) high mild
Eradicate/Detection/#'key': 1 + 1,
time: [79.604 ns 79.865 ns 80.135 ns]
change: [-69.787% -69.667% -69.549%] (p = 0.00 < 0.05)
Performance has improved.
```
## Summary
The parser now uses the raw source code as global context and slices
into it to parse debug text. It turns out we were always passing in the
_old_ source code, so when code was fixed, we were making invalid
accesses. This PR modifies the call to use the _fixed_ source code,
which will always be consistent with the tokens.
Closes https://github.com/astral-sh/ruff/issues/7711.
## Test Plan
`cargo test`
## Summary
This wasn't necessary in the past, since we _only_ applied this rule to
bodies that contained two statements, one of which was a `pass`. Now
that it applies to any `pass` in a block with multiple statements, we
can run into situations in which we remove both passes, and so need to
apply the fixes in isolation.
See:
https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741107573.
## Summary
The markdown documentation was present, but in the wrong place, so was
not displaying on the website. I moved it and added some references.
Related to #2646.
## Test Plan
`python scripts/check_docs_formatted.py`
## Summary
Extend `unnecessary-pass` (`PIE790`) to trigger on all unnecessary
`pass` statements by checking for `pass` statements in any class or
function body with more than one statement.
Closes#7600.
## Test Plan
`cargo test`
Part of #1646.
## Summary
Implement `S505`
([`weak_cryptographic_key`](https://bandit.readthedocs.io/en/latest/plugins/b505_weak_cryptographic_key.html))
rule from `bandit`.
For this rule, `bandit` [reports the issue
with](https://github.com/PyCQA/bandit/blob/1.7.5/bandit/plugins/weak_cryptographic_key.py#L47-L56):
- medium severity for DSA/RSA < 2048 bits and EC < 224 bits
- high severity for DSA/RSA < 1024 bits and EC < 160 bits
Since Ruff does not handle severities for `bandit`-related rules, we
could either report the issue if we have lower values than medium
severity, or lower values than high one. Two reasons led me to choose
the first option:
- a medium severity issue is still a security issue we would want to
report to the user, who can then decide to either handle the issue or
ignore it
- `bandit` [maps the EC key algorithms to their respective key lengths
in
bits](https://github.com/PyCQA/bandit/blob/1.7.5/bandit/plugins/weak_cryptographic_key.py#L112-L133),
but there is no value below 160 bits, so technically `bandit` would
never report medium severity issues for EC keys, only high ones
Another consideration is that as shared just above, for EC key
algorithms, `bandit` has a mapping to map the algorithms to their
respective key lengths. In the implementation in Ruff, I rather went
with an explicit list of EC algorithms known to be vulnerable (which
would thus be reported) rather than implementing a mapping to retrieve
the associated key length and comparing it with the minimum value.
## Test Plan
Snapshot tests from
https://github.com/PyCQA/bandit/blob/1.7.5/examples/weak_cryptographic_key_sizes.py.
## Summary
Extend the `task-tags` checking logic to ignore TODO tags (with or
without parentheses). For example,
```python
# TODO(tjkuson): Rewrite in Rust
```
is no longer flagged as commented-out code.
Closes#7031.
I also updated the documentation to inform users that the rule is prone
to false positives like this!
EDIT: Accidentally linked to the wrong issue when first opening this PR,
now corrected.
## Test Plan
`cargo test`
Closes#7434
Replaces the `PREVIEW` selector (removed in #7389) with a configuration
option `explicit-preview-rules` which requires selectors to use exact
rule codes for all preview rules. This allows users to enable preview
without opting into all preview rules at once.
## Test plan
Unit tests
## Summary
`PGH002`, which checks for use of deprecated `logging.warn` calls, did
not check for calls made on the attribute `warn` yet. Since
https://github.com/astral-sh/ruff/pull/7521 we check both cases for
similar rules wherever possible. To be consistent this PR expands PGH002
to do the same.
## Test Plan
Expanded existing fixtures with `logger.warn()` calls
## Issue links
Fixes final inconsistency mentioned in
https://github.com/astral-sh/ruff/issues/7502
## Summary
As we bind the `ast::ExprCall` in the big `match expr` in
`expression.rs`
```rust
Expr::Call(
call @ ast::ExprCall {
...
```
There is no need for additional `let/if let` checks on `ExprCall` in
downstream rules. Found a few older rules which still did this while
working on something else. This PR removes the redundant check from
these rules.
## Test Plan
`cargo test`
## Summary
This PR adds support for named expressions when analyzing `__all__`
assignments, as per https://github.com/astral-sh/ruff/issues/7672. It
also loosens the enforcement around assignments like: `__all__ =
list(some_other_expression)`. We shouldn't flag these as invalid, even
though we can't analyze the members, since we _know_ they evaluate to a
`list`.
Closes https://github.com/astral-sh/ruff/issues/7672.
## Test Plan
`cargo test`
## Summary
Fixes#7616 by ensuring that
[B006](https://docs.astral.sh/ruff/rules/mutable-argument-default/#mutable-argument-default-b006)
fixes are inserted after module imports.
I have created a new test file, `B006_5.py`. This is mainly because I
have been working on this on and off, and the merge conflicts were
easier to handle in a separate file. If needed, I can move it into
another file.
## Test Plan
`cargo test`
## Summary
Expands several rules to also check for `Expr::Name` values. As they
would previously not consider:
```python
from logging import error
error("foo")
```
as potential violations
```python
import logging
logging.error("foo")
```
as potential violations leading to inconsistent behaviour.
The rules impacted are:
- `BLE001`
- `TRY400`
- `TRY401`
- `PLE1205`
- `PLE1206`
- `LOG007`
- `G001`-`G004`
- `G101`
- `G201`
- `G202`
## Test Plan
Fixtures for all impacted rules expanded.
## Issue Link
Refers: https://github.com/astral-sh/ruff/issues/7502
## Summary
Pass around a `Settings` struct instead of individual members to
simplify function signatures and to make it easier to add new settings.
This PR was suggested in [this
comment](https://github.com/astral-sh/ruff/issues/1567#issuecomment-1734182803).
## Note on the choices
I chose which functions to modify based on which seem most likely to use
new settings, but suggestions on my choices are welcome!
**Summary** Check that `closefd` and `opener` aren't being used with
`builtin.open()` before suggesting `Path.open()` because pathlib doesn't
support these arguments.
Closes#7620
**Test Plan** New cases in the fixture.
## Summary
This is a follow-up to #7469 that attempts to achieve similar gains, but
without introducing malachite. Instead, this PR removes the `BigInt`
type altogether, instead opting for a simple enum that allows us to
store small integers directly and only allocate for values greater than
`i64`:
```rust
/// A Python integer literal. Represents both small (fits in an `i64`) and large integers.
#[derive(Clone, PartialEq, Eq, Hash)]
pub struct Int(Number);
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum Number {
/// A "small" number that can be represented as an `i64`.
Small(i64),
/// A "large" number that cannot be represented as an `i64`.
Big(Box<str>),
}
impl std::fmt::Display for Number {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Number::Small(value) => write!(f, "{value}"),
Number::Big(value) => write!(f, "{value}"),
}
}
}
```
We typically don't care about numbers greater than `isize` -- our only
uses are comparisons against small constants (like `1`, `2`, `3`, etc.),
so there's no real loss of information, except in one or two rules where
we're now a little more conservative (with the worst-case being that we
don't flag, e.g., an `itertools.pairwise` that uses an extremely large
value for the slice start constant). For simplicity, a few diagnostics
now show a dedicated message when they see integers that are out of the
supported range (e.g., `outdated-version-block`).
An additional benefit here is that we get to remove a few dependencies,
especially `num-bigint`.
## Test Plan
`cargo test`
## Summary
This PR fixes the autofix behavior for `PT022` to create an additional
edit for the return type if it's present. The edit will update the
return type from `Generator[T, ...]` to `T`. As per the [official
documentation](https://docs.python.org/3/library/typing.html?highlight=typing%20generator#typing.Generator),
the first position is the yield type, so we can ignore other positions.
```python
typing.Generator[YieldType, SendType, ReturnType]
```
## Test Plan
Add new test cases, `cargo test` and review the snapshots.
fixes: #7610
## Summary
Implement
[`simplify-print`](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/print.py)
as `print-empty-string` (`FURB105`).
Extends the original rule in that it also checks for multiple empty
string positional arguments with an empty string separator.
Related to #1348.
## Test Plan
`cargo test`
## Summary
B005 only flags `.strip()` calls for which the argument includes
duplicate characters. This is consistent with bugbear, but isn't
explained in the documentation.
## Summary
This is only used for the `level` field in relative imports (e.g., `from
..foo import bar`). It seems unnecessary to use a wrapper here, so this
PR changes to a `u32` directly.
## Summary
## Stack Summary
This stack splits `Settings` into `FormatterSettings` and `LinterSettings` and moves it into `ruff_workspace`. This change is necessary to add the `FormatterSettings` to `Settings` without adding `ruff_python_formatter` as a dependency to `ruff_linter` (and the linter should not contain the formatter settings).
A quick overview of our settings struct at play:
* `Options`: 1:1 representation of the options in the `pyproject.toml` or `ruff.toml`. Used for deserialization.
* `Configuration`: Resolved `Options`, potentially merged from multiple configurations (when using `extend`). The representation is very close if not identical to the `Options`.
* `Settings`: The resolved configuration that uses a data format optimized for reading. Optional fields are initialized with their default values. Initialized by `Configuration::into_settings` .
The goal of this stack is to split `Settings` into tool-specific resolved `Settings` that are independent of each other. This comes at the advantage that the individual crates don't need to know anything about the other tools. The downside is that information gets duplicated between `Settings`. Right now the duplication is minimal (`line-length`, `tab-width`) but we may need to come up with a solution if more expensive data needs sharing.
This stack focuses on `Settings`. Splitting `Configuration` into some smaller structs is something I'll follow up on later.
## PR Summary
This PR moves the `ResolverSettings` and `Settings` struct to `ruff_workspace`. `LinterSettings` remains in `ruff_linter` because it gets passed to lint rules, the `Checker` etc.
## Test Plan
`cargo test`
## Stack Summary
This stack splits `Settings` into `FormatterSettings` and `LinterSettings` and moves it into `ruff_workspace`. This change is necessary to add the `FormatterSettings` to `Settings` without adding `ruff_python_formatter` as a dependency to `ruff_linter` (and the linter should not contain the formatter settings).
A quick overview of our settings struct at play:
* `Options`: 1:1 representation of the options in the `pyproject.toml` or `ruff.toml`. Used for deserialization.
* `Configuration`: Resolved `Options`, potentially merged from multiple configurations (when using `extend`). The representation is very close if not identical to the `Options`.
* `Settings`: The resolved configuration that uses a data format optimized for reading. Optional fields are initialized with their default values. Initialized by `Configuration::into_settings` .
The goal of this stack is to split `Settings` into tool-specific resolved `Settings` that are independent of each other. This comes at the advantage that the individual crates don't need to know anything about the other tools. The downside is that information gets duplicated between `Settings`. Right now the duplication is minimal (`line-length`, `tab-width`) but we may need to come up with a solution if more expensive data needs sharing.
This stack focuses on `Settings`. Splitting `Configuration` into some smaller structs is something I'll follow up on later.
## PR Summary
This PR extracts the linter-specific settings into a new `LinterSettings` struct and adds it as a `linter` field to the `Settings` struct. This is in preparation for moving `Settings` from `ruff_linter` to `ruff_workspace`
## Test Plan
`cargo test`
## Stack Summary
This stack splits `Settings` into `FormatterSettings` and `LinterSettings` and moves it into `ruff_workspace`. This change is necessary to add the `FormatterSettings` to `Settings` without adding `ruff_python_formatter` as a dependency to `ruff_linter` (and the linter should not contain the formatter settings).
A quick overview of our settings struct at play:
* `Options`: 1:1 representation of the options in the `pyproject.toml` or `ruff.toml`. Used for deserialization.
* `Configuration`: Resolved `Options`, potentially merged from multiple configurations (when using `extend`). The representation is very close if not identical to the `Options`.
* `Settings`: The resolved configuration that uses a data format optimized for reading. Optional fields are initialized with their default values. Initialized by `Configuration::into_settings` .
The goal of this stack is to split `Settings` into tool-specific resolved `Settings` that are independent of each other. This comes at the advantage that the individual crates don't need to know anything about the other tools. The downside is that information gets duplicated between `Settings`. Right now the duplication is minimal (`line-length`, `tab-width`) but we may need to come up with a solution if more expensive data needs sharing.
This stack focuses on `Settings`. Splitting `Configuration` into some smaller structs is something I'll follow up on later.
## PR Summary
This PR extracts a `ResolverSettings` struct that holds all the resolver-relevant fields (uninteresting for the `Formatter` or `Linter`). This will allow us to move the `ResolverSettings` out of `ruff_linter` further up in the stack.
## Test Plan
`cargo test`
(I'll to more extensive testing at the top of this stack)
## Summary
This PR fixes the way NoQA range is inserted to the `NoqaMapping`.
Previously, the way the mapping insertion logic worked was as follows:
1. If the range which is to be inserted _touched_ the previous range, meaning
that the end of the previous range was the same as the start of the new
range, then the new range was added in addition to the previous range.
2. Else, if the new range intersected the previous range, then the previous
range was replaced with the new _intersection_ of the two ranges.
The problem with this logic is that it does not work for the following case:
```python
assert foo, \
"""multi-line
string"""
```
Now, the comments cannot be added to the same line which ends with a continuation
character. So, the `NoQA` directive has to be added to the next line. But, the
next line is also a triple-quoted string, so the `NoQA` directive for that line
needs to be added to the next line. This creates a **union** pattern instead of an
**intersection** pattern.
But, only union doesn't suffice because (1) means that for the edge case where
the range touch only at the end, the union won't take place.
### Solution
1. Replace '<=' with '<' to have a _strict_ insertion case
2. Use union instead of intersection
## Test Plan
Add a new test case. Run the test suite to ensure that nothing is broken.
### Integration
1. Make a `test.py` file with the following contents:
```python
assert foo, \
"""multi-line
string"""
```
2. Run the following command:
```console
$ cargo run --bin ruff -- check --isolated --no-cache --select=F821 test.py
/Users/dhruv/playground/ruff/fstring.py:1:8: F821 Undefined name `foo`
Found 1 error.
```
3. Use `--add-noqa`:
```console
$ cargo run --bin ruff -- check --isolated --no-cache --select=F821 --add-noqa test.py
Added 1 noqa directive.
```
4. Check that the NoQA directive was added in the correct position:
```python
assert foo, \
"""multi-line
string""" # noqa: F821
```
5. Run the `check` command to ensure that the NoQA directive is respected:
```console
$ cargo run --bin ruff -- check --isolated --no-cache --select=F821 test.py
```
fixes: #7530
Charlie Marsh
Auguste Lalande
Dhruv Manilawala
Uriya Harpeness
Alex Waygood
Dylan
Micha Reiser
Mateusz Sokół
TomerBin
ukyen
cake-monotone
Tim Chan
Matthias
Gaétan Lepage
Evan Rittenhouse
epenet
Trim21
Javier Kauer
Mathieu Kniewallner
Tom Kuson
Gilles Peiffer
Sergey Chudov
T-256
renovate[bot]
Tibor Reiss
Denny Wong
Eric Nielsen
dedebenui
Adrin Jalali
psychedelicious
Gilles Peiffer
Philipp Thiel
Aleksei Latyshev
aditya pillai
Embers-of-the-Fire
Max Muoto
Alex
Tushar Sadhwani
Tobias Fischer
plredmond
Christoph Hasse
Vitaliy
Tomas R
Akshet Pandey
Ahmed Ilyas
Fergus Longley
Amar Paul
Dmitry Bogorad
Evan Kohilas
Jane Lewis
Nicolas Jeker
Jaap Roes
Dimitri Papadopoulos Orfanos
Douglas Thor
Ahmed Ilyas
Jelle Zijlstra
Carl Meyer
Steve C
bersbersbers
Sid
Ottavio Hartman
KotlinIsland
Jonathan Plasse
James Frost
Zanie Blue
Hoël Bagard
Sebastian Pipping
wolfgangshi
Martin Imre
Carl Meyer
Bohdan
hikaru-kajita
Mix
Peter A. Jonsson
Filipe Laíns
Alexey Preobrazhenskiy
yt2b
Ryan May
Vasco Schiavo
Robin Caloudis
Guilherme Vasconcelos
Gautier Moin
Samuel Cormier-Iijima
Eero Vaher
ooo oo
Jeremy Hiatt
trag1c
Meheret
Greenstar
Michael Merickel
Mikko Leppänen
Shaygan Hooshyari
Justin Sexton
Seo Sanghyeon
Arjun Munji
Daniël van Noord
Adam Kuhn
Adrien Ball
Asger Hautop Drewsen
Hashem
Jack McIvor
Mikael Arguedas
Alex Gaynor
Christopher Covington
Bartosz Sławecki
Akira Noda
Thomas M Kehrenberg
yataka
Chammika Mannakkara
Jane Lewis
qdegraaf
Fabio Valentini
Shantanu
dependabot[bot]
Mike Bernard
Andrew Gallant
asafamr-mm
Tuomas Siipola
konsti
Chris Hipple
Simon Brugman
Sai-Suraj-27
Ofek Lev
Philipp A
Joffrey Bluthé
Samuel Searles-Bryant
Chaojie
Adrian
Jelmer Vernooij
maltevesper
Ezra Shaw
Iipin
Alex Bieg
Jonas Haag
Alan Du
Tuomas Siipola
Jelmer Vernooij
Alan Du
Jonathan Plasse
Charlie Wilson
Bodo Graumann
Jesse Serrao
Jake Park
doolio
Felix Williams
Kar Petrosyan
Lukasz Piatkowski
Hugo van Kemenade
Deepyaman Datta
Andrew Shannon Brown
Harutaka Kawamura
Lukas Burgholzer
Spencer Brown
Weijie Guo
Claudio Jolowicz
Flowrey
Clément Schreiner
Valeriy Savchenko
Tony Lykke
Ahmed Ashraf
Sven Hager
Victor Hugo Gomes
Henry Schreiner
Simon Høxbro Hansen
Timo Brembeck
jan Apisu
Daniel Parizher