mirror of https://github.com/astral-sh/ruff
[`flake8-pyi`] Implement `PYI046` (#6098)
## Summary Checks for the presence of unused private `typing.Protocol` definitions. ref #848 ## Test Plan Snapshots and manual runs of flake8.
This commit is contained in:
parent
d04367a042
commit
86539c1fc5
|
|
@ -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: ...
|
||||||
|
|
@ -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: ...
|
||||||
|
|
@ -12,6 +12,7 @@ pub(crate) fn bindings(checker: &mut Checker) {
|
||||||
Rule::UnconventionalImportAlias,
|
Rule::UnconventionalImportAlias,
|
||||||
Rule::UnusedPrivateTypeVar,
|
Rule::UnusedPrivateTypeVar,
|
||||||
Rule::UnusedVariable,
|
Rule::UnusedVariable,
|
||||||
|
Rule::UnusedPrivateProtocol,
|
||||||
]) {
|
]) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
@ -71,6 +72,13 @@ pub(crate) fn bindings(checker: &mut Checker) {
|
||||||
checker.diagnostics.push(diagnostic);
|
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);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -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, "043") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::TSuffixedTypeAlias),
|
||||||
(Flake8Pyi, "044") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::FutureAnnotationsInStub),
|
(Flake8Pyi, "044") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::FutureAnnotationsInStub),
|
||||||
(Flake8Pyi, "045") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::IterMethodReturnIterable),
|
(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, "048") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::StubBodyMultipleStatements),
|
||||||
(Flake8Pyi, "050") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NoReturnArgumentAnnotationInStub),
|
(Flake8Pyi, "050") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NoReturnArgumentAnnotationInStub),
|
||||||
(Flake8Pyi, "052") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnannotatedAssignmentInStub),
|
(Flake8Pyi, "052") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnannotatedAssignmentInStub),
|
||||||
|
|
|
||||||
|
|
@ -95,6 +95,8 @@ mod tests {
|
||||||
#[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.pyi"))]
|
#[test_case(Rule::UnsupportedMethodCallOnAll, Path::new("PYI056.pyi"))]
|
||||||
#[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.py"))]
|
#[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.py"))]
|
||||||
#[test_case(Rule::UnusedPrivateTypeVar, Path::new("PYI018.pyi"))]
|
#[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<()> {
|
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
||||||
let diagnostics = test_path(
|
let diagnostics = test_path(
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,7 @@ use crate::checkers::ast::Checker;
|
||||||
///
|
///
|
||||||
/// ## Why is this bad?
|
/// ## Why is this bad?
|
||||||
/// A private `TypeVar` that is defined but not used is likely a mistake, and
|
/// 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
|
/// ## Example
|
||||||
/// ```python
|
/// ```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
|
/// PYI018
|
||||||
pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> Option<Diagnostic> {
|
pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> Option<Diagnostic> {
|
||||||
if !(binding.kind.is_assignment() && binding.is_private_variable()) {
|
if !(binding.kind.is_assignment() && binding.is_private_declaration()) {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
if binding.is_used() {
|
if binding.is_used() {
|
||||||
|
|
@ -64,3 +106,35 @@ pub(crate) fn unused_private_type_var(checker: &Checker, binding: &Binding) -> O
|
||||||
binding.range,
|
binding.range,
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// PYI046
|
||||||
|
pub(crate) fn unused_private_protocol(checker: &Checker, binding: &Binding) -> Option<Diagnostic> {
|
||||||
|
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,
|
||||||
|
))
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff/src/rules/flake8_pyi/mod.rs
|
||||||
|
---
|
||||||
|
|
||||||
|
|
@ -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
|
||||||
|
|
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -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"`)
|
/// (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)
|
self.flags.contains(BindingFlags::PRIVATE_DECLARATION)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -2389,6 +2389,7 @@
|
||||||
"PYI043",
|
"PYI043",
|
||||||
"PYI044",
|
"PYI044",
|
||||||
"PYI045",
|
"PYI045",
|
||||||
|
"PYI046",
|
||||||
"PYI048",
|
"PYI048",
|
||||||
"PYI05",
|
"PYI05",
|
||||||
"PYI050",
|
"PYI050",
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue