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

[wps-light] Implement list-multiplication (WPS435) #5

Open
wants to merge 7 commits into
base: wps-light-rules
Choose a base branch
from
Open
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
23 changes: 23 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/wps_light/WPS435.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Example 1: Multiplying a list of mutable strings
row = [""] * 3 # Creates a list with 3 references to the same empty string
tic_tac_toe = [row] * 3 # This will create 3 references to the same `row` list
tic_tac_toe[0][0] = 'X' # This affects all rows because they refer to the same `row` list

# Example 2: Multiplying a list of mutable dictionaries
board = [{"cell": ""}] * 3 # Creates a list with 3 references to the same dictionary
tic_tac_toe_dict = [board] * 3 # This will create 3 references to the same `board` list
tic_tac_toe_dict[0][0]["cell"] = 'X' # This affects all dictionaries because they refer to the same object

# Example 3: Multiplying a list of sets
set_a = set() # Create an empty set
set_list = [set_a] * 3 # Creates a list with 3 references to the same set
set_list[0].add(1) # This affects all sets because they refer to the same set

# Example 4: Nested lists of mutable objects (list of lists)
nested_board = [[""] * 3 for _ in range(3)] # This creates separate lists for each row
tic_tac_toe_nested = [nested_board] * 3 # This will create 3 references to the same `nested_board` list
tic_tac_toe_nested[0][0][0] = 'X' # This affects all nested lists because they refer to the same object

# Example 5: Correct approach using list comprehension (no violation)
tic_tac_toe_fixed = [[''] * 3 for _ in range(3)] # Creates separate lists for each row
tic_tac_toe_fixed[0][0] = 'X' # Only the first row should be affected
11 changes: 10 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::rules::{
flake8_future_annotations, flake8_gettext, flake8_implicit_str_concat, flake8_logging,
flake8_logging_format, flake8_pie, flake8_print, flake8_pyi, flake8_pytest_style, flake8_self,
flake8_simplify, flake8_tidy_imports, flake8_type_checking, flake8_use_pathlib, flynt, numpy,
pandas_vet, pep8_naming, pycodestyle, pyflakes, pylint, pyupgrade, refurb, ruff,
pandas_vet, pep8_naming, pycodestyle, pyflakes, pylint, pyupgrade, refurb, ruff, wps_light,
};
use crate::settings::types::PythonVersion;

Expand Down Expand Up @@ -1272,6 +1272,15 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}
}
Expr::BinOp(
binary_op @ ast::ExprBinOp {
op: Operator::Mult, ..
},
) => {
if checker.enabled(Rule::ListMultiplication) {
wps_light::rules::list_multiplication(checker, binary_op);
}
}
Expr::UnaryOp(
unary_op @ ast::ExprUnaryOp {
op,
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 @@ -935,6 +935,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {

// wps-light
(WpsLight, "000") => (RuleGroup::Preview, rules::wps_light::rules::Dummy),
(WpsLight, "435") => (RuleGroup::Preview, rules::wps_light::rules::ListMultiplication),

// ruff
(Ruff, "001") => (RuleGroup::Stable, rules::ruff::rules::AmbiguousUnicodeCharacterString),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/wps_light/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod tests {
use crate::{assert_messages, settings};

#[test_case(Rule::Dummy, Path::new("WPS000.py"))]
#[test_case(Rule::ListMultiplication, Path::new("WPS435.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
155 changes: 155 additions & 0 deletions crates/ruff_linter/src/rules/wps_light/rules/list_multiplication.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, ExprBinOp, ExprCall, ExprName, Operator};
use ruff_python_semantic::analyze::typing::find_binding_value;
use ruff_python_semantic::{
analyze::{
type_inference::{NumberLike, PythonType, ResolvedPythonType},
typing,
},
BindingId, SemanticModel,
};

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

/// ## What it does
/// Checks for multiplication of list of mutable objects
///
/// ## Why is this bad?
/// Mutable objects in a list can lead to unexpected behavior when the list is multiplied.
/// Each element in the resulting list will refer to the same mutable object. Any modification
/// to one element affects all instances in the multiplied list. This can lead to bugs that are difficult to trace.
///
/// ## Example
/// ```python
/// row = [""] * 3
/// tic_tac_toe = [row] * 3
///
/// tic_tac_toe[0][0] = "X"
/// tic_tac_toe # [["X", "", ""], ["X", "", ""], ["", "", ""]]
/// ```
///
/// Use instead:
///
/// ```python
/// row = [[""] * 3 for _ in range(3)]
///
/// tic_tac_toe[0][0] = "X"
/// tic_tac_toe # [["X", "", ""], ["", "", ""], ["", "", ""]]
/// ```
#[violation]
pub struct ListMultiplication;

impl Violation for ListMultiplication {
#[derive_message_formats]
fn message(&self) -> String {
"Multiplication of list of mutable objects".to_string()
}
}

/// WPS435
pub(crate) fn list_multiplication(checker: &mut Checker, expr: &ExprBinOp) {
let ExprBinOp {
range, left, right, ..
} = expr;
let semantic = checker.semantic();

let operands = [left, right];
if operands
.into_iter()
.filter(|expr| is_integer(expr, semantic))
.count()
!= 1
{
return;
}
if operands
.into_iter()
.filter(|expr| is_nested_list_like(expr, semantic))
.count()
!= 1
{
return;
}

checker
.diagnostics
.push(Diagnostic::new(ListMultiplication, *range));
}

fn is_integer(expr: &Expr, semantic: &SemanticModel) -> bool {
// Check if the expression directly resolves to an integer type
if matches!(
ResolvedPythonType::from(expr),
ResolvedPythonType::Atom(PythonType::Number(NumberLike::Integer))
) {
return true;
}

// Check if the expression is a name and resolve its binding
let Some(expr_name) = expr.as_name_expr() else {
return false;
};
let Some(id) = semantic.only_binding(expr_name) else {
return false;
};

// Verify if the binding resolves to an integer
typing::is_int(semantic.binding(id), semantic)
}

fn is_nested_list_like(expr: &Expr, semantic: &SemanticModel) -> bool {
match expr {
Expr::ListComp(expr_list_comp) => is_terminal_list_like(&expr_list_comp.elt, semantic),
Expr::Name(expr_name) => get_name_value(expr_name, semantic)
.is_some_and(|value| is_nested_list_like(value, semantic)),
Expr::List(expr_list) => expr_list
.into_iter()
.any(|item| is_terminal_list_like(item, semantic)),
_ => false,
}
}

fn is_terminal_list_like(expr: &Expr, semantic: &SemanticModel) -> bool {
match expr {
Expr::BinOp(ExprBinOp {
op: Operator::Mult,
left,
right,
..
}) => {
let operands = [left, right];
if operands
.into_iter()
.filter(|expr| is_integer(expr, semantic))
.count()
!= 1
{
return false;
}
if operands
.into_iter()
.filter(|expr| is_terminal_list_like(expr, semantic))
.count()
!= 1
{
return false;
}
true
}
Expr::Call(ExprCall { func, .. }) => semantic.match_builtin_expr(func, "range"),
Expr::Name(expr_name) => get_name_value(expr_name, semantic)
.is_some_and(|value| is_terminal_list_like(value, semantic)),
_ => typing::is_mutable_expr(expr, semantic),
}
}

fn get_name_value<'a>(name: &ExprName, semantic: &'a SemanticModel) -> Option<&'a Expr> {
let scope = semantic.current_scope();
let bindings: Vec<BindingId> = scope.get_all(name.id()).collect();
let [binding_id] = bindings.as_slice() else {
return None;
};
let binding = semantic.binding(*binding_id);
find_binding_value(binding, semantic)
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/wps_light/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
pub(crate) use dummy::*;
pub(crate) use list_multiplication::*;

mod dummy;
mod list_multiplication;

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
source: crates/ruff_linter/src/rules/wps_light/mod.rs
snapshot_kind: text
---
WPS435.py:3:15: WPS435 Multiplication of list of mutable objects
|
1 | # Example 1: Multiplying a list of mutable strings
2 | row = [""] * 3 # Creates a list with 3 references to the same empty string
3 | tic_tac_toe = [row] * 3 # This will create 3 references to the same `row` list
| ^^^^^^^^^ WPS435
4 | tic_tac_toe[0][0] = 'X' # This affects all rows because they refer to the same `row` list
|

WPS435.py:7:9: WPS435 Multiplication of list of mutable objects
|
6 | # Example 2: Multiplying a list of mutable dictionaries
7 | board = [{"cell": ""}] * 3 # Creates a list with 3 references to the same dictionary
| ^^^^^^^^^^^^^^^^^^ WPS435
8 | tic_tac_toe_dict = [board] * 3 # This will create 3 references to the same `board` list
9 | tic_tac_toe_dict[0][0]["cell"] = 'X' # This affects all dictionaries because they refer to the same object
|

WPS435.py:8:20: WPS435 Multiplication of list of mutable objects
|
6 | # Example 2: Multiplying a list of mutable dictionaries
7 | board = [{"cell": ""}] * 3 # Creates a list with 3 references to the same dictionary
8 | tic_tac_toe_dict = [board] * 3 # This will create 3 references to the same `board` list
| ^^^^^^^^^^^ WPS435
9 | tic_tac_toe_dict[0][0]["cell"] = 'X' # This affects all dictionaries because they refer to the same object
|

WPS435.py:18:22: WPS435 Multiplication of list of mutable objects
|
16 | # Example 4: Nested lists of mutable objects (list of lists)
17 | nested_board = [[""] * 3 for _ in range(3)] # This creates separate lists for each row
18 | tic_tac_toe_nested = [nested_board] * 3 # This will create 3 references to the same `nested_board` list
| ^^^^^^^^^^^^^^^^^^ WPS435
19 | tic_tac_toe_nested[0][0][0] = 'X' # This affects all nested lists because they refer to the same object
|

This file was deleted.

3 changes: 3 additions & 0 deletions ruff.schema.json

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

Loading