flake8-pyi PYI006 bad version info comparison (#3291)

Implement PYI006 "bad version info comparison"

## What it does

Ensures that you only `<` and `>=` for version info comparisons with
`sys.version_info` in `.pyi` files. All other comparisons such as
`<`, `<=` and `==` are banned.

## Why is this bad?

```python
>>> import sys
>>> print(sys.version_info)
sys.version_info(major=3, minor=8, micro=10, releaselevel='final', serial=0)
>>> print(sys.version_info > (3, 8))
True
>>> print(sys.version_info == (3, 8))
False
>>> print(sys.version_info <= (3, 8))
False
>>> print(sys.version_info in (3, 8))
False
```

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
konstin 2023-03-01 18:58:57 +01:00 committed by GitHub
parent a032b66c2e
commit 2168404fc2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 216 additions and 2 deletions

View file

@ -0,0 +1,14 @@
import sys
from sys import version_info as python_version
if sys.version_info < (3, 9): ... # OK
if sys.version_info >= (3, 9): ... # OK
if sys.version_info == (3, 9): ... # OK
if sys.version_info <= (3, 10): ... # OK
if sys.version_info > (3, 10): ... # OK
if python_version > (3, 10): ... # OK

View file

@ -0,0 +1,18 @@
import sys
from sys import version_info as python_version
if sys.version_info < (3, 9): ... # OK
if sys.version_info >= (3, 9): ... # OK
if sys.version_info == (3, 9): ... # OK
if sys.version_info == (3, 9): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
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
if sys.version_info > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons
if python_version > (3, 10): ... # Error: PYI006 Use only `<` and `>=` for version info comparisons

View file

@ -233,6 +233,14 @@ impl<'a> Checker<'a> {
.map_or(false, |binding| binding.kind.is_builtin())
}
/// Resolves the call path, e.g. if you have a file
///
/// ```python
/// from sys import version_info as python_version
/// print(python_version)
/// ```
///
/// then `python_version` from the print statement will resolve to `sys.version_info`.
pub fn resolve_call_path<'b>(&'a self, value: &'b Expr) -> Option<CallPath<'a>>
where
'b: 'a,
@ -3391,6 +3399,16 @@ where
comparators,
);
}
if self.settings.rules.enabled(&Rule::BadVersionInfoComparison) {
flake8_pyi::rules::bad_version_info_comparison(
self,
expr,
left,
ops,
comparators,
);
}
}
}
ExprKind::Constant {

View file

@ -492,6 +492,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
// flake8-pyi
(Flake8Pyi, "001") => Rule::PrefixTypeParams,
(Flake8Pyi, "006") => Rule::BadVersionInfoComparison,
(Flake8Pyi, "007") => Rule::UnrecognizedPlatformCheck,
(Flake8Pyi, "008") => Rule::UnrecognizedPlatformName,
(Flake8Pyi, "009") => Rule::PassStatementStubBody,

View file

@ -466,6 +466,7 @@ ruff_macros::register_rules!(
rules::flake8_errmsg::rules::DotFormatInException,
// flake8-pyi
rules::flake8_pyi::rules::PrefixTypeParams,
rules::flake8_pyi::rules::BadVersionInfoComparison,
rules::flake8_pyi::rules::UnrecognizedPlatformCheck,
rules::flake8_pyi::rules::UnrecognizedPlatformName,
rules::flake8_pyi::rules::PassStatementStubBody,

View file

@ -15,6 +15,8 @@ mod tests {
#[test_case(Rule::PrefixTypeParams, Path::new("PYI001.pyi"))]
#[test_case(Rule::PrefixTypeParams, Path::new("PYI001.py"))]
#[test_case(Rule::BadVersionInfoComparison, Path::new("PYI006.pyi"))]
#[test_case(Rule::BadVersionInfoComparison, Path::new("PYI006.py"))]
#[test_case(Rule::UnrecognizedPlatformCheck, Path::new("PYI007.pyi"))]
#[test_case(Rule::UnrecognizedPlatformCheck, Path::new("PYI007.py"))]
#[test_case(Rule::UnrecognizedPlatformName, Path::new("PYI008.pyi"))]

View file

@ -0,0 +1,83 @@
use rustpython_parser::ast::{Cmpop, Expr};
use ruff_macros::{define_violation, derive_message_formats};
use crate::checkers::ast::Checker;
use crate::registry::Diagnostic;
use crate::violation::Violation;
use crate::Range;
define_violation!(
/// ## What it does
/// Checks for usages of comparators other than `<` and `>=` for
/// `sys.version_info` checks in `.pyi` files. All other comparators, such
/// as `>`, `<=`, and `==`, are banned.
///
/// ## Why is this bad?
/// Comparing `sys.version_info` with `==` or `<=` has unexpected behavior
/// and can lead to bugs.
///
/// For example, `sys.version_info > (3, 8)` will also match `3.8.10`,
/// while `sys.version_info <= (3, 8)` will _not_ match `3.8.10`:
///
/// ```python
/// >>> import sys
/// >>> print(sys.version_info)
/// sys.version_info(major=3, minor=8, micro=10, releaselevel='final', serial=0)
/// >>> print(sys.version_info > (3, 8))
/// True
/// >>> print(sys.version_info == (3, 8))
/// False
/// >>> print(sys.version_info <= (3, 8))
/// False
/// >>> print(sys.version_info in (3, 8))
/// False
/// ```
///
/// ## Example
/// ```python
/// import sys
///
/// if sys.version_info > (3, 8):
/// ...
/// ```
///
/// Use instead:
/// ```python
/// import sys
///
/// if sys.version_info >= (3, 9):
/// ...
/// ```
pub struct BadVersionInfoComparison;
);
impl Violation for BadVersionInfoComparison {
#[derive_message_formats]
fn message(&self) -> String {
format!("Use `<` or `>=` for version info comparisons")
}
}
/// PYI006
pub fn bad_version_info_comparison(
checker: &mut Checker,
expr: &Expr,
left: &Expr,
ops: &[Cmpop],
comparators: &[Expr],
) {
let ([op], [_right]) = (ops, comparators) else {
return;
};
if !checker.resolve_call_path(left).map_or(false, |call_path| {
call_path.as_slice() == ["sys", "version_info"]
}) {
return;
}
if !matches!(op, Cmpop::Lt | Cmpop::GtE) {
let diagnostic = Diagnostic::new(BadVersionInfoComparison, Range::from_located(expr));
checker.diagnostics.push(diagnostic);
}
}

View file

@ -1,3 +1,4 @@
pub use bad_version_info_comparison::{bad_version_info_comparison, BadVersionInfoComparison};
pub use docstring_in_stubs::{docstring_in_stubs, DocstringInStub};
pub use non_empty_stub_body::{non_empty_stub_body, NonEmptyStubBody};
pub use pass_statement_stub_body::{pass_statement_stub_body, PassStatementStubBody};
@ -10,10 +11,10 @@ pub use unrecognized_platform::{
unrecognized_platform, UnrecognizedPlatformCheck, UnrecognizedPlatformName,
};
mod bad_version_info_comparison;
mod docstring_in_stubs;
mod non_empty_stub_body;
mod pass_statement_stub_body;
mod prefix_type_params;
mod unrecognized_platform;
mod simple_defaults;
mod unrecognized_platform;

View file

@ -0,0 +1,6 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
expression: diagnostics
---
[]

View file

@ -0,0 +1,65 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
expression: diagnostics
---
- kind:
BadVersionInfoComparison: ~
location:
row: 8
column: 3
end_location:
row: 8
column: 29
fix: ~
parent: ~
- kind:
BadVersionInfoComparison: ~
location:
row: 10
column: 3
end_location:
row: 10
column: 29
fix: ~
parent: ~
- kind:
BadVersionInfoComparison: ~
location:
row: 12
column: 3
end_location:
row: 12
column: 30
fix: ~
parent: ~
- kind:
BadVersionInfoComparison: ~
location:
row: 14
column: 3
end_location:
row: 14
column: 30
fix: ~
parent: ~
- kind:
BadVersionInfoComparison: ~
location:
row: 16
column: 3
end_location:
row: 16
column: 29
fix: ~
parent: ~
- kind:
BadVersionInfoComparison: ~
location:
row: 18
column: 3
end_location:
row: 18
column: 27
fix: ~
parent: ~

1
ruff.schema.json generated
View file

@ -1928,6 +1928,7 @@
"PYI0",
"PYI00",
"PYI001",
"PYI006",
"PYI007",
"PYI008",
"PYI009",

View file

@ -3,11 +3,15 @@ import argparse
import re
import shutil
import subprocess
import sys
from pathlib import Path
from typing import NamedTuple
import yaml
if sys.version_info < (3, 9):
raise RuntimeError("You need at least python 3.9 to run this script")
class Section(NamedTuple):
"""A section to include in the MkDocs documentation."""