Skip to content

Commit

Permalink
Use single rule
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Jan 7, 2023
1 parent a24fa58 commit 11fbff1
Show file tree
Hide file tree
Showing 9 changed files with 226 additions and 346 deletions.
4 changes: 2 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,8 +551,8 @@ For more, see [Pyflakes](https://pypi.org/project/pyflakes/2.5.0/) on PyPI.
| F524 | StringDotFormatMissingArguments | '...'.format(...) is missing argument(s) for placeholder(s): ... | |
| F525 | StringDotFormatMixingAutomatic | '...'.format(...) mixes automatic and manual numbering | |
| F541 | FStringMissingPlaceholders | f-string without any placeholders | 🛠 |
| F601 | MultiValueRepeatedKeyLiteral | Dictionary key literal `...` repeated | 🛠 |
| F602 | MultiValueRepeatedKeyVariable | Dictionary key `...` repeated | 🛠 |
| F601 | MultiValueRepeatedKeyLiteral | Dictionary key literal `...` repeated | |
| F602 | MultiValueRepeatedKeyVariable | Dictionary key `...` repeated | |
| F621 | ExpressionsInStarAssignment | Too many expressions in star-unpacking assignment | |
| F622 | TwoStarredExpressions | Two starred expressions in assignment | |
| F631 | AssertTuple | Assert test is a non-empty tuple, which is always `True` | |
Expand Down
3 changes: 2 additions & 1 deletion resources/test/fixtures/pyflakes/F601.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,5 @@
a: 4,
}

x = {"b": 2, "a": 1, "b": 2}
x = {"a": 1, "a": 1}
x = {"a": 1, "b": 2, "a": 1}
3 changes: 2 additions & 1 deletion resources/test/fixtures/pyflakes/F602.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,5 @@
a: 4,
}

x = {b: 2, a: 1, b: 2}
x = {a: 1, a: 1}
x = {a: 1, b: 2, a: 1}
9 changes: 4 additions & 5 deletions src/checkers/ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2386,11 +2386,10 @@ where
}
}
ExprKind::Dict { keys, values } => {
if self.settings.enabled.contains(&CheckCode::F601) {
pyflakes::plugins::repeated_literal_keys(self, keys, values);
}
if self.settings.enabled.contains(&CheckCode::F602) {
pyflakes::plugins::repeated_variable_keys(self, keys, values);
if self.settings.enabled.contains(&CheckCode::F601)
|| self.settings.enabled.contains(&CheckCode::F602)
{
pyflakes::plugins::repeated_keys(self, keys, values);
}
}
ExprKind::Yield { .. } => {
Expand Down
2 changes: 1 addition & 1 deletion src/pyflakes/plugins/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pub use if_tuple::if_tuple;
pub use invalid_literal_comparisons::invalid_literal_comparison;
pub use invalid_print_syntax::invalid_print_syntax;
pub use raise_not_implemented::raise_not_implemented;
pub use repeated_keys::{repeated_literal_keys, repeated_variable_keys};
pub use repeated_keys::repeated_keys;
pub(crate) use strings::{
percent_format_expected_mapping, percent_format_expected_sequence,
percent_format_extra_named_arguments, percent_format_missing_arguments,
Expand Down
140 changes: 68 additions & 72 deletions src/pyflakes/plugins/repeated_keys.rs
View file
Original file line number Diff line number Diff line change
@@ -1,97 +1,93 @@
use rustpython_ast::{Constant, Expr, ExprKind};
use std::hash::{BuildHasherDefault, Hash};

use rustc_hash::FxHashMap;
use rustpython_ast::{Expr, ExprKind};

use crate::ast::cmp;
use crate::ast::helpers::unparse_expr;
use crate::ast::types::Range;
use crate::autofix::Fix;
use crate::checkers::ast::Checker;
use crate::registry::{Check, CheckCode, CheckKind};
use crate::registry::{Check, CheckCode};
use crate::source_code_style::SourceCodeStyleDetector;
use crate::violations;

#[derive(Debug, PartialEq)]
#[derive(Debug, Eq, PartialEq, Hash)]
enum DictionaryKey<'a> {
Constant(&'a Constant),
Constant(String),
Variable(&'a str),
}

impl<'a> TryFrom<&'a Expr> for DictionaryKey<'a> {
type Error = ();

fn try_from(expr: &'a Expr) -> Result<Self, Self::Error> {
match &expr.node {
ExprKind::Constant { value, .. } => Ok(DictionaryKey::Constant(value)),
ExprKind::Name { id, .. } => Ok(DictionaryKey::Variable(id)),
_ => Err(()),
}
fn into_dictionary_key<'a>(
expr: &'a Expr,
stylist: &SourceCodeStyleDetector,
) -> Option<DictionaryKey<'a>> {
match &expr.node {
ExprKind::Constant { .. } => Some(DictionaryKey::Constant(unparse_expr(expr, stylist))),
ExprKind::Name { id, .. } => Some(DictionaryKey::Variable(id)),
_ => None,
}
}

/// F601
pub fn repeated_literal_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) {
let num_keys = keys.len();
for i in 0..num_keys {
let k1: Option<DictionaryKey> = (&keys[i]).try_into().ok();
for j in i + 1..num_keys {
let k2: Option<DictionaryKey> = (&keys[j]).try_into().ok();
if let (Some(DictionaryKey::Constant(v1)), Some(DictionaryKey::Constant(v2))) =
(&k1, &k2)
{
if v1 == v2 {
let mut check = Check::new(
violations::MultiValueRepeatedKeyLiteral(unparse_expr(
&keys[j],
checker.style,
)),
Range::from_located(&keys[j]),
);
if checker.patch(&CheckCode::F601) {
if cmp::expr(&values[i], &values[j]) {
check.amend(if j < num_keys - 1 {
Fix::deletion(keys[j].location, keys[j + 1].location)
/// F601, F602
pub fn repeated_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) {
// Generate a map from key to (index, value).
let mut seen: FxHashMap<DictionaryKey, Vec<&Expr>> =
FxHashMap::with_capacity_and_hasher(keys.len(), BuildHasherDefault::default());

// Detect duplicate keys.
for (i, key) in keys.iter().enumerate() {
if let Some(key) = into_dictionary_key(key, checker.style) {
if let Some(seen_values) = seen.get_mut(&key) {
match key {
DictionaryKey::Constant(key) => {
if checker.settings.enabled.contains(&CheckCode::F601) {
let repeated_value =
seen_values.iter().any(|value| cmp::expr(value, &values[i]));
let mut check = Check::new(
violations::MultiValueRepeatedKeyLiteral(key, repeated_value),
Range::from_located(&keys[i]),
);
if repeated_value {
if checker.patch(&CheckCode::F601) {
check.amend(Fix::deletion(
values[i - 1].end_location.unwrap(),
values[i].end_location.unwrap(),
));
}
} else {
Fix::deletion(
values[j - 1].end_location.unwrap(),
values[j].end_location.unwrap(),
)
});
seen_values.push(&values[i]);
}
checker.checks.push(check);
}
}
checker.checks.push(check);
}
}
}
}
}

/// F602
pub fn repeated_variable_keys(checker: &mut Checker, keys: &[Expr], values: &[Expr]) {
let num_keys = keys.len();
for i in 0..num_keys {
let k1: Option<DictionaryKey> = (&keys[i]).try_into().ok();
for j in i + 1..num_keys {
let k2: Option<DictionaryKey> = (&keys[j]).try_into().ok();
if let (Some(DictionaryKey::Variable(v1)), Some(DictionaryKey::Variable(v2))) =
(&k1, &k2)
{
if v1 == v2 {
let mut check = Check::new(
violations::MultiValueRepeatedKeyVariable((*v2).to_string()),
Range::from_located(&keys[j]),
);
if checker.patch(&CheckCode::F602) {
if cmp::expr(&values[i], &values[j]) {
check.amend(if j < num_keys - 1 {
Fix::deletion(keys[j].location, keys[j + 1].location)
DictionaryKey::Variable(key) => {
if checker.settings.enabled.contains(&CheckCode::F602) {
let repeated_value =
seen_values.iter().any(|value| cmp::expr(value, &values[i]));
let mut check = Check::new(
violations::MultiValueRepeatedKeyVariable(
key.to_string(),
repeated_value,
),
Range::from_located(&keys[i]),
);
if repeated_value {
if checker.patch(&CheckCode::F602) {
check.amend(Fix::deletion(
values[i - 1].end_location.unwrap(),
values[i].end_location.unwrap(),
));
}
} else {
Fix::deletion(
values[j - 1].end_location.unwrap(),
values[j].end_location.unwrap(),
)
});
seen_values.push(&values[i]);
}
checker.checks.push(check);
}
}
checker.checks.push(check);
}
} else {
seen.insert(key, vec![&values[i]]);
}
}
}
Expand Down
Loading

0 comments on commit 11fbff1

Please sign in to comment.