Skip to content
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
dicts = [{"key": "value"}]

# OK
{**dictionary for dictionary in dicts}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
xs = [[1], [2]]

list(*x for x in xs)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
xs = [[1], [2]]

set(*x for x in xs)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
xs = [[1], [2]]

set([*x for x in xs])
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
xs = [[1], [2]]

tuple([*x for x in xs])
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
xs = [[1], [2]]

list([*x for x in xs])
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
dicts = [{"a": 1}, {"b": 2}]

dict({**d for d in dicts})
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
xs = [[1], [2]]

all([*x for x in xs])
any({*x for x in xs})
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
xs = [[1], [2]]

[*x for x in xs][0]
list(*x for x in xs)[0]
4 changes: 3 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1863,7 +1863,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
pylint::rules::unnecessary_dict_index_lookup_comprehension(checker, expr);
}

if checker.is_rule_enabled(Rule::UnnecessaryComprehension) {
if checker.is_rule_enabled(Rule::UnnecessaryComprehension)
&& let Some(key) = key
{
flake8_comprehensions::rules::unnecessary_dict_comprehension(
checker, expr, key, value, generators,
);
Expand Down
4 changes: 3 additions & 1 deletion crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1769,7 +1769,9 @@ impl<'a> Visitor<'a> for Checker<'a> {
node_index: _,
}) => {
self.visit_generators(GeneratorKind::DictComprehension, generators);
self.visit_expr(key);
if let Some(key) = key {
self.visit_expr(key);
}
self.visit_expr(value);
}
Expr::Lambda(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,9 @@ fn find_runtime_varying_call<'a>(
value,
generators,
..
}) => find_runtime_varying_call(key, semantic)
}) => key
.as_deref()
.and_then(|key| find_runtime_varying_call(key, semantic))
.or_else(|| find_runtime_varying_call(value, semantic))
.or_else(|| {
generators.iter().find_map(|generator| {
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ mod tests {
Path::new("B912.py"),
PythonVersion::PY313
)]
#[test_case(
Rule::StaticKeyDictComprehension,
Path::new("B035_py315.py"),
PythonVersion::PY315
)]
fn rules_with_target_version(
rule_code: Rule,
path: &Path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,9 @@ impl<'a> Visitor<'a> for GroupNameFinder<'a> {
}
if !self.overridden {
self.nested = true;
visitor::walk_expr(self, key);
if let Some(key) = key {
visitor::walk_expr(self, key);
}
visitor::walk_expr(self, value);
self.nested = false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ impl Violation for StaticKeyDictComprehension {

/// B035, RUF011
pub(crate) fn static_key_dict_comprehension(checker: &Checker, dict_comp: &ast::ExprDictComp) {
let Some(key) = dict_comp.key.as_deref() else {
return;
};

// Collect the bound names in the comprehension's generators.
let names = {
let mut visitor = StoredNameFinder::default();
Expand All @@ -58,12 +62,12 @@ pub(crate) fn static_key_dict_comprehension(checker: &Checker, dict_comp: &ast::
visitor.names
};

if is_constant(&dict_comp.key, &names) {
if is_constant(key, &names) {
checker.report_diagnostic(
StaticKeyDictComprehension {
key: SourceCodeSnippet::from_str(checker.locator().slice(dict_comp.key.as_ref())),
key: SourceCodeSnippet::from_str(checker.locator().slice(key)),
},
dict_comp.key.range(),
key.range(),
);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---

41 changes: 40 additions & 1 deletion crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ mod tests {
use std::path::Path;

use anyhow::Result;
use ruff_python_ast::PythonVersion;
use test_case::test_case;

use crate::assert_diagnostics;
use crate::registry::Rule;
use crate::settings::LinterSettings;
use crate::settings::types::PreviewMode;
use crate::test::test_path;
use crate::{assert_diagnostics, assert_diagnostics_diff};

#[test_case(Rule::UnnecessaryCallAroundSorted, Path::new("C413.py"))]
#[test_case(Rule::UnnecessaryCollectionCall, Path::new("C408.py"))]
Expand Down Expand Up @@ -51,6 +52,22 @@ mod tests {
Ok(())
}

#[test_case(Rule::UnnecessaryGeneratorList, Path::new("C400_py315.py"))]
#[test_case(Rule::UnnecessaryGeneratorSet, Path::new("C401_py315.py"))]
#[test_case(Rule::UnnecessaryListComprehensionSet, Path::new("C403_py315.py"))]
#[test_case(Rule::UnnecessaryListCall, Path::new("C411_py315.py"))]
#[test_case(Rule::UnnecessaryLiteralWithinDictCall, Path::new("C418_py315.py"))]
#[test_case(Rule::UnnecessaryComprehensionInCall, Path::new("C419_py315.py"))]
fn rules_py315(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Path::new("flake8_comprehensions").join(path).as_path(),
&LinterSettings::for_rule(rule_code).with_target_version(PythonVersion::PY315),
)?;
assert_diagnostics!(snapshot, diagnostics);
Ok(())
}

#[test_case(Rule::UnnecessaryLiteralWithinTupleCall, Path::new("C409.py"))]
#[test_case(Rule::UnnecessaryComprehensionInCall, Path::new("C419_1.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
Expand All @@ -70,6 +87,28 @@ mod tests {
Ok(())
}

#[test_case(Rule::UnnecessaryLiteralWithinTupleCall, Path::new("C409_py315.py"))]
fn preview_rules_py315(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
assert_diagnostics_diff!(
snapshot,
Path::new("flake8_comprehensions").join(path).as_path(),
&LinterSettings {
preview: PreviewMode::Disabled,
..LinterSettings::for_rule(rule_code).with_target_version(PythonVersion::PY315)
},
&LinterSettings {
preview: PreviewMode::Enabled,
..LinterSettings::for_rule(rule_code).with_target_version(PythonVersion::PY315)
},
);
Ok(())
}

#[test_case(Rule::UnnecessaryCollectionCall, Path::new("C408.py"))]
fn allow_dict_calls_with_keyword_arguments(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ pub(crate) fn unnecessary_comprehension_in_call(
}
};
if args.len() == 1 {
if elt.is_starred_expr() {
// The LibCST-based fixer does not yet support PEP 798 unpacking comprehensions.
return;
}
// If there's only one argument, remove the list or set brackets.
diagnostic.try_set_fix(|| {
fixes::fix_unnecessary_comprehension_in_call(expr, checker.locator(), checker.stylist())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,12 @@ pub(crate) fn unnecessary_dict_comprehension_for_iterable(
return;
}

let Some(key) = dict_comp.key.as_deref() else {
return;
};

// Don't suggest `dict.keys` if the target is not the same as the key.
if ComparableExpr::from(&generator.target) != ComparableExpr::from(dict_comp.key.as_ref()) {
if ComparableExpr::from(&generator.target) != ComparableExpr::from(key) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use ruff_python_ast::{Arguments, Expr, ExprCall};
use ruff_python_ast::{self as ast, Arguments, Expr, ExprCall};

use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::rules::flake8_comprehensions::fixes;
use crate::{AlwaysFixableViolation, Fix};
use crate::{Fix, FixAvailability, Violation};

use crate::rules::flake8_comprehensions::helpers;

Expand All @@ -32,14 +32,16 @@ use crate::rules::flake8_comprehensions::helpers;
#[violation_metadata(stable_since = "v0.0.73")]
pub(crate) struct UnnecessaryListCall;

impl AlwaysFixableViolation for UnnecessaryListCall {
impl Violation for UnnecessaryListCall {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
"Unnecessary `list()` call (remove the outer call to `list()`)".to_string()
}

fn fix_title(&self) -> String {
"Remove outer `list()` call".to_string()
fn fix_title(&self) -> Option<String> {
Some("Remove outer `list()` call".to_string())
}
}

Expand Down Expand Up @@ -77,6 +79,13 @@ pub(crate) fn unnecessary_list_call(checker: &Checker, expr: &Expr, call: &ExprC
return;
}
let mut diagnostic = checker.report_diagnostic(UnnecessaryListCall, expr.range());
if matches!(
argument,
Expr::ListComp(ast::ExprListComp { elt, .. }) if elt.is_starred_expr()
) {
// The LibCST-based fixer does not yet support PEP 798 unpacking comprehensions.
return;
}
diagnostic.try_set_fix(|| {
fixes::fix_unnecessary_list_call(expr, checker.locator(), checker.stylist())
.map(Fix::unsafe_edit)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::{AlwaysFixableViolation, Edit, Fix};
use crate::{Edit, Fix, FixAvailability, Violation};

use crate::rules::flake8_comprehensions::helpers;

Expand Down Expand Up @@ -40,15 +40,17 @@ pub(crate) struct UnnecessaryLiteralWithinDictCall {
kind: DictKind,
}

impl AlwaysFixableViolation for UnnecessaryLiteralWithinDictCall {
impl Violation for UnnecessaryLiteralWithinDictCall {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
let UnnecessaryLiteralWithinDictCall { kind } = self;
format!("Unnecessary dict {kind} passed to `dict()` (remove the outer call to `dict()`)")
}

fn fix_title(&self) -> String {
"Remove outer `dict()` call".to_string()
fn fix_title(&self) -> Option<String> {
Some("Remove outer `dict()` call".to_string())
}
}

Expand Down Expand Up @@ -79,6 +81,14 @@ pub(crate) fn unnecessary_literal_within_dict_call(checker: &Checker, call: &ast
call.range(),
);

if matches!(
argument,
Expr::DictComp(ast::ExprDictComp { key: None, .. })
) {
// The LibCST-based fixer does not yet support PEP 798 unpacking comprehensions.
return;
}

// Convert `dict({"a": 1})` to `{"a": 1}`
diagnostic.set_fix({
// Delete from the start of the call to the start of the argument.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use ruff_text_size::{Ranged, TextRange, TextSize};
use crate::checkers::ast::Checker;
use crate::preview::is_check_comprehensions_in_tuple_call_enabled;
use crate::rules::flake8_comprehensions::fixes;
use crate::{AlwaysFixableViolation, Edit, Fix};
use crate::{Edit, Fix, FixAvailability, Violation};

use crate::rules::flake8_comprehensions::helpers;

Expand Down Expand Up @@ -53,7 +53,9 @@ pub(crate) struct UnnecessaryLiteralWithinTupleCall {
literal_kind: TupleLiteralKind,
}

impl AlwaysFixableViolation for UnnecessaryLiteralWithinTupleCall {
impl Violation for UnnecessaryLiteralWithinTupleCall {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
match self.literal_kind {
Expand All @@ -72,13 +74,13 @@ impl AlwaysFixableViolation for UnnecessaryLiteralWithinTupleCall {
}
}

fn fix_title(&self) -> String {
fn fix_title(&self) -> Option<String> {
let title = match self.literal_kind {
TupleLiteralKind::List => "Rewrite as a tuple literal",
TupleLiteralKind::Tuple => "Remove the outer call to `tuple()`",
TupleLiteralKind::ListComp => "Rewrite as a generator",
};
title.to_string()
Some(title.to_string())
}
}

Expand Down Expand Up @@ -156,6 +158,10 @@ pub(crate) fn unnecessary_literal_within_tuple_call(
if any_over_expr(elt, &Expr::is_await_expr) {
return;
}
if elt.is_starred_expr() {
// The LibCST-based fixer does not yet support PEP 798 unpacking comprehensions.
return;
}
// Convert `tuple([x for x in range(10)])` to `tuple(x for x in range(10))`
diagnostic.try_set_fix(|| {
fixes::fix_unnecessary_comprehension_in_call(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs
---
C400 [*] Unnecessary generator (rewrite as a list comprehension)
--> C400_py315.py:3:1
|
1 | xs = [[1], [2]]
2 |
3 | list(*x for x in xs)
| ^^^^^^^^^^^^^^^^^^^^
|
help: Rewrite as a list comprehension
1 | xs = [[1], [2]]
2 |
- list(*x for x in xs)
3 + [*x for x in xs]
note: This is an unsafe fix and may change runtime behavior
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs
---
C401 [*] Unnecessary generator (rewrite as a set comprehension)
--> C401_py315.py:3:1
|
1 | xs = [[1], [2]]
2 |
3 | set(*x for x in xs)
| ^^^^^^^^^^^^^^^^^^^
|
help: Rewrite as a set comprehension
1 | xs = [[1], [2]]
2 |
- set(*x for x in xs)
3 + {*x for x in xs}
note: This is an unsafe fix and may change runtime behavior
Loading
Loading