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 0000000000..42fb031ebd --- /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 0000000000..fb9292ed85 --- /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 306d954788..1c8804272e 100644 --- a/crates/ruff/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff/src/checkers/ast/analyze/bindings.rs @@ -12,6 +12,7 @@ pub(crate) fn bindings(checker: &mut Checker) { Rule::UnconventionalImportAlias, Rule::UnusedPrivateTypeVar, Rule::UnusedVariable, + Rule::UnusedPrivateProtocol, ]) { return; } @@ -71,6 +72,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/codes.rs b/crates/ruff/src/codes.rs index a1df10006c..49c6710545 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -650,6 +650,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 b39ae31186..2cbb90c3e0 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -95,6 +95,8 @@ mod tests { #[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.pyi"))] #[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.py"))] #[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.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/unused_private_type_definition.rs b/crates/ruff/src/rules/flake8_pyi/rules/unused_private_type_definition.rs index 5f0fff3923..89eafeb545 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 @@ -31,9 +31,51 @@ impl Violation for UnusedPrivateTypeVar { } } +/// ## 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 either be used, made public, or 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!("Private protocol `{name}` is never used") + } +} + /// 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() { @@ -64,3 +106,35 @@ pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> O binding.range, )) } + +/// PYI046 +pub(crate) fn unused_private_protocol(checker: &Checker, binding: &Binding) -> Option { + if !(binding.kind.is_class_definition() && binding.is_private_declaration()) { + return None; + } + if binding.is_used() { + return None; + } + + 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: name.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 0000000000..d1aa2e9116 --- /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 0000000000..e21a217bdf --- /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 Private protocol `_Foo` is never used + | +5 | class _Foo(object, Protocol): + | ^^^^ PYI046 +6 | bar: int + | + +PYI046.pyi:9:7: PYI046 Private 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 783dd9ca36..4b38a927d1 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) } diff --git a/ruff.schema.json b/ruff.schema.json index 50c1d7367e..3a24a19a4a 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2389,6 +2389,7 @@ "PYI043", "PYI044", "PYI045", + "PYI046", "PYI048", "PYI05", "PYI050",