diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI006.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI006.py new file mode 100644 index 0000000000..89420bc50f --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI006.py @@ -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 diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI006.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI006.pyi new file mode 100644 index 0000000000..a0037bde07 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI006.pyi @@ -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 diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index 7be628fcdc..f54e7a3587 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -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> 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 { diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index e424e0173c..2edc581260 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -492,6 +492,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { // flake8-pyi (Flake8Pyi, "001") => Rule::PrefixTypeParams, + (Flake8Pyi, "006") => Rule::BadVersionInfoComparison, (Flake8Pyi, "007") => Rule::UnrecognizedPlatformCheck, (Flake8Pyi, "008") => Rule::UnrecognizedPlatformName, (Flake8Pyi, "009") => Rule::PassStatementStubBody, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 7583340025..cb8bfced83 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -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, diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index bd73a8e55f..a699938131 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -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"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/bad_version_info_comparison.rs b/crates/ruff/src/rules/flake8_pyi/rules/bad_version_info_comparison.rs new file mode 100644 index 0000000000..a6bb51018e --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/bad_version_info_comparison.rs @@ -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); + } +} diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index ea634b4ed8..bda523cf1d 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -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; diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI006_PYI006.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI006_PYI006.py.snap new file mode 100644 index 0000000000..efcc2d0c99 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI006_PYI006.py.snap @@ -0,0 +1,6 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +expression: diagnostics +--- +[] + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI006_PYI006.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI006_PYI006.pyi.snap new file mode 100644 index 0000000000..95de308401 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI006_PYI006.pyi.snap @@ -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: ~ + diff --git a/ruff.schema.json b/ruff.schema.json index 6b5dcf84ec..b20a4489d6 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1928,6 +1928,7 @@ "PYI0", "PYI00", "PYI001", + "PYI006", "PYI007", "PYI008", "PYI009", diff --git a/scripts/generate_mkdocs.py b/scripts/generate_mkdocs.py index 5793858e91..c857ca4a2b 100644 --- a/scripts/generate_mkdocs.py +++ b/scripts/generate_mkdocs.py @@ -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."""