From a8e50a7f40b2883b904d9ba347ef01c466179a75 Mon Sep 17 00:00:00 2001 From: Guilherme Vasconcelos <49197151+Guilherme-Vasconcelos@users.noreply.github.com> Date: Thu, 14 Mar 2024 20:18:03 -0300 Subject: [PATCH 1/7] [RUF008] Make it clearer that a mutable default in a dataclass is only valid if it is typed as a ClassVar (#10395) ## Summary The previous documentation sounded as if typing a mutable default as a `ClassVar` were optional. However, it is not, as not doing so causes a `ValueError`. The snippet below was tested in Python's interactive shell: ``` >>> from dataclasses import dataclass >>> @dataclass ... class A: ... mutable_default: list[int] = [] ... Traceback (most recent call last): File "", line 1, in File "/usr/lib/python3.11/dataclasses.py", line 1230, in dataclass return wrap(cls) ^^^^^^^^^ File "/usr/lib/python3.11/dataclasses.py", line 1220, in wrap return _process_class(cls, init, repr, eq, order, unsafe_hash, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/dataclasses.py", line 958, in _process_class cls_fields.append(_get_field(cls, name, type, kw_only)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/dataclasses.py", line 815, in _get_field raise ValueError(f'mutable default {type(f.default)} for field ' ValueError: mutable default for field mutable_default is not allowed: use default_factory >>> ``` This behavior is also documented in Python's docs, see [here](https://docs.python.org/3/library/dataclasses.html#mutable-default-values): > [...] the [dataclass()](https://docs.python.org/3/library/dataclasses.html#dataclasses.dataclass) decorator will raise a [ValueError](https://docs.python.org/3/library/exceptions.html#ValueError) if it detects an unhashable default parameter. The assumption is that if a value is unhashable, it is mutable. This is a partial solution, but it does protect against many common errors. And [here](https://docs.python.org/3/library/dataclasses.html#class-variables) it is documented why it works if it is typed as a `ClassVar`: > One of the few places where [dataclass()](https://docs.python.org/3/library/dataclasses.html#dataclasses.dataclass) actually inspects the type of a field is to determine if a field is a class variable as defined in [PEP 526](https://peps.python.org/pep-0526/). It does this by checking if the type of the field is typing.ClassVar. If a field is a ClassVar, it is excluded from consideration as a field and is ignored by the dataclass mechanisms. Such ClassVar pseudo-fields are not returned by the module-level [fields()](https://docs.python.org/3/library/dataclasses.html#dataclasses.fields) function. In this PR I have changed the documentation to make it a little bit clearer that not using `ClassVar` makes the code invalid. --- .../src/rules/ruff/rules/mutable_dataclass_default.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/mutable_dataclass_default.rs b/crates/ruff_linter/src/rules/ruff/rules/mutable_dataclass_default.rs index f12d8cf87a1281..9eebf896b523f6 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mutable_dataclass_default.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mutable_dataclass_default.rs @@ -19,8 +19,8 @@ use crate::rules::ruff::rules::helpers::{is_class_var_annotation, is_dataclass}; /// Instead of sharing mutable defaults, use the `field(default_factory=...)` /// pattern. /// -/// If the default value is intended to be mutable, it should be annotated with -/// `typing.ClassVar`. +/// If the default value is intended to be mutable, it must be annotated with +/// `typing.ClassVar`; otherwise, a `ValueError` will be raised. /// /// ## Examples /// ```python @@ -29,6 +29,8 @@ use crate::rules::ruff::rules::helpers::{is_class_var_annotation, is_dataclass}; /// /// @dataclass /// class A: +/// # A list without a `default_factory` or `ClassVar` annotation +/// # will raise a `ValueError`. /// mutable_default: list[int] = [] /// ``` /// @@ -44,7 +46,7 @@ use crate::rules::ruff::rules::helpers::{is_class_var_annotation, is_dataclass}; /// /// Or: /// ```python -/// from dataclasses import dataclass, field +/// from dataclasses import dataclass /// from typing import ClassVar /// /// From 10ace88e9a7423271441ba314e788daee53e00f0 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 14 Mar 2024 17:45:46 -0700 Subject: [PATCH 2/7] Track conditional deletions in the semantic model (#10415) ## Summary Given `del X`, we'll typically add a `BindingKind::Deletion` to `X` to shadow the current binding. However, if the deletion is inside of a conditional operation, we _won't_, as in: ```python def f(): global X if X > 0: del X ``` We will, however, track it as a reference to the binding. This PR adds the expression context to those resolved references, so that we can detect that the `X` in `global X` was "assigned to". Closes https://github.com/astral-sh/ruff/issues/10397. --- .../pylint/global_variable_not_assigned.py | 7 +++ .../checkers/ast/analyze/deferred_scopes.rs | 30 ++++++++--- crates/ruff_linter/src/checkers/ast/mod.rs | 9 +++- crates/ruff_linter/src/renamer.rs | 1 + .../runtime_import_in_type_checking_block.rs | 8 ++- .../rules/typing_only_runtime_import.rs | 2 +- .../src/rules/pylint/rules/non_ascii_name.rs | 1 + crates/ruff_python_semantic/src/binding.rs | 34 ++++++++++-- crates/ruff_python_semantic/src/model.rs | 52 ++++++++++++++----- crates/ruff_python_semantic/src/reference.rs | 34 +++++++----- 10 files changed, 133 insertions(+), 45 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/global_variable_not_assigned.py b/crates/ruff_linter/resources/test/fixtures/pylint/global_variable_not_assigned.py index e6514b35d0cb46..2109c29ad3f97d 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/global_variable_not_assigned.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/global_variable_not_assigned.py @@ -11,6 +11,13 @@ def f(): print(X) +def f(): + global X + + if X > 0: + del X + + ### # Non-errors. ### diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs index c35cf53a77a3a1..df2598b33ba395 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -1,6 +1,6 @@ use ruff_diagnostics::{Diagnostic, Fix}; use ruff_python_semantic::analyze::visibility; -use ruff_python_semantic::{Binding, BindingKind, Imported, ScopeKind}; +use ruff_python_semantic::{Binding, BindingKind, Imported, ResolvedReference, ScopeKind}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -91,13 +91,29 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { if checker.enabled(Rule::GlobalVariableNotAssigned) { for (name, binding_id) in scope.bindings() { let binding = checker.semantic.binding(binding_id); + // If the binding is a `global`, then it's a top-level `global` that was never + // assigned in the current scope. If it were assigned, the `global` would be + // shadowed by the assignment. if binding.kind.is_global() { - diagnostics.push(Diagnostic::new( - pylint::rules::GlobalVariableNotAssigned { - name: (*name).to_string(), - }, - binding.range(), - )); + // If the binding was conditionally deleted, it will include a reference within + // a `Del` context, but won't be shadowed by a `BindingKind::Deletion`, as in: + // ```python + // if condition: + // del var + // ``` + if binding + .references + .iter() + .map(|id| checker.semantic.reference(*id)) + .all(ResolvedReference::is_load) + { + diagnostics.push(Diagnostic::new( + pylint::rules::GlobalVariableNotAssigned { + name: (*name).to_string(), + }, + binding.range(), + )); + } } } } diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 43aaf86ccf3ead..562cb4e37c7d20 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -540,7 +540,11 @@ impl<'a> Visitor<'a> for Checker<'a> { for name in names { if let Some((scope_id, binding_id)) = self.semantic.nonlocal(name) { // Mark the binding as "used". - self.semantic.add_local_reference(binding_id, name.range()); + self.semantic.add_local_reference( + binding_id, + ExprContext::Load, + name.range(), + ); // Mark the binding in the enclosing scope as "rebound" in the current // scope. @@ -2113,7 +2117,8 @@ impl<'a> Checker<'a> { // Mark anything referenced in `__all__` as used. // TODO(charlie): `range` here should be the range of the name in `__all__`, not // the range of `__all__` itself. - self.semantic.add_global_reference(binding_id, range); + self.semantic + .add_global_reference(binding_id, ExprContext::Load, range); } else { if self.semantic.global_scope().uses_star_imports() { if self.enabled(Rule::UndefinedLocalWithImportStarUsage) { diff --git a/crates/ruff_linter/src/renamer.rs b/crates/ruff_linter/src/renamer.rs index 8f4d560afe6ed2..8571d7f53b067b 100644 --- a/crates/ruff_linter/src/renamer.rs +++ b/crates/ruff_linter/src/renamer.rs @@ -255,6 +255,7 @@ impl Renamer { | BindingKind::ClassDefinition(_) | BindingKind::FunctionDefinition(_) | BindingKind::Deletion + | BindingKind::ConditionalDeletion(_) | BindingKind::UnboundException(_) => { Some(Edit::range_replacement(target.to_string(), binding.range())) } diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index a30f8feaffe3a1..d9d7b45e2e5c63 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs @@ -121,8 +121,7 @@ pub(crate) fn runtime_import_in_type_checking_block( checker .semantic() .reference(reference_id) - .context() - .is_runtime() + .in_runtime_context() }) { let Some(node_id) = binding.source else { @@ -155,8 +154,7 @@ pub(crate) fn runtime_import_in_type_checking_block( if checker.settings.flake8_type_checking.quote_annotations && binding.references().all(|reference_id| { let reference = checker.semantic().reference(reference_id); - reference.context().is_typing() - || reference.in_runtime_evaluated_annotation() + reference.in_typing_context() || reference.in_runtime_evaluated_annotation() }) { actions @@ -268,7 +266,7 @@ fn quote_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) .flat_map(|ImportBinding { binding, .. }| { binding.references.iter().filter_map(|reference_id| { let reference = checker.semantic().reference(*reference_id); - if reference.context().is_runtime() { + if reference.in_runtime_context() { Some(quote_annotation( reference.expression_id()?, checker.semantic(), diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index 13c2ba673ae050..75d8f544504382 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -499,7 +499,7 @@ fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) -> .flat_map(|ImportBinding { binding, .. }| { binding.references.iter().filter_map(|reference_id| { let reference = checker.semantic().reference(*reference_id); - if reference.context().is_runtime() { + if reference.in_runtime_context() { Some(quote_annotation( reference.expression_id()?, checker.semantic(), diff --git a/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs b/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs index 92bcd799078270..2023c58ad14a81 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs @@ -67,6 +67,7 @@ pub(crate) fn non_ascii_name(binding: &Binding, locator: &Locator) -> Option { return None; } diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index cff3c7ce75d1c7..fff0d852bbe381 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -75,6 +75,11 @@ impl<'a> Binding<'a> { self.flags.intersects(BindingFlags::GLOBAL) } + /// Return `true` if this [`Binding`] was deleted. + pub const fn is_deleted(&self) -> bool { + self.flags.intersects(BindingFlags::DELETED) + } + /// Return `true` if this [`Binding`] represents an assignment to `__all__` with an invalid /// value (e.g., `__all__ = "Foo"`). pub const fn is_invalid_all_format(&self) -> bool { @@ -165,6 +170,7 @@ impl<'a> Binding<'a> { // Deletions, annotations, `__future__` imports, and builtins are never considered // redefinitions. BindingKind::Deletion + | BindingKind::ConditionalDeletion(_) | BindingKind::Annotation | BindingKind::FutureImport | BindingKind::Builtin => { @@ -265,6 +271,19 @@ bitflags! { /// ``` const GLOBAL = 1 << 4; + /// The binding was deleted (i.e., the target of a `del` statement). + /// + /// For example, the binding could be `x` in: + /// ```python + /// del x + /// ``` + /// + /// The semantic model will typically shadow a deleted binding via an additional binding + /// with [`BindingKind::Deletion`]; however, conditional deletions (e.g., + /// `if condition: del x`) do _not_ generate a shadow binding. This flag is thus used to + /// detect whether a binding was _ever_ deleted, even conditionally. + const DELETED = 1 << 5; + /// The binding represents an export via `__all__`, but the assigned value uses an invalid /// expression (i.e., a non-container type). /// @@ -272,7 +291,7 @@ bitflags! { /// ```python /// __all__ = 1 /// ``` - const INVALID_ALL_FORMAT = 1 << 5; + const INVALID_ALL_FORMAT = 1 << 6; /// The binding represents an export via `__all__`, but the assigned value contains an /// invalid member (i.e., a non-string). @@ -281,7 +300,7 @@ bitflags! { /// ```python /// __all__ = [1] /// ``` - const INVALID_ALL_OBJECT = 1 << 6; + const INVALID_ALL_OBJECT = 1 << 7; /// The binding represents a private declaration. /// @@ -289,7 +308,7 @@ bitflags! { /// ```python /// _T = "This is a private variable" /// ``` - const PRIVATE_DECLARATION = 1 << 7; + const PRIVATE_DECLARATION = 1 << 8; /// The binding represents an unpacked assignment. /// @@ -297,7 +316,7 @@ bitflags! { /// ```python /// (x, y) = 1, 2 /// ``` - const UNPACKED_ASSIGNMENT = 1 << 8; + const UNPACKED_ASSIGNMENT = 1 << 9; } } @@ -512,6 +531,13 @@ pub enum BindingKind<'a> { /// ``` Deletion, + /// A binding for a deletion, like `x` in: + /// ```python + /// if x > 0: + /// del x + /// ``` + ConditionalDeletion(BindingId), + /// A binding to bind an exception to a local variable, like `x` in: /// ```python /// try: diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index f6575656fdf576..f9f4fdc4716029 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -5,7 +5,7 @@ use rustc_hash::FxHashMap; use ruff_python_ast::helpers::from_relative_import; use ruff_python_ast::name::{QualifiedName, UnqualifiedName}; -use ruff_python_ast::{self as ast, Expr, Operator, Stmt}; +use ruff_python_ast::{self as ast, Expr, ExprContext, Operator, Stmt}; use ruff_python_stdlib::path::is_python_stub_file; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -271,7 +271,7 @@ impl<'a> SemanticModel<'a> { .get(symbol) .map_or(true, |binding_id| { // Treat the deletion of a name as a reference to that name. - self.add_local_reference(binding_id, range); + self.add_local_reference(binding_id, ExprContext::Del, range); self.bindings[binding_id].is_unbound() }); @@ -296,8 +296,9 @@ impl<'a> SemanticModel<'a> { let reference_id = self.resolved_references.push( ScopeId::global(), self.node_id, - name.range, + ExprContext::Load, self.flags, + name.range, ); self.bindings[binding_id].references.push(reference_id); @@ -308,8 +309,9 @@ impl<'a> SemanticModel<'a> { let reference_id = self.resolved_references.push( ScopeId::global(), self.node_id, - name.range, + ExprContext::Load, self.flags, + name.range, ); self.bindings[binding_id].references.push(reference_id); } @@ -365,8 +367,9 @@ impl<'a> SemanticModel<'a> { let reference_id = self.resolved_references.push( self.scope_id, self.node_id, - name.range, + ExprContext::Load, self.flags, + name.range, ); self.bindings[binding_id].references.push(reference_id); @@ -377,8 +380,9 @@ impl<'a> SemanticModel<'a> { let reference_id = self.resolved_references.push( self.scope_id, self.node_id, - name.range, + ExprContext::Load, self.flags, + name.range, ); self.bindings[binding_id].references.push(reference_id); } @@ -426,6 +430,15 @@ impl<'a> SemanticModel<'a> { return ReadResult::UnboundLocal(binding_id); } + BindingKind::ConditionalDeletion(binding_id) => { + self.unresolved_references.push( + name.range, + self.exceptions(), + UnresolvedReferenceFlags::empty(), + ); + return ReadResult::UnboundLocal(binding_id); + } + // If we hit an unbound exception that shadowed a bound name, resole to the // bound name. For example, given: // @@ -446,8 +459,9 @@ impl<'a> SemanticModel<'a> { let reference_id = self.resolved_references.push( self.scope_id, self.node_id, - name.range, + ExprContext::Load, self.flags, + name.range, ); self.bindings[binding_id].references.push(reference_id); @@ -458,8 +472,9 @@ impl<'a> SemanticModel<'a> { let reference_id = self.resolved_references.push( self.scope_id, self.node_id, - name.range, + ExprContext::Load, self.flags, + name.range, ); self.bindings[binding_id].references.push(reference_id); } @@ -548,6 +563,7 @@ impl<'a> SemanticModel<'a> { match self.bindings[binding_id].kind { BindingKind::Annotation => continue, BindingKind::Deletion | BindingKind::UnboundException(None) => return None, + BindingKind::ConditionalDeletion(binding_id) => return Some(binding_id), BindingKind::UnboundException(Some(binding_id)) => return Some(binding_id), _ => return Some(binding_id), } @@ -1315,18 +1331,28 @@ impl<'a> SemanticModel<'a> { } /// Add a reference to the given [`BindingId`] in the local scope. - pub fn add_local_reference(&mut self, binding_id: BindingId, range: TextRange) { + pub fn add_local_reference( + &mut self, + binding_id: BindingId, + ctx: ExprContext, + range: TextRange, + ) { let reference_id = self.resolved_references - .push(self.scope_id, self.node_id, range, self.flags); + .push(self.scope_id, self.node_id, ctx, self.flags, range); self.bindings[binding_id].references.push(reference_id); } /// Add a reference to the given [`BindingId`] in the global scope. - pub fn add_global_reference(&mut self, binding_id: BindingId, range: TextRange) { + pub fn add_global_reference( + &mut self, + binding_id: BindingId, + ctx: ExprContext, + range: TextRange, + ) { let reference_id = self.resolved_references - .push(ScopeId::global(), self.node_id, range, self.flags); + .push(ScopeId::global(), self.node_id, ctx, self.flags, range); self.bindings[binding_id].references.push(reference_id); } @@ -1700,7 +1726,6 @@ bitflags! { /// only required by the Python interpreter, but by runtime type checkers too. const RUNTIME_REQUIRED_ANNOTATION = 1 << 2; - /// The model is in a type definition. /// /// For example, the model could be visiting `int` in: @@ -1886,7 +1911,6 @@ bitflags! { /// ``` const COMPREHENSION_ASSIGNMENT = 1 << 19; - /// The model is in a module / class / function docstring. /// /// For example, the model could be visiting either the module, class, diff --git a/crates/ruff_python_semantic/src/reference.rs b/crates/ruff_python_semantic/src/reference.rs index 6bb807e1c85230..d6735f4232dc88 100644 --- a/crates/ruff_python_semantic/src/reference.rs +++ b/crates/ruff_python_semantic/src/reference.rs @@ -3,10 +3,10 @@ use std::ops::Deref; use bitflags::bitflags; use ruff_index::{newtype_index, IndexSlice, IndexVec}; +use ruff_python_ast::ExprContext; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange}; -use crate::context::ExecutionContext; use crate::scope::ScopeId; use crate::{Exceptions, NodeId, SemanticModelFlags}; @@ -18,10 +18,12 @@ pub struct ResolvedReference { node_id: Option, /// The scope in which the reference is defined. scope_id: ScopeId, - /// The range of the reference in the source code. - range: TextRange, + /// The expression context in which the reference occurs (e.g., `Load`, `Store`, `Del`). + ctx: ExprContext, /// The model state in which the reference occurs. flags: SemanticModelFlags, + /// The range of the reference in the source code. + range: TextRange, } impl ResolvedReference { @@ -35,13 +37,19 @@ impl ResolvedReference { self.scope_id } - /// The [`ExecutionContext`] of the reference. - pub const fn context(&self) -> ExecutionContext { - if self.flags.intersects(SemanticModelFlags::TYPING_CONTEXT) { - ExecutionContext::Typing - } else { - ExecutionContext::Runtime - } + /// Return `true` if the reference occurred in a `Load` operation. + pub const fn is_load(&self) -> bool { + self.ctx.is_load() + } + + /// Return `true` if the context is in a typing context. + pub const fn in_typing_context(&self) -> bool { + self.flags.intersects(SemanticModelFlags::TYPING_CONTEXT) + } + + /// Return `true` if the context is in a runtime context. + pub const fn in_runtime_context(&self) -> bool { + !self.flags.intersects(SemanticModelFlags::TYPING_CONTEXT) } /// Return `true` if the context is in a typing-only type annotation. @@ -108,14 +116,16 @@ impl ResolvedReferences { &mut self, scope_id: ScopeId, node_id: Option, - range: TextRange, + ctx: ExprContext, flags: SemanticModelFlags, + range: TextRange, ) -> ResolvedReferenceId { self.0.push(ResolvedReference { node_id, scope_id, - range, + ctx, flags, + range, }) } } From 9675e1867a73e536a609e971f2e823c799771afc Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Fri, 15 Mar 2024 03:55:57 +0000 Subject: [PATCH 3/7] Allow trailing ellipsis in `typing.TYPE_CHECKING` (#10413) ## Summary Trailing ellipses in objects defined in `typing.TYPE_CHECKING` might be meaningful (it might be declaring a stub). Thus, we should skip the `unnecessary-placeholder` (`PIE970`) rule in such contexts. Closes #10358. ## Test Plan `cargo nextest run` --- .../resources/test/fixtures/flake8_pie/PIE790.py | 8 ++++++++ .../src/rules/flake8_pie/rules/unnecessary_placeholder.rs | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py index 267d8a3c3dd214..f1b3d253018065 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py @@ -227,3 +227,11 @@ def func(self) -> str: def impl(self) -> str: """Docstring""" return self.func() + + +import typing + +if typing.TYPE_CHECKING: + def contains_meaningful_ellipsis() -> list[int]: + """Allow this in a TYPE_CHECKING block.""" + ... diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_placeholder.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_placeholder.rs index 576cb21e35d356..09a58552d6357a 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_placeholder.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_placeholder.rs @@ -87,6 +87,12 @@ pub(crate) fn unnecessary_placeholder(checker: &mut Checker, body: &[Stmt]) { let kind = match stmt { Stmt::Pass(_) => Placeholder::Pass, Stmt::Expr(expr) if expr.value.is_ellipsis_literal_expr() => { + // In a type-checking block, a trailing ellipsis might be meaningful. A + // user might be using the type-checking context to declare a stub. + if checker.semantic().in_type_checking_block() { + return; + } + // Ellipses are significant in protocol methods and abstract methods. Specifically, // Pyright uses the presence of an ellipsis to indicate that a method is a stub, // rather than a default implementation. From 7e652e8fcb7e9a4c331d8b8829adbf135154fa0f Mon Sep 17 00:00:00 2001 From: hikaru-kajita <61526956+boolean-light@users.noreply.github.com> Date: Fri, 15 Mar 2024 23:34:18 +0900 Subject: [PATCH 4/7] [`flake8_comprehensions`] Handled special case for `C400` which also matches `C416` (#10419) ## Summary Short-circuit implementation mentioned in #10403. I implemented this by extending C400: - Made `UnnecessaryGeneratorList` have information of whether the the short-circuiting occurred (to put diagnostic) - Add additional check for whether in `unnecessary_generator_list` function. Please give me suggestions if you think this isn't the best way to handle this :) ## Test Plan Extended `C400.py` a little, and written the cases where: - Code could be converted to one single conversion to `list` e.g. `list(x for x in range(3))` -> `list(range(3))` - Code couldn't be converted to one single conversion to `list` e.g. `list(2 * x for x in range(3))` -> `[2 * x for x in range(3)]` - `list` function is not built-in, and should not modify the code in any way. --- .../fixtures/flake8_comprehensions/C400.py | 11 ++- .../rules/unnecessary_generator_list.rs | 97 ++++++++++++++----- ...8_comprehensions__tests__C400_C400.py.snap | 96 +++++++++++++----- 3 files changed, 155 insertions(+), 49 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C400.py b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C400.py index ce76df9a8f7e3a..16e29e4fb33648 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C400.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C400.py @@ -1,11 +1,20 @@ +# Cannot combine with C416. Should use list comprehension here. +even_nums = list(2 * x for x in range(3)) +odd_nums = list( + 2 * x + 1 for x in range(3) +) + + +# Short-circuit case, combine with C416 and should produce x = list(range(3)) x = list(x for x in range(3)) x = list( x for x in range(3) ) - +# Not built-in list. def list(*args, **kwargs): return None +list(2 * x for x in range(3)) list(x for x in range(3)) diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs index 8de73493b01369..7b9bb5e4bffd1b 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs @@ -1,6 +1,8 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; +use ruff_python_ast::comparable::ComparableExpr; +use ruff_python_ast::ExprGenerator; use ruff_text_size::{Ranged, TextSize}; use crate::checkers::ast::Checker; @@ -9,37 +11,53 @@ use super::helpers; /// ## What it does /// Checks for unnecessary generators that can be rewritten as `list` -/// comprehensions. +/// comprehensions (or with `list` directly). /// /// ## Why is this bad? /// It is unnecessary to use `list` around a generator expression, since /// there are equivalent comprehensions for these types. Using a /// comprehension is clearer and more idiomatic. /// +/// Further, if the comprehension can be removed entirely, as in the case of +/// `list(x for x in foo)`, it's better to use `list(foo)` directly, since it's +/// even more direct. +/// /// ## Examples /// ```python /// list(f(x) for x in foo) +/// list(x for x in foo) /// ``` /// /// Use instead: /// ```python /// [f(x) for x in foo] +/// list(foo) /// ``` /// /// ## Fix safety /// This rule's fix is marked as unsafe, as it may occasionally drop comments /// when rewriting the call. In most cases, though, comments will be preserved. #[violation] -pub struct UnnecessaryGeneratorList; +pub struct UnnecessaryGeneratorList { + short_circuit: bool, +} impl AlwaysFixableViolation for UnnecessaryGeneratorList { #[derive_message_formats] fn message(&self) -> String { - format!("Unnecessary generator (rewrite as a `list` comprehension)") + if self.short_circuit { + format!("Unnecessary generator (rewrite using `list()`") + } else { + format!("Unnecessary generator (rewrite as a `list` comprehension)") + } } fn fix_title(&self) -> String { - "Rewrite as a `list` comprehension".to_string() + if self.short_circuit { + "Rewrite using `list()`".to_string() + } else { + "Rewrite as a `list` comprehension".to_string() + } } } @@ -56,28 +74,59 @@ pub(crate) fn unnecessary_generator_list(checker: &mut Checker, call: &ast::Expr if !checker.semantic().is_builtin("list") { return; } - if argument.is_generator_expr() { - let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorList, call.range()); - // Convert `list(x for x in y)` to `[x for x in y]`. - diagnostic.set_fix({ - // Replace `list(` with `[`. - let call_start = Edit::replacement( - "[".to_string(), - call.start(), - call.arguments.start() + TextSize::from(1), - ); + let Some(ExprGenerator { + elt, generators, .. + }) = argument.as_generator_expr() + else { + return; + }; + + // Short-circuit: given `list(x for x in y)`, generate `list(y)` (in lieu of `[x for x in y]`). + if let [generator] = generators.as_slice() { + if generator.ifs.is_empty() && !generator.is_async { + if ComparableExpr::from(elt) == ComparableExpr::from(&generator.target) { + let mut diagnostic = Diagnostic::new( + UnnecessaryGeneratorList { + short_circuit: true, + }, + call.range(), + ); + let iterator = format!("list({})", checker.locator().slice(generator.iter.range())); + diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( + iterator, + call.range(), + ))); + checker.diagnostics.push(diagnostic); + return; + } + } + } + + // Convert `list(f(x) for x in y)` to `[f(x) for x in y]`. + let mut diagnostic = Diagnostic::new( + UnnecessaryGeneratorList { + short_circuit: false, + }, + call.range(), + ); + diagnostic.set_fix({ + // Replace `list(` with `[`. + let call_start = Edit::replacement( + "[".to_string(), + call.start(), + call.arguments.start() + TextSize::from(1), + ); - // Replace `)` with `]`. - let call_end = Edit::replacement( - "]".to_string(), - call.arguments.end() - TextSize::from(1), - call.end(), - ); + // Replace `)` with `]`. + let call_end = Edit::replacement( + "]".to_string(), + call.arguments.end() - TextSize::from(1), + call.end(), + ); - Fix::unsafe_edits(call_start, [call_end]) - }); + Fix::unsafe_edits(call_start, [call_end]) + }); - checker.diagnostics.push(diagnostic); - } + checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C400_C400.py.snap b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C400_C400.py.snap index 56e0f8d95a866c..ba92bc7d57530c 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C400_C400.py.snap +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C400_C400.py.snap @@ -1,42 +1,90 @@ --- source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs --- -C400.py:1:5: C400 [*] Unnecessary generator (rewrite as a `list` comprehension) +C400.py:2:13: C400 [*] Unnecessary generator (rewrite as a `list` comprehension) | -1 | x = list(x for x in range(3)) - | ^^^^^^^^^^^^^^^^^^^^^^^^^ C400 -2 | x = list( -3 | x for x in range(3) +1 | # Cannot combine with C416. Should use list comprehension here. +2 | even_nums = list(2 * x for x in range(3)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C400 +3 | odd_nums = list( +4 | 2 * x + 1 for x in range(3) | = help: Rewrite as a `list` comprehension ℹ Unsafe fix -1 |-x = list(x for x in range(3)) - 1 |+x = [x for x in range(3)] -2 2 | x = list( -3 3 | x for x in range(3) -4 4 | ) +1 1 | # Cannot combine with C416. Should use list comprehension here. +2 |-even_nums = list(2 * x for x in range(3)) + 2 |+even_nums = [2 * x for x in range(3)] +3 3 | odd_nums = list( +4 4 | 2 * x + 1 for x in range(3) +5 5 | ) -C400.py:2:5: C400 [*] Unnecessary generator (rewrite as a `list` comprehension) +C400.py:3:12: C400 [*] Unnecessary generator (rewrite as a `list` comprehension) | -1 | x = list(x for x in range(3)) -2 | x = list( - | _____^ -3 | | x for x in range(3) -4 | | ) +1 | # Cannot combine with C416. Should use list comprehension here. +2 | even_nums = list(2 * x for x in range(3)) +3 | odd_nums = list( + | ____________^ +4 | | 2 * x + 1 for x in range(3) +5 | | ) | |_^ C400 | = help: Rewrite as a `list` comprehension ℹ Unsafe fix -1 1 | x = list(x for x in range(3)) -2 |-x = list( - 2 |+x = [ -3 3 | x for x in range(3) -4 |-) - 4 |+] -5 5 | +1 1 | # Cannot combine with C416. Should use list comprehension here. +2 2 | even_nums = list(2 * x for x in range(3)) +3 |-odd_nums = list( + 3 |+odd_nums = [ +4 4 | 2 * x + 1 for x in range(3) +5 |-) + 5 |+] 6 6 | -7 7 | def list(*args, **kwargs): +7 7 | +8 8 | # Short-circuit case, combine with C416 and should produce x = list(range(3)) +C400.py:9:5: C400 [*] Unnecessary generator (rewrite using `list()` + | + 8 | # Short-circuit case, combine with C416 and should produce x = list(range(3)) + 9 | x = list(x for x in range(3)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^ C400 +10 | x = list( +11 | x for x in range(3) + | + = help: Rewrite using `list()` +ℹ Unsafe fix +6 6 | +7 7 | +8 8 | # Short-circuit case, combine with C416 and should produce x = list(range(3)) +9 |-x = list(x for x in range(3)) + 9 |+x = list(range(3)) +10 10 | x = list( +11 11 | x for x in range(3) +12 12 | ) + +C400.py:10:5: C400 [*] Unnecessary generator (rewrite using `list()` + | + 8 | # Short-circuit case, combine with C416 and should produce x = list(range(3)) + 9 | x = list(x for x in range(3)) +10 | x = list( + | _____^ +11 | | x for x in range(3) +12 | | ) + | |_^ C400 +13 | +14 | # Not built-in list. + | + = help: Rewrite using `list()` + +ℹ Unsafe fix +7 7 | +8 8 | # Short-circuit case, combine with C416 and should produce x = list(range(3)) +9 9 | x = list(x for x in range(3)) +10 |-x = list( +11 |- x for x in range(3) +12 |-) + 10 |+x = list(range(3)) +13 11 | +14 12 | # Not built-in list. +15 13 | def list(*args, **kwargs): From 740c08b033d835f071e4887cea2f608d6cc662c6 Mon Sep 17 00:00:00 2001 From: Steve C Date: Fri, 15 Mar 2024 10:43:55 -0400 Subject: [PATCH 5/7] [`pylint`] - implement `redeclared-assigned-name` (`W0128`) (#9268) ## Summary Implements [`W0128`/`redeclared-assigned-name`](https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/redeclared-assigned-name.html) See: #970 ## Test Plan `cargo test` --- .../pylint/redeclared_assigned_name.py | 6 ++ .../src/checkers/ast/analyze/statement.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/pylint/mod.rs | 1 + .../ruff_linter/src/rules/pylint/rules/mod.rs | 2 + .../pylint/rules/redeclared_assigned_name.rs | 76 +++++++++++++++++++ ...__PLW0128_redeclared_assigned_name.py.snap | 59 ++++++++++++++ ruff.schema.json | 1 + 8 files changed, 149 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/pylint/redeclared_assigned_name.py create mode 100644 crates/ruff_linter/src/rules/pylint/rules/redeclared_assigned_name.rs create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0128_redeclared_assigned_name.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/redeclared_assigned_name.py b/crates/ruff_linter/resources/test/fixtures/pylint/redeclared_assigned_name.py new file mode 100644 index 00000000000000..50b3a27925c362 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/redeclared_assigned_name.py @@ -0,0 +1,6 @@ +FIRST, FIRST = (1, 2) # PLW0128 +FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 +FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + +FIRST, SECOND, _, _, _ignored = (1, 2, 3, 4, 5) # OK diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 191456bf0766b3..d4f6287bf95e90 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1389,6 +1389,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } } Stmt::Assign(assign @ ast::StmtAssign { targets, value, .. }) => { + if checker.enabled(Rule::RedeclaredAssignedName) { + pylint::rules::redeclared_assigned_name(checker, targets); + } if checker.enabled(Rule::LambdaAssignment) { if let [target] = &targets[..] { pycodestyle::rules::lambda_assignment(checker, target, value, None, stmt); diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index c471c890a0f1a7..e6900d54d24c57 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -294,6 +294,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "W0108") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryLambda), (Pylint, "W0120") => (RuleGroup::Stable, rules::pylint::rules::UselessElseOnLoop), (Pylint, "W0127") => (RuleGroup::Stable, rules::pylint::rules::SelfAssigningVariable), + (Pylint, "W0128") => (RuleGroup::Preview, rules::pylint::rules::RedeclaredAssignedName), (Pylint, "W0129") => (RuleGroup::Stable, rules::pylint::rules::AssertOnStringLiteral), (Pylint, "W0131") => (RuleGroup::Stable, rules::pylint::rules::NamedExprWithoutContext), (Pylint, "W0133") => (RuleGroup::Preview, rules::pylint::rules::UselessExceptionStatement), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 0cbd284ddbc8f0..ebd20c35ff90c4 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -95,6 +95,7 @@ mod tests { #[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.py"))] #[test_case(Rule::NonSlotAssignment, Path::new("non_slot_assignment.py"))] #[test_case(Rule::PropertyWithParameters, Path::new("property_with_parameters.py"))] + #[test_case(Rule::RedeclaredAssignedName, Path::new("redeclared_assigned_name.py"))] #[test_case( Rule::RedefinedArgumentFromLocal, Path::new("redefined_argument_from_local.py") diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 8f6d40554d2ad8..0f7c0e9e983ea1 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -47,6 +47,7 @@ pub(crate) use non_slot_assignment::*; pub(crate) use nonlocal_without_binding::*; pub(crate) use potential_index_error::*; pub(crate) use property_with_parameters::*; +pub(crate) use redeclared_assigned_name::*; pub(crate) use redefined_argument_from_local::*; pub(crate) use redefined_loop_name::*; pub(crate) use repeated_equality_comparison::*; @@ -136,6 +137,7 @@ mod non_slot_assignment; mod nonlocal_without_binding; mod potential_index_error; mod property_with_parameters; +mod redeclared_assigned_name; mod redefined_argument_from_local; mod redefined_loop_name; mod repeated_equality_comparison; diff --git a/crates/ruff_linter/src/rules/pylint/rules/redeclared_assigned_name.rs b/crates/ruff_linter/src/rules/pylint/rules/redeclared_assigned_name.rs new file mode 100644 index 00000000000000..cb0dbc5175f2b9 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/redeclared_assigned_name.rs @@ -0,0 +1,76 @@ +use ruff_python_ast::{self as ast, Expr}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for declared assignments to the same variable multiple times +/// in the same assignment. +/// +/// ## Why is this bad? +/// Assigning a variable multiple times in the same assignment is redundant, +/// as the final assignment to the variable is what the value will be. +/// +/// ## Example +/// ```python +/// a, b, a = (1, 2, 3) +/// print(a) # 3 +/// ``` +/// +/// Use instead: +/// ```python +/// # this is assuming you want to assign 3 to `a` +/// _, b, a = (1, 2, 3) +/// print(a) # 3 +/// ``` +/// +#[violation] +pub struct RedeclaredAssignedName { + name: String, +} + +impl Violation for RedeclaredAssignedName { + #[derive_message_formats] + fn message(&self) -> String { + let RedeclaredAssignedName { name } = self; + format!("Redeclared variable `{name}` in assignment") + } +} + +/// PLW0128 +pub(crate) fn redeclared_assigned_name(checker: &mut Checker, targets: &Vec) { + let mut names: Vec = Vec::new(); + + for target in targets { + check_expr(checker, target, &mut names); + } +} + +fn check_expr(checker: &mut Checker, expr: &Expr, names: &mut Vec) { + match expr { + Expr::Tuple(ast::ExprTuple { elts, .. }) => { + for target in elts { + check_expr(checker, target, names); + } + } + Expr::Name(ast::ExprName { id, .. }) => { + if checker.settings.dummy_variable_rgx.is_match(id) { + // Ignore dummy variable assignments + return; + } + if names.contains(id) { + checker.diagnostics.push(Diagnostic::new( + RedeclaredAssignedName { + name: id.to_string(), + }, + expr.range(), + )); + } + names.push(id.to_string()); + } + _ => {} + } +} diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0128_redeclared_assigned_name.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0128_redeclared_assigned_name.py.snap new file mode 100644 index 00000000000000..eda5b8ea9f3531 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0128_redeclared_assigned_name.py.snap @@ -0,0 +1,59 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +redeclared_assigned_name.py:1:8: PLW0128 Redeclared variable `FIRST` in assignment + | +1 | FIRST, FIRST = (1, 2) # PLW0128 + | ^^^^^ PLW0128 +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 + | + +redeclared_assigned_name.py:2:9: PLW0128 Redeclared variable `FIRST` in assignment + | +1 | FIRST, FIRST = (1, 2) # PLW0128 +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 + | ^^^^^ PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 +4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + | + +redeclared_assigned_name.py:3:9: PLW0128 Redeclared variable `FIRST` in assignment + | +1 | FIRST, FIRST = (1, 2) # PLW0128 +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 + | ^^^^^ PLW0128 +4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + | + +redeclared_assigned_name.py:3:32: PLW0128 Redeclared variable `FIRST` in assignment + | +1 | FIRST, FIRST = (1, 2) # PLW0128 +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 + | ^^^^^ PLW0128 +4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + | + +redeclared_assigned_name.py:4:23: PLW0128 Redeclared variable `FIRST` in assignment + | +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 +4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + | ^^^^^ PLW0128 +5 | +6 | FIRST, SECOND, _, _, _ignored = (1, 2, 3, 4, 5) # OK + | + +redeclared_assigned_name.py:4:30: PLW0128 Redeclared variable `SECOND` in assignment + | +2 | FIRST, (FIRST, SECOND) = (1, (1, 2)) # PLW0128 +3 | FIRST, (FIRST, SECOND, (THIRD, FIRST)) = (1, (1, 2)) # PLW0128 +4 | FIRST, SECOND, THIRD, FIRST, SECOND = (1, 2, 3, 4) # PLW0128 + | ^^^^^^ PLW0128 +5 | +6 | FIRST, SECOND, _, _, _ignored = (1, 2, 3, 4, 5) # OK + | + + diff --git a/ruff.schema.json b/ruff.schema.json index 7b65b8f158596f..9f06af7bc68b03 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3330,6 +3330,7 @@ "PLW012", "PLW0120", "PLW0127", + "PLW0128", "PLW0129", "PLW013", "PLW0131", From 608df9a1bc0e6025049add877d1d833f1739e966 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Fri, 15 Mar 2024 12:51:49 -0500 Subject: [PATCH 6/7] Bump version to 0.3.3 (#10425) Co-authored-by: Alex Waygood --- CHANGELOG.md | 46 ++++++++++++++++++++++++++++++- Cargo.lock | 6 ++-- README.md | 2 +- crates/ruff/Cargo.toml | 2 +- crates/ruff_linter/Cargo.toml | 2 +- crates/ruff_shrinking/Cargo.toml | 2 +- docs/integrations.md | 6 ++-- pyproject.toml | 2 +- scripts/benchmarks/pyproject.toml | 2 +- 9 files changed, 57 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a621b0c6fa1e3..00252923232189 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,49 @@ # Changelog +## 0.3.3 + +### Preview features + +- \[`flake8-bandit`\]: Implement `S610` rule ([#10316](https://github.com/astral-sh/ruff/pull/10316)) +- \[`pycodestyle`\] Implement `blank-line-at-end-of-file` (`W391`) ([#10243](https://github.com/astral-sh/ruff/pull/10243)) +- \[`pycodestyle`\] Implement `redundant-backslash` (`E502`) ([#10292](https://github.com/astral-sh/ruff/pull/10292)) +- \[`pylint`\] - implement `redeclared-assigned-name` (`W0128`) ([#9268](https://github.com/astral-sh/ruff/pull/9268)) + +### Rule changes + +- \[`flake8_comprehensions`\] Handled special case for `C400` which also matches `C416` ([#10419](https://github.com/astral-sh/ruff/pull/10419)) +- \[`flake8-bandit`\] Implement upstream updates for `S311`, `S324` and `S605` ([#10313](https://github.com/astral-sh/ruff/pull/10313)) +- \[`pyflakes`\] Remove `F401` fix for `__init__` imports by default and allow opt-in to unsafe fix ([#10365](https://github.com/astral-sh/ruff/pull/10365)) +- \[`pylint`\] Implement `invalid-bool-return-type` (`E304`) ([#10377](https://github.com/astral-sh/ruff/pull/10377)) +- \[`pylint`\] Include builtin warnings in useless-exception-statement (`PLW0133`) ([#10394](https://github.com/astral-sh/ruff/pull/10394)) + +### CLI + +- Add message on success to `ruff check` ([#8631](https://github.com/astral-sh/ruff/pull/8631)) + +### Bug fixes + +- \[`PIE970`\] Allow trailing ellipsis in `typing.TYPE_CHECKING` ([#10413](https://github.com/astral-sh/ruff/pull/10413)) +- Avoid `TRIO115` if the argument is a variable ([#10376](https://github.com/astral-sh/ruff/pull/10376)) +- \[`F811`\] Avoid removing shadowed imports that point to different symbols ([#10387](https://github.com/astral-sh/ruff/pull/10387)) +- Fix `F821` and `F822` false positives in `.pyi` files ([#10341](https://github.com/astral-sh/ruff/pull/10341)) +- Fix `F821` false negatives in `.py` files when `from __future__ import annotations` is active ([#10362](https://github.com/astral-sh/ruff/pull/10362)) +- Fix case where `Indexer` fails to identify continuation preceded by newline #10351 ([#10354](https://github.com/astral-sh/ruff/pull/10354)) +- Sort hash maps in `Settings` display ([#10370](https://github.com/astral-sh/ruff/pull/10370)) +- Track conditional deletions in the semantic model ([#10415](https://github.com/astral-sh/ruff/pull/10415)) +- \[`C413`\] Wrap expressions in parentheses when negating ([#10346](https://github.com/astral-sh/ruff/pull/10346)) +- \[`pycodestyle`\] Do not ignore lines before the first logical line in blank lines rules. ([#10382](https://github.com/astral-sh/ruff/pull/10382)) +- \[`pycodestyle`\] Do not trigger `E225` and `E275` when the next token is a ')' ([#10315](https://github.com/astral-sh/ruff/pull/10315)) +- \[`pylint`\] Avoid false-positive slot non-assignment for `__dict__` (`PLE0237`) ([#10348](https://github.com/astral-sh/ruff/pull/10348)) +- Gate f-string struct size test for Rustc \< 1.76 ([#10371](https://github.com/astral-sh/ruff/pull/10371)) + +### Documentation + +- Use `ruff.toml` format in README ([#10393](https://github.com/astral-sh/ruff/pull/10393)) +- \[`RUF008`\] Make it clearer that a mutable default in a dataclass is only valid if it is typed as a ClassVar ([#10395](https://github.com/astral-sh/ruff/pull/10395)) +- \[`pylint`\] Extend docs and test in `invalid-str-return-type` (`E307`) ([#10400](https://github.com/astral-sh/ruff/pull/10400)) +- Remove `.` from `check` and `format` commands ([#10217](https://github.com/astral-sh/ruff/pull/10217)) + ## 0.3.2 ### Preview features @@ -1199,7 +1243,7 @@ Read Ruff's new [versioning policy](https://docs.astral.sh/ruff/versioning/). - \[`refurb`\] Add `single-item-membership-test` (`FURB171`) ([#7815](https://github.com/astral-sh/ruff/pull/7815)) - \[`pylint`\] Add `and-or-ternary` (`R1706`) ([#7811](https://github.com/astral-sh/ruff/pull/7811)) -_New rules are added in [preview](https://docs.astral.sh/ruff/preview/)._ +*New rules are added in [preview](https://docs.astral.sh/ruff/preview/).* ### Configuration diff --git a/Cargo.lock b/Cargo.lock index b8345646d05ace..007fc032dcd86f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2003,7 +2003,7 @@ dependencies = [ [[package]] name = "ruff" -version = "0.3.2" +version = "0.3.3" dependencies = [ "anyhow", "argfile", @@ -2167,7 +2167,7 @@ dependencies = [ [[package]] name = "ruff_linter" -version = "0.3.2" +version = "0.3.3" dependencies = [ "aho-corasick", "annotate-snippets 0.9.2", @@ -2448,7 +2448,7 @@ dependencies = [ [[package]] name = "ruff_shrinking" -version = "0.3.2" +version = "0.3.3" dependencies = [ "anyhow", "clap", diff --git a/README.md b/README.md index fbf0cdb3372878..0ce12e24728636 100644 --- a/README.md +++ b/README.md @@ -151,7 +151,7 @@ Ruff can also be used as a [pre-commit](https://pre-commit.com/) hook via [`ruff ```yaml - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.3.2 + rev: v0.3.3 hooks: # Run the linter. - id: ruff diff --git a/crates/ruff/Cargo.toml b/crates/ruff/Cargo.toml index 7e83ca7df16090..4e9880808d01f3 100644 --- a/crates/ruff/Cargo.toml +++ b/crates/ruff/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ruff" -version = "0.3.2" +version = "0.3.3" publish = false authors = { workspace = true } edition = { workspace = true } diff --git a/crates/ruff_linter/Cargo.toml b/crates/ruff_linter/Cargo.toml index 1c3ad680913343..bf0e07c41c2a96 100644 --- a/crates/ruff_linter/Cargo.toml +++ b/crates/ruff_linter/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ruff_linter" -version = "0.3.2" +version = "0.3.3" publish = false authors = { workspace = true } edition = { workspace = true } diff --git a/crates/ruff_shrinking/Cargo.toml b/crates/ruff_shrinking/Cargo.toml index 24b3223a178660..07eec8f76ceb8b 100644 --- a/crates/ruff_shrinking/Cargo.toml +++ b/crates/ruff_shrinking/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ruff_shrinking" -version = "0.3.2" +version = "0.3.3" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/docs/integrations.md b/docs/integrations.md index 45f393763e0c83..b39b9fd9b667b8 100644 --- a/docs/integrations.md +++ b/docs/integrations.md @@ -14,7 +14,7 @@ Ruff can be used as a [pre-commit](https://pre-commit.com) hook via [`ruff-pre-c ```yaml - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.3.2 + rev: v0.3.3 hooks: # Run the linter. - id: ruff @@ -27,7 +27,7 @@ To enable lint fixes, add the `--fix` argument to the lint hook: ```yaml - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.3.2 + rev: v0.3.3 hooks: # Run the linter. - id: ruff @@ -41,7 +41,7 @@ To run the hooks over Jupyter Notebooks too, add `jupyter` to the list of allowe ```yaml - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.3.2 + rev: v0.3.3 hooks: # Run the linter. - id: ruff diff --git a/pyproject.toml b/pyproject.toml index 18744d4a0b334e..806990f5413832 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "maturin" [project] name = "ruff" -version = "0.3.2" +version = "0.3.3" description = "An extremely fast Python linter and code formatter, written in Rust." authors = [{ name = "Astral Software Inc.", email = "hey@astral.sh" }] readme = "README.md" diff --git a/scripts/benchmarks/pyproject.toml b/scripts/benchmarks/pyproject.toml index 1ec5344b536bca..e1d1d0a24e963d 100644 --- a/scripts/benchmarks/pyproject.toml +++ b/scripts/benchmarks/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "scripts" -version = "0.3.2" +version = "0.3.3" description = "" authors = ["Charles Marsh "] From 861998612300520f2c18dbc7b8a6226300ceb508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ho=C3=ABl=20Bagard?= <34478245+hoel-bagard@users.noreply.github.com> Date: Sun, 17 Mar 2024 23:18:29 +0900 Subject: [PATCH 7/7] Add `repos/` to the gitignore (#10435) ## Summary I would like to add `repos/` to the gitignore since it is given as an example for the cache directory path in [the ecosystem check's README](https://github.com/astral-sh/ruff/tree/main/python/ruff-ecosystem#development): ```console ruff-ecosystem check ruff "./target/debug/ruff" --cache ./repos ``` --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index a7912ac7c94615..4302ff30a762a4 100644 --- a/.gitignore +++ b/.gitignore @@ -92,6 +92,7 @@ coverage.xml .hypothesis/ .pytest_cache/ cover/ +repos/ # Translations *.mo