mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 05:15:12 +00:00
[flake8-pyi] Implement PYI066 (#11541)
## Summary - Implements `Y066` from `flake8-pyi` as `PYI066` - Fixes `PYI006` not being raised for `elif` clauses. This would have conflicted with PYI006's implementation, so decided to do it in the same PR. ## Test Plan `cargo test` / `cargo insta review`
This commit is contained in:
Tushar Sadhwani
GitHub
parent
e0169d8dea
commit
531ae5227c
11 changed files with 301 additions and 19 deletions
|
|
@ -14,5 +14,7 @@ if sys.version_info <= (3, 10): ... # Error: PYI006 Use only `<` and `>=` for v
|
|||
if sys.version_info <= (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
|
||||
if sys.version_info > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
elif sys.version_info > (3, 11): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
|
||||
if python_version > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
elif python_version == (3, 11): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
|
|
|
|||
54
crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI066.py
vendored
Normal file
54
crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI066.py
vendored
Normal file
|
|
@ -0,0 +1,54 @@
|
|||
import sys
|
||||
|
||||
if sys.version_info < (3, 10): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
|
||||
def foo(x): ...
|
||||
else:
|
||||
def foo(x, *, bar=True): ...
|
||||
|
||||
if sys.version_info < (3, 8): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 8)"
|
||||
def bar(x): ...
|
||||
elif sys.version_info < (3, 9): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 9)"
|
||||
def bar(x, *, bar=True): ...
|
||||
elif sys.version_info < (3, 11): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
|
||||
def bar(x, *, bar=True, baz=False): ...
|
||||
else:
|
||||
def bar(x, *, bar=True, baz=False, qux=1): ...
|
||||
|
||||
|
||||
if sys.version_info >= (3, 5):
|
||||
...
|
||||
elif sys.version_info < (3, 9): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
|
||||
...
|
||||
else:
|
||||
...
|
||||
|
||||
# Negative cases
|
||||
|
||||
if sys.version_info[0] == 2: ...
|
||||
if sys.version_info[:1] == (2,): ...
|
||||
if sys.version_info[:1] == (True,): ...
|
||||
if sys.version_info < ('3', '0'): ...
|
||||
if sys.version_info >= (3, 4, 3): ...
|
||||
if sys.version_info == (3, 4): ...
|
||||
if sys.version_info < (3, 5): ...
|
||||
if sys.version_info >= (3, 5): ...
|
||||
if (2, 7) <= sys.version_info < (3, 5): ...
|
||||
|
||||
|
||||
if sys.version_info >= (3, 5):
|
||||
...
|
||||
else:
|
||||
...
|
||||
|
||||
if sys.version_info >= (3, 10):
|
||||
def foo1(x, *, bar=True, baz=False): ...
|
||||
elif sys.version_info >= (3, 9):
|
||||
def foo1(x, *, bar=True): ...
|
||||
else:
|
||||
def foo1(x): ...
|
||||
|
||||
if sys.version_info < (3, 9):
|
||||
def foo2(x): ...
|
||||
elif sys.version_info < (3, 10):
|
||||
def foo2(x, *, bar=True): ...
|
||||
# no else case, no raise
|
||||
54
crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI066.pyi
vendored
Normal file
54
crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI066.pyi
vendored
Normal file
|
|
@ -0,0 +1,54 @@
|
|||
import sys
|
||||
|
||||
if sys.version_info < (3, 10): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
|
||||
def foo(x): ...
|
||||
else:
|
||||
def foo(x, *, bar=True): ...
|
||||
|
||||
if sys.version_info < (3, 8): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 8)"
|
||||
def bar(x): ...
|
||||
elif sys.version_info < (3, 9): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 9)"
|
||||
def bar(x, *, bar=True): ...
|
||||
elif sys.version_info < (3, 11): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
|
||||
def bar(x, *, bar=True, baz=False): ...
|
||||
else:
|
||||
def bar(x, *, bar=True, baz=False, qux=1): ...
|
||||
|
||||
|
||||
if sys.version_info >= (3, 5):
|
||||
...
|
||||
elif sys.version_info < (3, 9): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
|
||||
...
|
||||
else:
|
||||
...
|
||||
|
||||
# Negative cases
|
||||
|
||||
if sys.version_info[0] == 2: ...
|
||||
if sys.version_info[:1] == (2,): ...
|
||||
if sys.version_info[:1] == (True,): ...
|
||||
if sys.version_info < ('3', '0'): ...
|
||||
if sys.version_info >= (3, 4, 3): ...
|
||||
if sys.version_info == (3, 4): ...
|
||||
if sys.version_info < (3, 5): ...
|
||||
if sys.version_info >= (3, 5): ...
|
||||
if (2, 7) <= sys.version_info < (3, 5): ...
|
||||
|
||||
|
||||
if sys.version_info >= (3, 5):
|
||||
...
|
||||
else:
|
||||
...
|
||||
|
||||
if sys.version_info >= (3, 10):
|
||||
def foo1(x, *, bar=True, baz=False): ...
|
||||
elif sys.version_info >= (3, 9):
|
||||
def foo1(x, *, bar=True): ...
|
||||
else:
|
||||
def foo1(x): ...
|
||||
|
||||
if sys.version_info < (3, 9):
|
||||
def foo2(x): ...
|
||||
elif sys.version_info < (3, 10):
|
||||
def foo2(x, *, bar=True): ...
|
||||
# no else case, no raise
|
||||
|
|
@ -1092,7 +1092,13 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
|||
ruff::rules::sort_dunder_all_aug_assign(checker, aug_assign);
|
||||
}
|
||||
}
|
||||
Stmt::If(if_ @ ast::StmtIf { test, .. }) => {
|
||||
Stmt::If(
|
||||
if_ @ ast::StmtIf {
|
||||
test,
|
||||
elif_else_clauses,
|
||||
..
|
||||
},
|
||||
) => {
|
||||
if checker.enabled(Rule::TooManyNestedBlocks) {
|
||||
pylint::rules::too_many_nested_blocks(checker, stmt);
|
||||
}
|
||||
|
|
@ -1172,13 +1178,38 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
|||
flake8_pyi::rules::unrecognized_platform(checker, test);
|
||||
}
|
||||
}
|
||||
if checker.enabled(Rule::BadVersionInfoComparison) {
|
||||
if let Expr::BoolOp(ast::ExprBoolOp { values, .. }) = test.as_ref() {
|
||||
for value in values {
|
||||
flake8_pyi::rules::bad_version_info_comparison(checker, value);
|
||||
if checker.any_enabled(&[Rule::BadVersionInfoComparison, Rule::BadVersionInfoOrder])
|
||||
{
|
||||
fn bad_version_info_comparison(
|
||||
checker: &mut Checker,
|
||||
test: &Expr,
|
||||
has_else_clause: bool,
|
||||
) {
|
||||
if let Expr::BoolOp(ast::ExprBoolOp { values, .. }) = test {
|
||||
for value in values {
|
||||
flake8_pyi::rules::bad_version_info_comparison(
|
||||
checker,
|
||||
value,
|
||||
has_else_clause,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
flake8_pyi::rules::bad_version_info_comparison(
|
||||
checker,
|
||||
test,
|
||||
has_else_clause,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
let has_else_clause =
|
||||
elif_else_clauses.iter().any(|clause| clause.test.is_none());
|
||||
|
||||
bad_version_info_comparison(checker, test.as_ref(), has_else_clause);
|
||||
for clause in elif_else_clauses {
|
||||
if let Some(test) = clause.test.as_ref() {
|
||||
bad_version_info_comparison(checker, test, has_else_clause);
|
||||
}
|
||||
} else {
|
||||
flake8_pyi::rules::bad_version_info_comparison(checker, test);
|
||||
}
|
||||
}
|
||||
if checker.enabled(Rule::ComplexIfStatementInStub) {
|
||||
|
|
|
|||
|
|
@ -812,6 +812,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
|||
(Flake8Pyi, "059") => (RuleGroup::Preview, rules::flake8_pyi::rules::GenericNotLastBaseClass),
|
||||
(Flake8Pyi, "062") => (RuleGroup::Preview, rules::flake8_pyi::rules::DuplicateLiteralMember),
|
||||
(Flake8Pyi, "064") => (RuleGroup::Preview, rules::flake8_pyi::rules::RedundantFinalLiteral),
|
||||
(Flake8Pyi, "066") => (RuleGroup::Preview, rules::flake8_pyi::rules::BadVersionInfoOrder),
|
||||
|
||||
// flake8-pytest-style
|
||||
(Flake8PytestStyle, "001") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestFixtureIncorrectParenthesesStyle),
|
||||
|
|
|
|||
|
|
@ -23,6 +23,8 @@ mod tests {
|
|||
#[test_case(Rule::BadExitAnnotation, Path::new("PYI036.pyi"))]
|
||||
#[test_case(Rule::BadVersionInfoComparison, Path::new("PYI006.py"))]
|
||||
#[test_case(Rule::BadVersionInfoComparison, Path::new("PYI006.pyi"))]
|
||||
#[test_case(Rule::BadVersionInfoOrder, Path::new("PYI066.py"))]
|
||||
#[test_case(Rule::BadVersionInfoOrder, Path::new("PYI066.pyi"))]
|
||||
#[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.py"))]
|
||||
#[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.pyi"))]
|
||||
#[test_case(Rule::ComplexAssignmentInStub, Path::new("PYI017.py"))]
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ use ruff_macros::{derive_message_formats, violation};
|
|||
use ruff_text_size::Ranged;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::registry::Rule;
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for uses of comparators other than `<` and `>=` for
|
||||
|
|
@ -57,8 +58,61 @@ impl Violation for BadVersionInfoComparison {
|
|||
}
|
||||
}
|
||||
|
||||
/// PYI006
|
||||
pub(crate) fn bad_version_info_comparison(checker: &mut Checker, test: &Expr) {
|
||||
/// ## What it does
|
||||
/// Checks for if-else statements with `sys.version_info` comparisons that use
|
||||
/// `<` comparators.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// As a convention, branches that correspond to newer Python versions should
|
||||
/// come first when using `sys.version_info` comparisons. This makes it easier
|
||||
/// to understand the desired behavior, which typically corresponds to the
|
||||
/// latest Python versions.
|
||||
///
|
||||
/// ## Example
|
||||
///
|
||||
/// ```python
|
||||
/// import sys
|
||||
///
|
||||
/// if sys.version_info < (3, 10):
|
||||
///
|
||||
/// def read_data(x, *, preserve_order=True):
|
||||
/// ...
|
||||
///
|
||||
/// else:
|
||||
///
|
||||
/// def read_data(x):
|
||||
/// ...
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
///
|
||||
/// ```python
|
||||
/// if sys.version_info >= (3, 10):
|
||||
///
|
||||
/// def read_data(x):
|
||||
/// ...
|
||||
///
|
||||
/// else:
|
||||
///
|
||||
/// def read_data(x, *, preserve_order=True):
|
||||
/// ...
|
||||
/// ```
|
||||
#[violation]
|
||||
pub struct BadVersionInfoOrder;
|
||||
|
||||
impl Violation for BadVersionInfoOrder {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Use `>=` when using `if`-`else` with `sys.version_info` comparisons")
|
||||
}
|
||||
}
|
||||
|
||||
/// PYI006, PYI066
|
||||
pub(crate) fn bad_version_info_comparison(
|
||||
checker: &mut Checker,
|
||||
test: &Expr,
|
||||
has_else_clause: bool,
|
||||
) {
|
||||
let Expr::Compare(ast::ExprCompare {
|
||||
left,
|
||||
ops,
|
||||
|
|
@ -81,11 +135,24 @@ pub(crate) fn bad_version_info_comparison(checker: &mut Checker, test: &Expr) {
|
|||
return;
|
||||
}
|
||||
|
||||
if matches!(op, CmpOp::Lt | CmpOp::GtE) {
|
||||
if matches!(op, CmpOp::GtE) {
|
||||
// No issue to be raised, early exit.
|
||||
return;
|
||||
}
|
||||
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(BadVersionInfoComparison, test.range()));
|
||||
if matches!(op, CmpOp::Lt) {
|
||||
if checker.enabled(Rule::BadVersionInfoOrder) {
|
||||
if has_else_clause {
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(BadVersionInfoOrder, test.range()));
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if checker.enabled(Rule::BadVersionInfoComparison) {
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(BadVersionInfoComparison, test.range()));
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -47,16 +47,30 @@ PYI006.pyi:16:4: PYI006 Use `<` or `>=` for `sys.version_info` comparisons
|
|||
15 |
|
||||
16 | if sys.version_info > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI006
|
||||
17 |
|
||||
18 | if python_version > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
17 | elif sys.version_info > (3, 11): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
|
|
||||
|
||||
PYI006.pyi:18:4: PYI006 Use `<` or `>=` for `sys.version_info` comparisons
|
||||
PYI006.pyi:17:6: PYI006 Use `<` or `>=` for `sys.version_info` comparisons
|
||||
|
|
||||
16 | if sys.version_info > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
17 |
|
||||
18 | if python_version > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ PYI006
|
||||
17 | elif sys.version_info > (3, 11): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI006
|
||||
18 |
|
||||
19 | if python_version > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
|
|
||||
|
||||
PYI006.pyi:19:4: PYI006 Use `<` or `>=` for `sys.version_info` comparisons
|
||||
|
|
||||
17 | elif sys.version_info > (3, 11): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
18 |
|
||||
19 | if python_version > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ PYI006
|
||||
20 | elif python_version == (3, 11): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
|
|
||||
|
||||
PYI006.pyi:20:6: PYI006 Use `<` or `>=` for `sys.version_info` comparisons
|
||||
|
|
||||
19 | if python_version > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
20 | elif python_version == (3, 11): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^ PYI006
|
||||
|
|
||||
|
|
|
|||
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
|
||||
---
|
||||
|
||||
|
|
@ -0,0 +1,52 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
|
||||
---
|
||||
PYI066.pyi:3:4: PYI066 Use `>=` when using `if`-`else` with `sys.version_info` comparisons
|
||||
|
|
||||
1 | import sys
|
||||
2 |
|
||||
3 | if sys.version_info < (3, 10): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI066
|
||||
4 | def foo(x): ...
|
||||
5 | else:
|
||||
|
|
||||
|
||||
PYI066.pyi:8:4: PYI066 Use `>=` when using `if`-`else` with `sys.version_info` comparisons
|
||||
|
|
||||
6 | def foo(x, *, bar=True): ...
|
||||
7 |
|
||||
8 | if sys.version_info < (3, 8): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 8)"
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^ PYI066
|
||||
9 | def bar(x): ...
|
||||
10 | elif sys.version_info < (3, 9): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 9)"
|
||||
|
|
||||
|
||||
PYI066.pyi:10:6: PYI066 Use `>=` when using `if`-`else` with `sys.version_info` comparisons
|
||||
|
|
||||
8 | if sys.version_info < (3, 8): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 8)"
|
||||
9 | def bar(x): ...
|
||||
10 | elif sys.version_info < (3, 9): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 9)"
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^ PYI066
|
||||
11 | def bar(x, *, bar=True): ...
|
||||
12 | elif sys.version_info < (3, 11): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
|
||||
|
|
||||
|
||||
PYI066.pyi:12:6: PYI066 Use `>=` when using `if`-`else` with `sys.version_info` comparisons
|
||||
|
|
||||
10 | elif sys.version_info < (3, 9): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 9)"
|
||||
11 | def bar(x, *, bar=True): ...
|
||||
12 | elif sys.version_info < (3, 11): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI066
|
||||
13 | def bar(x, *, bar=True, baz=False): ...
|
||||
14 | else:
|
||||
|
|
||||
|
||||
PYI066.pyi:20:6: PYI066 Use `>=` when using `if`-`else` with `sys.version_info` comparisons
|
||||
|
|
||||
18 | if sys.version_info >= (3, 5):
|
||||
19 | ...
|
||||
20 | elif sys.version_info < (3, 9): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^ PYI066
|
||||
21 | ...
|
||||
22 | else:
|
||||
|
|
||||
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
|
@ -3581,6 +3581,7 @@
|
|||
"PYI06",
|
||||
"PYI062",
|
||||
"PYI064",
|
||||
"PYI066",
|
||||
"Q",
|
||||
"Q0",
|
||||
"Q00",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue