diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF048.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF048.py new file mode 100644 index 0000000000000..59cd1f3b00beb --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF048.py @@ -0,0 +1,17 @@ +__version__ = (0, 1, 0) + + +tuple(map(int, __version__.split("."))) +list(map(int, __version__.split("."))) + +# `sep` passed as keyword argument +for part in map(int, __version__.split(sep=".")): + print(part) + +# Comma +tuple(map(int, __version__.split(","))) +list(map(int, __version__.split(","))) + +# Multiple arguments +tuple(map(int, __version__.split(".", 1))) +list(map(int, __version__.split(".", maxsplit=2))) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF048_1.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF048_1.py new file mode 100644 index 0000000000000..8ba9457ae91cb --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF048_1.py @@ -0,0 +1,27 @@ +from foo import __version__ +import bar + + +tuple(map(int, __version__.split("."))) +list(map(int, __version__.split("."))) +tuple(map(int, bar.__version__.split("."))) +list(map(int, bar.__version__.split("."))) + +# `sep` passed as keyword argument +for part in map(int, bar.__version__.split(sep=".")): + print(part) + +for part in map(int, __version__.split(sep=".")): + print(part) + +# Comma +tuple(map(int, __version__.split(","))) +list(map(int, __version__.split(","))) +tuple(map(int, bar.__version__.split(","))) +list(map(int, bar.__version__.split(","))) + +# Multiple arguments +tuple(map(int, __version__.split(",", 1))) +list(map(int, __version__.split(",", maxsplit = 2))) +tuple(map(int, bar.__version__.split(",", 1))) +list(map(int, bar.__version__.split(",", maxsplit = 2))) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 73c37257efb2e..930996a56fd36 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1055,6 +1055,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::UnsafeMarkupUse) { ruff::rules::unsafe_markup_call(checker, call); } + if checker.enabled(Rule::MapIntVersionParsing) { + ruff::rules::map_int_version_parsing(checker, call); + } } Expr::Dict(dict) => { if checker.any_enabled(&[ diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 3a7c8c56f28f7..0b9e625ca2468 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -971,6 +971,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "035") => (RuleGroup::Preview, rules::ruff::rules::UnsafeMarkupUse), (Ruff, "036") => (RuleGroup::Preview, rules::ruff::rules::NoneNotAtEndOfUnion), (Ruff, "038") => (RuleGroup::Preview, rules::ruff::rules::RedundantBoolLiteral), + (Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::MapIntVersionParsing), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA), diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 8030ab296819c..541dc77083c27 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -397,6 +397,8 @@ mod tests { Path::new("RUF009_attrs.py") )] #[test_case(Rule::MutableDataclassDefault, Path::new("RUF008_attrs.py"))] + #[test_case(Rule::MapIntVersionParsing, Path::new("RUF048.py"))] + #[test_case(Rule::MapIntVersionParsing, Path::new("RUF048_1.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( "preview__{}_{}", diff --git a/crates/ruff_linter/src/rules/ruff/rules/map_int_version_parsing.rs b/crates/ruff_linter/src/rules/ruff/rules/map_int_version_parsing.rs new file mode 100644 index 0000000000000..553f1f9c7cdab --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/map_int_version_parsing.rs @@ -0,0 +1,142 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_python_semantic::SemanticModel; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for calls of the form `map(int, __version__.split("."))`. +/// +/// ## Why is this bad? +/// `__version__` does not always contain integral-like elements. +/// +/// ```python +/// import matplotlib # `__version__ == "3.9.1.post-1"` in our environment +/// +/// # ValueError: invalid literal for int() with base 10: 'post1' +/// tuple(map(int, matplotlib.__version__.split("."))) +/// ``` +/// +/// See also [*Version specifiers* | Packaging spec][version-specifier]. +/// +/// ## Example +/// ```python +/// tuple(map(int, matplotlib.__version__.split("."))) +/// ``` +/// +/// Use instead: +/// ```python +/// import packaging.version as version +/// +/// version.parse(matplotlib.__version__) +/// ``` +/// +/// [version-specifier]: https://packaging.python.org/en/latest/specifications/version-specifiers/#version-specifiers +#[violation] +pub struct MapIntVersionParsing; + +impl Violation for MapIntVersionParsing { + #[derive_message_formats] + fn message(&self) -> String { + "`__version__` may contain non-integral-like elements".to_string() + } +} + +/// RUF048 +pub(crate) fn map_int_version_parsing(checker: &mut Checker, call: &ast::ExprCall) { + let semantic = checker.semantic(); + + let Some((first, second)) = map_call_with_two_arguments(semantic, call) else { + return; + }; + + if is_dunder_version_split_dot(second) && semantic.match_builtin_expr(first, "int") { + checker + .diagnostics + .push(Diagnostic::new(MapIntVersionParsing, call.range())); + } +} + +fn map_call_with_two_arguments<'a>( + semantic: &SemanticModel, + call: &'a ast::ExprCall, +) -> Option<(&'a ast::Expr, &'a ast::Expr)> { + let ast::ExprCall { + func, + arguments: + ast::Arguments { + args, + keywords, + range: _, + }, + range: _, + } = call; + + if !keywords.is_empty() { + return None; + } + + let [first, second] = &**args else { + return None; + }; + + if !semantic.match_builtin_expr(func, "map") { + return None; + }; + + Some((first, second)) +} + +/// Whether `expr` has the form `__version__.split(".")` or `something.__version__.split(".")`. +fn is_dunder_version_split_dot(expr: &ast::Expr) -> bool { + let ast::Expr::Call(ast::ExprCall { + func, arguments, .. + }) = expr + else { + return false; + }; + + if arguments.len() != 1 { + return false; + } + + let Some(ast::Expr::StringLiteral(ast::ExprStringLiteral { value, range: _ })) = + arguments.find_argument("sep", 0) + else { + return false; + }; + + if value.to_str() != "." { + return false; + } + + is_dunder_version_split(func) +} + +fn is_dunder_version_split(func: &ast::Expr) -> bool { + // foo.__version__.split(".") + // ---- value ---- ^^^^^ attr + let ast::Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func else { + return false; + }; + if attr != "split" { + return false; + } + is_dunder_version(value) +} + +fn is_dunder_version(expr: &ast::Expr) -> bool { + if let ast::Expr::Name(ast::ExprName { id, .. }) = expr { + return id == "__version__"; + } + + // foo.__version__.split(".") + // ^^^^^^^^^^^ attr + let ast::Expr::Attribute(ast::ExprAttribute { attr, .. }) = expr else { + return false; + }; + + attr == "__version__" +} diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 8ca14f31c526b..331a77e0a6b40 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -12,6 +12,7 @@ pub(crate) use incorrectly_parenthesized_tuple_in_subscript::*; pub(crate) use invalid_formatter_suppression_comment::*; pub(crate) use invalid_index_type::*; pub(crate) use invalid_pyproject_toml::*; +pub(crate) use map_int_version_parsing::*; pub(crate) use missing_fstring_syntax::*; pub(crate) use mutable_class_default::*; pub(crate) use mutable_dataclass_default::*; @@ -52,6 +53,7 @@ mod incorrectly_parenthesized_tuple_in_subscript; mod invalid_formatter_suppression_comment; mod invalid_index_type; mod invalid_pyproject_toml; +mod map_int_version_parsing; mod missing_fstring_syntax; mod mutable_class_default; mod mutable_dataclass_default; diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF048_RUF048.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF048_RUF048.py.snap new file mode 100644 index 0000000000000..fe1dcd4691fb4 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF048_RUF048.py.snap @@ -0,0 +1,26 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF048.py:4:7: RUF048 `__version__` may contain non-integral-like elements + | +4 | tuple(map(int, __version__.split("."))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048 +5 | list(map(int, __version__.split("."))) + | + +RUF048.py:5:6: RUF048 `__version__` may contain non-integral-like elements + | +4 | tuple(map(int, __version__.split("."))) +5 | list(map(int, __version__.split("."))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048 +6 | +7 | # `sep` passed as keyword argument + | + +RUF048.py:8:13: RUF048 `__version__` may contain non-integral-like elements + | +7 | # `sep` passed as keyword argument +8 | for part in map(int, __version__.split(sep=".")): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048 +9 | print(part) + | diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF048_RUF048_1.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF048_RUF048_1.py.snap new file mode 100644 index 0000000000000..a78add9e5d36b --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF048_RUF048_1.py.snap @@ -0,0 +1,55 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF048_1.py:5:7: RUF048 `__version__` may contain non-integral-like elements + | +5 | tuple(map(int, __version__.split("."))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048 +6 | list(map(int, __version__.split("."))) +7 | tuple(map(int, bar.__version__.split("."))) + | + +RUF048_1.py:6:6: RUF048 `__version__` may contain non-integral-like elements + | +5 | tuple(map(int, __version__.split("."))) +6 | list(map(int, __version__.split("."))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048 +7 | tuple(map(int, bar.__version__.split("."))) +8 | list(map(int, bar.__version__.split("."))) + | + +RUF048_1.py:7:7: RUF048 `__version__` may contain non-integral-like elements + | +5 | tuple(map(int, __version__.split("."))) +6 | list(map(int, __version__.split("."))) +7 | tuple(map(int, bar.__version__.split("."))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048 +8 | list(map(int, bar.__version__.split("."))) + | + +RUF048_1.py:8:6: RUF048 `__version__` may contain non-integral-like elements + | + 6 | list(map(int, __version__.split("."))) + 7 | tuple(map(int, bar.__version__.split("."))) + 8 | list(map(int, bar.__version__.split("."))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048 + 9 | +10 | # `sep` passed as keyword argument + | + +RUF048_1.py:11:13: RUF048 `__version__` may contain non-integral-like elements + | +10 | # `sep` passed as keyword argument +11 | for part in map(int, bar.__version__.split(sep=".")): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048 +12 | print(part) + | + +RUF048_1.py:14:13: RUF048 `__version__` may contain non-integral-like elements + | +12 | print(part) +13 | +14 | for part in map(int, __version__.split(sep=".")): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048 +15 | print(part) + | diff --git a/ruff.schema.json b/ruff.schema.json index 83f02006c77bb..0db62b2ef8ea8 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3836,6 +3836,8 @@ "RUF035", "RUF036", "RUF038", + "RUF04", + "RUF048", "RUF1", "RUF10", "RUF100",