Add support for `int`, `float`, `bool` in `UP018` (#6013)

## Summary

This pull request add supports for `int`, `float` and `bool` types in
`UP018`
rule to convert empty call to the default value of the type or remove
the call
if a value of the same type is provided as an argument.

## Test Plan

Added tests for `int`, `float` and `bool` types.

Partially resolves #5988
This commit is contained in:
Dhruv Manilawala 2023-07-24 07:09:43 +05:30 committed by GitHub
parent 95e6258d5d
commit 742f615792
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 342 additions and 151 deletions

View File

@ -15,7 +15,22 @@ bytes("foo", **a)
bytes(b"foo"
b"bar")
bytes("foo")
bytes(1)
f"{f'{str()}'}"
int(1.0)
int("1")
int(b"11")
int(10, base=2)
int("10", base=2)
int("10", 2)
float("1.0")
float(b"1.0")
bool(1)
bool(0)
bool("foo")
bool("")
bool(b"")
bool(1.0)
# These become string or byte literals
str()
@ -27,3 +42,10 @@ bytes(b"foo")
bytes(b"""
foo""")
f"{str()}"
int()
int(1)
float()
float(1.0)
bool()
bool(True)
bool(False)

View File

@ -1,5 +1,7 @@
use std::fmt;
use std::str::FromStr;
use num_bigint::BigInt;
use rustpython_parser::ast::{self, Constant, Expr, Keyword, Ranged};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
@ -10,9 +12,54 @@ use crate::checkers::ast::Checker;
use crate::registry::AsRule;
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
pub(crate) enum LiteralType {
enum LiteralType {
Str,
Bytes,
Int,
Float,
Bool,
}
impl FromStr for LiteralType {
type Err = ();
fn from_str(value: &str) -> Result<Self, Self::Err> {
match value {
"str" => Ok(LiteralType::Str),
"bytes" => Ok(LiteralType::Bytes),
"int" => Ok(LiteralType::Int),
"float" => Ok(LiteralType::Float),
"bool" => Ok(LiteralType::Bool),
_ => Err(()),
}
}
}
impl From<LiteralType> for Constant {
fn from(value: LiteralType) -> Self {
match value {
LiteralType::Str => Constant::Str(String::new()),
LiteralType::Bytes => Constant::Bytes(vec![]),
LiteralType::Int => Constant::Int(BigInt::from(0)),
LiteralType::Float => Constant::Float(0.0),
LiteralType::Bool => Constant::Bool(false),
}
}
}
impl TryFrom<&Constant> for LiteralType {
type Error = ();
fn try_from(value: &Constant) -> Result<Self, Self::Error> {
match value {
Constant::Str(_) => Ok(LiteralType::Str),
Constant::Bytes(_) => Ok(LiteralType::Bytes),
Constant::Int(_) => Ok(LiteralType::Int),
Constant::Float(_) => Ok(LiteralType::Float),
Constant::Bool(_) => Ok(LiteralType::Bool),
_ => Err(()),
}
}
}
impl fmt::Display for LiteralType {
@ -20,16 +67,19 @@ impl fmt::Display for LiteralType {
match self {
LiteralType::Str => fmt.write_str("str"),
LiteralType::Bytes => fmt.write_str("bytes"),
LiteralType::Int => fmt.write_str("int"),
LiteralType::Float => fmt.write_str("float"),
LiteralType::Bool => fmt.write_str("bool"),
}
}
}
/// ## What it does
/// Checks for unnecessary calls to `str` and `bytes`.
/// Checks for unnecessary calls to `str`, `bytes`, `int`, `float`, and `bool`.
///
/// ## Why is this bad?
/// The `str` and `bytes` constructors can be replaced with string and bytes
/// literals, which are more readable and idiomatic.
/// The mentioned constructors can be replaced with their respective literal
/// forms, which are more readable and idiomatic.
///
/// ## Example
/// ```python
@ -44,6 +94,9 @@ impl fmt::Display for LiteralType {
/// ## References
/// - [Python documentation: `str`](https://docs.python.org/3/library/stdtypes.html#str)
/// - [Python documentation: `bytes`](https://docs.python.org/3/library/stdtypes.html#bytes)
/// - [Python documentation: `int`](https://docs.python.org/3/library/functions.html#int)
/// - [Python documentation: `float`](https://docs.python.org/3/library/functions.html#float)
/// - [Python documentation: `bool`](https://docs.python.org/3/library/functions.html#bool)
#[violation]
pub struct NativeLiterals {
literal_type: LiteralType,
@ -53,7 +106,7 @@ impl AlwaysAutofixableViolation for NativeLiterals {
#[derive_message_formats]
fn message(&self) -> String {
let NativeLiterals { literal_type } = self;
format!("Unnecessary call to `{literal_type}`")
format!("Unnecessary `{literal_type}` call (rewrite as a literal)")
}
fn autofix_title(&self) -> String {
@ -61,6 +114,9 @@ impl AlwaysAutofixableViolation for NativeLiterals {
match literal_type {
LiteralType::Str => "Replace with empty string".to_string(),
LiteralType::Bytes => "Replace with empty bytes".to_string(),
LiteralType::Int => "Replace with 0".to_string(),
LiteralType::Float => "Replace with 0.0".to_string(),
LiteralType::Bool => "Replace with `False`".to_string(),
}
}
}
@ -81,9 +137,9 @@ pub(crate) fn native_literals(
return;
}
if !matches!(id.as_str(), "str" | "bytes") {
let Ok(literal_type) = LiteralType::from_str(id.as_str()) else {
return;
}
};
if !checker.semantic().is_builtin(id) {
return;
@ -104,22 +160,9 @@ pub(crate) fn native_literals(
match args.get(0) {
None => {
let mut diagnostic = Diagnostic::new(
NativeLiterals {
literal_type: if id == "str" {
LiteralType::Str
} else {
LiteralType::Bytes
},
},
expr.range(),
);
let mut diagnostic = Diagnostic::new(NativeLiterals { literal_type }, expr.range());
if checker.patch(diagnostic.kind.rule()) {
let constant = if id == "bytes" {
Constant::Bytes(vec![])
} else {
Constant::Str(String::new())
};
let constant = Constant::from(literal_type);
let content = checker.generator().constant(&constant);
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
content,
@ -129,48 +172,28 @@ pub(crate) fn native_literals(
checker.diagnostics.push(diagnostic);
}
Some(arg) => {
// Look for `str("")`.
if id == "str"
&& !matches!(
&arg,
Expr::Constant(ast::ExprConstant {
value: Constant::Str(_),
..
}),
)
{
let Expr::Constant(ast::ExprConstant { value, .. }) = arg else {
return;
};
let Ok(arg_literal_type) = LiteralType::try_from(value) else {
return;
};
if arg_literal_type != literal_type {
return;
}
// Look for `bytes(b"")`
if id == "bytes"
&& !matches!(
&arg,
Expr::Constant(ast::ExprConstant {
value: Constant::Bytes(_),
..
}),
)
{
return;
}
let arg_code = checker.locator().slice(arg.range());
// Skip implicit string concatenations.
let arg_code = checker.locator().slice(arg.range());
if is_implicit_concatenation(arg_code) {
if matches!(arg_literal_type, LiteralType::Str | LiteralType::Bytes)
&& is_implicit_concatenation(arg_code)
{
return;
}
let mut diagnostic = Diagnostic::new(
NativeLiterals {
literal_type: if id == "str" {
LiteralType::Str
} else {
LiteralType::Bytes
},
},
expr.range(),
);
let mut diagnostic = Diagnostic::new(NativeLiterals { literal_type }, expr.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
arg_code.to_string(),

View File

@ -1,148 +1,294 @@
---
source: crates/ruff/src/rules/pyupgrade/mod.rs
---
UP018.py:21:1: UP018 [*] Unnecessary call to `str`
UP018.py:36:1: UP018 [*] Unnecessary `str` call (rewrite as a literal)
|
20 | # These become string or byte literals
21 | str()
35 | # These become string or byte literals
36 | str()
| ^^^^^ UP018
22 | str("foo")
23 | str("""
37 | str("foo")
38 | str("""
|
= help: Replace with empty string
Fix
18 18 | f"{f'{str()}'}"
19 19 |
20 20 | # These become string or byte literals
21 |-str()
21 |+""
22 22 | str("foo")
23 23 | str("""
24 24 | foo""")
33 33 | bool(1.0)
34 34 |
35 35 | # These become string or byte literals
36 |-str()
36 |+""
37 37 | str("foo")
38 38 | str("""
39 39 | foo""")
UP018.py:22:1: UP018 [*] Unnecessary call to `str`
UP018.py:37:1: UP018 [*] Unnecessary `str` call (rewrite as a literal)
|
20 | # These become string or byte literals
21 | str()
22 | str("foo")
35 | # These become string or byte literals
36 | str()
37 | str("foo")
| ^^^^^^^^^^ UP018
23 | str("""
24 | foo""")
38 | str("""
39 | foo""")
|
= help: Replace with empty string
Fix
19 19 |
20 20 | # These become string or byte literals
21 21 | str()
22 |-str("foo")
22 |+"foo"
23 23 | str("""
24 24 | foo""")
25 25 | bytes()
34 34 |
35 35 | # These become string or byte literals
36 36 | str()
37 |-str("foo")
37 |+"foo"
38 38 | str("""
39 39 | foo""")
40 40 | bytes()
UP018.py:23:1: UP018 [*] Unnecessary call to `str`
UP018.py:38:1: UP018 [*] Unnecessary `str` call (rewrite as a literal)
|
21 | str()
22 | str("foo")
23 | / str("""
24 | | foo""")
36 | str()
37 | str("foo")
38 | / str("""
39 | | foo""")
| |_______^ UP018
25 | bytes()
26 | bytes(b"foo")
40 | bytes()
41 | bytes(b"foo")
|
= help: Replace with empty string
Fix
20 20 | # These become string or byte literals
21 21 | str()
22 22 | str("foo")
23 |-str("""
24 |-foo""")
23 |+"""
24 |+foo"""
25 25 | bytes()
26 26 | bytes(b"foo")
27 27 | bytes(b"""
35 35 | # These become string or byte literals
36 36 | str()
37 37 | str("foo")
38 |-str("""
39 |-foo""")
38 |+"""
39 |+foo"""
40 40 | bytes()
41 41 | bytes(b"foo")
42 42 | bytes(b"""
UP018.py:25:1: UP018 [*] Unnecessary call to `bytes`
UP018.py:40:1: UP018 [*] Unnecessary `bytes` call (rewrite as a literal)
|
23 | str("""
24 | foo""")
25 | bytes()
38 | str("""
39 | foo""")
40 | bytes()
| ^^^^^^^ UP018
26 | bytes(b"foo")
27 | bytes(b"""
41 | bytes(b"foo")
42 | bytes(b"""
|
= help: Replace with empty bytes
Fix
22 22 | str("foo")
23 23 | str("""
24 24 | foo""")
25 |-bytes()
25 |+b""
26 26 | bytes(b"foo")
27 27 | bytes(b"""
28 28 | foo""")
37 37 | str("foo")
38 38 | str("""
39 39 | foo""")
40 |-bytes()
40 |+b""
41 41 | bytes(b"foo")
42 42 | bytes(b"""
43 43 | foo""")
UP018.py:26:1: UP018 [*] Unnecessary call to `bytes`
UP018.py:41:1: UP018 [*] Unnecessary `bytes` call (rewrite as a literal)
|
24 | foo""")
25 | bytes()
26 | bytes(b"foo")
39 | foo""")
40 | bytes()
41 | bytes(b"foo")
| ^^^^^^^^^^^^^ UP018
27 | bytes(b"""
28 | foo""")
42 | bytes(b"""
43 | foo""")
|
= help: Replace with empty bytes
Fix
23 23 | str("""
24 24 | foo""")
25 25 | bytes()
26 |-bytes(b"foo")
26 |+b"foo"
27 27 | bytes(b"""
28 28 | foo""")
29 29 | f"{str()}"
38 38 | str("""
39 39 | foo""")
40 40 | bytes()
41 |-bytes(b"foo")
41 |+b"foo"
42 42 | bytes(b"""
43 43 | foo""")
44 44 | f"{str()}"
UP018.py:27:1: UP018 [*] Unnecessary call to `bytes`
UP018.py:42:1: UP018 [*] Unnecessary `bytes` call (rewrite as a literal)
|
25 | bytes()
26 | bytes(b"foo")
27 | / bytes(b"""
28 | | foo""")
40 | bytes()
41 | bytes(b"foo")
42 | / bytes(b"""
43 | | foo""")
| |_______^ UP018
29 | f"{str()}"
44 | f"{str()}"
45 | int()
|
= help: Replace with empty bytes
Fix
24 24 | foo""")
25 25 | bytes()
26 26 | bytes(b"foo")
27 |-bytes(b"""
28 |-foo""")
27 |+b"""
28 |+foo"""
29 29 | f"{str()}"
39 39 | foo""")
40 40 | bytes()
41 41 | bytes(b"foo")
42 |-bytes(b"""
43 |-foo""")
42 |+b"""
43 |+foo"""
44 44 | f"{str()}"
45 45 | int()
46 46 | int(1)
UP018.py:29:4: UP018 [*] Unnecessary call to `str`
UP018.py:44:4: UP018 [*] Unnecessary `str` call (rewrite as a literal)
|
27 | bytes(b"""
28 | foo""")
29 | f"{str()}"
42 | bytes(b"""
43 | foo""")
44 | f"{str()}"
| ^^^^^ UP018
45 | int()
46 | int(1)
|
= help: Replace with empty string
Fix
26 26 | bytes(b"foo")
27 27 | bytes(b"""
28 28 | foo""")
29 |-f"{str()}"
29 |+f"{''}"
41 41 | bytes(b"foo")
42 42 | bytes(b"""
43 43 | foo""")
44 |-f"{str()}"
44 |+f"{''}"
45 45 | int()
46 46 | int(1)
47 47 | float()
UP018.py:45:1: UP018 [*] Unnecessary `int` call (rewrite as a literal)
|
43 | foo""")
44 | f"{str()}"
45 | int()
| ^^^^^ UP018
46 | int(1)
47 | float()
|
= help: Replace with 0
Fix
42 42 | bytes(b"""
43 43 | foo""")
44 44 | f"{str()}"
45 |-int()
45 |+0
46 46 | int(1)
47 47 | float()
48 48 | float(1.0)
UP018.py:46:1: UP018 [*] Unnecessary `int` call (rewrite as a literal)
|
44 | f"{str()}"
45 | int()
46 | int(1)
| ^^^^^^ UP018
47 | float()
48 | float(1.0)
|
= help: Replace with 0
Fix
43 43 | foo""")
44 44 | f"{str()}"
45 45 | int()
46 |-int(1)
46 |+1
47 47 | float()
48 48 | float(1.0)
49 49 | bool()
UP018.py:47:1: UP018 [*] Unnecessary `float` call (rewrite as a literal)
|
45 | int()
46 | int(1)
47 | float()
| ^^^^^^^ UP018
48 | float(1.0)
49 | bool()
|
= help: Replace with 0.0
Fix
44 44 | f"{str()}"
45 45 | int()
46 46 | int(1)
47 |-float()
47 |+0.0
48 48 | float(1.0)
49 49 | bool()
50 50 | bool(True)
UP018.py:48:1: UP018 [*] Unnecessary `float` call (rewrite as a literal)
|
46 | int(1)
47 | float()
48 | float(1.0)
| ^^^^^^^^^^ UP018
49 | bool()
50 | bool(True)
|
= help: Replace with 0.0
Fix
45 45 | int()
46 46 | int(1)
47 47 | float()
48 |-float(1.0)
48 |+1.0
49 49 | bool()
50 50 | bool(True)
51 51 | bool(False)
UP018.py:49:1: UP018 [*] Unnecessary `bool` call (rewrite as a literal)
|
47 | float()
48 | float(1.0)
49 | bool()
| ^^^^^^ UP018
50 | bool(True)
51 | bool(False)
|
= help: Replace with `False`
Fix
46 46 | int(1)
47 47 | float()
48 48 | float(1.0)
49 |-bool()
49 |+False
50 50 | bool(True)
51 51 | bool(False)
UP018.py:50:1: UP018 [*] Unnecessary `bool` call (rewrite as a literal)
|
48 | float(1.0)
49 | bool()
50 | bool(True)
| ^^^^^^^^^^ UP018
51 | bool(False)
|
= help: Replace with `False`
Fix
47 47 | float()
48 48 | float(1.0)
49 49 | bool()
50 |-bool(True)
50 |+True
51 51 | bool(False)
UP018.py:51:1: UP018 [*] Unnecessary `bool` call (rewrite as a literal)
|
49 | bool()
50 | bool(True)
51 | bool(False)
| ^^^^^^^^^^^ UP018
|
= help: Replace with `False`
Fix
48 48 | float(1.0)
49 49 | bool()
50 50 | bool(True)
51 |-bool(False)
51 |+False