## Summary
Part of #970.
This adds Pylint's [R0244
empty_comment](https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/empty-comment.html)
lint as well as an always-safe fix.
## Test Plan
The included snapshot verifies the following:
- A line containing only a non-empty comment is not changed
- A line containing leading whitespace before a non-empty comment is not
changed
- A line containing only an empty comment has its content deleted
- A line containing only leading whitespace before an empty comment has
its content deleted
- A line containing only leading and trailing whitespace on an empty
comment has its content deleted
- A line containing trailing whitespace after a non-empty comment is not
changed
- A line containing only a single newline character (i.e. a blank line)
is not changed
- A line containing code followed by a non-empty comment is not changed
- A line containing code followed by an empty comment has its content
deleted after the last non-whitespace character
- Lines containing code and no comments are not changed
- Empty comment lines within block comments are ignored
- Empty comments within triple-quoted sections are ignored
## Comparison to Pylint
Running Ruff and Pylint 3.0.3 with Python 3.12.0 against the
`empty_comment.py` file added in this PR, we see the following:
* Identical behavior:
* empty_comment.py:3:0: R2044: Line with empty comment (empty-comment)
* empty_comment.py:4:0: R2044: Line with empty comment (empty-comment)
* empty_comment.py:5:0: R2044: Line with empty comment (empty-comment)
* empty_comment.py:18:0: R2044: Line with empty comment (empty-comment)
* Differing behavior:
* Pylint doesn't ignore empty comments in block comments commonly used
for visual spacing; I decided these were fine in this implementation
since many projects use these and likely do not want them removed.
* empty_comment.py:28:0: R2044: Line with empty comment (empty-comment)
* Pylint detects "empty comments" within the triple-quoted section at
the bottom of the file, which is arguably a bug in the Pylint
implementation since these are not truly comments. These are ignored by
this implementation.
* empty_comment.py:37:0: R2044: Line with empty comment (empty-comment)
* empty_comment.py:38:0: R2044: Line with empty comment (empty-comment)
* empty_comment.py:39:0: R2044: Line with empty comment (empty-comment)
## Summary
Hey there 👋 thanks for this great project!
On python code looking like the following
```
import yaml
from yaml.loader import SafeLoader
with MY_FILE_PATH.open("r") as my_file:
my_data = yaml.load(my_file, Loader=SafeLoader)
```
ruff reports this error:
```
S506 Probable use of unsafe loader `SafeLoader` with `yaml.load`. Allows instantiation of arbitrary objects. Consider `yaml.safe_load`.
```
This PR is an attempt to support SafeLoader being imported for either
`yaml` or `yaml.loader`
Disclaimer:
I am not familiar with Rust so this is likely not the better way of
doing it. Interested in hearing how to adapt this PR to provide similar
behavior in a better way
## Test Plan
The S506.py file was updated accordingly to cover the use cases and test
were confirmed to pass with this change.
## Summary
If `RUF100` is ignored via `per-file-ignores`, we need to avoid raising
it. `RUF100` has special "self-ignore" logic, since the rule itself
deals with `# noqa` directives. This PR wires up `per-file-ignores` to
that "self-ignore" logic.
Closes https://github.com/astral-sh/ruff/issues/9297.
We should avoid adding `-> None` to stubs in `.pyi` files, along with a
few other cases. (We already ignore abstract methods.)
Closes https://github.com/astral-sh/ruff/issues/9270.
## Summary
This PR adds some helper structs to the linter paths to enable passing
in the pre-computed tokens and parsed source code during benchmarking,
to remove lexing and parsing from the overall linter benchmark
measurement. We already remove parsing for the formatter, and we have
separate benchmarks for the lexer and the parser, so this should make it
much easier to measure linter performance changes.
## Summary
Adds a rule to detect unions that include `typing.NoReturn` or
`typing.Never`. In such cases, the use of the bottom type is redundant.
Closes https://github.com/astral-sh/ruff/issues/9113.
## Test Plan
`cargo test`
## Summary
fixes#6956
details in issue
Following an advice in
https://github.com/astral-sh/ruff/issues/6956#issuecomment-1817672585,
this change separates expressions to 3 levels of "constant likelihood":
* literals, empty dict and tuples... (definitely constant, level 2)
* CONSTANT_CASE vars (probably constant, 1)
* all other expressions (0)
a comparison is marked yoda if the level is strictly higher on its left
hand side
following
https://github.com/astral-sh/ruff/issues/6956#issuecomment-1697107822
marking compound expressions of literals (e.g. `60 * 60` ) as constants
this change current behaviour on
`SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)` in the fixture
from error to ok
## Summary
Given a function like:
```python
def func(x: int):
if not x:
raise ValueError
else:
raise TypeError
```
We now correctly use `NoReturn` as the return type, rather than `None`.
Closes https://github.com/astral-sh/ruff/issues/9201.
## Summary
Part of #8771. flake8-pyi will emit a Y018 error for unused TypeVars,
ParamSpecs or TypeVarTuples; Ruff currently only emits PYI018 for unused
TypeVars.
This is my first "proper" Ruff PR -- let me know if there's a better way
of doing this! Not sure if the repeated calls to `match_typing_expr()`
are ideal.
## Test Plan
I manually updated the fixtures to add some unused ParamSpecs and
TypeVarTuples, and then updated the snapshots using `cargo insta
review`. All tests then passed when run using `cargo test`.
## Summary
Given `Callable[[Callable[_P, _R]], Callable[_P, _R]]` from the
originating issue, when quoting `Callable`, we quoted the inner
`[Callable[_P, _R]]`, and then created a separate edit for the outer
`Callable`. Since there's an extra level of nesting in the subscript,
the edit for `[Callable[_P, _R]]` correctly did _not_ expand to the
entire expression. However, in this case, we should discard the inner
edit, since the expression is getting quoted by the outer edit anyway.
Closes https://github.com/astral-sh/ruff/issues/9162.
## Summary
A common mistake is to add quotes around one member in an `X | Y`-style
type union, as in:
```python
contract_versions_list: list[ContractVersion] | 'QuerySet[ContractVersion]' | None = None
```
However, doing so will lead to a runtime error if the annotation is
runtime-evaluated. This PR lints against such patterns.
Closes https://github.com/astral-sh/ruff/issues/9139.
## Summary
Fix dropped union expressions for piped non-types in `PYI055` autofix
If you had `type[int] | type[str] | str`, it would have dropped the
`str`, which breaks the type!
Closes#9156
## Test Plan
`cargo test`
Fix#9080
Example, where `[]` is a 2 byte non-breaking space:
```
def f():
""" Docstring header
^^^^ Real indentation is 4 chars
docstring body, over-indented
^^^^^^ Over-indentation is 6 - 4 = 2 chars due to this line
[] [] docstring body 2, further indented
^^^^^ We take these 4 chars/5 bytes to match the docstring ...
^^^ ... and these 2 chars/3 bytes to remove the `over_indented_size` ...
^^ ... but preserve this real indent
```
Given:
```python
x: DataFrame[
int
] = 1
```
We currently wrap the annotation in single quotes, which leads to a
syntax error:
```python
x: "DataFrame[
int
]" = 1
```
There are a few options for what to suggest for users here... Use triple
quotes:
```python
x: """DataFrame[
int
]""" = 1
```
Or, use an implicit string concatenation (which may require
parentheses):
```python
x: ("DataFrame["
"int"
"]") = 1
```
The solution I settled on here is to use the `Generator`, which
effectively means we write it out on a single line, like:
```python
x: "DataFrame[int]" = 1
```
It's kind of the "least opinionated" solution, but it does mean we'll
expand to a very long line in some cases.
Closes https://github.com/astral-sh/ruff/issues/9136.
This PR adds a `as_slice` method to all the string nodes which returns
all the parts of the nodes as a slice. This will be useful in the next
PR to split the string formatting to use this method to extract the
_single node_ or _implicitly concanated nodes_.
## Summary
Adds support for sarif v2.1.0 output to cli, usable via the
output-format paramter.
`ruff . --output-format=sarif`
Includes a few changes I wasn't sure of, namely:
* Adds a few derives for Clone & Copy, which I think could be removed
with a little extra work as well.
## Test Plan
I built and ran this against several large open source projects and
verified that the output sarif was valid, using [Microsoft's SARIF
validator tool](https://sarifweb.azurewebsites.net/Validation)
I've also attached an output of the sarif generated by this version of
ruff on the main branch of django at commit: b287af5dc9
[django_main_b287af5dc9_sarif.json](https://github.com/astral-sh/ruff/files/13626222/django_main_b287af5dc9_sarif.json)
Note: this needs to be regenerated with the latest changes and
confirmed.
## Open Points
[ ] Convert to just using all Rules all the time
[ ] Fix the issue with getting the file URI when compiling for web
assembly
## Summary
This allows us to fix usages like:
```python
from pandas import DataFrame
def baz() -> DataFrame:
...
```
By quoting the `DataFrame` in `-> DataFrame`. Without quotes, moving
`from pandas import DataFrame` into an `if TYPE_CHECKING:` block will
fail at runtime, since Python tries to evaluate the annotation to add it
to the function's `__annotations__`.
Unfortunately, this does require us to split our "annotation kind" flags
into three categories, rather than two:
- `typing-only`: The annotation is only evaluated at type-checking-time.
- `runtime-evaluated`: Python will evaluate the annotation at runtime
(like above) -- but we're willing to quote it.
- `runtime-required`: Python will evaluate the annotation at runtime
(like above), and some library (like Pydantic) needs it to be available
at runtime, so we _can't_ quote it.
This functionality is gated behind a setting
(`flake8-type-checking.quote-annotations`).
Closes https://github.com/astral-sh/ruff/issues/5559.
## Summary
Adds `find_assigned_value` a function which gets the `&Expr` assigned to
a given `id` if one exists in the semantic model.
Open TODOs:
- [ ] Handle `binding.kind.is_unpacked_assignment()`: I am bit confused
by this one. The snippet from its documentation does not appear to be
counted as an unpacked assignment and the only ones I could find for
which that was true were invalid Python like:
```python
x, y = 1
```
- [ ] How to handle AugAssign. Can we combine statements like:
```python
(a, b) = [(1, 2, 3), (4,)]
a += (6, 7)
```
to get the full value for a? Code currently just returns `None` for
these assign types
- [ ] Multi target assigns
```python
m_c = (m_d, m_e) = (0, 0)
trio.sleep(m_c) # OK
trio.sleep(m_d) # TRIO115
trio.sleep(m_e) # TRIO115
```
## Test Plan
Used the function in two rules:
- `TRIO115`
- `PERF101`
Expanded both their fixtures for explicit multi target check
## Summary
A fairly common pattern which triggers F841 is unused variables from
tuple assignments, e.g.:
user, created = User.objects.get_or_create(...)
^ F841: Local variable `created` is assigned to but never used
This error is currently not auto-fixable.
This PR adds support for fixing the error automatically by renaming the
unused variable to have a leading underscore (i.e. `_created`) **iff**
the `dummy-variable-rgx` setting would match it.
I considered using `renamers::Renamer` here, but because by the nature
of the error there should be no references to it, that seemed like
overkill. Also note that the fix might break by shadowing the new name
if it is already used elsewhere in the scope. I left it as is because
1. the renamed variable matches the "unused" regex, so it should
hopefully not already be used,
2. the fix is marked as unsafe so it should be reviewed manually
anyways, and
3. I'm not actually sure how to check the scope for the new variable
name 😅
Hides hints about unsafe fixes when they are disabled e.g. with
`--no-unsafe-fixes` or `unsafe-fixes = false`. By default, unsafe fix
hints are still displayed. This seems like a nice way to remove the nag
for users who have chosen not to apply unsafe fixes.
Inspired by comment at
https://github.com/astral-sh/ruff/issues/9063#issuecomment-1850289675
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Check floating-point numbers similarly to integers in FURB163. For
example, both `math.log(x, 10)` and `math.log(x, 10.0)` should be
changed to `math.log10(x)`.
## Test Plan
<!-- How was it tested? -->
Added couple of test cases.
## Summary
E274 currently flags any keyword at the start of a line indented with
tabs. This turns out to be due to a bug in `Whitespace::trailing` that
never considers any whitespace containing a tab as indentation.
## Test Plan
Added a simple test case.
This PR allows `matplotlib.use` calls to intersperse imports without
triggering `E402`. This is a pragmatic choice as it's common to require
`matplotlib.use` calls prior to importing from within `matplotlib`
itself.
Closes https://github.com/astral-sh/ruff/issues/9091.
## Summary
Fixes#8863 : Detect asyncio-dangling-task (RUF006) when discarding
return value
## Test Plan
added new two testcases, changed result of an old one that was made more
specific
## Summary
Fix a couple typos:
- I'm certain about `It's is` → `It is`.
- Not sure about `is it's` → `if it's` because I don't understand the
sentence.
## Test Plan
No tests.
## Summary
This PR updates the logic for `is_magic_cell` to include certain cell
magics. These cell magics would contain Python code following the line
defining the command. The code could define a variable which can then be
referenced in other cells. Currently, we would ignore the cell
completely leading to undefined-name violation.
As discussed in
https://github.com/astral-sh/ruff/issues/8354#issuecomment-1832221009
## Test Plan
Add new test case to validate this scenario.
## Summary
It's common to interleave a `sys.path` modification between imports at
the top of a file. This is a frequent cause of `# noqa: E402` false
positives, as seen in the ecosystem checks. This PR modifies E402 to
omit such modifications when determining the "import boundary".
(We could consider linting against `sys.path` modifications, but that
should be a separate rule.)
Closes: https://github.com/astral-sh/ruff/issues/5557.
## Summary
This PR introduces a new `StringLike` enum which is a narrow type to
indicate string-like nodes. These includes the string literals, bytes
literals, and the literal parts of f-strings.
The main motivation behind this is to avoid repetition of rule calling
in the AST checker. We add a new `analyze::string_like` function which
takes in the enum and calls all the respective rule functions which
expects atleast 2 of the variants of this enum.
I'm open to discarding this if others think it's not that useful at this
stage as currently only 3 rules require these nodes.
As suggested
[here](https://github.com/astral-sh/ruff/pull/8835#discussion_r1414746934)
and
[here](https://github.com/astral-sh/ruff/pull/8835#discussion_r1414750204).
## Test Plan
`cargo test`
Rebase of #6365 authored by @davidszotten.
## Summary
This PR updates the AST structure for an f-string elements.
The main **motivation** behind this change is to have a dedicated node
for the string part of an f-string. Previously, the existing
`ExprStringLiteral` node was used for this purpose which isn't exactly
correct. The `ExprStringLiteral` node should include the quotes as well
in the range but the f-string literal element doesn't include the quote
as it's a specific part within an f-string. For example,
```python
f"foo {x}"
# ^^^^
# This is the literal part of an f-string
```
The introduction of `FStringElement` enum is helpful which represent
either the literal part or the expression part of an f-string.
### Rule Updates
This means that there'll be two nodes representing a string depending on
the context. One for a normal string literal while the other is a string
literal within an f-string. The AST checker is updated to accommodate
this change. The rules which work on string literal are updated to check
on the literal part of f-string as well.
#### Notes
1. The `Expr::is_literal_expr` method would check for
`ExprStringLiteral` and return true if so. But now that we don't
represent the literal part of an f-string using that node, this improves
the method's behavior and confines to the actual expression. We do have
the `FStringElement::is_literal` method.
2. We avoid checking if we're in a f-string context before adding to
`string_type_definitions` because the f-string literal is now a
dedicated node and not part of `Expr`.
3. Annotations cannot use f-string so we avoid changing any rules which
work on annotation and checks for `ExprStringLiteral`.
## Test Plan
- All references of `Expr::StringLiteral` were checked to see if any of
the rules require updating to account for the f-string literal element
node.
- New test cases are added for rules which check against the literal
part of an f-string.
- Check the ecosystem results and ensure it remains unchanged.
## Performance
There's a performance penalty in the parser. The reason for this remains
unknown as it seems that the generated assembly code is now different
for the `__reduce154` function. The reduce function body is just popping
the `ParenthesizedExpr` on top of the stack and pushing it with the new
location.
- The size of `FStringElement` enum is the same as `Expr` which is what
it replaces in `FString::format_spec`
- The size of `FStringExpressionElement` is the same as
`ExprFormattedValue` which is what it replaces
I tried reducing the `Expr` enum from 80 bytes to 72 bytes but it hardly
resulted in any performance gain. The difference can be seen here:
- Original profile: https://share.firefox.dev/3Taa7ES
- Profile after boxing some node fields:
https://share.firefox.dev/3GsNXpD
### Backtracking
I tried backtracking the changes to see if any of the isolated change
produced this regression. The problem here is that the overall change is
so small that there's only a single checkpoint where I can backtrack and
that checkpoint results in the same regression. This checkpoint is to
revert using `Expr` to the `FString::format_spec` field. After this
point, the change would revert back to the original implementation.
## Review process
The review process is similar to #7927. The first set of commits update
the node structure, parser, and related AST files. Then, further commits
update the linter and formatter part to account for the AST change.
---------
Co-authored-by: David Szotten <davidszotten@gmail.com>
## Summary
Occasionally, valid code needs to use `argparse._SubParsersAction` in a
type annotation. This isn't great, but it's indicative of the fact that
public interfaces can return private types. If you accessed that private
type via a private interface, then we should be flagging the call site,
rather than the annotation.
Closes https://github.com/astral-sh/ruff/issues/9013.
## Summary
This PR updates the `ANN201`, `ANN202`, `ANN205`, and `ANN206` rules to
not create a fix for the return type when it's an abstract method and
the function body is empty i.e., it only contains either a pass
statement, docstring or an ellipsis literal.
fixes: #9004
## Test Plan
Add the following test cases:
- Abstract method with pass statement
- Abstract method with docstring
- Abstract method with ellipsis literal
- Abstract method with possible return type
This PR renames the semantic model flag `LITERAL` to `TYPING_LITERAL` to
better reflect its purpose. The main motivation behind this change is to
avoid any confusion with the "literal" terminology used in the AST for
literal nodes like string, bytes, numbers, etc.
## Summary
If a string has a Unicode prefix, we can't add the `r` prefix on top of
that -- we need to remove and replace it. (The Unicode prefix is
redundant anyway in Python 3.)
Closes https://github.com/astral-sh/ruff/issues/8967.
## Summary
Check PEP 695 type alias definitions for `snake-case-type-alias`
(`PYI042`) and `t-suffixed-type-alias` (`PYI043`)
Related to #8771.
## Test Plan
`cargo test`
## Summary
For `t-suffixed-type-alias` to trigger, the type alias needs to be
marked as such using the `typing.TypeAlias` annotation and the name of
the alias must be marked as private using a leading underscore. The
documentation example was of an unannotated type alias that was not
marked as private, which was misleading.
## Test Plan
The current example doesn't trigger the rule; the example in this merge
request does.
## Summary
- Adds `add_argument` similar to existing `remove_argument` utility to
safely add arguments to functions.
- Adds autofix for `PLW1514` as per specs requested in
https://github.com/astral-sh/ruff/issues/8883 as a test
## Test Plan
Checks on existing fixtures as well as additional test and fixture for
Python 3.9 and lower fix
## Issue Link
Closes: https://github.com/astral-sh/ruff/issues/8883
## Summary
Adds detection for branches without a `return` or `raise`, so that we
can properly `Optional` the return types. I'd like to remove this and
replace it with our code graph analysis from the `unreachable.rs` rule,
but it at least fixes the worst offenders.
Closes#8942.
## Summary
Triggers `no-return-argument-annotation-in-stub` (`PYI050`) for vararg
and kwarg `NoReturn` type annotations.
Related to #8771.
## Test Plan
`cargo test`
## Summary
When a function uses `@functools.singledispatch`, we need to treat the
first argument of any implementations as runtime-required.
Closes https://github.com/astral-sh/ruff/issues/6849.
Mike Bernard
Charlie Marsh
Mikael Arguedas
Steve C
Shantanu
dependabot[bot]
Andrew Gallant
Micha Reiser
asafamr-mm
Alex Waygood
Tuomas Siipola
konsti
Dhruv Manilawala
Chris Hipple
qdegraaf
T-256
Samuel Cormier-Iijima
Zanie Blue
Simon Brugman
Sai-Suraj-27
Dimitri Papadopoulos Orfanos
Ofek Lev
Philipp A
Tom Kuson