## Summary
The code later in this file that checks for slices relies on the stack
of brackets to determine the position. I'm not sure why format strings
were being excluded from this, but the tests still pass with these match
guards removed.
Closes#10278
## Test Plan
~Still needs a test.~ Test case added for this example.
## Summary
This is a follow-up to https://github.com/astral-sh/ruff/pull/10238 to
offer fixes for the f-string rule regardless of the line length of the
resulting fix. To quote Alex in the linked PR:
> Yes, from the user's perspective I'd rather have a fix that may lead
to line length issues than have to fix them myself :-) Cleaning up line
lengths is easier than changing from `"".format()` to `f""`
I agree with this position, which is that if we're going to offer a
diagnostic, we should really be offering the user the ability to fix it
-- otherwise, we're just inconveniencing them.
## Summary
Given a format string like `"{x} {x}".format(x=foo())`, we should avoid
converting to an f-string, since doing so would require repeating the
function call (`f"{foo()} {foo()}"`), which could introduce side
effects.
Closes https://github.com/astral-sh/ruff/issues/10258.
## Summary
Fixes https://github.com/astral-sh/ruff/issues/10235
This PR changes `UP032` to flag all `"".format` calls that can
technically be rewritten to an f-string, even if rewritting it to an
fstring, at least automatically, exceeds the line length (or increases
the amount by which it goes over the line length).
I looked at the Git history to understand whether the check prevents
some false positives (reported by an issue), but i haven't found a
compelling reason to limit the rule to only flag format calls that stay
in the line length limit:
* https://github.com/astral-sh/ruff/pull/7818 Changed the heuristic to
determine if the fix fits to address
https://github.com/astral-sh/ruff/discussions/7810
* https://github.com/astral-sh/ruff/pull/1905 first version of the rule
I did take a look at pyupgrade and couldn't find a similar check, at
least not in the rule code (maybe it's checked somewhere else?)
https://github.com/asottile/pyupgrade/blob/main/pyupgrade/_plugins/fstrings.py
## Breaking Change?
This could be seen as a breaking change according to ruff's [versioning
policy](https://docs.astral.sh/ruff/versioning/):
> The behavior of a stable rule is changed
* The scope of a stable rule is significantly increased
* The intent of the rule changes
* Does not include bug fixes that follow the original intent of the rule
It does increase the scope of the rule, but it is in the original intent
of the rule (so it's not).
## Test Plan
See changed test output
## Summary
When you try to remove an internal representation leaking into another
type and end up rewriting a simple version of `smallvec`.
The goal of this PR is to replace the `Box<[&'a str]>` with
`Box<QualifiedName>` to avoid that the internal `QualifiedName`
representation leaks (and it gives us a nicer API too). However, doing
this when `QualifiedName` uses `SmallVec` internally gives us all sort
of funny lifetime errors. I was lost but @BurntSushi came to rescue me.
He figured out that `smallvec` has a variance problem which is already
tracked in https://github.com/servo/rust-smallvec/issues/146
To fix the variants problem, I could use the smallvec-2-alpha-4 or
implement our own smallvec. I went with implementing our own small vec
for this specific problem. It obviously isn't as sophisticated as
smallvec (only uses safe code), e.g. it doesn't perform any size
optimizations, but it does its job.
Other changes:
* Removed `Imported::qualified_name` (the version that returns a
`String`). This can be replaced by calling `ToString` on the qualified
name.
* Renamed `Imported::call_path` to `qualified_name` and changed its
return type to `&QualifiedName`.
* Renamed `QualifiedName::imported` to `user_defined` which is the more
common term when talking about builtins vs the rest/user defined
functions.
## Test plan
`cargo test`
## Summary
Fixes https://github.com/astral-sh/ruff/issues/10039
The [recommendation for typing stub
files](https://typing.readthedocs.io/en/latest/source/stubs.html#blank-lines)
is to use **one** blank line to group related definitions and
otherwise omit blank lines.
The newly added blank line rules (`E3*`) didn't account for typing stub
files and enforced two empty lines at the top level and one empty line
otherwise, making it impossible to group related definitions.
This PR implements the `E3*` rules to:
* Not enforce blank lines. The use of blank lines in typing definitions
is entirely up to the user.
* Allow at most one empty line, including between top level statements.
## Test Plan
Added unit tests (It may look odd that many snapshots are empty but the
point is that the rule should no longer emit diagnostics)
## Summary
This PR changes the `E3*` rules to respect the `isort`
`lines-after-imports` and `lines-between-types` settings. Specifically,
the following rules required changing
* `TooManyBlannkLines` : Respects both settings.
* `BlankLinesTopLevel`: Respects `lines-after-imports`. Doesn't need to
respect `lines-between-types` because it only applies to classes and
functions
The downside of this approach is that `isort` and the blank line rules
emit a diagnostic when there are too many blank lines. The fixes aren't
identical, the blank line is less opinionated, but blank lines accepts
the fix of `isort`.
<details>
<summary>Outdated approach</summary>
Fixes
https://github.com/astral-sh/ruff/issues/10077#issuecomment-1961266981
This PR changes the blank line rules to not enforce the number of blank
lines after imports (top-level) if isort is enabled and leave it to
isort to enforce the right number of lines (depends on the
`isort.lines-after-imports` and `isort.lines-between-types` settings).
The reason to give `isort` precedence over the blank line rules is that
they are configurable. Users that always want to blank lines after
imports can use `isort.lines-after-imports=2` to enforce that
(specifically for imports).
This PR does not fix the incompatibility with the formatter in pyi files
that only uses 0 to 1 blank lines. I'll address this separately.
</details>
## Review
The first commit is a small refactor that simplified implementing the
fix (and makes it easier to reason about what's mutable and what's not).
## Test Plan
I added a new test and verified that it fails with an error that the fix
never converges. I verified the snapshot output after implementing the
fix.
---------
Co-authored-by: Hoël Bagard <34478245+hoel-bagard@users.noreply.github.com>
## Summary
When users provide configurations via `--config`, we use `shellexpand`
to ensure that we expand signifiers like `~` and environment variables.
In https://github.com/astral-sh/ruff/pull/9599, we modified `--config`
to accept either a path or an arbitrary setting. However, the detection
(to determine whether the value is a path or a setting) was lacking the
`shellexpand` behavior -- it was downstream. So we were always treating
paths like `~/ruff.toml` as values, not paths.
Closes https://github.com/astral-sh/ruff-vscode/issues/413.
The expression types in our AST are called `ExprYield`, `ExprAwait`,
`ExprStringLiteral` etc, except `ExprNamedExpr`, `ExprIfExpr` and
`ExprGenratorExpr`. This seems to align with [Python AST's
naming](https://docs.python.org/3/library/ast.html) but feels
inconsistent and excessive.
This PR removes the `Expr` postfix from `ExprNamedExpr`, `ExprIfExpr`,
and `ExprGeneratorExpr`.
## Summary
Charlie can probably explain this better than I but it turns out,
`CallPath` is used for two different things:
* To represent unqualified names like `version` where `version` can be a
local variable or imported (e.g. `from sys import version` where the
full qualified name is `sys.version`)
* To represent resolved, full qualified names
This PR splits `CallPath` into two types to make this destinction clear.
> Note: I haven't renamed all `call_path` variables to `qualified_name`
or `unqualified_name`. I can do that if that's welcomed but I first want
to get feedback on the approach and naming overall.
## Test Plan
`cargo test`
## Summary
This PR fixes for `invalid-first-argument` rules.
The fixes rename the first argument of methods and class methods to the
valid one. References to this argument are also renamed.
Fixes are skipped when another argument is named as the valid first
argument.
The fix is marked as unsafe due
The functions for the `N804` and `N805` rules are now merged, as they
only differ by the name of the valid first argument.
The rules were moved from the AST iteration to the deferred scopes to be
in the function scope while creating the fix.
## Test Plan
`cargo test`
## Summary
Allows `required-version` to be set with a version specifier, like
`>=0.3.1`.
If a single version is provided, falls back to assuming `==0.3.1`, for
backwards compatibility.
Closes https://github.com/astral-sh/ruff/issues/10192.
## Summary
This PR removes the unneeded lifetime `'b` from many of our `Visitor`
implementations.
The lifetime is unneeded because it is only constraint by `'a`, so we
can use `'a` directly.
## Test Plan
`cargo build`
## Summary
This PR changes the `CallPath` type alias to a newtype wrapper.
A newtype wrapper allows us to limit the API and to experiment with
alternative ways to implement matching on `CallPath`s.
## Test Plan
`cargo test`
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
Removes the unnecessary `exc` variable in `TRY300`'s docs example.
## Test Plan
```
python scripts/generate_mkdocs.py; mkdocs serve -f mkdocs.public.yml
```
## 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
This is a not-unpopular directory name, and it's led to tons of issues
and user confusion (most recently:
https://github.com/astral-sh/ruff-pre-commit/issues/69). I've wanted to
remove it for a long time, but we need to do so as part of a minor
release.
## 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
_This is preview only feature and is available using the `--preview`
command-line flag._
With the implementation of [PEP 701] in Python 3.12, f-strings can now
be broken into multiple lines, can contain comments, and can re-use the
same quote character. Currently, no other Python formatter formats the
f-strings so there's some discussion which needs to happen in defining
the style used for f-string formatting. Relevant discussion:
https://github.com/astral-sh/ruff/discussions/9785
The goal for this PR is to add minimal support for f-string formatting.
This would be to format expression within the replacement field without
introducing any major style changes.
### Newlines
The heuristics for adding newline is similar to that of
[Prettier](https://prettier.io/docs/en/next/rationale.html#template-literals)
where the formatter would only split an expression in the replacement
field across multiple lines if there was already a line break within the
replacement field.
In other words, the formatter would not add any newlines unless they
were already present i.e., they were added by the user. This makes
breaking any expression inside an f-string optional and in control of
the user. For example,
```python
# We wouldn't break this
aaaaaaaaaaa = f"asaaaaaaaaaaaaaaaa { aaaaaaaaaaaa + bbbbbbbbbbbb + ccccccccccccccc } cccccccccc"
# But, we would break the following as there's already a newline
aaaaaaaaaaa = f"asaaaaaaaaaaaaaaaa {
aaaaaaaaaaaa + bbbbbbbbbbbb + ccccccccccccccc } cccccccccc"
```
If there are comments in any of the replacement field of the f-string,
then it will always be a multi-line f-string in which case the formatter
would prefer to break expressions i.e., introduce newlines. For example,
```python
x = f"{ # comment
a }"
```
### Quotes
The logic for formatting quotes remains unchanged. The existing logic is
used to determine the necessary quote char and is used accordingly.
Now, if the expression inside an f-string is itself a string like, then
we need to make sure to preserve the existing quote and not change it to
the preferred quote unless it's 3.12. For example,
```python
f"outer {'inner'} outer"
# For pre 3.12, preserve the single quote
f"outer {'inner'} outer"
# While for 3.12 and later, the quotes can be changed
f"outer {"inner"} outer"
```
But, for triple-quoted strings, we can re-use the same quote char unless
the inner string is itself a triple-quoted string.
```python
f"""outer {"inner"} outer""" # valid
f"""outer {'''inner'''} outer""" # preserve the single quote char for the inner string
```
### Debug expressions
If debug expressions are present in the replacement field of a f-string,
then the whitespace needs to be preserved as they will be rendered as it
is (for example, `f"{ x = }"`. If there are any nested f-strings, then
the whitespace in them needs to be preserved as well which means that
we'll stop formatting the f-string as soon as we encounter a debug
expression.
```python
f"outer { x = !s :.3f}"
# ^^
# We can remove these whitespaces
```
Now, the whitespace doesn't need to be preserved around conversion spec
and format specifiers, so we'll format them as usual but we won't be
formatting any nested f-string within the format specifier.
### Miscellaneous
- The
[`hug_parens_with_braces_and_square_brackets`](https://github.com/astral-sh/ruff/issues/8279)
preview style isn't implemented w.r.t. the f-string curly braces.
- The
[indentation](https://github.com/astral-sh/ruff/discussions/9785#discussioncomment-8470590)
is always relative to the f-string containing statement
## Test Plan
* Add new test cases
* Review existing snapshot changes
* Review the ecosystem changes
[PEP 701]: https://peps.python.org/pep-0701/
## 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 is a small refactor to extract out the logic for normalizing
string in the formatter from the `StringPart` struct. It also separates
the quote selection into a separate method on the new
`StringNormalizer`. Both of these will help in the f-string formatting
to use `StringPart` and `choose_quotes` irrespective of normalization.
The reason for having separate quote selection and normalization step is
so that the f-string formatting can perform quote selection on its own.
Unlike string and byte literals, the f-string formatting would require
that the normalization happens only for the literal elements of it i.e.,
the "foo" and "bar" in `f"foo {x + y} bar"`. This will automatically be
handled by the already separate `normalize_string` function.
Another use-case in the f-string formatting is to extract out the
relevant information from the `StringPart` like quotes and prefix which
is to be passed as context while formatting each element of an f-string.
## Test Plan
Ensure that clippy is happy and all tests pass.
## 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 fixes the `DebugText` implementation to use the expression range
instead of the parenthesized range.
Taking the following code snippet as an example:
```python
x = 1
print(f"{ ( x ) = }")
```
The output of running it would be:
```
( x ) = 1
```
Notice that the whitespace between the parentheses and the expression is
preserved as is.
Currently, we don't preserve this information in the AST which defeats
the purpose of `DebugText` as the main purpose of the struct is to
preserve whitespaces _around_ the expression.
This is also problematic when generating the code from the AST node as
then the generator has no information about the parentheses the
whitespaces between them and the expression which would lead to the
removal of the parentheses in the generated code.
I noticed this while working on the f-string formatting where the debug
text would be used to preserve the text surrounding the expression in
the presence of debug expression. The parentheses were being dropped
then which made me realize that the problem is instead in the parser.
## Test Plan
1. Add a test case for the parser
2. Add a test case for the generator
## 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
This PR adds the `AnyNode` and `AnyNodeRef` implementation for
`FStringFormatSpec` node which will be required in the f-string
formatting.
The main usage for this is so that we can pass in the node directly to
`suppressed_node` in case debug expression is used to format is as
verbatim text.
<!--
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
`max-positional-args` defaults to `max-args` if it's not specified and
the default to `max-args` is 5, so saying that the default is 3 is
definitely wrong. Ideally, we wouldn't specify a default at all for this
config option, but I don't think that's possible?
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
<!-- How was it tested? -->
Not sure.
## 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.
It turns out that for ASCII identifiers, this is nearly 2x faster:
```
Parser/before time: [15.388 ns 15.395 ns 15.406 ns]
Parser/after time: [8.3786 ns 8.5821 ns 8.7715 ns]
```
<!--
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>
## Summary
This was causing build failures for #9599. We were referencing the
command line overrides instead of the merged configuration data, hence
the issue.
## Test Plan
A snapshot test was added.
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.
Extends #9682 to error if the nursery selector is used or nursery rules
are selected without preview.
Part of #7992 — we will remove this in 0.3.0 instead so we can provide
nice errors in 0.2.0.
# Conflicts:
# crates/ruff/tests/integration_test.rs
# crates/ruff_workspace/src/configuration.rs
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 PR implements the `blank_line_after_nested_stub_class` preview
style in the formatter.
The logic is divided into 3 parts:
1. In between preceding and following nodes at top level and nested
suite
2. When there's a trailing comment after the class
3. When there is no following node from (1) which is the case when it's
the last or the only node in a suite
We handle (3) with `FormatLeadingAlternateBranchComments`.
## Test Plan
- Add new test cases and update existing snapshots
- Checked the `typeshed` diff
fixes: #8891
## 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.
Changes our warning for combined use of `--preview` and `--select
NURSERY` to a hard error.
This should go out _before_ #9680 where we will ban use of `NURSERY`
outside of preview as well (see #9683).
Part of https://github.com/astral-sh/ruff/issues/7992
## 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
```
Previously, without the 'wrap_help' feature enabled, Clap would not do
any auto-wrapping of help text. For help text with long lines, this
tends to lead to non-ideal formatting. It can be especially difficult to
read when the width of the terminal is smaller.
This commit enables 'wrap_help', which will automatically cause Clap to
query the terminal size and wrap according to that. Or, if the terminal
size cannot be determined, it will default to a maximum line width of
100.
Ref https://github.com/astral-sh/ruff/pull/9599#discussion_r1464992692
## Summary
If you paste in the TOML for our default configuration (from the docs),
it's rejected by our JSON Schema:

It seems like the issue is with:
```toml
# Set the line length limit used when formatting code snippets in
# docstrings.
#
# This only has an effect when the `docstring-code-format` setting is
# enabled.
docstring-code-line-length = "dynamic"
```
Specifically, since that value uses a custom Serde implementation, I
guess Schemars bails out? This PR adds a custom representation to allow
`"dynamic"` (but no other strings):

This seems like it should work but I don't have a great way to test it.
Closes https://github.com/astral-sh/ruff/issues/9630.
## 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
When determining whether _any_ settings have namespace packages, we need
to consider the global settings (as would be provided via `--config`).
This was a subtle fallout of a refactor.
Closes https://github.com/astral-sh/ruff/issues/9579.
## Test Plan
Tested locally by compiling Ruff and running against this
[namespace-test](https://github.com/gokay05/namespace-test) repo.
## 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
Right now, if you run with `explicit-preview-rules`, and use something
like `select = ["RUF017"]`, we won't actually enable fixing for that
rule, because `fixable = ["ALL"]` (the default) won't include `RUF017`
due to the `explicit-preview-rules`.
The framing in this PR is that `explicit-preview-rules` should only
affect the enablement selectors, whereas the fixable selectors should
just include all possible matching rules. I think this will lead to the
most intuitive behavior.
Closes https://github.com/astral-sh/ruff/issues/9282. (An alternative to
https://github.com/astral-sh/ruff/pull/9284.)
## 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
I found that `cargo benchmark lexer` didn't work as expected:
```shell
❯ cargo benchmark lexer
Finished bench [optimized] target(s) in 0.08s
Running benches/formatter.rs (target/release/deps/formatter-4e1d9bf9d3ba529d)
Running benches/linter.rs (target/release/deps/linter-e449086ddfd8ad8c)
```
Turns out that `cargo bench -p ruff_benchmark` is now recommended over
`cargo benchmark`, so updating the docs to reflect that.
## 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.
## Summary
Sort of a random PR to make the coupling between `pyproject_config` and
`resolver` more explicit by passing it to the `Resolver`, rather than
threading it through to each individual method.
Saves 2% on Airflow:
```shell
❯ hyperfine --warmup 20 -i "./target/release/main format ../airflow" "./target/release/ruff format ../airflow"
Benchmark 1: ./target/release/main format ../airflow
Time (mean ± σ): 72.7 ms ± 0.4 ms [User: 48.7 ms, System: 75.5 ms]
Range (min … max): 72.0 ms … 73.7 ms 40 runs
Benchmark 2: ./target/release/ruff format ../airflow
Time (mean ± σ): 71.4 ms ± 0.6 ms [User: 46.2 ms, System: 76.2 ms]
Range (min … max): 70.3 ms … 73.8 ms 41 runs
Summary
'./target/release/ruff format ../airflow' ran
1.02 ± 0.01 times faster than './target/release/main format ../airflow'
```
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
This is similar to https://github.com/astral-sh/ruff/pull/8876, but more
limited in scope:
1. It only applies to `# fmt: skip` (like Black). Like `# isort: on`, `#
fmt: on` needs to be on its own line (still).
2. It only delimits on `#`, so you can do `# fmt: skip # noqa`, but not
`# fmt: skip - some other content` or `# fmt: skip; noqa`.
If we want to support the `;`-delimited version, we should revisit
later, since we don't support that in the linter (so `# fmt: skip; noqa`
wouldn't register a `noqa`).
Closes https://github.com/astral-sh/ruff/issues/8892.
When formatting notebooks, we populate the `id` field for cells that do
not have one. Previously, we generated a UUID v4 which resulted in
non-deterministic formatting. Here, we generate the UUID from a seeded
random number generator instead of using true randomness. For example,
here are the first five ids it would generate:
```
7fb27b94-1602-401d-9154-2211134fc71a
acae54e3-7e7d-407b-bb7b-55eff062a284
9a63283c-baf0-4dbc-ab1f-6479b197f3a8
8dd0d809-2fe7-4a7c-9628-1538738b07e2
72eea511-9410-473a-a328-ad9291626812
```
We also add a check that an id is not present in another cell to prevent
accidental introduction of duplicate ids.
The specification is lax, and we could just use incrementing integers
e.g. `0`, `1`, ... but I have a minor preference for retaining the UUID
format. Some discussion
[here](https://github.com/astral-sh/ruff/pull/9359#discussion_r1439607121)
— I'm happy to go either way though.
Discovered via #9293
## 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
Given:
```python
F"{"ڤ
```
We try to locate the "unclosed left brace" error by subtracting the
quote size from the lexer offset -- so we subtract 1 from the end of the
source, which puts us in the middle of a Unicode character. I don't
think we should try to adjust the offset in this way, since there can be
content _after_ the quote. For example, with the advent of PEP 701, this
string could reasonably be fixed as:
```python
F"{"ڤ"}"
````
Closes https://github.com/astral-sh/ruff/issues/9379.
## 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.
## Summary
The `ureq` dev-dependency in the ruff_cli workspace member is unused.
There are no code references to `ureq` in that crate.
## Test Plan
ruff and its tests continues to compile with the dependency removed. :)
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
We stopped releasing this a while ago and no longer advertise it
anywhere. IMO, we should remove it so that we stop paying the cost of
maintaining it. If we want to revive it, we can always do so from Git.
## 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
This helps a bit with (but does not close) the issues described in
https://github.com/astral-sh/ruff/issues/9311. E.g., now, we at least
see: `error: Failed to format main.py: source contains syntax errors:
invalid syntax. Got unexpected token '=' at byte offset 20`.
## 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
If a rule ends with a trailing placeholder (like "Use {target}"), that
gets interpreted as an HTML attribute adding, `target="target"` to the
node. This PR escapes such cases. In reality, they're rare, since we
almost always wrap placeholders in backticks, which avoids this problem
-- but in some cases, they are in fact correct to be un-backticked.
Closes https://github.com/astral-sh/ruff/issues/9288.
## Test Plan
<img width="673" alt="Screen Shot 2023-12-28 at 9 33 40 AM"
src="https://github.com/astral-sh/ruff/assets/1309177/14aaa168-c802-4258-b82d-217a85b42ebf">
## 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`.
Bumps [wasm-bindgen-test](https://github.com/rustwasm/wasm-bindgen) from
0.3.38 to 0.3.39.
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a
href="https://github.com/rustwasm/wasm-bindgen/commits">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
New messages for "format" mode.
Fixes#9132
## Test Plan
<!-- How was it tested? -->
I ran the tests specified in `CONTRIBUTING.md`
```bash
cargo run -p ruff_cli -- check /path/to/some_files.py --no-cache
cargo run -p ruff_cli -- format --check /path/to/some_files.py --no-cache
cargo clippy --workspace --all-targets --all-features -- -D warnings
RUFF_UPDATE_SCHEMA=1 cargo test
pre-commit run --all-files --show-diff-on-failure
```
**Note:** In case no files are detected, either correctly formatted,
changed, or unchanged, it does not display a message. Wouldn't it be
better to show some message in this case?
## 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
```
The example below used to panic because we tried to split at 2 bytes in
the 4-bytes character `转`.
```python
def sample_func(xx):
"""
转置 (transpose)
"""
return xx.T
```
Fixes#9145
Fixes https://github.com/astral-sh/ruff-vscode/issues/362
The second commit is a small test refactoring.
This sets `lto = "thin"` instead of using "fat" LTO, and sets
`codegen-units = 16`. These are the defaults for Cargo's `release`
profile, and I think it may give us faster iteration times, especially
when benchmarking. The point of this PR is to see what kind of impact
this has on benchmarks. It is expected that benchmarks may regress to
some extent.
I did some quick ad hoc experiments to quantify this change in compile
times. Namely, I ran:
cargo build --profile release -p ruff_cli
Then I ran
touch crates/ruff_python_formatter/src/expression/string/docstring.rs
(because that's where i've been working lately) and re-ran
cargo build --profile release -p ruff_cli
This last command is what I timed, since it reflects how much time one
has to wait between making a change and getting a compiled artifact.
Here are my results:
* With status quo `release` profile, build takes 77s
* with `release` but `lto = "thin"`, build takes 41s
* with `release`, but `lto = false`, build takes 19s
* with `release`, but `lto = false` **and** `codegen-units = 16`, build
takes 7s
* with `release`, but `lto = "thin"` **and** `codegen-units = 16`, build
takes 16s (i believe this is the default `release` configuration)
This PR represents the last option. It's not the fastest to compile, but
it's nearly a whole minute faster! The idea is that with `codegen-units
= 16`, we still make use of parallelism, but keep _some_ level of LTO on
to try and re-gain what we lose by increasing the number of codegen
units.
## Summary
Add new test cases for `with_item` and `match` sequence that demonstrate how long headers break.
Removes one use of `optional_parentheses` in a position where it is know that the parentheses always need to be added.
## Test Plan
cargo test
## Summary
Add some more documentation to `can_omit_optional_parentheses` because it is realy hard to understand.
Restrict the `Attribute` and `None` `OperatorPrecedence` branches to ensure they only get applyied to the intended nodes.
## Test Plan
Ecosystem check reports no differences. The compatibility index remains unchanged.
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 splits the string formatting code in the formatter to be handled
by the respective nodes.
Previously, the string formatting was done through a single
`FormatString` interface. Now, the nodes themselves are responsible for
formatting.
The following changes were made:
1. Remove `StringLayout::ImplicitStringConcatenationInBinaryLike` and
inline the call to `FormatStringContinuation`. After the refactor, the
binary like formatting would delegate to `FormatString` which would then
delegate to `FormatStringContinuation`. This removes the intermediary
steps.
2. Add formatter implementation for `FStringPart` which delegates it to
the respective string literal or f-string node.
3. Add `ExprStringLiteralKind` which is either `String` or `Docstring`.
If it's a docstring variant, then the string expression would not be
implicitly concatenated. This is guaranteed by the
`DocstringStmt::try_from_expression` constructor.
4. Add `StringLiteralKind` which is either a `String`, `Docstring` or
`InImplicitlyConcatenatedFString`. The last variant is for when the
string literal is implicitly concatenated with an f-string (`"foo" f"bar
{x}"`).
5. Remove `FormatString`.
6. Extract the f-string quote detection as a standalone function which
is public to the crate. This is used to detect the quote to be used for
an f-string at the expression level (`ExprFString` or
`FormatStringContinuation`).
### Formatter ecosystem result
**This PR**
| project | similarity index | total files | changed files |
|----------------|------------------:|------------------:|------------------:|
| cpython | 0.75804 | 1799 | 1648 |
| django | 0.99984 | 2772 | 34 |
| home-assistant | 0.99955 | 10596 | 214 |
| poetry | 0.99905 | 321 | 15 |
| transformers | 0.99967 | 2657 | 324 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99980 | 3669 | 18 |
| warehouse | 0.99976 | 654 | 14 |
| zulip | 0.99958 | 1459 | 36 |
**main**
| project | similarity index | total files | changed files |
|----------------|------------------:|------------------:|------------------:|
| cpython | 0.75804 | 1799 | 1648 |
| django | 0.99984 | 2772 | 34 |
| home-assistant | 0.99955 | 10596 | 214 |
| poetry | 0.99905 | 321 | 15 |
| transformers | 0.99967 | 2657 | 324 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99980 | 3669 | 18 |
| warehouse | 0.99976 | 654 | 14 |
| zulip | 0.99958 | 1459 | 36 |
This fixes a bug where the current indent level was not calculated
correctly for doctests. Namely, it didn't account for the extra indent
level (in terms of ASCII spaces) used by by the PS1 (`>>> `) and PS2
(`... `) prompts. As a result, lines could extend up to 4 spaces beyond
the configured line length limit.
We fix that by passing the `CodeExampleKind` to the `format` routine
instead of just the code itself. In this way, `format` can query whether
there will be any extra indent added _after_ formatting the code and
take that into account for its line length setting.
We add a few regression tests, taken directly from @stinodego's
examples.
Fixes#9126
This PR does the plumbing to make a new formatting option,
`docstring-code-format`, available in the configuration for end users.
It is disabled by default (opt-in). It is opt-in at least initially to
reflect a conservative posture. The intent is to make it opt-out at some
point in the future.
This was split out from #8811 in order to make #8811 easier to merge.
Namely, once this is merged, docstring code snippet formatting will
become available to end users. (See comments below for how we arrived at
the name.)
Closes#7146
## Test Plan
Other than the standard test suite, I ran the formatter over the CPython
and polars projects to ensure both that the result looked sensible and
that tests still passed. At time of writing, one issue that currently
appears is that reformatting code snippets trips the long line lint:
https://github.com/BurntSushi/polars/actions/runs/7006619426/job/19058868021
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 😅
## Summary
This PR changes the internal `docstring-code-line-width` setting to
additionally accept a string value `dynamic`. When `dynamic` is set, the
line width is dynamically adjusted when reformatting code snippets in
docstrings based on the indent level of the docstring. The result is
that the reformatted lines from the code snippet should not exceed the
"global" line width configuration for the surrounding source.
This PR does not change the default behavior, although I suspect the
default should probably be `dynamic`.
## Test Plan
I added a new configuration to the existing docstring code tests and
also added a new set of tests dedicated to the new `dynamic` mode.
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
This does the light plumbing necessary to add a new internal option that
permits setting the line width of code examples in docstrings. The plan
is to add the corresponding user facing knob in #8854.
Note that this effectively removes the `same-as-global` configuration
style discussed [in this
comment](https://github.com/astral-sh/ruff/issues/8855#issuecomment-1847230440).
It replaces it with the `{integer}` configuration style only.
There are a lot of commits here, but they are each tiny to make review
easier because of the changes to snapshots.
## Test Plan
I added a new docstring test configuration that sets
`docstring-code-line-width = 60` and examined the differences.
Bumps
[serde-wasm-bindgen](https://github.com/RReverser/serde-wasm-bindgen)
from 0.6.1 to 0.6.3.
<details>
<summary>Commits</summary>
<ul>
<li><a
href="e65f027ed7"><code>e65f027</code></a>
chore: Release</li>
<li><a
href="0cf8879399"><code>0cf8879</code></a>
Fix find-replace typo in docs</li>
<li><a
href="ff83666343"><code>ff83666</code></a>
Fix doc annotation</li>
<li><a
href="014e415d41"><code>014e415</code></a>
chore: Release</li>
<li><a
href="34aab01dcb"><code>34aab01</code></a>
Use Wasm target for docs.rs</li>
<li><a
href="455d55645f"><code>455d556</code></a>
More consistent docs + hide internal fields</li>
<li><a
href="ce7669e1d1"><code>ce7669e</code></a>
Use field indices for struct deserialization</li>
<li><a
href="a7e4c5b5aa"><code>a7e4c5b</code></a>
Bump deps</li>
<li><a
href="b4b4965c63"><code>b4b4965</code></a>
Don't use --profiling for benchmarks</li>
<li><a
href="3dfe7271ba"><code>3dfe727</code></a>
Speed up integer decoding</li>
<li>Additional commits viewable in <a
href="https://github.com/RReverser/serde-wasm-bindgen/compare/v0.6.1...v0.6.3">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
## Summary
In https://github.com/astral-sh/ruff/pull/8921, we changed our parameter
formatting behavior to add a trailing comma whenever a single-argument
function breaks. This introduced a deviation in the case that a function
contains a single argument, but _also_ includes a positional-only or
keyword-only separator.
Closes https://github.com/astral-sh/ruff/issues/9074.
## 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.
(This is not possible to actually use until
https://github.com/astral-sh/ruff/pull/8854 is merged.)
This commit slots in support for formatting Markdown fenced code
blocks[1]. With the refactoring done for reStructuredText previously,
this ended up being pretty easy to add. Markdown code blocks are also
quite a bit easier to parse and recognize correctly.
One point of contention in #8860 is whether to assume that unlabeled
Markdown code fences are Python or not by default. In this PR, we make
such an assumption. This follows what `rustdoc` does. The mitigation
here is that if an unlabeled code block isn't Python, then it probably
won't parse as Python. And we'll end up skipping it. So in the vast
majority of cases, the worst thing that can happen is a little bit of
wasted work.
Closes#8860
[1]: https://spec.commonmark.org/0.30/#fenced-code-blocks
## 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 is not possible to actually use until
https://github.com/astral-sh/ruff/pull/8854 is merged.)
ruff_python_formatter: add reStructuredText docstring formatting support
This commit makes use of the refactoring done in prior commits to slot
in reStructuredText support. Essentially, we add a new type of code
example and look for *both* literal blocks and code block directives.
Literal blocks are treated as Python by default because it seems to be a
[common
practice](https://github.com/adamchainz/blacken-docs/issues/195).
That is, literal blocks like this:
```
def example():
"""
Here's an example::
foo( 1 )
All done.
"""
pass
```
Will get reformatted. And code blocks (via reStructuredText directives)
will also get reformatted:
```
def example():
"""
Here's an example:
.. code-block:: python
foo( 1 )
All done.
"""
pass
```
When looking for a code block, it is possible for it to become invalid.
In which case, we back out of looking for a code example and print the
lines out as they are. As with doctest formatting, if reformatting the
code would result in invalid Python or if the code collected from the
block is invalid, then formatting is also skipped.
A number of tests have been added to check both the formatting and
resetting behavior. Mixed indentation is also tested a fair bit, since
one of my initial attempts at dealing with mixed indentation ended up
not working.
I recommend working through this PR commit-by-commit. There is in
particular a somewhat gnarly refactoring before reST support is added.
Closes#8859
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
We should avoid inlining the ellipsis in:
```python
def h():
...
# bye
```
Just as we omit the ellipsis in:
```python
def h():
# bye
...
```
Closes https://github.com/astral-sh/ruff/issues/8905.
## 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.
In the source of working on #8859, I made a number of smallish refactors
to how code snippet formatting works. Most or all of these were
motivated by writing in support for reStructuredText blocks. They have
some fundamentally different requirements than doctests, and there are a
lot more ways for reStructuredText blocks to become invalid.
(Commit-by-commit review is recommended as the commit messages provide
further context on each change. I split this off from ongoing work to
make review more manageable.)
## 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>
<!--
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
Our `SoftKeywordTokenizer` only respected soft keywords in compound
statement positions -- for example, at the start of a logical line:
```python
type X = int
```
However, type aliases can also appear in simple statement positions,
like:
```python
class Class: type X = int
```
(Note that `match` and `case` are _not_ valid keywords in such
positions.)
This PR upgrades the tokenizer to track both kinds of valid positions.
Closes https://github.com/astral-sh/ruff/issues/8900.
Closes https://github.com/astral-sh/ruff/issues/8899.
## Test Plan
`cargo test`
## 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
Given `with (a := b): pass`, we truncate the `WithItem` range by one on
both sides such that the parentheses are part of the statement, rather
than the item. However, for `with (a := b) as x: pass`, we want to avoid
this trick.
Closes https://github.com/astral-sh/ruff/issues/8913.
## 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
This PR fixes the bug in the lexer where the `Mode::Ipython` wasn't
being considered when initializing the soft keyword transformer which
wraps the lexer. This means that if the source code starts with either
`match` or `type` keyword, then the keywords were being considered as
name tokens instead. For example,
```python
match foo:
case bar:
pass
```
This would transform the `match` keyword into an identifier if the mode
is `Ipython`.
The fix is to reverse the condition in the soft keyword initializer so
that any new modes are by default considered as the lexer being at start
of line.
## Test Plan
Add a new test case for `Mode::Ipython` and verify the snapshot.
fixes: #8870
This PR removes several uses of `unsafe`. I generally limited myself to
low hanging fruit that I could see. There are still a few remaining uses
of `unsafe` that looked a bit more difficult to remove (if possible at
all). But this gets rid of a good chunk of them.
I put each `unsafe` removal into its own commit with a justification for
why I did it. So I would encourage reviewing this PR commit-by-commit.
That way, we can legislate them independently. It's no problem to drop a
commit if we feel the `unsafe` should stay in that case.
## 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
This turns `string` into a parent module with a `docstring` sub-module.
I arranged things this way because there are parts of the `string`
module that the `docstring` module wants to know about (such as a
`NormalizedString`). The alternative I think would be to make
`docstring` a sibling module and expose more of `string`'s internals.
I think I overall like this change because it gives docstring handling a
bit more room to breath. It has grown quite a bit with the addition of
code snippet formatting.
[This was suggested by
@charliermarsh.](https://github.com/astral-sh/ruff/pull/8811#discussion_r1401169531)
## Summary
This PR adds opt-in support for formatting doctests in docstrings. This
reflects initial support and it is intended to add support for Markdown
and reStructuredText Python code blocks in the future. But I believe
this PR lays the groundwork, and future additions for Markdown and reST
should be less costly to add.
It's strongly recommended to review this PR commit-by-commit. The last
few commits in particular implement the bulk of the work here and
represent the denser portions.
Some things worth mentioning:
* The formatter is itself not perfect, and it is possible for it to
produce invalid Python code. Because of this, reformatted code snippets
are checked for Python validity. If they aren't valid, then we
(unfortunately silently) bail on formatting that code snippet.
* There are a couple places where it would be nice to at least warn the
user that doctest formatting failed, but it wasn't clear to me what the
best way to do that is.
* I haven't yet run this in anger on a real world code base. I think
that should happen before merging.
Closes#7146
## Test Plan
* [x] Pass the local test suite.
* [x] Scrutinize ecosystem changes.
* [x] Run this formatter on extant code and scrutinize the results.
(e.g., CPython, numpy.)
A follow-up to auto-generate the `FormatNodeRule` implementation for the
string part nodes. This is just a dummy implementation that is
unreachable because it's handled by the parent nodes.
## 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.
## Summary
This refactors the `ruff_cli` integration tests to create a new
`RuffCheck` struct -- this holds options to configure the "common case"
flags that we want to pass to Ruff (e.g. `--no-cache`, `--isolated`,
etc). This helps reduce the boilerplate and (IMO) makes it more obvious
what the core logic of each test is by keeping only the "interesting"
parameters.
## Test Plan
`cargo test`
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.
Closes https://github.com/astral-sh/ruff/issues/7347Closes#3970 via use of `include`
We could update examples in our documentation, but I worry since we do
not have versioned documentation users on older versions would be
confused. Instead, I'll open an issue to track updating use of `ruff
check .` in the documentation sometime in the future.
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`
Bumps [js-sys](https://github.com/rustwasm/wasm-bindgen) from 0.3.64 to
0.3.65.
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a
href="https://github.com/rustwasm/wasm-bindgen/commits">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
## 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.
Fix an instability where await was followed by a breaking fluent style
expression:
```python
test_data = await (
Stream.from_async(async_data)
.flat_map_async()
.map()
.filter_async(is_valid_data)
.to_list()
)
```
Note that this technically a minor style change (see ecosystem check)
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.
## Summary
When running ruff in verbose mode with `-v`, the first debug logs show
where the config settings are taken from. For example:
```
❯ ruff check ./some_file.py -v
[2023-11-17][00:16:25][ruff_cli::resolve][DEBUG] Using pyproject.toml (parent) at /Users/vince/demo/ruff.toml
```
This threw me off for a second because I knew I had no python project
there, and therefore no `pyproject.toml` file. Then I realised it was
actually reading a `ruff.toml` file (obvious when you read the whole
print I suppose) and that the pyproject.toml is a hardcoded string in
the debug log.
I think it would be nice to tweak the wording slightly so it is clear
that the settings don't neccessarily have to come from a
`pyproject.toml` file.
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.
I checked for ipython-specific builtins on python 3.11 using
```python
import json
from subprocess import check_output
builtins_python = json.loads(check_output(["python3", "-c" "import json; print(json.dumps(dir(__builtins__)))"]))
builtins_ipython = json.loads(check_output(["ipython3", "-c" "import json; print(json.dumps(dir(__builtins__)))"]))
print(sorted(set(builtins_ipython) - set(builtins_python)))
```
and updated the relevant constant and match. The list changes from
`display`
to
`__IPYTHON__`, `display`, `get_ipython`.
Followup to #8707
## 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.
Testing the compatibility with the future stable black style, i realized
the `ruff_python_formatter` dev main was lacking the
`--skip-magic-trailing-comma` option. This does not affect `ruff
format`.
Usage:
```shell
cargo run --bin ruff_python_formatter -p ruff_python_formatter -- --skip-magic-trailing-comma --emit stdout scratch.py
```
## 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 implements validation in the formatter tests to ensure that we
don't modify the AST during formatting. Black has similar logic.
In implementing this, I learned that Black actually _does_ modify the
AST, and their test infrastructure normalizes the AST to wipe away those
differences. Specifically, Black changes the indentation of docstrings,
which _does_ modify the AST; and it also inserts parentheses in `del`
statements, which changes the AST too.
Ruff also does both these things, so we _also_ implement the same
normalization using a new visitor that allows for modifying the AST.
Closes https://github.com/astral-sh/ruff/issues/8184.
## Test Plan
`cargo test`
## 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`
It seems as though using `include!(...)` to avoid the source code copy
breaks rust-analzer. Namely, it treats the included file as unlinked,
and so any part of analysis (e.g., goto-definition) that needs that file
to reason about the code ends up failing.
<!--
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? -->
## Test Plan
<!-- How was it tested? -->
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.
## Summary
This fixes#2606 by moving where we apply the convention ignores --
instead of applying that at the very end, e track, we now track which
rules have been specifically enabled (via `Specificity::Rule`). If they
have, then we do *not* apply the docstring overrides at the end.
## Test Plan
Added unit tests to `ruff_workspace` and an integration test to
`ruff_cli`
## Summary
This PR updates the formatter to preserve trailing semicolon for Jupyter
Notebooks.
The motivation behind the change is that semicolons in notebooks are
typically used to hide the output, for example when plotting. This is
highlighted in the linked issue.
The conditions required as to when the trailing semicolon should be
preserved are:
1. It should be a top-level statement which is last in the module.
2. For statement, it can be either assignment, annotated assignment, or
augmented assignment. Here, the target should only be a single
identifier i.e., multiple assignments or tuple unpacking isn't
considered.
3. For expression, it can be any.
## Test Plan
Add a new integration test in `ruff_cli`. The test notebook basically
acts as a document as to which trailing semicolons are to be preserved.
fixes: #8254
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
When you run Ruff via stdin, and pass `format` or `check --fix`, we
typically write the changed or unchanged contents to stdout. It turns
out we forgot to do this when the file is _excluded_, so if you run
`ruff format /path/to/excluded/file.py`, we don't write _anything_ to
`stdout`. This led to a bug in the LSP whereby we deleted file contents
for third-party files.
The right thing to do here is write back the unchanged contents, as it
should always be safe to write the output of stdout back to a file.
## 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
Being able to set `--no-cache` without touching the command line makes
comparing formatter speed with e.g. Hyperfine a lot easier; Black allows
one to set `BLACK_CACHE_DIR=/dev/null`, but setting
`RUFF_CACHE_DIR=/dev/null` has Ruff choke:
```
error: Failed to initialize cache at /dev/null: Not a directory (os error 20)
error: Failed to initialize cache at /dev/null: Not a directory (os error 20)
warning: Failed to open cache file '/dev/null/0.1.4/18160934645386409287': Not a directory (os error 20)
```
Alternately, we could make a `/dev/null` (or `nul` on Windows) cache
directory imply `--no-cache`?
## Test Plan
None yet.