From da4391d0780985369a8b0e2b57f3633d1876451e Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Wed, 26 Jul 2023 12:05:35 -0300 Subject: [PATCH 1/5] Implement `PYI046` ## What it does Checks for the presence of unused private `typing.Protocol` definitions. ## Why is this bad? A private `typing.Protocol` that is defined but not used is likely a mistake, and should be removed to avoid confusion. ## Example ```python import typing class _PrivateProtocol(typing.Protocol): foo: int ``` Use instead: ```python import typing class _PrivateProtocol(typing.Protocol): foo: int def func(arg: _PrivateProtocol) -> None: ... ``` --- .../test/fixtures/flake8_pyi/PYI046.py | 18 ++++++ .../test/fixtures/flake8_pyi/PYI046.pyi | 18 ++++++ .../ruff/src/checkers/ast/analyze/bindings.rs | 8 +++ crates/ruff/src/checkers/ast/mod.rs | 15 ++++- crates/ruff/src/codes.rs | 1 + crates/ruff/src/rules/flake8_pyi/mod.rs | 2 + crates/ruff/src/rules/flake8_pyi/rules/mod.rs | 2 + .../rules/unused_private_type_declaration.rs | 62 +++++++++++++++++++ ...__flake8_pyi__tests__PYI046_PYI046.py.snap | 4 ++ ..._flake8_pyi__tests__PYI046_PYI046.pyi.snap | 18 ++++++ crates/ruff_python_semantic/src/binding.rs | 16 +++++ ruff.schema.json | 1 + 12 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI046.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI046.pyi create mode 100644 crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_declaration.rs create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.py.snap create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.pyi.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI046.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI046.py new file mode 100644 index 0000000000000..42fb031ebdb0d --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI046.py @@ -0,0 +1,18 @@ +import typing +from typing import Protocol + + +class _Foo(Protocol): + bar: int + + +class _Bar(typing.Protocol): + bar: int + + +# OK +class _UsedPrivateProtocol(Protocol): + bar: int + + +def uses__UsedPrivateProtocol(arg: _UsedPrivateProtocol) -> None: ... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI046.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI046.pyi new file mode 100644 index 0000000000000..fb9292ed857fa --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI046.pyi @@ -0,0 +1,18 @@ +import typing +from typing import Protocol + + +class _Foo(object, Protocol): + bar: int + + +class _Bar(typing.Protocol): + bar: int + + +# OK +class _UsedPrivateProtocol(Protocol): + bar: int + + +def uses__UsedPrivateProtocol(arg: _UsedPrivateProtocol) -> None: ... diff --git a/crates/ruff/src/checkers/ast/analyze/bindings.rs b/crates/ruff/src/checkers/ast/analyze/bindings.rs index 0d6d096114111..fc62d28f883fb 100644 --- a/crates/ruff/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff/src/checkers/ast/analyze/bindings.rs @@ -11,6 +11,7 @@ pub(crate) fn bindings(checker: &mut Checker) { Rule::UnaliasedCollectionsAbcSetImport, Rule::UnconventionalImportAlias, Rule::UnusedVariable, + Rule::UnusedPrivateProtocol, ]) { return; } @@ -63,6 +64,13 @@ pub(crate) fn bindings(checker: &mut Checker) { checker.diagnostics.push(diagnostic); } } + if checker.enabled(Rule::UnusedPrivateProtocol) { + if let Some(diagnostic) = + flake8_pyi::rules::unused_private_protocol(checker, binding) + { + checker.diagnostics.push(diagnostic); + } + } } } } diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index e02b026833f2f..d9eb331757703 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -722,16 +722,27 @@ where BindingFlags::empty(), ); } - Stmt::ClassDef(ast::StmtClassDef { name, .. }) => { + Stmt::ClassDef(ast::StmtClassDef { name, bases, .. }) => { + let mut flags = BindingFlags::empty(); let scope_id = self.semantic.scope_id; self.deferred.scopes.push(scope_id); self.semantic.pop_scope(); self.semantic.pop_definition(); + + if name.starts_with('_') { + if bases + .iter() + .any(|base| self.semantic.match_typing_expr(base, "Protocol")) + { + flags |= BindingFlags::PRIVATE_TYPE_PROTOCOL; + } + } + self.add_binding( name, stmt.identifier(), BindingKind::ClassDefinition(scope_id), - BindingFlags::empty(), + flags, ); } _ => {} diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index c08c86733a1dd..ca93ef27904de 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -649,6 +649,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "043") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::TSuffixedTypeAlias), (Flake8Pyi, "044") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::FutureAnnotationsInStub), (Flake8Pyi, "045") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::IterMethodReturnIterable), + (Flake8Pyi, "046") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnusedPrivateProtocol), (Flake8Pyi, "048") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::StubBodyMultipleStatements), (Flake8Pyi, "050") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NoReturnArgumentAnnotationInStub), (Flake8Pyi, "052") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnannotatedAssignmentInStub), diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index c43a023ae0591..7f7c5ed5c07b2 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -93,6 +93,8 @@ mod tests { #[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.pyi"))] #[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.py"))] #[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.pyi"))] + #[test_case(Rule::UnusedPrivateProtocol, Path::new("PYI046.py"))] + #[test_case(Rule::UnusedPrivateProtocol, Path::new("PYI046.pyi"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 4b8d229df9ecd..2fced18d7238f 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -29,6 +29,7 @@ pub(crate) use unnecessary_literal_union::*; pub(crate) use unrecognized_platform::*; pub(crate) use unrecognized_version_info::*; pub(crate) use unsupported_method_call_on_all::*; +pub(crate) use unused_private_type_declaration::*; mod any_eq_ne_annotation; mod bad_version_info_comparison; @@ -61,3 +62,4 @@ mod unnecessary_literal_union; mod unrecognized_platform; mod unrecognized_version_info; mod unsupported_method_call_on_all; +mod unused_private_type_declaration; diff --git a/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_declaration.rs b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_declaration.rs new file mode 100644 index 0000000000000..77323c82c2738 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_declaration.rs @@ -0,0 +1,62 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_semantic::Binding; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for the presence of unused private `typing.Protocol` definitions. +/// +/// ## Why is this bad? +/// A private `typing.Protocol` that is defined but not used is likely a mistake, and should +/// be removed to avoid confusion. +/// +/// ## Example +/// ```python +/// import typing +/// +/// class _PrivateProtocol(typing.Protocol): +/// foo: int +/// ``` +/// +/// Use instead: +/// ```python +/// import typing +/// +/// class _PrivateProtocol(typing.Protocol): +/// foo: int +/// +/// def func(arg: _PrivateProtocol) -> None: ... +/// ``` +#[violation] +pub struct UnusedPrivateProtocol { + name: String, +} + +impl Violation for UnusedPrivateProtocol { + #[derive_message_formats] + fn message(&self) -> String { + let UnusedPrivateProtocol { name } = self; + format!("Protocol `{name}` is never used") + } +} + +/// PYI046 +pub(crate) fn unused_private_protocol(checker: &Checker, binding: &Binding) -> Option { + if !binding.kind.is_class_definition() { + return None; + } + if !binding.is_private_protocol() { + return None; + } + if binding.is_used() { + return None; + } + + Some(Diagnostic::new( + UnusedPrivateProtocol { + name: binding.name(checker.locator()).to_string(), + }, + binding.range, + )) +} diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.py.snap new file mode 100644 index 0000000000000..d1aa2e9116558 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.pyi.snap new file mode 100644 index 0000000000000..54dafc64b32f6 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.pyi.snap @@ -0,0 +1,18 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI046.pyi:5:7: PYI046 Protocol `_Foo` is never used + | +5 | class _Foo(object, Protocol): + | ^^^^ PYI046 +6 | bar: int + | + +PYI046.pyi:9:7: PYI046 Protocol `_Bar` is never used + | + 9 | class _Bar(typing.Protocol): + | ^^^^ PYI046 +10 | bar: int + | + + diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index eb93d6ffbd235..9dc33399be17f 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -94,6 +94,11 @@ impl<'a> Binding<'a> { ) } + /// Return `true` if this [`Binding`] represents a `typing.Protocol` definition. + pub const fn is_private_protocol(&self) -> bool { + self.flags.intersects(BindingFlags::PRIVATE_TYPE_PROTOCOL) + } + /// Return `true` if this binding redefines the given binding. pub fn redefines(&self, existing: &'a Binding) -> bool { match &self.kind { @@ -264,6 +269,17 @@ bitflags! { /// __all__ = [1] /// ``` const INVALID_ALL_OBJECT = 1 << 6; + + /// The binding represents a private `typing.Protocol`. + /// + /// For example, the binding could be `_PrivateProtocol` in: + /// ```python + /// import typing + /// + /// class _PrivateProtocol(typing.Protocol): + /// foo: int + /// ``` + const PRIVATE_TYPE_PROTOCOL = 1 << 7; } } diff --git a/ruff.schema.json b/ruff.schema.json index f1c40fa7c085f..34d32cbbe819d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2387,6 +2387,7 @@ "PYI043", "PYI044", "PYI045", + "PYI046", "PYI048", "PYI05", "PYI050", From df5e81942c97841cbb81a0228d0cadf16e1ad416 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Wed, 26 Jul 2023 18:46:16 -0300 Subject: [PATCH 2/5] review update --- crates/ruff/src/rules/flake8_pyi/rules/mod.rs | 4 ++-- ...laration.rs => unused_private_type_definition.rs} | 12 ++++++++---- ..._rules__flake8_pyi__tests__PYI046_PYI046.pyi.snap | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) rename crates/ruff/src/rules/flake8_pyi/rules/{unused_private_type_declaration.rs => unused_private_type_definition.rs} (88%) diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 2fced18d7238f..6ee3f89e8a74e 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -29,7 +29,7 @@ pub(crate) use unnecessary_literal_union::*; pub(crate) use unrecognized_platform::*; pub(crate) use unrecognized_version_info::*; pub(crate) use unsupported_method_call_on_all::*; -pub(crate) use unused_private_type_declaration::*; +pub(crate) use unused_private_type_definition::*; mod any_eq_ne_annotation; mod bad_version_info_comparison; @@ -62,4 +62,4 @@ mod unnecessary_literal_union; mod unrecognized_platform; mod unrecognized_version_info; mod unsupported_method_call_on_all; -mod unused_private_type_declaration; +mod unused_private_type_definition; diff --git a/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_declaration.rs b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs similarity index 88% rename from crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_declaration.rs rename to crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs index 77323c82c2738..605a059f9cc07 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_declaration.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs @@ -8,13 +8,14 @@ use crate::checkers::ast::Checker; /// Checks for the presence of unused private `typing.Protocol` definitions. /// /// ## Why is this bad? -/// A private `typing.Protocol` that is defined but not used is likely a mistake, and should -/// be removed to avoid confusion. +/// A private `typing.Protocol` that is defined but not used is likely a mistake, consider +/// making it public. /// /// ## Example /// ```python /// import typing /// +/// /// class _PrivateProtocol(typing.Protocol): /// foo: int /// ``` @@ -23,10 +24,13 @@ use crate::checkers::ast::Checker; /// ```python /// import typing /// +/// /// class _PrivateProtocol(typing.Protocol): /// foo: int /// -/// def func(arg: _PrivateProtocol) -> None: ... +/// +/// def func(arg: _PrivateProtocol) -> None: +/// ... /// ``` #[violation] pub struct UnusedPrivateProtocol { @@ -37,7 +41,7 @@ impl Violation for UnusedPrivateProtocol { #[derive_message_formats] fn message(&self) -> String { let UnusedPrivateProtocol { name } = self; - format!("Protocol `{name}` is never used") + format!("Private protocol `{name}` is never used") } } diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.pyi.snap index 54dafc64b32f6..e21a217bdfb80 100644 --- a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.pyi.snap +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI046_PYI046.pyi.snap @@ -1,14 +1,14 @@ --- source: crates/ruff/src/rules/flake8_pyi/mod.rs --- -PYI046.pyi:5:7: PYI046 Protocol `_Foo` is never used +PYI046.pyi:5:7: PYI046 Private protocol `_Foo` is never used | 5 | class _Foo(object, Protocol): | ^^^^ PYI046 6 | bar: int | -PYI046.pyi:9:7: PYI046 Protocol `_Bar` is never used +PYI046.pyi:9:7: PYI046 Private protocol `_Bar` is never used | 9 | class _Bar(typing.Protocol): | ^^^^ PYI046 From 392995aeaf2310048e9696f775df50c8ae44cbb1 Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Wed, 26 Jul 2023 22:36:57 -0300 Subject: [PATCH 3/5] review update --- .../ruff/src/checkers/ast/analyze/bindings.rs | 2 +- crates/ruff/src/checkers/ast/mod.rs | 15 ++-------- .../rules/unused_private_type_definition.rs | 28 ++++++++++++------- crates/ruff_python_semantic/src/binding.rs | 4 +-- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/crates/ruff/src/checkers/ast/analyze/bindings.rs b/crates/ruff/src/checkers/ast/analyze/bindings.rs index 6e8e3d19a5b52..1c8804272ebef 100644 --- a/crates/ruff/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff/src/checkers/ast/analyze/bindings.rs @@ -72,7 +72,7 @@ pub(crate) fn bindings(checker: &mut Checker) { checker.diagnostics.push(diagnostic); } } - if checker.enabled(Rule::UnusedPrivateProtocol) { + if checker.enabled(Rule::UnusedPrivateProtocol) { if let Some(diagnostic) = flake8_pyi::rules::unused_private_protocol(checker, binding) { diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 7bc12dad419dd..06b0b3d47ca9a 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -724,27 +724,16 @@ where BindingFlags::empty(), ); } - Stmt::ClassDef(ast::StmtClassDef { name, bases, .. }) => { - let mut flags = BindingFlags::empty(); + Stmt::ClassDef(ast::StmtClassDef { name, .. }) => { let scope_id = self.semantic.scope_id; self.deferred.scopes.push(scope_id); self.semantic.pop_scope(); self.semantic.pop_definition(); - - if name.starts_with('_') { - if bases - .iter() - .any(|base| self.semantic.match_typing_expr(base, "Protocol")) - { - flags |= BindingFlags::PRIVATE_TYPE_PROTOCOL; - } - } - self.add_binding( name, stmt.identifier(), BindingKind::ClassDefinition(scope_id), - flags, + BindingFlags::empty(), ); } _ => {} diff --git a/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs index 68f74ca69fc9f..36321445b93b7 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs @@ -74,7 +74,7 @@ impl Violation for UnusedPrivateProtocol { /// PYI018 pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> Option { - if !(binding.kind.is_assignment() && binding.is_private_variable()) { + if !(binding.kind.is_assignment() && binding.is_private_declaration()) { return None; } if binding.is_used() { @@ -99,29 +99,37 @@ pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> O } Some(Diagnostic::new( - UnusedPrivateTypeVar { - name: id.to_string(), - }, + UnusedPrivateTypeVar { name: id.to_string() }, binding.range, )) } /// PYI046 pub(crate) fn unused_private_protocol(checker: &Checker, binding: &Binding) -> Option { - if !binding.kind.is_class_definition() { + if !(binding.kind.is_class_definition() && binding.is_private_declaration()) { return None; } - if !binding.is_private_protocol() { + if binding.is_used() { return None; } - if binding.is_used() { + + let Some(source) = binding.source else { + return None; + }; + let Stmt::ClassDef(ast::StmtClassDef {name, bases, ..}) = checker.semantic().stmts[source] + else { + return None; + }; + + if !bases + .iter() + .any(|base| checker.semantic().match_typing_expr(base, "Protocol")) + { return None; } Some(Diagnostic::new( - UnusedPrivateProtocol { - name: binding.name(checker.locator()).to_string(), - }, + UnusedPrivateProtocol { name: name.to_string() }, binding.range, )) } diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 783dd9ca367fd..4b38a927d1f41 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -94,9 +94,9 @@ impl<'a> Binding<'a> { ) } - /// Return `true` if this [`Binding`] represents an private variable + /// Return `true` if this [`Binding`] represents an private declaration /// (e.g., `_x` in `_x = "private variable"`) - pub const fn is_private_variable(&self) -> bool { + pub const fn is_private_declaration(&self) -> bool { self.flags.contains(BindingFlags::PRIVATE_DECLARATION) } From 48dcb2a47e511043fbd4124810fc77ba9902f95b Mon Sep 17 00:00:00 2001 From: LaBatata101 Date: Wed, 26 Jul 2023 23:16:02 -0300 Subject: [PATCH 4/5] fix formatting --- .../flake8_pyi/rules/unused_private_type_definition.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs index 36321445b93b7..ad299dade73d8 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs @@ -99,7 +99,9 @@ pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> O } Some(Diagnostic::new( - UnusedPrivateTypeVar { name: id.to_string() }, + UnusedPrivateTypeVar { + name: id.to_string(), + }, binding.range, )) } @@ -129,7 +131,9 @@ pub(crate) fn unused_private_protocol(checker: &Checker, binding: &Binding) -> O } Some(Diagnostic::new( - UnusedPrivateProtocol { name: name.to_string() }, + UnusedPrivateProtocol { + name: name.to_string(), + }, binding.range, )) } From f3ceb34c20c790abe7654ed64ad166242b8056e4 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 26 Jul 2023 22:28:25 -0400 Subject: [PATCH 5/5] Tweak docs --- .../flake8_pyi/rules/unused_private_type_definition.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs index ad299dade73d8..89eafeb545a13 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs @@ -10,7 +10,7 @@ use crate::checkers::ast::Checker; /// /// ## Why is this bad? /// A private `TypeVar` that is defined but not used is likely a mistake, and -/// should be removed to avoid confusion. +/// should either be used, made public, or removed to avoid confusion. /// /// ## Example /// ```python @@ -35,8 +35,9 @@ impl Violation for UnusedPrivateTypeVar { /// Checks for the presence of unused private `typing.Protocol` definitions. /// /// ## Why is this bad? -/// A private `typing.Protocol` that is defined but not used is likely a mistake, consider -/// making it public. +/// A private `typing.Protocol` that is defined but not used is likely a +/// mistake, and should either be used, made public, or removed to avoid +/// confusion. /// /// ## Example /// ```python @@ -118,7 +119,7 @@ pub(crate) fn unused_private_protocol(checker: &Checker, binding: &Binding) -> O let Some(source) = binding.source else { return None; }; - let Stmt::ClassDef(ast::StmtClassDef {name, bases, ..}) = checker.semantic().stmts[source] + let Stmt::ClassDef(ast::StmtClassDef { name, bases, .. }) = checker.semantic().stmts[source] else { return None; };