Skip to content

Commit

Permalink
[ruff] tuple(map(int, package.__version__.split('.'))) (RUF048)
Browse files Browse the repository at this point in the history
  • Loading branch information
InSyncWithFoo committed Nov 16, 2024
1 parent a48d779 commit 8cdfabb
Show file tree
Hide file tree
Showing 10 changed files with 269 additions and 0 deletions.
13 changes: 13 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF048.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
__version__ = (0, 1, 0)


tuple(map(int, __version__.split(".")))
list(map(int, __version__.split(".")))

# 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)))
20 changes: 20 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF048_1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
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(".")))

# 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)))
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,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::TupleMapIntVersionParsing) {
ruff::rules::tuple_map_int_version_parsing(checker, call);
}
}
Expr::Dict(dict) => {
if checker.any_enabled(&[
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "034") => (RuleGroup::Preview, rules::ruff::rules::UselessIfElse),
(Ruff, "035") => (RuleGroup::Preview, rules::ruff::rules::UnsafeMarkupUse),
(Ruff, "036") => (RuleGroup::Preview, rules::ruff::rules::NoneNotAtEndOfUnion),
(Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::TupleMapIntVersionParsing),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),

Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,8 @@ mod tests {
Path::new("RUF009_attrs.py")
)]
#[test_case(Rule::MutableDataclassDefault, Path::new("RUF008_attrs.py"))]
#[test_case(Rule::TupleMapIntVersionParsing, Path::new("RUF048.py"))]
#[test_case(Rule::TupleMapIntVersionParsing, Path::new("RUF048_1.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub(crate) use sort_dunder_slots::*;
pub(crate) use static_key_dict_comprehension::*;
#[cfg(any(feature = "test-rules", test))]
pub(crate) use test_rules::*;
pub(crate) use tuple_map_int_version_parsing::*;
pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
pub(crate) use unnecessary_key_check::*;
pub(crate) use unsafe_markup_use::*;
Expand Down Expand Up @@ -68,6 +69,7 @@ mod static_key_dict_comprehension;
mod suppression_comment_visitor;
#[cfg(any(feature = "test-rules", test))]
pub(crate) mod test_rules;
mod tuple_map_int_version_parsing;
mod unnecessary_iterable_allocation_for_first_element;
mod unnecessary_key_check;
mod unsafe_markup_use;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
use std::ops::Deref;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, ExprAttribute, ExprCall, ExprName};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for calls of the form `tuple(map(int, __version__.split(".")))`.
///
/// ## Why is this bad?
/// `__version__` does not always contain integral-like elements.
///
/// ```python
/// import matplotlib # 3.9.1.post-1
///
/// # ValueError: invalid literal for int() with base 10: 'post1'
/// tuple(map(int, matplotlib.__version__.split(".")))
/// ```
///
/// See also [PEP 440].
///
/// ## Example
/// ```python
/// tuple(map(int, matplotlib.__version__.split(".")))
/// ```
///
/// Use instead:
/// ```python
/// import packaging.version as version
///
/// version.parse(matplotlib.__version__)
/// ```
///
/// [PEP 440]: https://peps.python.org/pep-0440/
#[violation]
pub struct TupleMapIntVersionParsing;

impl Violation for TupleMapIntVersionParsing {
#[derive_message_formats]
fn message(&self) -> String {
"`__version__` may contain non-integral-like elements".to_string()
}
}

/// RUF048
pub(crate) fn tuple_map_int_version_parsing(checker: &mut Checker, call: &ExprCall) {
let semantic = checker.semantic();

let Some(Expr::Call(child_call)) = tuple_like_call_with_single_argument(semantic, call) else {
return;
};

let Some((first, second)) = map_call_with_two_arguments(semantic, child_call) else {
return;
};

if !semantic.match_builtin_expr(first, "int") || !is_dunder_version_split_dot(second) {
return;
}

let diagnostic = Diagnostic::new(TupleMapIntVersionParsing, call.range());

checker.diagnostics.push(diagnostic);
}

fn tuple_like_call_with_single_argument<'a>(
semantic: &SemanticModel,
call: &'a ExprCall,
) -> Option<&'a Expr> {
let Some((func, positionals)) = func_and_positionals(call) else {
return None;
};

let func_is = |symbol: &str| semantic.match_builtin_expr(func, symbol);

if !func_is("tuple") && !func_is("list") || positionals.len() != 1 {
return None;
};

positionals.first()
}

fn map_call_with_two_arguments<'a>(
semantic: &SemanticModel,
call: &'a ExprCall,
) -> Option<(&'a Expr, &'a Expr)> {
let Some((func, positionals)) = func_and_positionals(call) else {
return None;
};

if !semantic.match_builtin_expr(func, "map") || positionals.len() != 2 {
return None;
};

Some((positionals.first().unwrap(), positionals.last().unwrap()))
}

/// Whether `expr` has the form `__version__.split(".")` or `something.__version__.split(".")`.
fn is_dunder_version_split_dot(expr: &Expr) -> bool {
let Expr::Call(call) = expr else {
return false;
};
let Some((func, arguments)) = func_and_positionals(call) else {
return false;
};
let argument = if arguments.len() == 1 {
arguments.first().unwrap()
} else {
return false;
};

is_dunder_version_split(func) && is_single_dot_string(argument)
}

fn is_dunder_version_split(func: &Expr) -> bool {
// foo.__version__.split(".")
// ---- value ---- ^^^^^ attr
let Expr::Attribute(ExprAttribute { attr, value, .. }) = func else {
return false;
};
if attr.as_str() != "split" {
return false;
}

is_dunder_version(value)
}

fn is_dunder_version(expr: &Expr) -> bool {
if let Expr::Name(ExprName { id, .. }) = expr {
return id.as_str() == "__version__";
}

// foo.__version__.split(".")
// ^^^^^^^^^^^ attr
let Expr::Attribute(ExprAttribute { attr, .. }) = expr else {
return false;
};

attr == "__version__"
}

fn is_single_dot_string(argument: &Expr) -> bool {
let Some(string) = argument.as_string_literal_expr() else {
return false;
};

let mut string_chars = string.value.chars();
let (first, second) = (string_chars.next(), string_chars.next());

matches!((first, second), (Some('.'), None))
}

/// Extracts the function being called and its positional arguments.
/// Returns `None` if there are keyword arguments.
fn func_and_positionals(expr: &ExprCall) -> Option<(&Expr, &[Expr])> {
let func = &expr.func;
let arguments = &expr.arguments;

if !arguments.keywords.is_empty() {
return None;
}

Some((func.deref(), arguments.args.deref()))
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
snapshot_kind: text
---
RUF048.py:4:1: RUF048 `__version__` may contain non-integral-like elements
|
4 | tuple(map(int, __version__.split(".")))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048
5 | list(map(int, __version__.split(".")))
|

RUF048.py:5:1: RUF048 `__version__` may contain non-integral-like elements
|
4 | tuple(map(int, __version__.split(".")))
5 | list(map(int, __version__.split(".")))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF048
6 |
7 | # Comma
|
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
snapshot_kind: text
---
RUF048_1.py:5:1: 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:1: 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:1: 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:1: 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 | # Comma
|
2 changes: 2 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 8cdfabb

Please sign in to comment.