Skip to content

Commit

Permalink
Merge branch 'main' into add-R1736
Browse files Browse the repository at this point in the history
  • Loading branch information
diceroll123 authored Oct 17, 2023
2 parents 7af33f0 + 5da0f91 commit d5422ed
Show file tree
Hide file tree
Showing 11 changed files with 278 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@
for item in {"apples", "lemons", "water"}: # flags in-line set literals
print(f"I like {item}.")

for item in {1,}:
print(f"I can count to {item}!")

for item in {
"apples", "lemons", "water"
}: # flags in-line set literals
print(f"I like {item}.")

numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions

numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Errors
1 in [1, 2, 3]
1 in (1, 2, 3)
1 in (
1, 2, 3
)

# OK
fruits = ["cherry", "grapes"]
"cherry" in fruits
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 @@ -1197,6 +1197,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::ComparisonWithItself) {
pylint::rules::comparison_with_itself(checker, left, ops, comparators);
}
if checker.enabled(Rule::LiteralMembership) {
pylint::rules::literal_membership(checker, compare);
}
if checker.enabled(Rule::ComparisonOfConstant) {
pylint::rules::comparison_of_constant(checker, left, ops, comparators);
}
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 @@ -255,6 +255,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "R1722") => (RuleGroup::Stable, rules::pylint::rules::SysExitAlias),
(Pylint, "R2004") => (RuleGroup::Stable, rules::pylint::rules::MagicValueComparison),
(Pylint, "R5501") => (RuleGroup::Stable, rules::pylint::rules::CollapsibleElseIf),
(Pylint, "R6201") => (RuleGroup::Preview, rules::pylint::rules::LiteralMembership),
#[allow(deprecated)]
(Pylint, "R6301") => (RuleGroup::Nursery, rules::pylint::rules::NoSelfUse),
(Pylint, "W0120") => (RuleGroup::Stable, rules::pylint::rules::UselessElseOnLoop),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ mod tests {
Rule::UnnecessaryListIndexLookup,
Path::new("unnecessary_list_index_lookup.py")
)]
#[test_case(Rule::LiteralMembership, Path::new("literal_membership.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
25 changes: 11 additions & 14 deletions crates/ruff_linter/src/rules/pylint/rules/iteration_over_set.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
use ast::ExprContext;
use ruff_python_ast::{self as ast, Expr};

use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_text_size::{Ranged, TextRange};
use ruff_python_ast::{self as ast, Expr};
use ruff_text_size::Ranged;

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

Expand Down Expand Up @@ -38,7 +36,7 @@ impl AlwaysFixableViolation for IterationOverSet {
}

fn fix_title(&self) -> String {
format!("Use a sequence type instead of a `set` when iterating over values")
format!("Convert to `tuple`")
}
}

Expand All @@ -54,15 +52,14 @@ pub(crate) fn iteration_over_set(checker: &mut Checker, expr: &Expr) {

let mut diagnostic = Diagnostic::new(IterationOverSet, expr.range());

let tuple = checker.generator().expr(&Expr::Tuple(ast::ExprTuple {
elts: elts.clone(),
ctx: ExprContext::Store,
range: TextRange::default(),
}));
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
format!("({tuple})"),
expr.range(),
)));
let tuple = if let [elt] = elts.as_slice() {
let elt = checker.locator().slice(elt);
format!("({elt},)")
} else {
let set = checker.locator().slice(expr);
format!("({})", &set[1..set.len() - 1])
};
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(tuple, expr.range())));

checker.diagnostics.push(diagnostic);
}
65 changes: 65 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/literal_membership.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, CmpOp, Expr};
use ruff_text_size::Ranged;

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

/// ## What it does
/// Checks for membership tests on `list` and `tuple` literals.
///
/// ## Why is this bad?
/// When testing for membership in a static sequence, prefer a `set` literal
/// over a `list` or `tuple`, as Python optimizes `set` membership tests.
///
/// ## Example
/// ```python
/// 1 in [1, 2, 3]
/// ```
///
/// Use instead:
/// ```python
/// 1 in {1, 2, 3}
/// ```
/// ## References
/// - [What’s New In Python 3.2](https://docs.python.org/3/whatsnew/3.2.html#optimizations)
#[violation]
pub struct LiteralMembership;

impl AlwaysFixableViolation for LiteralMembership {
#[derive_message_formats]
fn message(&self) -> String {
format!("Use a `set` literal when testing for membership")
}

fn fix_title(&self) -> String {
format!("Convert to `set`")
}
}

/// PLR6201
pub(crate) fn literal_membership(checker: &mut Checker, compare: &ast::ExprCompare) {
let [op] = compare.ops.as_slice() else {
return;
};

if !matches!(op, CmpOp::In | CmpOp::NotIn) {
return;
}

let [right] = compare.comparators.as_slice() else {
return;
};

if !matches!(right, Expr::List(_) | Expr::Tuple(_)) {
return;
}

let mut diagnostic = Diagnostic::new(LiteralMembership, right.range());

let literal = checker.locator().slice(right);
let set = format!("{{{}}}", &literal[1..literal.len() - 1]);
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(set, right.range())));

checker.diagnostics.push(diagnostic);
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub(crate) use invalid_envvar_value::*;
pub(crate) use invalid_str_return::*;
pub(crate) use invalid_string_characters::*;
pub(crate) use iteration_over_set::*;
pub(crate) use literal_membership::*;
pub(crate) use load_before_global_declaration::*;
pub(crate) use logging::*;
pub(crate) use magic_value_comparison::*;
Expand Down Expand Up @@ -87,6 +88,7 @@ mod invalid_envvar_value;
mod invalid_str_return;
mod invalid_string_characters;
mod iteration_over_set;
mod literal_membership;
mod load_before_global_declaration;
mod logging;
mod magic_value_comparison;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ iteration_over_set.py:3:13: PLC0208 [*] Use a sequence type instead of a `set` w
| ^^^ PLC0208
4 | print(f"I can count to {item}!")
|
= help: Use a sequence type instead of a `set` when iterating over values
= help: Convert to `tuple`

Fix
1 1 | # Errors
Expand All @@ -28,7 +28,7 @@ iteration_over_set.py:6:13: PLC0208 [*] Use a sequence type instead of a `set` w
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC0208
7 | print(f"I like {item}.")
|
= help: Use a sequence type instead of a `set` when iterating over values
= help: Convert to `tuple`

Fix
3 3 | for item in {1}:
Expand All @@ -38,90 +38,136 @@ iteration_over_set.py:6:13: PLC0208 [*] Use a sequence type instead of a `set` w
6 |+for item in ("apples", "lemons", "water"): # flags in-line set literals
7 7 | print(f"I like {item}.")
8 8 |
9 9 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
9 9 | for item in {1,}:

iteration_over_set.py:9:28: PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
iteration_over_set.py:9:13: PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
|
7 | print(f"I like {item}.")
8 |
9 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
| ^^^^^^^^^ PLC0208
10 |
11 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
9 | for item in {1,}:
| ^^^^ PLC0208
10 | print(f"I can count to {item}!")
|
= help: Use a sequence type instead of a `set` when iterating over values
= help: Convert to `tuple`

Fix
6 6 | for item in {"apples", "lemons", "water"}: # flags in-line set literals
7 7 | print(f"I like {item}.")
8 8 |
9 |-numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
9 |+numbers_list = [i for i in (1, 2, 3)] # flags sets in list comprehensions
10 10 |
11 11 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
12 12 |
9 |-for item in {1,}:
9 |+for item in (1,):
10 10 | print(f"I can count to {item}!")
11 11 |
12 12 | for item in {

iteration_over_set.py:11:27: PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
iteration_over_set.py:12:13: PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
|
9 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
10 |
11 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
| ^^^^^^^^^ PLC0208
12 |
13 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
10 | print(f"I can count to {item}!")
11 |
12 | for item in {
| _____________^
13 | | "apples", "lemons", "water"
14 | | }: # flags in-line set literals
| |_^ PLC0208
15 | print(f"I like {item}.")
|
= help: Use a sequence type instead of a `set` when iterating over values
= help: Convert to `tuple`

ℹ Fix
8 8 |
9 9 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
10 10 |
11 |-numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
11 |+numbers_set = {i for i in (1, 2, 3)} # flags sets in set comprehensions
12 12 |
13 13 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
14 14 |

iteration_over_set.py:13:36: PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
9 9 | for item in {1,}:
10 10 | print(f"I can count to {item}!")
11 11 |
12 |-for item in {
12 |+for item in (
13 13 | "apples", "lemons", "water"
14 |-}: # flags in-line set literals
14 |+): # flags in-line set literals
15 15 | print(f"I like {item}.")
16 16 |
17 17 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions

iteration_over_set.py:17:28: PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
|
11 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
12 |
13 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
| ^^^^^^^^^ PLC0208
14 |
15 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions
15 | print(f"I like {item}.")
16 |
17 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
| ^^^^^^^^^ PLC0208
18 |
19 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
|
= help: Use a sequence type instead of a `set` when iterating over values
= help: Convert to `tuple`

ℹ Fix
10 10 |
11 11 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
12 12 |
13 |-numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
13 |+numbers_dict = {str(i): i for i in (1, 2, 3)} # flags sets in dict comprehensions
14 14 |
15 15 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions
14 14 | }: # flags in-line set literals
15 15 | print(f"I like {item}.")
16 16 |
17 |-numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
17 |+numbers_list = [i for i in (1, 2, 3)] # flags sets in list comprehensions
18 18 |
19 19 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
20 20 |

iteration_over_set.py:15:27: PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
iteration_over_set.py:19:27: PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
|
13 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
14 |
15 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions
17 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
18 |
19 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
| ^^^^^^^^^ PLC0208
16 |
17 | # Non-errors
20 |
21 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
|
= help: Use a sequence type instead of a `set` when iterating over values
= help: Convert to `tuple`

Fix
12 12 |
13 13 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
14 14 |
15 |-numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions
15 |+numbers_gen = (i for i in (1, 2, 3)) # flags sets in generator expressions
16 16 |
17 17 | # Non-errors
17 17 | numbers_list = [i for i in {1, 2, 3}] # flags sets in list comprehensions
18 18 |
19 |-numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
19 |+numbers_set = {i for i in (1, 2, 3)} # flags sets in set comprehensions
20 20 |
21 21 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
22 22 |

iteration_over_set.py:21:36: PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
|
19 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
20 |
21 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
| ^^^^^^^^^ PLC0208
22 |
23 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions
|
= help: Convert to `tuple`

Fix
18 18 |
19 19 | numbers_set = {i for i in {1, 2, 3}} # flags sets in set comprehensions
20 20 |
21 |-numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
21 |+numbers_dict = {str(i): i for i in (1, 2, 3)} # flags sets in dict comprehensions
22 22 |
23 23 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions
24 24 |

iteration_over_set.py:23:27: PLC0208 [*] Use a sequence type instead of a `set` when iterating over values
|
21 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
22 |
23 | numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions
| ^^^^^^^^^ PLC0208
24 |
25 | # Non-errors
|
= help: Convert to `tuple`

Fix
20 20 |
21 21 | numbers_dict = {str(i): i for i in {1, 2, 3}} # flags sets in dict comprehensions
22 22 |
23 |-numbers_gen = (i for i in {1, 2, 3}) # flags sets in generator expressions
23 |+numbers_gen = (i for i in (1, 2, 3)) # flags sets in generator expressions
24 24 |
25 25 | # Non-errors
26 26 |


Loading

0 comments on commit d5422ed

Please sign in to comment.