Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ruff] Add rule forbidding map(int, package.__version__.split('.')) (RUF048) #14373

Merged
merged 8 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 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,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)))
27 changes: 27 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,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)))
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 @@ -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(&[
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 @@ -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),

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 @@ -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__{}_{}",
Expand Down
142 changes: 142 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/map_int_version_parsing.rs
Original file line number Diff line number Diff line change
@@ -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__"
}
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 @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
|
Original file line number Diff line number Diff line change
@@ -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)
|
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.

Loading