<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
Resolves#20033
## Test Plan
unit tests added to the new split function, existing snapshot test
updated.
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Fixes#19887
- flynt(FLY002): When joining only string constants, upgrade raw
single-quoted strings to raw triple-quoted if the resulting
content contains a newline.
- Choose a safe triple-quote delimiter by switching to the opposite
quote style if the preferred triple appears inside the
content.
- Update FLY002 snapshot to include the `\n'.join([r'line1','line2'])`
case.
## Test Plan
I've added one test case to FLY002.py.
<!-- How was it tested? -->
---------
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Fixes#20255
Mark single-item-membership-test fixes as always unsafe
- Always set `Applicability::Unsafe` for FURB171 fixes
- Update “Fix safety” docs to reflect always-unsafe behavior
- Expand tests (not in, nested set/frozenset, commented args)
## Test Plan
<!-- How was it tested? -->
I have added new test cases to
`crates/ruff_linter/resources/test/fixtures/refurb/FURB171_0.py` and
`crates/ruff_linter/resources/test/fixtures/refurb/FURB171_1.py`.
---------
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
## Summary
Fixes https://github.com/astral-sh/ruff/issues/20134
## Test Plan
`cargo nextest run flake8_use_pathlib`
---------
Co-authored-by: Dan Parizher <danparizher@users.noreply.github.com>
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
## Summary
Part of https://github.com/astral-sh/ruff/issues/2331
## Test Plan
`cargo nextest run flake8_use_pathlib`
---------
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
- Convert panics to diagnostics with id `Panic`, severity `Fatal`, and
the error as the diagnostic message, annotated with a `Span` with empty
code block and no range.
- Updates the post-linting message diagnostic handling to track the
maximum severity seen, and then prints the "report a bug in ruff"
message only if the max severity was `Fatal`
This depends on the sorting changes since it creates diagnostics with no
range specified.
## Summary
Resolves#20266
Definition of the frozen dataclass attribute can be instantiation of a
nested frozen dataclass as well as a non-nested one.
### Problem explanation
The `function_call_in_dataclass_default` function is invoked during the
"defined scope" stage, after all scopes have been processed. At this
point, the semantic references the top-level scope. When
`SemanticModel::lookup_attribute` executes, it searches for bindings in
the top-level module scope rather than the class scope, resulting in an
error.
To solve this issue, the lookup should be evaluated through the class
scope.
## Test Plan
- Added test case from issue
Co-authored-by: Igor Drokin <drokinii1017@gmail.com>
## Summary
Fixes#19842
Prevent infinite loop with I002 and UP026
- Implement isort-aware handling for UP026 (deprecated mock import):
- Add CLI integration tests in crates/ruff/tests/lint.rs:
## Test Plan
I have added two integration tests
`pyupgrade_up026_respects_isort_required_import_fix` and
`pyupgrade_up026_respects_isort_required_import_from_fix` in
`crates/ruff/tests/lint.rs`.
## Summary
Resolves#20282
Makes the rule fix always unsafe, because the replacement may not be
semantically equivalent to the original expression, potentially changing
the behavior of the code.
Updated docstring with examples.
## Test Plan
- Added two tests from issue and regenerated the snapshot
---------
Co-authored-by: Igor Drokin <drokinii1017@gmail.com>
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
## Summary
Fixes#20204
Recognize t-strings, generators, and lambdas in RUF016
- Accept boolean literals as valid index and slice bounds.
- Add TString, Generator, and Lambda to `CheckableExprType`.
- Expand RUF016.py fixture and update snapshots accordingly.
Our token-based rules and `noqa` extraction used an `Indexer` that kept
track of f-string ranges but not t-strings. We've updated the `Indexer`
and downstream uses thereof to handle both f-strings and t-strings.
Most of the diff is renaming and adding tests.
Note that much of the "new" logic gets to be naive because the lexer has
already ensured that f and t-string "starts" are paired with their
respective "ends", even amidst nesting and so on.
Finally: one could imagine wanting to know if a given interpolated
string range corresponds to an f-string or a t-string, but I didn't find
a place where we actually needed this.
Closes#20310
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Fixes#20235
• Fix `RUF102` to properly handle rule redirects when validating noqa
codes
• Update `code_is_valid` to check redirect targets before determining
validity
• Add test case for rule redirects (TCH002 in this case)
## Test Plan
<!-- How was it tested? -->
I have added a test case for rule redirects to
`crates/ruff_linter/resources/test/fixtures/ruff/RUF102.py`.
- **Stabilize `airflow3-suggested-update` (`AIR311`)**
- **Stabilize `airflow3-suggested-to-move-to-provider` (`AIR312`)**
- **Stabilize `airflow3-removal` (`AIR301`)**
- **Stabilize `airflow3-moved-to-provider` (`AIR302`)**
- **Stabilize `airflow-dag-no-schedule-argument` (`AIR002`)**
I put this all in one PR to make it easier to double check with @Lee-W
before we merge this. I also made a few minor documentation changes and
updated one error message that I want to make sure are okay. But for the
most part this just moves the rules from `RuleGroup::Preview` to
`RuleGroup::Stable`!
Fixes#17749
This stabilizes the behavior introduced in #16565 which (roughly) tries
to match an import like `import a.b.c` to an actual directory path
`a/b/c` in order to label it as first-party, rather than simply looking
for a directory `a`.
Mainly this affects the sorting of imports in the presence of namespace
packages, but a few other rules are affected as well.
This one has been a bit contentious in the past. It usually uncovers
~700 ecosystem hits. See:
- https://github.com/astral-sh/ruff/pull/16657
- https://github.com/astral-sh/ruff/issues/16690
But I think there's consensus that it's okay to merge as-is. We'd love
an
autofix since it's so common, but we can't reliably tell what a user
meant. The
pattern is ambiguous after all 😆
This is the first rule that actually needed its test case relocated, but
the
docs looked good.
## Summary
This PR Removes deprecated UP038 as per instructed in #18727closes#18727
## Test Plan
I have run tests non of them failing
One Question i have is do we have to document that UP038 is removed?
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
## Summary
closes#7710
## Test Plan
It is is removal so i don't think we have to add tests otherwise i have
followed test plan mentioned in contributing.md
---------
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
Summary
--
Rule and test/snapshot updated, the docs look good
My one hesitation here is that we could hold off stabilizing the rule
until its fix is also ready for stabilization, but this is also the only
preview PTH rule, so I think it's okay to stabilize the rule and later
(probably in the next minor release) stabilize the fixes together.
The tests looked good. For the docs, I added a `## See also` section
pointing to
the closely-related F841 (unused-variable) and the corresponding section
to F841
pointing back to RUF059. It seems like you'd probably want both of these
active
or at least to know about the other when reading the docs.
## Summary
Resolves#19357
Skip UP008 diagnostic for `builtins.super(P, self)` calls when
`__class__` is not referenced locally, preventing incorrect fixes.
**Note:** I haven't found concrete information about which cases
`__class__` will be loaded into the scope. Let me know if anyone has
references, it would be useful to enhance the implementation. I did a
lot of tests to determine when `__class__` is loaded. Considered
sources:
1. [Python doc
super](https://docs.python.org/3/library/functions.html#super)
2. [Python doc classes](https://docs.python.org/3/tutorial/classes.html)
3. [pep-3135](https://peps.python.org/pep-3135/#specification)
As I understand it, Python will inject at runtime into local scope a
`__class__` variable if it detects references to `super` or `__class__`.
This allows calling `super()` and passing appropriate parameters.
However, the compiler doesn't do the same for `builtins.super`, so we
need to somehow introduce `__class__` into the local scope.
I figured out `__class__` will be in scope with valid value when two
conditions are met:
1. `super` or `__class__` names have been loaded within function scope
4. `__class__` is not overridden.
I think my solution isn't elegant, so I would be appreciate a detailed
review.
## Test Plan
Added 19 test cases, updated snapshots.
---------
Co-authored-by: Igor Drokin <drokinii1017@gmail.com>
Specifically, the [`if_not_else`] lint will sometimes flag
code to change the order of `if` and `else` bodies if this
would allow a `!` to be removed. While perhaps tasteful in
some cases, there are many cases in my experience where this
bows to other competing concerns that impact readability.
(Such as the relative sizes of the `if` and `else` bodies,
or perhaps an ordering that just makes the code flow in a
more natural way.)
[`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#/if_not_else
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
Noticed this was not escaped when writing a project that parses the
result of `ruff rule --outputformat json`. This is visible here:
<https://docs.astral.sh/ruff/rules/mixed-case-variable-in-global-scope/#why-is-this-bad>
## Test Plan
documentation only
---------
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
### Why
Removal should be grouped into the same category. It doesn't matter
whether it's from a provider or not (and the only case we used to have
was not anyway).
`ProviderReplacement` is used to indicate that we have a replacement and
we might need to install an extra Python package to cater to it.
### What
Move `airflow.operators.postgres_operator.Mapping` from AIR302 to AIR301
and get rid of `ProviderReplace::None`
## Test Plan
<!-- How was it tested? -->
Update the test fixtures accordingly in the first commit and reorganize
them in the second commit
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
This PR implements
https://docs.astral.sh/ruff/rules/yield-from-in-async-function/ as a
syntax semantic error
## Test Plan
<!-- How was it tested? -->
I have written a simple inline test as directed in
[https://github.com/astral-sh/ruff/issues/17412](https://github.com/astral-sh/ruff/issues/17412)
---------
Signed-off-by: 11happy <soni5happy@gmail.com>
Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
update the argument `datasets` as `assets`
## Test Plan
<!-- How was it tested? -->
update fixture accordingly
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
### What
Change the message from "DAG should have an explicit `schedule`
argument" to "`DAG` or `@dag` should have an explicit `schedule`
argument"
### Why
We're trying to get rid of the idea that DAG in airflow was Directed
acyclic graph. Thus, change it to refer to the class `DAG` or the
decorator `@dag` might help a bit.
## Test Plan
<!-- How was it tested? -->
update the test fixtures accordly
## Summary
This PR fixes#7352 by exposing the `show_fix_diff` option used in our
snapshot tests in the CLI. As the issue suggests, we plan to make this
the default output format in the future, so this is added to the `full`
output format in preview for now.
This turned out to be pretty straightforward. I just used our existing
`Applicability` settings to determine whether or not to print the diff.
The snapshot differences are because we now set
`Applicability::DisplayOnly` for our snapshot tests. This
`Applicability` is also used to determine whether or not the fix icon
(`[*]`) is rendered, so this is now shown for display-only fixes in our
snapshots. This was already the case previously, but we were only
setting `Applicability::Unsafe` in these tests and ignoring the
`Applicability` when rendering fix diffs. CLI users can't enable
display-only fixes, so this is only a test change for now, but this
should work smoothly if we decide to expose a `--display-only-fixes`
flag or similar in the future.
I also deleted the `PrinterFlags::SHOW_FIX_DIFF` flag. This was
completely unused before, and it seemed less confusing just to delete it
than to enable it in the right place and check it along with the
`OutputFormat` and `preview`.
## Test Plan
I only added one CLI test for now. I'm kind of assuming that we have
decent coverage of the cases where this shouldn't be firing, especially
the `output_format` CLI test, which shows that this definitely doesn't
affect non-preview `full` output. I'm happy to add more tests with
different combinations of options, if we're worried about any in
particular. I did try `--diff` and `--preview` and a few other
combinations manually.
And here's a screenshot using our trusty UP049 example from the design
discussion confirming that all the colors and other formatting still
look as expected:
<img width="786" height="629" alt="image"
src="https://github.com/user-attachments/assets/94e408bc-af7b-4573-b546-a5ceac2620f2"
/>
And one with an unsafe fix to see the footer:
<img width="782" height="367" alt="image"
src="https://github.com/user-attachments/assets/bbb29e47-310b-4293-b2c2-cc7aee3baff4"
/>
## Related issues and PR
- https://github.com/astral-sh/ruff/issues/7352
- https://github.com/astral-sh/ruff/pull/12595
- https://github.com/astral-sh/ruff/issues/12598
- https://github.com/astral-sh/ruff/issues/12599
- https://github.com/astral-sh/ruff/issues/12600
I think we could probably close all of these issues now. I think we've
either resolved or avoided most of them, and if we encounter them again
with the new output format, it would probably make sense to open new
ones anyway.
This pull request fixes the bug described in issue
[#19153](https://github.com/astral-sh/ruff/issues/19153).
The issue occurred when `PERF403` incorrectly flagged cases involving
tuple unpacking in a for loop. For example:
```python
def f():
v = {}
for (o, p), x in [("op", "x")]:
v[x] = o, p
```
This code was wrongly suggested to be rewritten into a dictionary
comprehension, which changes the semantics.
Changes in this PR:
Updated the `PERF403` rule to correctly handle tuple unpacking in loop
targets.
Added regression tests to ensure this case (and similar ones) are no
longer flagged incorrectly.
Why:
This ensures that `PERF403` only triggers when a dictionary
comprehension is semantically equivalent to the original loop,
preventing false positives.
---------
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
## Summary
Adds new rule to catch use of builtins `input()` in async functions.
Issue #8451
## Test Plan
New snapshosts in `ASYNC250.py` with `cargo insta test`.
## Summary
I spun this off from #19919 to separate the rendering code change and
snapshot updates from the (much smaller) changes to expose this in the
CLI. I grouped all of the `ruff_linter` snapshot changes in the final
commit in an effort to make this easier to review. The code changes are
in [this
range](619395eb41).
I went through all of the snapshots, albeit fairly quickly, and they all
looked correct to me. In the last few commits I was trying to resolve an
existing issue in the alignment of the line number separator:
73720c73be/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C409_C409.py.snap (L87-L89)
In the snapshot above on `main`, you can see that a double-digit line
number at the end of the context lines for a snippet was causing a
misalignment with the other separators. That's now resolved. The one
downside is that this can lead to a mismatch with the diagnostic above:
```
C409 [*] Unnecessary list literal passed to `tuple()` (rewrite as a tuple literal)
--> C409.py:4:6
|
2 | t2 = tuple([1, 2])
3 | t3 = tuple((1, 2))
4 | t4 = tuple([
| ______^
5 | | 1,
6 | | 2
7 | | ])
| |__^
8 | t5 = tuple(
9 | (1, 2)
|
help: Rewrite as a tuple literal
1 | t1 = tuple([])
2 | t2 = tuple([1, 2])
3 | t3 = tuple((1, 2))
- t4 = tuple([
4 + t4 = (
5 | 1,
6 | 2
- ])
7 + )
8 | t5 = tuple(
9 | (1, 2)
10 | )
note: This is an unsafe fix and may remove comments or change runtime behavior
```
But I don't think we can avoid that without really reworking this
rendering to make the diagnostic and diff rendering aware of each other.
Anyway, this should only happen in relatively rare cases where the
diagnostic is near a digit boundary and also near a context boundary.
Most of our diagnostics line up nicely.
Another potential downside of the new rendering format is its handling
of long stretches of `+` or `-` lines:
```
help: Replace with `Literal[...] | None`
21 | ...
22 |
23 |
- def func6(arg1: Literal[
- "hello",
- None # Comment 1
- , "world"
- ]):
24 + def func6(arg1: Literal["hello", "world"] | None):
25 | ...
26 |
27 |
note: This is an unsafe fix and may remove comments or change runtime behavior
```
To me it just seems a little hard to tell what's going on with just a
long streak of `-`-prefixed lines. I saw an even more exaggerated
example at some point, but I think this is also fairly rare. Most of the
snapshots seem more like the examples we looked at on Discord with
plenty of `|` lines and pairs of `+` and `-` lines.
## Test Plan
Existing tests plus one new test in `ruff_db` to isolate a line
separator alignment issue
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Fixes#19664
Fix allowed unused imports matching for top-level modules.
I've simply replaced `from_dotted_name` with `user_defined`. Since
QualifiedName for imports is created in
crates/ruff_python_semantic/src/imports.rs, I guess it's acceptable to
use `user_defined` here. Please tell me if there is better way.
0c5089ed9e/crates/ruff_python_semantic/src/imports.rs (L62)
## Test Plan
<!-- How was it tested? -->
I've added a snapshot test
`f401_allowed_unused_imports_top_level_module`.
## Summary
Adds new rule to find and report use of `httpx.Client` in synchronous
functions.
See issue #8451
## Test Plan
New snapshots for `ASYNC212.py` with `cargo insta test`.
## Summary
Fixes#19581
I decided to add in a `indent_first_line` function into
[`textwrap.rs`](https://github.com/astral-sh/ruff/blob/main/crates/ruff_python_trivia/src/textwrap.rs),
as it solely focuses on text manipulation utilities. It follows the same
design as `indent()`, and there may be situations in the future where it
can be reused as well.
---------
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Extend the following rules.
### AIR311
* `airflow.sensors.base.BaseSensorOperator` →
airflow.sdk.bases.sensor.BaseSensorOperator`
* `airflow.sensors.base.PokeReturnValue` →
airflow.sdk.bases.sensor.PokeReturnValue`
* `airflow.sensors.base.poke_mode_only` →
airflow.sdk.bases.sensor.poke_mode_only`
* `airflow.decorators.base.DecoratedOperator` →
airflow.sdk.bases.decorator.DecoratedOperator`
* `airflow.models.param.Param` → airflow.sdk.definitions.param.Param`
* `airflow.decorators.base.DecoratedMappedOperator` →
`airflow.sdk.bases.decorator.DecoratedMappedOperator`
* `airflow.decorators.base.DecoratedOperator` →
`airflow.sdk.bases.decorator.DecoratedOperator`
* `airflow.decorators.base.TaskDecorator` →
`airflow.sdk.bases.decorator.TaskDecorator`
* `airflow.decorators.base.get_unique_task_id` →
`airflow.sdk.bases.decorator.get_unique_task_id`
* `airflow.decorators.base.task_decorator_factory` →
`airflow.sdk.bases.decorator.task_decorator_factory`
### AIR312
* `airflow.sensors.bash.BashSensor` →
`airflow.providers.standard.sensor.bash.BashSensor`
* `airflow.sensors.python.PythonSensor` →
`airflow.providers.standard.sensors.python.PythonSensor`
## Test Plan
<!-- How was it tested? -->
update the test fixture accordingly in the second commit and reorg in
the third
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
Part of https://github.com/astral-sh/ruff/pull/20100 |
https://github.com/astral-sh/ruff/pull/20100#issuecomment-3225349156
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
Fixes https://github.com/astral-sh/ruff/issues/20088
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
`cargo nextest run flake8_use_pathlib`
---------
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
Closes#19302
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
This adds an auto-fix for `Logging statement uses f-string` Ruff G004,
so users don't have to resolve it manually.
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
I ran the auto-fixes on a Python file locally and and it worked as
expected.
<!-- How was it tested? -->
---------
Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
Summary
--
This PR aims to resolve (or help to resolve) #18442 and #19357 by
encoding the CPython semantics around the `__class__` cell in our
semantic model. Namely,
> `__class__` is an implicit closure reference created by the compiler
if any methods in a class body refer to either `__class__` or super.
from the Python
[docs](https://docs.python.org/3/reference/datamodel.html#creating-the-class-object).
As noted in the variant docs by @AlexWaygood, we don't fully model this
behavior, opting always to create the `__class__` cell binding in a new
`ScopeKind::DunderClassCell` around each method definition, without
checking if any method in the class body actually refers to `__class__`
or `super`.
As such, this PR fixes#18442 but not #19357.
Test Plan
--
Existing tests, plus the tests from #19783, which now pass without any
rule-specific code.
Note that we opted not to alter the behavior of F841 here because
flagging `__class__` in these cases still seems helpful. See the
discussion in
https://github.com/astral-sh/ruff/pull/20048#discussion_r2296252395 and
in the test comments for more information.
---------
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Mikko Leppänen <mleppan23@gmail.com>
## Summary
Removes the `module_ptr` field from `AstNodeRef` in release mode, and
change `NodeIndex` to a `NonZeroU32` to reduce the size of
`Option<AstNodeRef<_>>` fields.
I believe CI runs in debug mode, so this won't show up in the memory
report, but this reduces memory by ~2% in release mode.
Adds a method to `TStringValue` to detect whether the t-string is empty
_as an iterable_. Note the subtlety here that, unlike f-strings, an
empty t-string is still truthy (i.e. `bool(t"")==True`).
Closes#19951
## Summary
Resolves#19561
Fixes the [unnecessary-future-import
(UP010)](https://docs.astral.sh/ruff/rules/unnecessary-future-import/)
rule to correctly identify when imported __future__ modules are actually
used in the code, preventing false positives.
I assume there is no way to check usage in `analyze::statements`,
because we don't have any usage bindings for imports. To determine
unused imports, we have to fully scan the file to create bindings and
then check usage, similar to [unused-import
(F401)](https://docs.astral.sh/ruff/rules/unused-import/#unused-import-f401).
So, `Rule::UnnecessaryFutureImport` was moved from the
`analyze::statements` to the `analyze::deferred_scopes` stage. This
caused the need to change the logic of future import handling to a
bindings-based approach.
Also, the diagnostic report was changed.
Before
```
|
1 | from __future__ import nested_scopes, generators
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP010
```
after
```
|
1 | from __future__ import nested_scopes, generators
| ^^^^^^^^^^^^^ UP010
```
I believe this is the correct way, because `generators` may be used, but
`nested_scopes` is not.
### Special case
I've found out about some specific case.
```python
from __future__ import nested_scopes
nested_scopes = 1
```
Here we can treat `nested_scopes` as an unused import because the
variable `nested_scopes` shadows it and we can safely remove the future
import (my fix does it).
But
[F401](https://docs.astral.sh/ruff/rules/unused-import/#unused-import-f401)
not triggered for such case
([sandbox](https://play.ruff.rs/296d9c7e-0f02-4659-b0c0-78cc21f3de76))
```
from foo import print_function
print_function = 1
```
In my mind, `print_function` here is an unused import and should be
deleted (my IDE highlight it). What do you think?
## Test Plan
Added test cases and snapshots:
- Split test file into separate _0 and _1 files for appropriate checks.
- Added test cases to verify fixes when future module are used.
---------
Co-authored-by: Igor Drokin <drokinii1017@gmail.com>
**Stacked on top of #19849; diff will include that PR until it is
merged.**
---
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
-->
## Summary
As part of #19849, I noticed this fix could be implemented.
## Test Plan
Tests added based on CPython behaviour.
## Summary
- Refactored `BLE001` logic for clarity and minor speed-up.
- Improved documentation and comments (previously, `BLE001` docs claimed
it catches bare `except:`s, but it doesn't).
- Fixed a false-positive bug with `from None` cause:
```python
# somefile.py
try:
pass
except BaseException as e:
raise e from None
```
### main branch
```
somefile.py:3:8: BLE001 Do not catch blind exception: `BaseException`
|
1 | try:
2 | pass
3 | except BaseException as e:
| ^^^^^^^^^^^^^ BLE001
4 | raise e from None
|
Found 1 error.
```
### this change
```cargo run -p ruff -- check somefile.py --no-cache --select=BLE001```
```
All checks passed!
```
## Test Plan
- Added a test case to cover `raise X from Y` clause
- Added a test case to cover `raise X from None` clause
<!--
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
Add "airflow.secrets.cache.SecretCache" →
"airflow.sdk.cache.SecretCache" rule
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
<!-- How was it tested? -->
---------
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
## Summary
This PR switches the `full` output format in Ruff over to use the
rendering code
in `ruff_db`. As proposed in the design doc, this involves a lot of
changes to the snapshot output.
I also had to comment out this assertion with a TODO to replace it after
https://github.com/astral-sh/ruff/issues/19688 because many of Ruff's
"file-level" annotations aren't actually file-level. They just happen to
occur at the start of the file, especially in tests with very short
snippets.
529d81daca/crates/ruff_annotate_snippets/src/renderer/display_list.rs (L1204-L1208)
I broke up the snapshot commits at the end into several blocks, but I
don't think it's enough to help with review. The first few (notebooks,
syntax errors, and test rules) are small enough to look at, but I
couldn't really think of other categories beyond that. I'm happy to
break those up or pick out specific examples beyond what I have below,
if that would help.
The minimal code changes are in this
[range](abd28f1e77),
with the snapshot commits following. Moving the `FullRenderer` and
updating the `EmitterFlags` aren't strictly necessary either. I even
dropped the renderer commit this morning but figured it made sense to
keep it since we have the `full` module for tests. I don't feel strongly
either way.
## Test Plan
I did actually click through all 1700 snapshots individually instead of
accepting them all at once, although I moved through them quickly. There
are a
few main categories:
### Lint diagnostics
```diff
-unused.py:8:19: F401 [*] `pathlib` imported but unused
+F401 [*] `pathlib` imported but unused
+ --> unused.py:8:19
|
7 | # Unused, _not_ marked as required (due to the alias).
8 | import pathlib as non_alias
- | ^^^^^^^^^ F401
+ | ^^^^^^^^^
9 |
10 | # Unused, marked as required.
|
- = help: Remove unused import: `pathlib`
+help: Remove unused import: `pathlib`
```
- The filename and line numbers are moved to the second line
- The second noqa code next to the underline is removed
### Syntax errors
These are much like the above.
```diff
- -:1:16: invalid-syntax: Expected one or more symbol names after import
+ invalid-syntax: Expected one or more symbol names after import
+ --> -:1:16
|
1 | from foo import
| ^
```
One thing I noticed while reviewing some of these, but I don't think is
strictly syntax-error-related, is that some of the new diagnostics have
a little less context after the error. I don't think this is a problem,
but it's one small discrepancy I hadn't noticed before. Here's a minor
example:
```diff
-syntax_errors.py:1:15: invalid-syntax: Expected one or more symbol names after import
+invalid-syntax: Expected one or more symbol names after import
+ --> syntax_errors.py:1:15
|
1 | from os import
| ^
2 |
3 | if call(foo
-4 | def bar():
|
```
And one of the biggest examples:
```diff
-E30_syntax_error.py:18:11: invalid-syntax: Expected ')', found newline
+invalid-syntax: Expected ')', found newline
+ --> E30_syntax_error.py:18:11
|
16 | pass
17 |
18 | foo = Foo(
| ^
-19 |
-20 |
-21 | def top(
|
```
Similarly, a few of the lint diagnostics showed that the cut indicator
calculation for overly long lines is also slightly different, but I
think that's okay too.
### Full-file diagnostics
```diff
-comment.py:1:1: I002 [*] Missing required import: `from __future__ import annotations`
+I002 [*] Missing required import: `from __future__ import annotations`
+--> comment.py:1:1
+help: Insert required import: `from __future__ import annotations`
+
```
As noted above, these will be much more rare after #19688 too. This case
isn't a true full-file diagnostic and will render a snippet in the
future, but you can see that we're now rendering the help message that
would have been discarded before. In contrast, this is a true full-file
diagnostic and should still look like this after #19688:
```diff
-__init__.py:1:1: A005 Module `logging` shadows a Python standard-library module
+A005 Module `logging` shadows a Python standard-library module
+--> __init__.py:1:1
```
### Jupyter notebooks
There's nothing particularly different about these, just showing off the
cell index again.
```diff
- Jupyter.ipynb:cell 3:1:7: F821 Undefined name `x`
+ F821 Undefined name `x`
+ --> Jupyter.ipynb:cell 3:1:7
|
1 | print(x)
- | ^ F821
+ | ^
|
```
PLE2513 --fix changes ESC and SUB to uppercase hexadecimal values such
as \x1B while the formatter changes them to lowercase \x1b
<!--
Thank you for contributing to Ruff/ty! 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? (Please prefix
with `[ty]` for ty pull
requests.)
- 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? -->
---------
Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
## Summary
This PR is a spin-off from https://github.com/astral-sh/ruff/pull/19415.
It enables replacing the severity and lint name in a ty-style
diagnostic:
```
error[unused-import]: `os` imported but unused
```
with the noqa code and optional fix availability icon for a Ruff
diagnostic:
```
F401 [*] `os` imported but unused
F821 Undefined name `a`
```
or nothing at all for a Ruff syntax error:
```
SyntaxError: Expected one or more symbol names after import
```
Ruff adds the `SyntaxError` prefix to these messages manually.
Initially (d912458), I just passed a `hide_severity` flag through a
bunch of calls to get it into `annotate-snippets`, but after looking at
it again today, I think reusing the `None` severity/level gave a nicer
result. As I note in a lengthy code comment, I think all of this code
should be temporary and reverted when Ruff gets real severities, so
hopefully it's okay if it feels a little hacky.
I think the main visible downside of this approach is that we can't
style the asterisk in the fix availabilty icon in cyan, as in Ruff's
current output. It's part of the message in this PR and any styling gets
overwritten in `annotate-snippets`.
<img width="400" height="342" alt="image"
src="https://github.com/user-attachments/assets/57542ec9-a81c-4a01-91c7-bd6d7ec99f99"
/>
Hmm, I guess reusing `Level::None` also means the `F401` isn't red
anymore. Maybe my initial approach was better after all. In any case,
the rest of the PR should be basically the same, it just depends how we
want to toggle the severity.
## Test Plan
New `ruff_db` tests. These snapshots should be compared to the two tests
just above them (`hide_severity_output` vs `output` and
`hide_severity_syntax_errors` against `syntax_errors`).
## Summary
This PR enhances the `BLE001` rule to correctly detect blind exception
handling in tuple exceptions. Previously, the rule only checked single
exception types, but Python allows catching multiple exceptions using
tuples like `except (Exception, ValueError):`.
## Test Plan
It fails the following (whereas the main branch does not):
```bash
cargo run -p ruff -- check somefile.py --no-cache --select=BLE001
```
```python
# somefile.py
try:
1/0
except (ValueError, Exception) as e:
print(e)
```
```
somefile.py:3:21: BLE001 Do not catch blind exception: `Exception`
|
1 | try:
2 | 1/0
3 | except (ValueError, Exception) as e:
| ^^^^^^^^^ BLE001
4 | print(e)
|
Found 1 error.
```
Dan Parizher
Frazer McLean
Takayuki Maeda
Micha Reiser
chiri
Andrew Gallant
Amethyst Reese
Igor Drokin
Dylan
Brent Westbrook
हिमांशु
arielle
Alex Waygood
Wei Lee
Bhuminjay Soni
Hans
Lior Weissman
Kot
Hamir Mahal
Ibraheem Ahmed
Vivek Dasari
Max Mynter
Andrey
Avasam
Roman Kitaev
Harutaka Kawamura
Sneha Prabhu
ember91