[ruff] Add rule forbidding `map(int, package.__version__.split('.')…

…)` (`RUF048`) (#14373)

Co-authored-by: Alex Waygood <[email protected]>

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)))

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)))

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(&[

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),
 

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__{}_{}",

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__"
+}

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;

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)
+  |

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)
+   |